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 a33a598
Show file tree
Hide file tree
Showing 5 changed files with 410 additions and 12 deletions.
31 changes: 28 additions & 3 deletions docs/src/integrate/nodejs-api.md
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
2 changes: 2 additions & 0 deletions lib/linter/linter.js
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
142 changes: 138 additions & 4 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -18,6 +18,9 @@
* { code: "{code}", errors: {numErrors} },
* { code: "{code}", errors: ["{errorMessage}"] },
* { code: "{code}", options: {options}, globals: {globals}, parser: "{parser}", settings: {settings}, errors: [{ message: "{errorMessage}", type: "{errorNodeType}"}] }
* ],
* fatal: [
* { code: "{code}", options: {options}, globals: {globals}, parser: "{parser}", settings: {settings}, error: { message: "{errorMessage}", name: "Error"} }
* ]
* });
*
Expand Down Expand Up @@ -96,6 +99,19 @@ const { SourceCode } = require("../source-code");
* @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 @@ -108,6 +124,13 @@ const { SourceCode } = require("../source-code");
* @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 All @@ -130,6 +153,7 @@ const RuleTesterParameters = [
"code",
"filename",
"options",
"error",
"errors",
"output",
"only"
Expand All @@ -151,6 +175,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 @@ -468,8 +501,8 @@ class RuleTester {

/**
* 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 @@ -522,7 +555,8 @@ class RuleTester {
* @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 @@ -679,7 +713,31 @@ class RuleTester {
}
}

validate(config, "rule-tester", id => (id === ruleName ? rule : null));
if (item.error) {

// Inside a fatal test case, which is expected to throw an error, so surround schema validation with try/catch.
try {
validate(config, "rule-tester", id => (id === ruleName ? rule : null));
} catch (err) {
if (err.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: err.messageForTest,
name: err.name === "Error" ? err.constructor.name : err.name
}]
};
}

// Not one of the relevant exceptions for testing.
throw err;
}
} else {
validate(config, "rule-tester", id => (id === ruleName ? rule : null));
}

// Verify the code.
const { getComments } = SourceCode.prototype;
Expand All @@ -688,6 +746,22 @@ class RuleTester {
try {
SourceCode.prototype.getComments = getCommentsDeprecation;
messages = linter.verify(code, config, 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 @@ -1019,6 +1093,55 @@ class RuleTester {
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 @@ -1045,6 +1168,17 @@ class RuleTester {
);
});
});

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
17 changes: 12 additions & 5 deletions lib/shared/config-validator.js
Expand Up @@ -127,6 +127,14 @@ function validateRuleSchema(rule, localOptions) {
}
}

/** Exception thrown when schema validation fails because a rule is provided invalid options. */
class SchemaValidationError extends Error {
constructor(message) {
super(message);
this.name = "SchemaValidationError";
}
}

/**
* Validates a rule's options against its schema.
* @param {{create: Function}|null} rule The rule that the config is being validated for
Expand All @@ -146,12 +154,11 @@ function validateRuleOptions(rule, ruleId, options, source = null) {
}
} catch (err) {
const enhancedMessage = `Configuration for rule "${ruleId}" is invalid:\n${err.message}`;
const enhancedMessageWithSource = source ? `${source}:\n\t${enhancedMessage}` : enhancedMessage;
const schemaValidationError = new SchemaValidationError(enhancedMessageWithSource);

if (typeof source === "string") {
throw new Error(`${source}:\n\t${enhancedMessage}`);
} else {
throw new Error(enhancedMessage);
}
schemaValidationError.messageForTest = err.message.trim(); // Original exception message without extra helper text for asserting in fatal test cases.
throw schemaValidationError;
}
}

Expand Down

0 comments on commit a33a598

Please sign in to comment.