From 5966b6ac7329878e9e16f5b1b88261c5b7f7e438 Mon Sep 17 00:00:00 2001 From: Craig Date: Wed, 9 Nov 2016 16:42:53 -0800 Subject: [PATCH] fix(cli): fix setting flag to false (#135) - This fixes `webdriver-manager update --gecko=false` - This does not fix `webdriver-manager update --gecko=0`. Minimist interprets 0 as true. - Add options and programs unit tests closes #110 --- lib/cli/options.ts | 25 +++++-- lib/cli/programs.ts | 6 +- spec/cli/options_spec.ts | 145 ++++++++++++++++++++++++++++++++++++++ spec/cli/programs_spec.ts | 82 +++++++++++++++++++++ spec/webdriver_spec.ts | 2 +- 5 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 spec/cli/options_spec.ts create mode 100644 spec/cli/programs_spec.ts diff --git a/lib/cli/options.ts b/lib/cli/options.ts index 3c38facd..ee8e78fb 100644 --- a/lib/cli/options.ts +++ b/lib/cli/options.ts @@ -16,7 +16,7 @@ export class Option { this.opt = opt; this.description = description; this.type = type; - if (defaultValue) { + if (defaultValue != null) { this.defaultValue = defaultValue; } } @@ -31,23 +31,34 @@ export class Option { getNumber(): number { let value = this.getValue_(); - if (value && typeof value === 'number') { + if (value != null && (typeof value === 'number' || typeof value === 'string')) { return +value; + } else { + return null; } - return null; } getString(): string { let value = this.getValue_(); - if (value && typeof value === 'string') { - return '' + value; + if (value != null) { + return '' + this.getValue_(); + } else { + return ''; } - return ''; } getBoolean(): boolean { let value = this.getValue_(); - return Boolean(value); + if (value != null) { + if (typeof value === 'string') { + return !(value === '0' || value === 'false'); + } else if (typeof value === 'number') { + return value !== 0; + } else { + return value; + } + } + return false; } } diff --git a/lib/cli/programs.ts b/lib/cli/programs.ts index 78743f30..ca97b945 100644 --- a/lib/cli/programs.ts +++ b/lib/cli/programs.ts @@ -78,19 +78,19 @@ export class Program { this.runMethod(this.options); } - private getValue_(key: string, json: JSON): string { + private getValue_(key: string, json: JSON): number|boolean|string { let keyList: string[] = key.split('.'); let tempJson: any = json; while (keyList.length > 0) { let keyItem = keyList[0]; - if (tempJson[keyItem]) { + if (tempJson[keyItem] != null) { tempJson = tempJson[keyItem]; keyList = keyList.slice(1); } else { return undefined; } } - return tempJson.toString(); + return tempJson; } /** diff --git a/spec/cli/options_spec.ts b/spec/cli/options_spec.ts new file mode 100644 index 00000000..1c1b4359 --- /dev/null +++ b/spec/cli/options_spec.ts @@ -0,0 +1,145 @@ +import {Option} from '../../lib/cli/options'; + + +describe('options', () => { + let option: Option; + + describe('get number', () => { + + describe('for this.value not set', () => { + it('should return the default value', () => { + option = new Option('fake opt', 'fake description', 'number', 10); + expect(option.getNumber()).toEqual(10); + + option = new Option('fake opt', 'fake description', 'number', 0); + expect(option.getNumber()).toEqual(0); + + option = new Option('fake opt', 'fake description', 'number', -5); + expect(option.getNumber()).toEqual(-5); + }); + + it('should return null if the default value is not set', () => { + option = new Option('fake opt', 'fake description', 'number'); + expect(option.getNumber()).toBeNull(); + }); + }); + + describe('for this.value set', () => { + beforeEach(() => { + option = new Option('fake opt', 'fake description', 'number', -10); + }); + + it('should return the this.value when this.value is a number', () => { + option.value = 20; + expect(option.getNumber()).toEqual(20); + }); + + it('should return a number of this.value when it is a string of a number', () => { + option.value = '10'; + expect(option.getNumber()).toEqual(10); + option.value = '0'; + expect(option.getNumber()).toEqual(0); + option.value = '-5'; + expect(option.getNumber()).toEqual(-5); + }); + + it('should return null if this.value is not a string or a number', () => { + option.value = true; + expect(option.getNumber()).toBeNull(); + option.value = false; + expect(option.getNumber()).toBeNull(); + }); + + it('should return NaN if this.value is a string but is not a number', () => { + option.value = 'foobar'; + expect(option.getNumber()).toEqual(NaN); + }); + }); + }); + + describe('get boolean', () => { + describe('for this.value not set', () => { + it('should return the default value', () => { + option = new Option('fake opt', 'fake description', 'boolean', true); + expect(option.getBoolean()).toBeTruthy(); + option = new Option('fake opt', 'fake description', 'boolean', false); + expect(option.getBoolean()).not.toBeTruthy(); + }); + + it('should return false if the default value is not defined', () => { + option = new Option('fake opt', 'fake description', 'boolean'); + expect(option.getBoolean()).not.toBeTruthy(); + }); + }); + + describe('for this.value set', () => { + beforeEach(() => { + option = new Option('fake opt', 'fake description', 'boolean'); + }); + + it('should return a boolean when this.value is a string', () => { + option.value = 'true'; + expect(option.getBoolean()).toBeTruthy(); + option.value = 'false'; + expect(option.getBoolean()).not.toBeTruthy(); + }); + + it('should return a boolean of this.value when this.value is a number', () => { + option.value = 1; + expect(option.getNumber()).toBeTruthy(); + option.value = 0; + expect(option.getNumber()).not.toBeTruthy(); + }); + + it('should return the boolean of this.value when this.value is a boolean', () => { + option.value = true; + expect(option.getNumber()).toBeNull(); + option.value = false; + expect(option.getNumber()).toBeNull(); + }); + }); + }); + + describe('get string', () => { + describe('for this.value not set', () => { + it('should return the default value', () => { + option = new Option('fake opt', 'fake description', 'string', 'foobar'); + expect(option.getString()).toBe('foobar'); + option = new Option('fake opt', 'fake description', 'string', ''); + expect(option.getString()).toBe(''); + }); + + it('should return an empty string if the default value is not defined', () => { + option = new Option('fake opt', 'fake description', 'string'); + expect(option.getString()).toBe(''); + }); + }); + + describe('for this.value set', () => { + beforeEach(() => { + option = new Option('fake opt', 'fake description', 'string', 'foo'); + }); + + it('should return this.value when this.value is a string', () => { + option.value = 'bar'; + expect(option.getString()).toEqual('bar'); + option.value = ''; + expect(option.getString()).toEqual(''); + }); + + it('should return the string of this.value when this.value is a number', () => { + option.value = 0; + expect(option.getString()).toEqual('0'); + option.value = 1; + expect(option.getString()).toEqual('1'); + }); + + it('should return the string of this.value when this.value is a boolean', () => { + option.value = false; + expect(option.getString()).toEqual('false'); + option.value = true; + expect(option.getString()).toEqual('true'); + }); + }); + }); +}); diff --git a/spec/cli/programs_spec.ts b/spec/cli/programs_spec.ts new file mode 100644 index 00000000..0400fece --- /dev/null +++ b/spec/cli/programs_spec.ts @@ -0,0 +1,82 @@ +import {Option, Options, Program} from '../../lib/cli'; + + +describe('program', () => { + let program: Program; + + beforeEach(() => { + program = new Program() + .command('fooCmd', 'fooDescription') + .addOption(new Option('fooString1', 'fooDescription', 'string', 'foo')) + .addOption(new Option('fooString1', 'fooDescription', 'string', 'foo')) + .addOption(new Option('fooBoolean1', 'fooDescription', 'boolean', false)) + .addOption(new Option('fooBoolean2', 'fooDescription', 'boolean', true)) + .addOption(new Option('fooNumber1', 'fooDescription', 'number', 1)) + .addOption(new Option('fooNumber2', 'fooDescription', 'number', 2)) + .addOption(new Option('fooNumber3', 'fooDescription', 'number', 3)) + }); + + it('should get minimist options', () => { + let json = JSON.parse(JSON.stringify(program.getMinimistOptions())); + expect(json.string.length).toEqual(1); + expect(json.boolean.length).toEqual(2); + expect(json.number.length).toEqual(3); + let length = 0; + for (let item in json.default) { + length++; + } + expect(length).toEqual(6); + expect(json.string[0]).toBe('fooString1'); + expect(json.boolean[0]).toBe('fooBoolean1'); + expect(json.boolean[1]).toBe('fooBoolean2'); + expect(json.number[0]).toBe('fooNumber1'); + expect(json.number[1]).toBe('fooNumber2'); + expect(json.number[2]).toBe('fooNumber3'); + }); + + it('should be able to extract the correct type and value', () => { + let testString: string; + let json = JSON.parse(JSON.stringify({ + '_': ['fooCmd'], + 'fooString1': 'bar', + 'fooBoolean1': true, + 'fooBoolean2': false, + 'fooNumber1': 10, + 'fooNumber2': 20, + 'fooNumber3': 30 + })); + let callbackTest = (options: Options) => { + expect(options['fooString1'].getString()).toEqual('bar'); + expect(options['fooBoolean1'].getBoolean()).toEqual(true); + expect(options['fooBoolean2'].getBoolean()).toEqual(false); + expect(options['fooNumber1'].getNumber()).toEqual(10); + expect(options['fooNumber2'].getNumber()).toEqual(20); + expect(options['fooNumber3'].getNumber()).toEqual(30); + } + program.action(callbackTest); + program.run(json); + }); + + it('should be able to extract the mixed type and get the right type', () => { + let testString: string; + let json = JSON.parse(JSON.stringify({ + '_': ['fooCmd'], + 'fooString1': 1, + 'fooBoolean1': 'true', + 'fooBoolean2': 0, + 'fooNumber1': '100', + 'fooNumber2': 'foo', + 'fooNumber3': true + })); + let callbackTest = (options: Options) => { + expect(options['fooString1'].getString()).toEqual('1'); + expect(options['fooBoolean1'].getBoolean()).toEqual(true); + expect(options['fooBoolean2'].getBoolean()).toEqual(false); + expect(options['fooNumber1'].getNumber()).toEqual(100); + expect(options['fooNumber2'].getNumber()).toEqual(NaN); + expect(options['fooNumber3'].getNumber()).toEqual(null); + } + program.action(callbackTest); + program.run(json); + }); +}); diff --git a/spec/webdriver_spec.ts b/spec/webdriver_spec.ts index 1d13fffd..7ca48aa0 100644 --- a/spec/webdriver_spec.ts +++ b/spec/webdriver_spec.ts @@ -14,7 +14,7 @@ describe('cli', () => { let lines = runSpawn('node', ['built/lib/webdriver.js', 'help']); // very specific to make sure the - let index = 0 + let index = 0; expect(lines[index++].indexOf('Usage:')).toBe(0); index++; expect(lines[index++].indexOf('Commands:')).toBe(0);