From acc77ba39d15dae94eb55bd81c76802f1e3da803 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Mon, 23 Jan 2023 21:11:45 -0500 Subject: [PATCH 1/9] feat: ability to test rule errors and invalid schemas per RFC-103 --- docs/src/integrate/nodejs-api.md | 31 ++- lib/config/rule-validator.js | 14 +- lib/linter/linter.js | 2 + lib/rule-tester/flat-rule-tester.js | 126 ++++++++++- lib/rule-tester/rule-tester.js | 142 +++++++++++- lib/shared/config-validator.js | 12 +- lib/shared/schema-validation-error.js | 11 + tests/lib/rule-tester/flat-rule-tester.js | 264 ++++++++++++++++++++++ tests/lib/rule-tester/rule-tester.js | 264 ++++++++++++++++++++++ 9 files changed, 846 insertions(+), 20 deletions(-) create mode 100644 lib/shared/schema-validation-error.js diff --git a/docs/src/integrate/nodejs-api.md b/docs/src/integrate/nodejs-api.md index 853abdc57b8..16241560cba 100644 --- a/docs/src/integrate/nodejs-api.md +++ b/docs/src/integrate/nodejs-api.md @@ -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', + }, + }, + ], }); ``` @@ -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 @@ -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 diff --git a/lib/config/rule-validator.js b/lib/config/rule-validator.js index 0b5858fb30f..567c22ce0e4 100644 --- a/lib/config/rule-validator.js +++ b/lib/config/rule-validator.js @@ -16,6 +16,7 @@ const { getRuleOptionsSchema } = require("./flat-config-helpers"); const ruleReplacements = require("../../conf/replacements.json"); +const SchemaValidationError = require("../shared/schema-validation-error"); //----------------------------------------------------------------------------- // Helpers @@ -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; } } } diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 233cbed5b5c..d296f7e1ba7 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -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); @@ -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); diff --git a/lib/rule-tester/flat-rule-tester.js b/lib/rule-tester/flat-rule-tester.js index 97055d104f4..ce46e2f7b1a 100644 --- a/lib/rule-tester/flat-rule-tester.js +++ b/lib/rule-tester/flat-rule-tester.js @@ -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 @@ -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 */ //------------------------------------------------------------------------------ @@ -100,6 +120,7 @@ const RuleTesterParameters = [ "filename", "options", "errors", + "error", "output", "only" ]; @@ -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. */ @@ -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") { @@ -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. @@ -671,6 +702,19 @@ 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; } @@ -678,6 +722,22 @@ class FlatRuleTester { 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; } @@ -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. @@ -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); + } + ); + }); + }); }); } } diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 8518299d0b0..caa8b0daf27 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -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"} } * ] * }); * @@ -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 @@ -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 */ //------------------------------------------------------------------------------ @@ -130,6 +153,7 @@ const RuleTesterParameters = [ "code", "filename", "options", + "error", "errors", "output", "only" @@ -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. */ @@ -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") { @@ -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. @@ -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; @@ -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; } @@ -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. @@ -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); + } + ); + }); + }); }); } } diff --git a/lib/shared/config-validator.js b/lib/shared/config-validator.js index 47353ac4814..0727574abff 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -31,7 +31,8 @@ const environments: BuiltInEnvironments } } = require("@eslint/eslintrc"), - { emitDeprecationWarning } = require("./deprecation-warnings"); + { emitDeprecationWarning } = require("./deprecation-warnings"), + SchemaValidationError = require("./schema-validation-error"); const ajv = require("./ajv")(); const ruleValidators = new WeakMap(); @@ -146,12 +147,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; } } diff --git a/lib/shared/schema-validation-error.js b/lib/shared/schema-validation-error.js new file mode 100644 index 00000000000..bb8b01b6bf4 --- /dev/null +++ b/lib/shared/schema-validation-error.js @@ -0,0 +1,11 @@ +"use strict"; + +/** Exception thrown when schema validation fails because a rule is provided invalid options. */ +class SchemaValidationError extends Error { + constructor(message) { + super(message); + this.name = "SchemaValidationError"; + } +} + +module.exports = SchemaValidationError; diff --git a/tests/lib/rule-tester/flat-rule-tester.js b/tests/lib/rule-tester/flat-rule-tester.js index f65c3bb1913..b06f7e62899 100644 --- a/tests/lib/rule-tester/flat-rule-tester.js +++ b/tests/lib/rule-tester/flat-rule-tester.js @@ -2246,6 +2246,261 @@ describe("FlatRuleTester", () => { }); }); + describe("fatal test cases", () => { + const rule = { + meta: { schema: [] }, + create: context => ({ + Literal(node) { + + /** */ + class CustomError extends Error {} // Custom Error class. + if (node.value === "foo") { + throw new CustomError("Some custom error message"); + } + if (node.value === "bar") { + context.report(node, "no baz"); + } + } + }) + }; + const ruleWithoutCustomErrorClass = { + meta: { schema: [] }, + create: context => ({ + Literal(node) { + + const error = new Error("Some custom error message"); // Standard Error class. + + error.name = "CustomError"; // This name comes from the `name` property, not the constructor's name. + + if (node.value === "foo") { + throw error; + } + if (node.value === "bar") { + context.report(node, "no baz"); + } + } + }) + }; + + it("should handle custom rule error exception", () => { + + ruleTester.run("rule", rule, { + valid: ["'baz'"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + error: { + + // both `message` and `name` + message: "Some custom error message", + name: "CustomError" + } + }, + { + code: "'foo'", + error: { + + // omit `message` + name: "CustomError" + } + }, + { + code: "'foo'", + error: { + + // omit `name` + message: "Some custom error message" + } + }, + { + code: "'foo'", + error: { + + // `message` as RegExp + message: /Some custom error message/u, + name: "CustomError" + } + } + ] + }); + + ruleTester.run("ruleWithoutCustomErrorClass", ruleWithoutCustomErrorClass, { + valid: ["'baz'"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + error: { + + // both `message` and `name` + message: "Some custom error message", + name: "CustomError" + } + } + ] + }); + }); + + it("should handle a schema validation error", () => { + ruleTester.run("no-bar", rule, { + valid: ["foo()"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // both `message` and `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.', + name: "SchemaValidationError" + } + }, + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // omit `message` + name: "SchemaValidationError" + } + }, + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // omit `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.' + } + }, + { + + // omit `code` + options: [{ foo: true }], + error: { + + // omit `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.' + } + } + ] + }); + }); + + it("throws when fatal test case does not match error thrown", () => { + + // error `name` does not match + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "'foo'", error: { name: "FakeErrorName" } }] + }); + }, 'Fatal error name should be "FakeErrorName" but got "CustomError" instead.'); + + // error `message` does not match + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "'foo'", error: { message: "Fake Message" } }] + }); + }); + + /* + * TODO: figure out why this assert message doesn't match: + * "Expected values to be strictly equal:\n+ actual - expected\n\n+ 'Some custom error message'\n- 'Fake Message'"); + */ + }); + + it("throws when fatal test case has no error", () => { + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "foo", error: { name: "Error" } }] + }); + }, "A fatal test case must throw an exception"); + }); + + it("throws with missing properties or wrong property types", () => { + + // empty `fatal` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{}] + }); + }, "Test case property 'error' must be an object"); + + // wrong type `code` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: true }] + }); + }, "Optional test case property 'code' must be a string"); + + // wrong type `name` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ name: true }] + }); + }, "Optional test case property 'name' must be a string"); + + // extra property in `fatal` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: "Error" }, code: "'foo'", foo: true }] + }); + }, 'ESLint configuration in rule-tester is invalid: Unexpected key "foo" found.'); + + // extra property in `error` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: "Error", foo: true }, code: "'foo'" }] + }); + }, "Invalid error property name 'foo'. Expected one of ['message', 'name']."); + + // empty `error` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: {} }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + + // wrong type `error.name` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: true } }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + + // wrong type `error.message` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { message: true } }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + }); + }); + /** * Asserts that a particular value will be emitted from an EventEmitter. * @param {EventEmitter} emitter The emitter that should emit a value @@ -2544,6 +2799,15 @@ describe("FlatRuleTester", () => { sinon.assert.calledWith(spyRuleTesterIt, code); }); + it("should label a fatal test case with no `code` correctly", () => { + ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), { + valid: [], + invalid: [], + fatal: [{ options: [{ foo: true }] }] + }); + sinon.assert.calledWith(spyRuleTesterIt, "(Test Case #1)"); + }); + }); describe("SourceCode#getComments()", () => { diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index a36edafd409..de19b3cea1a 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -2230,6 +2230,261 @@ describe("RuleTester", () => { }); }); + describe("fatal test cases", () => { + const rule = { + meta: { schema: [] }, + create: context => ({ + Literal(node) { + + /** */ + class CustomError extends Error {} // Custom Error class. + if (node.value === "foo") { + throw new CustomError("Some custom error message"); + } + if (node.value === "bar") { + context.report(node, "no baz"); + } + } + }) + }; + const ruleWithoutCustomErrorClass = { + meta: { schema: [] }, + create: context => ({ + Literal(node) { + + const error = new Error("Some custom error message"); // Standard Error class. + + error.name = "CustomError"; // This name comes from the `name` property, not the constructor's name. + + if (node.value === "foo") { + throw error; + } + if (node.value === "bar") { + context.report(node, "no baz"); + } + } + }) + }; + + it("should handle custom rule error exception", () => { + + ruleTester.run("rule", rule, { + valid: ["'baz'"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + error: { + + // both `message` and `name` + message: "Some custom error message", + name: "CustomError" + } + }, + { + code: "'foo'", + error: { + + // omit `message` + name: "CustomError" + } + }, + { + code: "'foo'", + error: { + + // omit `name` + message: "Some custom error message" + } + }, + { + code: "'foo'", + error: { + + // `message` as RegExp + message: /Some custom error message/u, + name: "CustomError" + } + } + ] + }); + + ruleTester.run("ruleWithoutCustomErrorClass", ruleWithoutCustomErrorClass, { + valid: ["'baz'"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + error: { + + // both `message` and `name` + message: "Some custom error message", + name: "CustomError" + } + } + ] + }); + }); + + it("should handle a schema validation error", () => { + ruleTester.run("no-bar", rule, { + valid: ["foo()"], + invalid: [{ code: "'bar'", errors: [{ message: "no baz" }] }], + fatal: [ + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // both `message` and `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.', + name: "SchemaValidationError" + } + }, + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // omit `message` + name: "SchemaValidationError" + } + }, + { + code: "'foo'", + options: [{ foo: true }], + error: { + + // omit `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.' + } + }, + { + + // omit `code` + options: [{ foo: true }], + error: { + + // omit `name` + message: 'Value [{"foo":true}] should NOT have more than 0 items.' + } + } + ] + }); + }); + + it("throws when fatal test case does not match error thrown", () => { + + // error `name` does not match + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "'foo'", error: { name: "FakeErrorName" } }] + }); + }, 'Fatal error name should be "FakeErrorName" but got "CustomError" instead.'); + + // error `message` does not match + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "'foo'", error: { message: "Fake Message" } }] + }); + }); + + /* + * TODO: figure out why this assert message doesn't match: + * "Expected values to be strictly equal:\n+ actual - expected\n\n+ 'Some custom error message'\n- 'Fake Message'"); + */ + }); + + it("throws when fatal test case has no error", () => { + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: "foo", error: { name: "Error" } }] + }); + }, "A fatal test case must throw an exception"); + }); + + it("throws with missing properties or wrong property types", () => { + + // empty `fatal` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{}] + }); + }, "Test case property 'error' must be an object"); + + // wrong type `code` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ code: true }] + }); + }, "Optional test case property 'code' must be a string"); + + // wrong type `name` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ name: true }] + }); + }, "Optional test case property 'name' must be a string"); + + // extra property in `fatal` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: "Error" }, code: "'foo'", foo: true }] + }); + }, 'ESLint configuration in rule-tester is invalid:\n\t- Unexpected top-level property "foo".'); + + // extra property in `error` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: "Error", foo: true }, code: "'foo'" }] + }); + }, "Invalid error property name 'foo'. Expected one of ['message', 'name']."); + + // empty `error` object + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: {} }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + + // wrong type `error.name` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { name: true } }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + + // wrong type `error.message` + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: { message: true } }] + }); + }, "Test case property 'error' object must provide a string 'name' property or string/regexp 'message' property"); + }); + }); + describe("deprecations", () => { let processStub; const ruleWithNoSchema = { @@ -2787,6 +3042,15 @@ describe("RuleTester", () => { sinon.assert.calledWith(spyRuleTesterIt, code); }); + it("should label a fatal test case with no `code` correctly", () => { + ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), { + valid: [], + invalid: [], + fatal: [{ options: [{ foo: true }] }] + }); + sinon.assert.calledWith(spyRuleTesterIt, "(Test Case #1)"); + }); + }); describe("SourceCode#getComments()", () => { From a61487b195365ec44d83c756c943becd770f4eaa Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:46:32 -0400 Subject: [PATCH 2/9] Update docs/src/integrate/nodejs-api.md Co-authored-by: Nicholas C. Zakas --- docs/src/integrate/nodejs-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/integrate/nodejs-api.md b/docs/src/integrate/nodejs-api.md index 16241560cba..94990026b8e 100644 --- a/docs/src/integrate/nodejs-api.md +++ b/docs/src/integrate/nodejs-api.md @@ -833,7 +833,7 @@ 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 the following test case array properties: `valid`, `invalid`, `fatal` (optional) +* An object containing `valid` and `invalid` properties. An optional `fatal` property may also be added. Each property is an array containing test cases. Valid and invalid test cases are objects with the following properties: From bd22fa53a6244e276eb6dc5f385b67fcb86543ec Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:46:43 -0400 Subject: [PATCH 3/9] Update lib/shared/schema-validation-error.js Co-authored-by: Nicholas C. Zakas --- lib/shared/schema-validation-error.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/shared/schema-validation-error.js b/lib/shared/schema-validation-error.js index bb8b01b6bf4..80f9ae56227 100644 --- a/lib/shared/schema-validation-error.js +++ b/lib/shared/schema-validation-error.js @@ -1,6 +1,8 @@ "use strict"; -/** Exception thrown when schema validation fails because a rule is provided invalid options. */ +/** + * Exception thrown when schema validation fails because a rule is provided invalid options. + */ class SchemaValidationError extends Error { constructor(message) { super(message); From 0f606e318bfb928ec6853c4ca98459388665e1a9 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:48:43 -0400 Subject: [PATCH 4/9] Update lib/rule-tester/rule-tester.js Co-authored-by: Milos Djermanovic --- lib/rule-tester/rule-tester.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index caa8b0daf27..beea49be3c0 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -1107,7 +1107,7 @@ class RuleTester { if (item.name) { assert.ok(typeof item.name === "string", "Optional test case property 'name' must be a string"); } - assert.ok(typeof item.error === "object", + assert.ok(typeof item.error === "object" && item.error !== null, "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, From f015c1b02328f1ef2706f0f89546349ab656aec1 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:49:17 -0400 Subject: [PATCH 5/9] null check --- lib/rule-tester/flat-rule-tester.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rule-tester/flat-rule-tester.js b/lib/rule-tester/flat-rule-tester.js index ce46e2f7b1a..8b464098f51 100644 --- a/lib/rule-tester/flat-rule-tester.js +++ b/lib/rule-tester/flat-rule-tester.js @@ -1083,7 +1083,7 @@ class FlatRuleTester { if (item.name) { assert.ok(typeof item.name === "string", "Optional test case property 'name' must be a string"); } - assert.ok(typeof item.error === "object", + assert.ok(typeof item.error === "object" && item.error !== null, "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, From f03dbb286b1e676c1bbf4f671053e07e9681bd95 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 14:15:47 -0400 Subject: [PATCH 6/9] Update lib/rule-tester/rule-tester.js Co-authored-by: Milos Djermanovic --- lib/rule-tester/rule-tester.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 7a556097152..65599dbe9e0 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -108,7 +108,10 @@ const { SourceCode } = require("../source-code"); * @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 {string} [parser] The absolute path for the parser. + * @property {{ [name: string]: any }} [parserOptions] Options for the parser. + * @property {{ [name: string]: "readonly" | "writable" | "off" }} [globals] The additional global variables. + * @property {{ [name: string]: boolean }} [env] Environments for the test case. * @property {boolean} [only] Run only this test case or the subset of test cases with this property. */ From 109fab0e09dd698bcda9fb91394d9e7fb745f1e3 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 14:17:23 -0400 Subject: [PATCH 7/9] fix jsdoc --- lib/rule-tester/flat-rule-tester.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rule-tester/flat-rule-tester.js b/lib/rule-tester/flat-rule-tester.js index da54b3e740e..7c0e537248e 100644 --- a/lib/rule-tester/flat-rule-tester.js +++ b/lib/rule-tester/flat-rule-tester.js @@ -69,7 +69,10 @@ const { ConfigArraySymbol } = require("@humanwhocodes/config-array"); * @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 {string} [parser] The absolute path for the parser. + * @property {{ [name: string]: any }} [parserOptions] Options for the parser. + * @property {{ [name: string]: "readonly" | "writable" | "off" }} [globals] The additional global variables. + * @property {{ [name: string]: boolean }} [env] Environments for the test case. * @property {boolean} [only] Run only this test case or the subset of test cases with this property. */ From de93fe692dccf5aa0ebf064355dd6b6a77dde602 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Sun, 23 Jul 2023 14:26:39 -0400 Subject: [PATCH 8/9] reword node api doc --- docs/src/integrate/nodejs-api.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/src/integrate/nodejs-api.md b/docs/src/integrate/nodejs-api.md index 3047b7ded26..705ddaca96c 100644 --- a/docs/src/integrate/nodejs-api.md +++ b/docs/src/integrate/nodejs-api.md @@ -798,7 +798,7 @@ ruleTester.run("my-rule", rule, { } ], - // Optional array for testing invalid rule options or custom exceptions thrown by a rule. + // Optional array for testing invalid rule options or custom errors thrown by a rule. fatal: [ // Test case for invalid rule options. Useful for complex schemas. { @@ -811,7 +811,7 @@ ruleTester.run("my-rule", rule, { }, }, - // Test case for a custom exception thrown by the rule. + // Test case for a custom errors thrown by the rule. { code: 'for(const x of [1, 2, 3]) {}', error: { @@ -859,7 +859,7 @@ 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). +Fatal test cases are optional and can be used to test custom errors (exceptions) thrown by a rule or invalid rule options. They have an optional `code` property (as it's irrelevant when testing rule options). They have an `error` object that should include one or both of the `message` of the error and the error class `name`. When testing invalid rule options, the error `name` will be `SchemaValidationError`, and the `message` will 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: From 2b39ad336b6bd34ba6b1dab4b702f0ed68b1fa5c Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Mon, 24 Jul 2023 11:05:35 -0400 Subject: [PATCH 9/9] test null check --- tests/lib/rule-tester/flat-rule-tester.js | 7 +++++++ tests/lib/rule-tester/rule-tester.js | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/tests/lib/rule-tester/flat-rule-tester.js b/tests/lib/rule-tester/flat-rule-tester.js index 4184a80ff61..1edd4fe3f0a 100644 --- a/tests/lib/rule-tester/flat-rule-tester.js +++ b/tests/lib/rule-tester/flat-rule-tester.js @@ -2437,6 +2437,13 @@ describe("FlatRuleTester", () => { fatal: [{}] }); }, "Test case property 'error' must be an object"); + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: null }] + }); + }, "Test case property 'error' must be an object"); // wrong type `code` assert.throws(() => { diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 8c3e70d1b05..da603db10b7 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -2421,6 +2421,13 @@ describe("RuleTester", () => { fatal: [{}] }); }, "Test case property 'error' must be an object"); + assert.throws(() => { + ruleTester.run("foo", rule, { + valid: [{ code: "'baz'" }], + invalid: [{ code: "'bar'", errors: [{ type: "Literal" }] }], + fatal: [{ error: null }] + }); + }, "Test case property 'error' must be an object"); // wrong type `code` assert.throws(() => {