Skip to content

Commit

Permalink
feat: ability to test rule errors and invalid schemas per RFC-103
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed May 22, 2023
1 parent 7a2a0be commit acc77ba
Show file tree
Hide file tree
Showing 9 changed files with 846 additions and 20 deletions.
31 changes: 28 additions & 3 deletions docs/src/integrate/nodejs-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,30 @@ ruleTester.run("my-rule", rule, {
code: "var invalidVariable = true",
errors: [{ message: /^Unexpected.+variable/ }]
}
]
],

// Optional array for testing invalid rule options or custom exceptions thrown by a rule.
fatal: [
// Test case for invalid rule options. Useful for complex schemas.
{
// `code` can be omitted as it's irrelevant when testing the schema.
options: [{ foo: true }],
error: {
// Only one property in this error object is required.
name: 'SchemaValidationError', // Error class name.
message: 'Value {"foo":true} should NOT have additional properties.', // Error message. Can be provided as string or RegExp.
},
},

// Test case for a custom exception thrown by the rule.
{
code: 'for(const x of [1, 2, 3]) {}',
error: {
name: 'NotYetImplementedError',
message: 'Not implemented',
},
},
],
});
```

Expand All @@ -810,9 +833,9 @@ The `RuleTester#run()` method is used to run the tests. It should be passed the

* The name of the rule (string)
* The rule object itself (see ["working with rules"](../extend/custom-rules))
* An object containing `valid` and `invalid` properties, each of which is an array containing test cases.
* An object containing the following test case array properties: `valid`, `invalid`, `fatal` (optional)

A test case is an object with the following properties:
Valid and invalid test cases are objects with the following properties:

* `name` (string, optional): The name to use for the test case, to make it easier to find
* `code` (string, required): The source code that the rule should be run on
Expand All @@ -836,6 +859,8 @@ In addition to the properties above, invalid test cases can also have the follow
If a string is provided as an error instead of an object, the string is used to assert the `message` of the error.
* `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.

Fatal test cases (which are optional) are the same as invalid test cases, except that `code` is optional (it may be irrelevant when testing rule options), and there's an `error` object instead of an `errors` array. The `error` object should include one or both of the `message` of the error and the error (exception) class `name`. Fatal test cases can be used to test custom errors thrown by the rule, or invalid rule options (in which case the error `name` will be `SchemaValidationError`, and the `message` will be come from JSON Schema -- note that strings from JSON Schema are subject to change with future upgrades).

Any additional properties of a test case will be passed directly to the linter as config options. For example, a test case can have a `parserOptions` property to configure parser behavior:

```js
Expand Down
14 changes: 10 additions & 4 deletions lib/config/rule-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const {
getRuleOptionsSchema
} = require("./flat-config-helpers");
const ruleReplacements = require("../../conf/replacements.json");
const SchemaValidationError = require("../shared/schema-validation-error");

//-----------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -143,11 +144,16 @@ class RuleValidator {
validateRule(ruleOptions.slice(1));

if (validateRule.errors) {
throw new Error(`Key "rules": Key "${ruleId}": ${
validateRule.errors.map(
error => `\tValue ${JSON.stringify(error.data)} ${error.message}.\n`
).join("")
const message = validateRule.errors.map(
error => `\tValue ${JSON.stringify(error.data)} ${error.message}.\n`
).join("");
const error = new SchemaValidationError(`Key "rules": Key "${ruleId}": ${
message
}`);

error.messageForTest = message.trim(); // Original exception message without extra helper text for asserting in fatal test cases.

throw error;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/linter/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,7 @@ class Linter {
providedOptions.physicalFilename
);
} catch (err) {
err.messageForTest = err.message; // Original exception message without extra helper text for asserting in fatal test cases.
err.message += `\nOccurred while linting ${options.filename}`;
debug("An error occurred while traversing");
debug("Filename:", options.filename);
Expand Down Expand Up @@ -1640,6 +1641,7 @@ class Linter {
providedOptions.physicalFilename
);
} catch (err) {
err.messageForTest = err.message; // Original exception message without extra helper text for asserting in fatal test cases.
err.message += `\nOccurred while linting ${options.filename}`;
debug("An error occurred while traversing");
debug("Filename:", options.filename);
Expand Down
126 changes: 123 additions & 3 deletions lib/rule-tester/flat-rule-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ const { ConfigArraySymbol } = require("@humanwhocodes/config-array");
* @property {boolean} [only] Run only this test case or the subset of test cases with this property.
*/

/**
* A test case that is expected to trigger a fatal error / exception.
* @typedef {Object} FatalTestCase
* @property {string} [name] Name for the test case.
* @property {string} [code] Code for the test case.
* @property {TestCaseFatalError} error Expected error/exception.
* @property {any[]} [options] Options for the test case.
* @property {{ [name: string]: any }} [settings] Settings for the test case.
* @property {string} [filename] The fake filename for the test case. Useful for rules that make assertion about filenames.
* @property {LanguageOptions} [languageOptions] The language options to use in the test case.
* @property {boolean} [only] Run only this test case or the subset of test cases with this property.
*/

/**
* A description of a reported error used in a rule tester test.
* @typedef {Object} TestCaseError
Expand All @@ -72,6 +85,13 @@ const { ConfigArraySymbol } = require("@humanwhocodes/config-array");
* @property {number} [endLine] The 1-based line number of the reported end location.
* @property {number} [endColumn] The 1-based column number of the reported end location.
*/

/**
* A description of an error/exception from a fatal rule tester test case.
* @typedef {Object} TestCaseFatalError
* @property {string | RegExp} [message] Error message.
* @property {string} [name] Error class name.
*/
/* eslint-enable jsdoc/valid-types -- https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/4#issuecomment-778805577 */

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -100,6 +120,7 @@ const RuleTesterParameters = [
"filename",
"options",
"errors",
"error",
"output",
"only"
];
Expand All @@ -120,6 +141,15 @@ const errorObjectParameters = new Set([
]);
const friendlyErrorObjectParameterList = `[${[...errorObjectParameters].map(key => `'${key}'`).join(", ")}]`;

/*
* All allowed property names in fatal error objects.
*/
const fatalErrorObjectParameters = new Set([
"message",
"name"
]);
const friendlyFatalErrorObjectParameterList = `[${[...fatalErrorObjectParameters].map(key => `'${key}'`).join(", ")}]`;

/*
* All allowed property names in suggestion objects.
*/
Expand Down Expand Up @@ -405,8 +435,8 @@ class FlatRuleTester {

/**
* Adds the `only` property to a test to run it in isolation.
* @param {string | ValidTestCase | InvalidTestCase} item A single test to run by itself.
* @returns {ValidTestCase | InvalidTestCase} The test with `only` set.
* @param {string | ValidTestCase | InvalidTestCase | FatalTestCase} item A single test to run by itself.
* @returns {ValidTestCase | InvalidTestCase | FatalTestCase} The test with `only` set.
*/
static only(item) {
if (typeof item === "string") {
Expand Down Expand Up @@ -450,7 +480,8 @@ class FlatRuleTester {
* @param {Function} rule The rule to test.
* @param {{
* valid: (ValidTestCase | string)[],
* invalid: InvalidTestCase[]
* invalid: InvalidTestCase[],
* fatal: FatalTestCase[]
* }} test The collection of tests to run.
* @throws {TypeError|Error} If non-object `test`, or if a required
* scenario of the given type is missing.
Expand Down Expand Up @@ -671,13 +702,42 @@ class FlatRuleTester {
configs.normalizeSync();
configs.getConfig("test.js");
} catch (error) {
if (item.error && error.messageForTest) {

// If this was a testable exception, return it so it can be compared against in the test case.
return {
messages: [{
ruleId: ruleName,
fatal: true,
message: error.messageForTest,
name: error.name === "Error" ? error.constructor.name : error.name
}]
};
}

error.message = `ESLint configuration in rule-tester is invalid: ${error.message}`;
throw error;
}

try {
SourceCode.prototype.getComments = getCommentsDeprecation;
messages = linter.verify(code, configs, filename);
} catch (err) {
if (item.error && err.messageForTest) {

// If this was a testable exception and we're executing a fatal test case, return it so the error can be compared against in the test case.
return {
messages: [{
ruleId: ruleName,
fatal: true,
message: err.messageForTest,
name: err.name === "Error" ? err.constructor.name : err.name
}]
};
}

// Not testing an exception.
throw err;
} finally {
SourceCode.prototype.getComments = getComments;
}
Expand Down Expand Up @@ -1009,6 +1069,55 @@ class FlatRuleTester {
assertASTDidntChange(result.beforeAST, result.afterAST);
}

/**
* Check if the template is fatal or not.
* All fatal cases go through this.
* @param {string|Object} item Item to run the rule against
* @returns {void}
* @private
*/
function testFatalTemplate(item) {
if (item.code) {
assert.ok(typeof item.code === "string", "Optional test case property 'code' must be a string");
}
if (item.name) {
assert.ok(typeof item.name === "string", "Optional test case property 'name' must be a string");
}
assert.ok(typeof item.error === "object",
"Test case property 'error' must be an object");

assert.ok(typeof item.error.name === "string" || typeof item.error.message === "string" || item.error.message instanceof RegExp,
"Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property");

assert.ok(typeof item.code === "string" || typeof item.options !== "undefined", "Test case must contain at least one of `code` and `options`");

const result = runRuleForItem(item);

assert.ok(result.messages.length === 1 && result.messages[0].fatal === true, "A fatal test case must throw an exception");

const errorActual = result.messages[0];
const errorExpected = item.error;

Object.keys(errorExpected).forEach(propertyName => {
assert.ok(
fatalErrorObjectParameters.has(propertyName),
`Invalid error property name '${propertyName}'. Expected one of ${friendlyFatalErrorObjectParameterList}.`
);
});

if (hasOwnProperty(errorExpected, "name")) {
assert.strictEqual(
errorActual.name,
errorExpected.name,
`Fatal error name should be "${errorExpected.name}" but got "${errorActual.name}" instead.`
);
}

if (hasOwnProperty(errorExpected, "message")) {
assertMessageMatches(errorActual.message, errorExpected.message);
}
}

/*
* This creates a mocha test suite and pipes all supplied info through
* one of the templates above.
Expand All @@ -1035,6 +1144,17 @@ class FlatRuleTester {
);
});
});

this.constructor.describe("fatal", () => {
(test.fatal || []).forEach((fatal, index) => {
this.constructor[fatal.only ? "itOnly" : "it"](
sanitize(fatal.name || fatal.code || `(Test Case #${index + 1})`),
() => {
testFatalTemplate(fatal);
}
);
});
});
});
}
}
Expand Down

0 comments on commit acc77ba

Please sign in to comment.