From 6fddd479e4f6b3cacf1d2d18831cc6afa30bdc01 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] feat: ability to test rule errors and invalid schemas per RFC-103 --- docs/src/integrate/nodejs-api.md | 25 +++++- lib/linter/linter.js | 2 + lib/rule-tester/rule-tester.js | 140 ++++++++++++++++++++++++++++++- lib/shared/config-validator.js | 17 ++-- 4 files changed, 174 insertions(+), 10 deletions(-) diff --git a/docs/src/integrate/nodejs-api.md b/docs/src/integrate/nodejs-api.md index 08583fc34487..6c000702e50e 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', + }, + }, + ], }); ``` diff --git a/lib/linter/linter.js b/lib/linter/linter.js index 0f1bd4f77611..41d7b8479b1c 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -1407,6 +1407,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); @@ -1706,6 +1707,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/rule-tester.js b/lib/rule-tester/rule-tester.js index 48df3b79b943..35279063f0d4 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,53 @@ 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"); + + 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 +1166,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 47353ac4814b..96d48369457d 100644 --- a/lib/shared/config-validator.js +++ b/lib/shared/config-validator.js @@ -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 @@ -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; } }