Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added configurable warning limits through .thresholdrc file #132

Merged
merged 1 commit into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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._

Expand Down Expand Up @@ -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.
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand Down
32 changes: 30 additions & 2 deletions src/cli-validator/runValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
117 changes: 102 additions & 15 deletions src/cli-validator/utils/processConfiguration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
};

barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
const validateConfigObject = function(configObject, chalk) {
const configErrors = [];
let validObject = true;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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;
3 changes: 3 additions & 0 deletions test/cli-validator/mockFiles/thresholds/fiveWarnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"warnings": 5
}
2 changes: 2 additions & 0 deletions test/cli-validator/mockFiles/thresholds/invalidJSON.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[
: }
5 changes: 5 additions & 0 deletions test/cli-validator/mockFiles/thresholds/invalidValues.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"errors": 0,
"warnings": "text",
"population": 10
}
3 changes: 3 additions & 0 deletions test/cli-validator/mockFiles/thresholds/zeroWarnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"warnings": 0
}
113 changes: 113 additions & 0 deletions test/cli-validator/tests/thresholdValidator.js
Original file line number Diff line number Diff line change
@@ -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.'
)
);
});
});