From 21edbfcabe8afaf4ba0dfb74db1b02fae3f0ea2d Mon Sep 17 00:00:00 2001 From: Barrett Schonefeld Date: Tue, 4 Feb 2020 15:46:36 -0600 Subject: [PATCH] feat: added configurable warning limits through .thresholdrc file - added code to process a .thresholdrc file to get warnings limit - used warning limit to set exit code to 1 and print a message if warning limit is exceeded - added validation on the user input from .thresholdrc to check if valid keys and values provided in .thresholdrc - set default limit to Number.MAX_VALUE if no .thresholdrc provided - added tests to test error handling for .thresholdrc file and expected output when warning limits exceeded - added test to check if error thrown when invalid JSON provided in .thresholdrc - refactored error printing in processConfiguration file into a printConfigErrors function - ran npm run fix to fix style - documented how to add a warnings limit in the README --- README.md | 20 ++- src/cli-validator/runValidator.js | 32 ++++- .../utils/processConfiguration.js | 117 +++++++++++++++--- .../mockFiles/thresholds/fiveWarnings.json | 3 + .../mockFiles/thresholds/invalidJSON.json | 2 + .../mockFiles/thresholds/invalidValues.json | 5 + .../mockFiles/thresholds/zeroWarnings.json | 3 + .../cli-validator/tests/thresholdValidator.js | 113 +++++++++++++++++ 8 files changed, 276 insertions(+), 19 deletions(-) create mode 100644 test/cli-validator/mockFiles/thresholds/fiveWarnings.json create mode 100644 test/cli-validator/mockFiles/thresholds/invalidJSON.json create mode 100644 test/cli-validator/mockFiles/thresholds/invalidValues.json create mode 100644 test/cli-validator/mockFiles/thresholds/zeroWarnings.json create mode 100644 test/cli-validator/tests/thresholdValidator.js diff --git a/README.md b/README.md index f74719d0d..8750994ba 100644 --- a/README.md +++ b/README.md @@ -146,11 +146,11 @@ The object will always have `errors` and `warnings` keys that map to arrays. If ## Configuration The command line validator is built so that each IBM validation can be configured. To get started configuring the validator, [set up](#setup) a [configuration file](#configuration-file) and continue reading this section. Specific validation "rules" can be turned off, or configured to trigger either errors or warnings in the validator. Some validations can be configured even further, such as switching the case convention to validate against for parameter names. -Additionally, certain files files can be ignored by the validator. Any glob placed in a file called `.validateignore` will always be ignored by the validator at runtime. This is set up like a `.gitignore` or a `.eslintignore` file. +Additionally, certain files can be ignored by the validator. Any glob placed in a file called `.validateignore` will always be ignored by the validator at runtime. This is set up like a `.gitignore` or a `.eslintignore` file. ### Setup To set up the configuration capability, simply run the command `lint-openapi init`. -This will create (or over-write) a `.validaterc` file with all rules set to their [default value](#default-values). This command does not create a `.validateignore`. That file must be created manually. These rules can then be changed to configure the validator. Continue reading for more details. +This will create (or overwrite) a `.validaterc` file with all rules set to their [default value](#default-values). This command does not create a `.validateignore`. That file must be created manually. These rules can then be changed to configure the validator. Continue reading for more details. _WARNING: If a `.validaterc` file already exists and has been customized, this command will reset all rules to their default values._ @@ -434,6 +434,22 @@ The default values for each rule are described below. | duplicate_sibling_description | warning | | incorrect_ref_pattern | warning | +## Warnings Limit + +You may impose a warning limit on your API definitions. If the number of warnings issued exceeds the warning limit, the **exit code will be set to 1**. If the Validator is part of your CI build, this will cause the build to fail. + +To impose a warnings limit on a project, add a `.thresholdrc` to your project. It is recommended to add this file to the root of the project. The validator recursively searches up the filesystem from whichever directory the validator is invoked, and the nearest `.thresholdrc` will be used. + +The format for the `.thresholdrc` file is a top-level JSON object with a `"warnings"` field (shown below). + + { + "warnings": 0 + } + +###### limits +| Limit | Default | +| ----------------------- | --------- | +| warnings | MAX_VALUE | ## Turning off `update-notifier` This package uses [`update-notifier`](https://github.com/yeoman/update-notifier) to alert users when new versions of the tool are available. To turn this feature _off_, follow [these instructions](https://github.com/yeoman/update-notifier/tree/8df01b35fbb8093e91d79fdf9900c344c2236f08#user-settings) from the package authors. It is recommended to keep this feature _on_ to help stay up to date with the latest changes. diff --git a/src/cli-validator/runValidator.js b/src/cli-validator/runValidator.js index d96e3f8c3..1cb5bbc56 100644 --- a/src/cli-validator/runValidator.js +++ b/src/cli-validator/runValidator.js @@ -41,6 +41,8 @@ const processInput = async function(program) { const configFileOverride = program.config; + const limitsFileOverride = program.limits; + // turn on coloring by default const colors = turnOffColoring ? false : true; @@ -140,6 +142,14 @@ const processInput = async function(program) { return Promise.reject(err); } + // get limits from .thresholdrc file + let limitsObject; + try { + limitsObject = await config.limits(chalk, limitsFileOverride); + } catch (err) { + return Promise.reject(err); + } + // define an exit code to return. this will tell the parent program whether // the validator passed or not let exitCode = 0; @@ -250,8 +260,26 @@ const processInput = async function(program) { originalFile, errorsOnly ); - // fail on errors, but not if there are only warnings - if (results.error) exitCode = 1; + // fail on errors or if number of warnings exceeds warnings limit + if (results.error) { + exitCode = 1; + } else { + // Calculate number of warnings and set exit code to 1 if warning limit exceeded + let numWarnings = 0; + for (const key of Object.keys(results.warnings)) { + numWarnings += results.warnings[key].length; + } + if (numWarnings > limitsObject.warnings) { + exitCode = 1; + console.log( + chalk.red( + `Number of warnings (${numWarnings}) exceeds warnings limit (${ + limitsObject.warnings + }).` + ) + ); + } + } } else { console.log(chalk.green(`\n${validFile} passed the validator`)); if (validFile === last(filesToValidate)) console.log(); diff --git a/src/cli-validator/utils/processConfiguration.js b/src/cli-validator/utils/processConfiguration.js index d2379c0d1..a2d0d73a7 100644 --- a/src/cli-validator/utils/processConfiguration.js +++ b/src/cli-validator/utils/processConfiguration.js @@ -13,6 +13,26 @@ const defaultObject = defaultConfig.defaults; const deprecatedRuleObject = defaultConfig.deprecated; const configOptions = defaultConfig.options; +const printConfigErrors = function(problems, chalk, fileName) { + const description = `Invalid configuration in ${chalk.underline( + fileName + )} file. See below for details.`; + + const message = []; + + // add all errors for printError + problems.forEach(function(problem) { + message.push( + `\n - ${chalk.red(problem.message)}\n ${chalk.magenta( + problem.correction + )}` + ); + }); + if (message.length) { + printError(chalk, description, message.join('\n')); + } +}; + const validateConfigObject = function(configObject, chalk) { const configErrors = []; let validObject = true; @@ -147,21 +167,8 @@ const validateConfigObject = function(configObject, chalk) { configObject.invalid = false; } else { // if the object is not valid, exit and tell the user why - const description = `Invalid configuration in ${chalk.underline( - '.validaterc' - )} file. See below for details.`; - const message = []; - - // concatenate all the error messages for the printError module - configErrors.forEach(function(problem) { - message.push( - `\n - ${chalk.red(problem.message)}\n ${chalk.magenta( - problem.correction - )}` - ); - }); + printConfigErrors(configErrors, chalk, '.validaterc'); - printError(chalk, description, message.join('\n')); configObject.invalid = true; } @@ -208,7 +215,6 @@ const getConfigObject = async function(defaultMode, chalk, configFileOverride) { configObject = defaultObject; } else { try { - // the config file must be in the root folder of the project const fileAsString = await readFile(configFile, 'utf8'); configObject = JSON.parse(fileAsString); } catch (err) { @@ -263,6 +269,85 @@ const getFilesToIgnore = async function() { return filesToIgnore; }; +const validateLimits = function(limitsObject, chalk) { + const allowedLimits = ['warnings']; + const limitErrors = []; + + Object.keys(limitsObject).forEach(function(key) { + if (!allowedLimits.includes(key)) { + // remove the entry and notify the user + delete limitsObject[key]; + limitErrors.push({ + message: `"${key}" limit not supported. This value will be ignored.`, + correction: `Valid limits for .thresholdrc are: ${allowedLimits.join( + ', ' + )}.` + }); + } else { + // valid limit option, ensure the limit given is a number + if (typeof limitsObject[key] !== 'number') { + // remove the entry and notify the user + delete limitsObject[key]; + limitErrors.push({ + message: `Value provided for ${key} limit is invalid.`, + correction: `${key} limit should be a number.` + }); + } + } + }); + + // give the user corrections for .thresholdrc file + if (limitErrors.length) { + printConfigErrors(limitErrors, chalk, '.thresholdrc'); + } + + // sets all limits options not defined by user to default + for (const limitOption of allowedLimits) { + if (!(limitOption in limitsObject)) { + limitsObject[limitOption] = Number.MAX_VALUE; + } + } + + return limitsObject; +}; + +const getLimits = async function(chalk, limitsFileOverride) { + let limitsObject = {}; + + const findUpOpts = {}; + let limitsFileName; + + if (limitsFileOverride) { + limitsFileName = path.basename(limitsFileOverride); + findUpOpts.cwd = path.dirname(limitsFileOverride); + } else { + limitsFileName = '.thresholdrc'; + } + + // search up the file system for the first instance + // of the threshold file + const limitsFile = await findUp(limitsFileName, findUpOpts); + + if (limitsFile !== null) { + try { + const fileAsString = await readFile(limitsFile, 'utf8'); + limitsObject = JSON.parse(fileAsString); + } catch (err) { + // this most likely means there is a problem in the json syntax itself + const description = + 'There is a problem with the .thresholdrc file. See below for details.'; + printError(chalk, description, err); + return Promise.reject(2); + } + } + + // returns complete limits object with all valid user settings + // and default values for undefined limits + limitsObject = validateLimits(limitsObject, chalk); + + return limitsObject; +}; + const validateConfigOption = function(userOption, defaultOption) { const result = { valid: true }; // determine what type of option it is @@ -286,3 +371,5 @@ module.exports.get = getConfigObject; module.exports.validate = validateConfigObject; module.exports.ignore = getFilesToIgnore; module.exports.validateOption = validateConfigOption; +module.exports.validateLimits = validateLimits; +module.exports.limits = getLimits; diff --git a/test/cli-validator/mockFiles/thresholds/fiveWarnings.json b/test/cli-validator/mockFiles/thresholds/fiveWarnings.json new file mode 100644 index 000000000..fcebb9e7b --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/fiveWarnings.json @@ -0,0 +1,3 @@ +{ + "warnings": 5 +} diff --git a/test/cli-validator/mockFiles/thresholds/invalidJSON.json b/test/cli-validator/mockFiles/thresholds/invalidJSON.json new file mode 100644 index 000000000..ed88a2560 --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/invalidJSON.json @@ -0,0 +1,2 @@ +[ + : } diff --git a/test/cli-validator/mockFiles/thresholds/invalidValues.json b/test/cli-validator/mockFiles/thresholds/invalidValues.json new file mode 100644 index 000000000..5b5049bf5 --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/invalidValues.json @@ -0,0 +1,5 @@ +{ + "errors": 0, + "warnings": "text", + "population": 10 +} diff --git a/test/cli-validator/mockFiles/thresholds/zeroWarnings.json b/test/cli-validator/mockFiles/thresholds/zeroWarnings.json new file mode 100644 index 000000000..1f14b6ccc --- /dev/null +++ b/test/cli-validator/mockFiles/thresholds/zeroWarnings.json @@ -0,0 +1,3 @@ +{ + "warnings": 0 +} diff --git a/test/cli-validator/tests/thresholdValidator.js b/test/cli-validator/tests/thresholdValidator.js new file mode 100644 index 000000000..64363d95c --- /dev/null +++ b/test/cli-validator/tests/thresholdValidator.js @@ -0,0 +1,113 @@ +// the rule names are all snake case and need to stay that way. don't lint them +/* eslint-disable camelcase */ + +const intercept = require('intercept-stdout'); +const expect = require('expect'); +const stripAnsiFrom = require('strip-ansi'); + +const commandLineValidator = require('../../../src/cli-validator/runValidator'); + +describe('test the .thresholdrc limits', function() { + it('should show error and set exit code to 1 when warning limit exceeded', async function() { + const capturedText = []; + + const program = {}; + program.args = ['./test/cli-validator/mockFiles/circularRefs.yml']; + program.limits = + './test/cli-validator/mockFiles/thresholds/fiveWarnings.json'; + program.default_mode = true; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + const exitCode = await commandLineValidator(program); + + unhookIntercept(); + + expect(exitCode).toEqual(1); + + expect(capturedText[capturedText.length - 1].slice(0, 18)).toEqual( + `Number of warnings` + ); + }); + + it('should print errors for unsupported limit options and invalid limit values', async function() { + const capturedText = []; + + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = + './test/cli-validator/mockFiles/thresholds/invalidValues.json'; + program.default_mode = true; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + const exitCode = await commandLineValidator(program); + + unhookIntercept(); + + // limit values invalid, so default limit, Number.MAX_VALUE, used + expect(exitCode).toEqual(0); + + const allOutput = capturedText.join(''); + + expect(allOutput.includes('"population" limit not supported.')).toEqual( + true + ); + + expect(allOutput.includes('Value provided for warnings')).toEqual(true); + }); + + it('should give exit code 0 when warnings limit not exceeded', async function() { + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = + './test/cli-validator/mockFiles/thresholds/zeroWarnings.json'; + program.default_mode = true; + + const capturedText = []; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + const exitCode = await commandLineValidator(program); + + unhookIntercept(); + + expect(exitCode).toEqual(0); + }); + + it('should give an error for invalid JSON', async function() { + const program = {}; + program.args = ['./test/cli-validator/mockFiles/clean.yml']; + program.limits = + './test/cli-validator/mockFiles/thresholds/invalidJSON.json'; + program.default_mode = true; + + const capturedText = []; + + const unhookIntercept = intercept(function(txt) { + capturedText.push(stripAnsiFrom(txt)); + return ''; + }); + + await expect(commandLineValidator(program)).rejects.toBe(2); + + unhookIntercept(); + + const allOutput = capturedText.join(''); + + expect( + allOutput.includes( + '[Error] There is a problem with the .thresholdrc file.' + ) + ); + }); +});