From 9795e907aada383f7a21664501de681a3db1d49d Mon Sep 17 00:00:00 2001 From: wimrijnders Date: Thu, 20 Jul 2017 22:37:31 +0200 Subject: [PATCH] Refactoring and unit testing of Validator module (#3106) * Added unit test for Validator, minimum viable version. * Added test-console to package list * Completed minimum viable unit test for Validator * Added Validator unit test for explicit 'undefined' --- lib/shared/Validator.js | 94 ++++++++------ test/Validator.test.js | 265 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 321 insertions(+), 38 deletions(-) create mode 100644 test/Validator.test.js diff --git a/lib/shared/Validator.js b/lib/shared/Validator.js index 70412c24..8ede9bc6 100644 --- a/lib/shared/Validator.js +++ b/lib/shared/Validator.js @@ -53,53 +53,62 @@ class Validator { static check(option, options, referenceOptions, path) { if (referenceOptions[option] === undefined && referenceOptions.__any__ === undefined) { Validator.getSuggestion(option, referenceOptions, path); + return; } - else if (referenceOptions[option] === undefined && referenceOptions.__any__ !== undefined) { + + let referenceOption = option; + let is_object = true; + + if (referenceOptions[option] === undefined && referenceOptions.__any__ !== undefined) { + // NOTE: This only triggers if the __any__ is in the top level of the options object. + // THAT'S A REALLY BAD PLACE TO ALLOW IT!!!! + // TODO: Examine if needed, remove if possible + // __any__ is a wildcard. Any value is accepted and will be further analysed by reference. - if (Validator.getType(options[option]) === 'object' && referenceOptions['__any__'].__type__ !== undefined) { - // if the any subgroup is not a predefined object int he configurator we do not look deeper into the object. - Validator.checkFields(option, options, referenceOptions, '__any__', referenceOptions['__any__'].__type__, path); - } - else { - Validator.checkFields(option, options, referenceOptions, '__any__', referenceOptions['__any__'], path); - } + referenceOption = '__any__'; + + // if the any-subgroup is not a predefined object in the configurator, + // we do not look deeper into the object. + is_object = (Validator.getType(options[option]) === 'object'); } else { - // Since all options in the reference are objects, we can check whether they are supposed to be object to look for the __type__ field. - if (referenceOptions[option].__type__ !== undefined) { - // if this should be an object, we check if the correct type has been supplied to account for shorthand options. - Validator.checkFields(option, options, referenceOptions, option, referenceOptions[option].__type__, path); - } - else { - Validator.checkFields(option, options, referenceOptions, option, referenceOptions[option], path); - } + // Since all options in the reference are objects, we can check whether + // they are supposed to be the object to look for the __type__ field. + // if this is an object, we check if the correct type has been supplied to account for shorthand options. + } + + let refOptionObj = referenceOptions[referenceOption]; + if (is_object && refOptionObj.__type__ !== undefined) { + refOptionObj = refOptionObj.__type__; } + + Validator.checkFields(option, options, referenceOptions, referenceOption, refOptionObj, path); } /** * - * @param {String} option | the option property - * @param {Object} options | The supplied options object - * @param {Object} referenceOptions | The reference options containing all options and their allowed formats - * @param {String} referenceOption | Usually this is the same as option, except when handling an __any__ tag. - * @param {String} refOptionType | This is the type object from the reference options - * @param {Array} path | where in the object is the option + * @param {String} option | the option property + * @param {Object} options | The supplied options object + * @param {Object} referenceOptions | The reference options containing all options and their allowed formats + * @param {String} referenceOption | Usually this is the same as option, except when handling an __any__ tag. + * @param {String} refOptionType | This is the type object from the reference options + * @param {Array} path | where in the object is the option */ static checkFields(option, options, referenceOptions, referenceOption, refOptionObj, path) { + let log = function(message) { + console.log('%c' + message + Validator.printLocation(path, option), printStyle); + }; + let optionType = Validator.getType(options[option]); let refOptionType = refOptionObj[optionType]; + if (refOptionType !== undefined) { // if the type is correct, we check if it is supposed to be one of a few select values - if (Validator.getType(refOptionType) === 'array') { - if (refOptionType.indexOf(options[option]) === -1) { - console.log('%cInvalid option detected in "' + option + '".' + - ' Allowed values are:' + Validator.print(refOptionType) + ' not "' + options[option] + '". ' + Validator.printLocation(path, option), printStyle); - errorFound = true; - } - else if (optionType === 'object' && referenceOption !== "__any__") { - path = util.copyAndExtendArray(path, option); - Validator.parse(options[option], referenceOptions[referenceOption], path); - } + if (Validator.getType(refOptionType) === 'array' && refOptionType.indexOf(options[option]) === -1) { + log('Invalid option detected in "' + option + '".' + + ' Allowed values are:' + Validator.print(refOptionType) + + ' not "' + options[option] + '". '); + errorFound = true; } else if (optionType === 'object' && referenceOption !== "__any__") { path = util.copyAndExtendArray(path, option); @@ -108,7 +117,9 @@ class Validator { } else if (refOptionObj['any'] === undefined) { // type of the field is incorrect and the field cannot be any - console.log('%cInvalid type received for "' + option + '". Expected: ' + Validator.print(Object.keys(refOptionObj)) + '. Received [' + optionType + '] "' + options[option] + '"' + Validator.printLocation(path, option), printStyle); + log('Invalid type received for "' + option + + '". Expected: ' + Validator.print(Object.keys(refOptionObj)) + + '. Received [' + optionType + '] "' + options[option] + '"'); errorFound = true; } } @@ -166,19 +177,26 @@ class Validator { let localSearchThreshold = 8; let globalSearchThreshold = 4; + let msg; if (localSearch.indexMatch !== undefined) { - console.log('%cUnknown option detected: "' + option + '" in ' + Validator.printLocation(localSearch.path, option,'') + 'Perhaps it was incomplete? Did you mean: "' + localSearch.indexMatch + '"?\n\n', printStyle); + msg = ' in ' + Validator.printLocation(localSearch.path, option,'') + + 'Perhaps it was incomplete? Did you mean: "' + localSearch.indexMatch + '"?\n\n'; } else if (globalSearch.distance <= globalSearchThreshold && localSearch.distance > globalSearch.distance) { - console.log('%cUnknown option detected: "' + option + '" in ' + Validator.printLocation(localSearch.path, option,'') + 'Perhaps it was misplaced? Matching option found at: ' + Validator.printLocation(globalSearch.path, globalSearch.closestMatch,''), printStyle); + msg = ' in ' + Validator.printLocation(localSearch.path, option,'') + + 'Perhaps it was misplaced? Matching option found at: ' + + Validator.printLocation(globalSearch.path, globalSearch.closestMatch,''); } else if (localSearch.distance <= localSearchThreshold) { - console.log('%cUnknown option detected: "' + option + '". Did you mean "' + localSearch.closestMatch + '"?' + Validator.printLocation(localSearch.path, option), printStyle); + msg = '. Did you mean "' + localSearch.closestMatch + '"?' + + Validator.printLocation(localSearch.path, option); } else { - console.log('%cUnknown option detected: "' + option + '". Did you mean one of these: ' + Validator.print(Object.keys(options)) + Validator.printLocation(path, option), printStyle); + msg = '. Did you mean one of these: ' + Validator.print(Object.keys(options)) + + Validator.printLocation(path, option); } + console.log('%cUnknown option detected: "' + option + '"' + msg, printStyle); errorFound = true; } @@ -298,4 +316,4 @@ class Validator { export default Validator; -export {printStyle} \ No newline at end of file +export {printStyle} diff --git a/test/Validator.test.js b/test/Validator.test.js new file mode 100644 index 00000000..692d9df4 --- /dev/null +++ b/test/Validator.test.js @@ -0,0 +1,265 @@ +/** + * The goal here is to have a minimum-viable test case here, in order to + * check changes in Validator. + * + * Changes in Validator should ideally be checked to see if they trigger here. + * + * test-console reference: https://github.com/jamesshore/test-console + */ +var assert = require('assert'); +var stdout = require('test-console').stdout; +var Validator = require("../lib/shared/Validator").default; + +// Copied from lib/network/options.js +let string = 'string'; +let bool = 'boolean'; +let number = 'number'; +let array = 'array'; +let object = 'object'; // should only be in a __type__ property +let dom = 'dom'; +let any = 'any'; + + +let allOptions = { + // simple options + enabled: { boolean: bool }, + inherit: { string: ['from', 'to', 'both'] }, + size: { number }, + filter: { 'function': 'function' }, + chosen: { + label: { boolean: bool }, + edge: { 'function': 'function' }, + __type__: { object } + }, + chosen2: { + label: { string }, + __type__: { object } + }, + + + // Tests with any. These have been tailored to test all paths in: + // - Validator.check() + // - Validator.checkFields() + __any__: { string }, // Any option name allowed here, but it must be a string + // NOTE: you can have as many new options as you want! IS THIS INTENTIONAL? + groups: { + generic: { any }, + __any__: { any }, + __type__: { object } + }, + + + // TODO: add combined options, e.g. + //inherit: { string: ['from', 'to', 'both'], boolean: bool }, + //filter: { boolean: bool, string, array, 'function': 'function' }, + __type__: { object } +}; + + +describe('Validator', function() { + + function run_validator(options, check_correct, definition = undefined) { + let errorFound; + let output; + + if (definition === undefined) { + definition = allOptions; + } + + output = stdout.inspectSync(function() { + errorFound = Validator.validate(options, definition); + }); + + if (check_correct) { + assert(!errorFound); + assert(output.length === 0, 'No error expected'); + } else { + //console.log(output); //sometimes useful here + assert(errorFound, 'Validation should have failed'); + assert(output.length !== 0, 'must return errors'); + } + + return output; + } + + + function testExpected(output, expectedErrors) { + for (let i = 0; i < expectedErrors.length; ++i) { + assert(expectedErrors[i].test(output[i]), 'Regular expression at index ' + i + ' failed'); + } + assert(output.length === expectedErrors.length, 'Number of expected errors does not match returned errors'); + } + + + it('handles regular options correctly', function(done) { + // Empty options should be accepted as well + run_validator({}, true); + + // test values for all options + var options = { + enabled: true, + inherit: 'from', + size : 123, + filter : function() { return true; }, + chosen : { + label: false, + edge :function() { return true; }, + }, + chosen2: { + label: "I am a string" + }, + + myNameDoesntMatter: "My type does", + groups : { + generic: "any type is good here", + dontCareAboutName: [0,1,2,3] // Type can also be anything + } + }; + + run_validator(options, true); + + done(); + }); + + + it('rejects incorrect options', function(done) { + // All of the options are wrong, all should generate an error + var options = { + iDontExist: 42, // name is 'any' but type must be string + enabled : 'boolean', + inherit : 'abc', + size : 'not a number', + filter : 42, + chosen : 'not an object', + chosen2 : { + label : 123, + + // Following test the working of Validator.getSuggestion() + iDontExist: 'asdf', + generic : "I'm not defined here", + labe : 42, // Incomplete name + labell : 123, + }, + + }; + + var output = run_validator(options, false); + // Sometimes useful: console.log(output); + + // Errors are in the order as the options are defined in the object + let expectedErrors = [ + /Invalid type received for "iDontExist"\. Expected: string\. Received \[number\]/, + /Invalid type received for "enabled"\. Expected: boolean\. Received \[string\]/, + /Invalid option detected in "inherit"\. Allowed values are:from, to, both not/, + /Invalid type received for "size"\. Expected: number\. Received \[string\]/, + /Invalid type received for "filter"\. Expected: function\. Received \[number\]/, + /Invalid type received for "chosen"\. Expected: object\. Received \[string\]/, + /Invalid type received for "label". Expected: string. Received \[number\]/, + + // Expected results of Validator.getSuggestion() + /Unknown option detected: "iDontExist"\. Did you mean one of these:/, + /Unknown option detected: "generic"[\s\S]*Perhaps it was misplaced\? Matching option found at:/gm, + /Unknown option detected: "labe"[\s\S]*Perhaps it was incomplete\? Did you mean:/gm, + /Unknown option detected: "labell"\. Did you mean "label"\?/ + ]; + testExpected(output, expectedErrors); + + done(); + }); + + + /** + * Explicit tests on explicit 'undefined', to be really sure it works as expected. + */ + it('properly handles explicit `undefined`', function(done) { + // Option definitions with 'undefined' + let undefinedOptions = { + width : { number, 'undefined': 'undefined' }, + undefOnly : { 'undefined': 'undefined' }, + colorOptions : { + fill : { string }, + stroke : { string, 'undefined': 'undefined' }, + strokeWidth: { number }, + __type__ : { string, object, 'undefined': 'undefined' } + }, + moreOptions : { + hello : { string }, + world : { string, 'undefined': 'undefined' }, + __type__ : { object } + } + } + + // + // Test good actual option values + // + let correct1 = { + width : 42, + colorOptions: 'I am a string', + moreOptions : { + hello: 'world', + world: '!' + } + } + var output = run_validator(correct1, true, undefinedOptions); + + let correct2 = { + width : undefined, + colorOptions: { + fill : 'I am a string', + stroke: 'I am a string' + }, + moreOptions : { + world: undefined + } + } + var output = run_validator(correct2, true, undefinedOptions); + + let correct3 = { + width : undefined, + undefOnly : undefined, + colorOptions: undefined + } + var output = run_validator(correct3, true, undefinedOptions); + + // + // Test bad actual option values + // + let bad1 = { + width : 'string', + undefOnly : 42, + colorOptions: 42, + moreOptions : undefined + } + var output = run_validator(bad1, false, undefinedOptions); + + let expectedErrors = [ + /Invalid type received for "width"\. Expected: number, undefined\. Received \[string\]/, + /Invalid type received for "undefOnly"\. Expected: undefined\. Received \[number\]/, + /Invalid type received for "colorOptions"\. Expected: string, object, undefined\. Received \[number\]/, + /Invalid type received for "moreOptions"\. Expected: object\. Received \[undefined\]/ + ]; + testExpected(output, expectedErrors); + + let bad2 = { + undefOnly : 'undefined', + colorOptions: { + fill: undefined + } , + moreOptions: { + hello: undefined, + world: 42 + } + } + var output = run_validator(bad2, false, undefinedOptions); + + let expectedErrors2= [ + /Invalid type received for "undefOnly"\. Expected: undefined\. Received \[string\]/, + /Invalid type received for "fill"\. Expected: string\. Received \[undefined\]/, + /Invalid type received for "hello"\. Expected: string\. Received \[undefined\]/, + /Invalid type received for "world"\. Expected: string, undefined\. Received \[number\]/ + ]; + testExpected(output, expectedErrors2); + + done(); + }); +});