fix: minBrowserVersion in SyncOption should be a number (#291)

Author: rerorero

lib/storage-client.js

@@ -7,6 +7,7 @@ import {
   X64,
   X86,
   APPLE_ARM_SUFFIXES,
+  convertToInt,
 } from './utils';
 import _ from 'lodash';
 import xpath from 'xpath';
@@ -296,9 +297,9 @@ class ChromedriverStorageClient {
       }
     }
 
-    if (_.isString(minBrowserVersion) && !Number.isNaN(minBrowserVersion)) {
+    const minBrowserVersionInt = convertToInt(minBrowserVersion);
+    if (minBrowserVersionInt !== null) {
       // Only select drivers that support the current browser whose major version number equals to `minBrowserVersion`
-      const minBrowserVersionInt = parseInt(minBrowserVersion, 10);
       log.debug(
         `Selecting chromedrivers whose minimum supported browser version matches to ${minBrowserVersionInt}`
       );

lib/utils.js

@@ -147,6 +147,25 @@ function generateLogPrefix(obj, sessionId = null) {
   );
 }
 
+/**
+ * Converts the given object to an integer number if possible
+ *
+ * @param {any} value to be converted
+ * @returns {number | null}
+ */
+function convertToInt(value) {
+  switch (typeof value) {
+    case 'number':
+      return Number.isNaN(value) ? null : value;
+    case 'string': {
+      const parsedAsInt = parseInt(value, 10);
+      return Number.isNaN(parsedAsInt) ? null : parsedAsInt;
+    }
+    default:
+      return null;
+  }
+}
+
 export {
   getChromeVersion,
   getChromedriverDir,
@@ -164,4 +183,5 @@ export {
   X86,
   APPLE_ARM_SUFFIXES,
   generateLogPrefix,
+  convertToInt,
 };

test/functional/storage-client-e2e-specs.js

@@ -41,7 +41,7 @@ describe('ChromedriverStorageClient', function () {
     });
     try {
       (await client.syncDrivers({
-        minBrowserVersion: '44',
+        minBrowserVersion: 44,
       })).length.should.be.greaterThan(0);
       (await fs.readdir(tmpRoot)).length.should.be.greaterThan(0);
     } finally {
@@ -56,7 +56,7 @@ describe('ChromedriverStorageClient', function () {
     });
     try {
       (await client.syncDrivers({
-        minBrowserVersion: '74',
+        minBrowserVersion: 74,
       })).length.should.be.greaterThan(0);
       (await fs.readdir(tmpRoot)).length.should.be.greaterThan(0);
     } finally {

test/unit/storage-client-specs.js

@@ -165,7 +165,7 @@ describe('ChromedriverStorageClient', function () {
         name: 'mac',
         arch: '64',
       }, {
-        minBrowserVersion: '60',
+        minBrowserVersion: 60,
       });
       selectedDrivers.should.eql([
         '76.0.3809.12/chromedriver_mac64.zip',
@@ -180,7 +180,7 @@ describe('ChromedriverStorageClient', function () {
         arch: '64',
       }, {
         versions: ['76.0.3809.12'],
-        minBrowserVersion: '60',
+        minBrowserVersion: 60,
       });
       selectedDrivers.should.eql([
         '76.0.3809.12/chromedriver_mac64.zip',

test/unit/utils-specs.js

@@ -0,0 +1,32 @@
+import {convertToInt} from '../../lib/utils';
+import chai from 'chai';
+
+chai.should();
+const should = chai.should();
+
+describe('utils', function () {
+  describe('convertToInt', function () {
+    it('should parse a number', function () {
+      convertToInt(0).should.eql(0);
+      convertToInt(1).should.eql(1);
+      convertToInt(100).should.eql(100);
+    });
+    it('should return null with NaN', function () {
+      should.not.exist(convertToInt(NaN));
+    });
+    it('should parse a number string', function () {
+      convertToInt('0').should.eql(0);
+      convertToInt('1.1').should.eql(1);
+      convertToInt('-123').should.eql(-123);
+    });
+    it('should return null if non numer string is given', function () {
+      should.not.exist(convertToInt(''));
+      should.not.exist(convertToInt('foo'));
+    });
+    it('should return null if unexpected type', function () {
+      should.not.exist(convertToInt({}));
+      should.not.exist(convertToInt(null));
+      should.not.exist(convertToInt(true));
+    });
+  });
+});