From fbce335c39d06b537660fa8b10d18e94e0f86d1a Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:57:07 -0500 Subject: [PATCH] feat!: detect duplicate test cases Part of: https://github.com/eslint/rfcs/tree/main/designs/2021-stricter-rule-test-validation#disallow-identical-test-cases --- lib/rule-tester/rule-tester.js | 33 ++++++++++++++++- tests/lib/rule-tester/rule-tester.js | 55 ++++++++++++++++++++++++++++ tests/lib/rules/newline-after-var.js | 4 +- 3 files changed, 89 insertions(+), 3 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index bff284ea8e61..96b1dc213512 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -16,7 +16,8 @@ const equal = require("fast-deep-equal"), Traverser = require("../shared/traverser"), { getRuleOptionsSchema } = require("../config/flat-config-helpers"), - { Linter, SourceCodeFixer, interpolate } = require("../linter"); + { Linter, SourceCodeFixer, interpolate } = require("../linter"), + stringify = require("json-stable-stringify-without-jsonify"); const { FlatConfigArray } = require("../config/flat-config-array"); const { defaultConfig } = require("../config/default-config"); @@ -499,6 +500,9 @@ class RuleTester { linter = this.linter, ruleId = `rule-to-test/${ruleName}`; + const seenValidTestCases = new Set(); + const seenInvalidTestCases = new Set(); + if (!rule || typeof rule !== "object" || typeof rule.create !== "function") { throw new TypeError("Rule must be an object with a `create` method"); } @@ -794,6 +798,29 @@ class RuleTester { } } + /** + * Check if this test case is a duplicate of one we have seen before. + * @param {Object} item test case object + * @param {Set} seenTestCases set of serialized test cases we have seen so far (managed by this function) + * @returns {void} + * @private + */ + function checkDuplicateTestCase(item, seenTestCases) { + if (["object", "function"].includes(typeof item.config) || item.settings || item.languageOptions || item.plugins) { + + // Don't check for duplicates if the test case has any properties that could be non-serializable. + return; + } + + const serializedTestCase = stringify(item); + + assert( + !seenTestCases.has(serializedTestCase), + "detected duplicate test case" + ); + seenTestCases.add(serializedTestCase); + } + /** * Check if the template is valid or not * all valid cases go through this @@ -809,6 +836,8 @@ class RuleTester { assert.ok(typeof item.name === "string", "Optional test case property 'name' must be a string"); } + checkDuplicateTestCase(item, seenValidTestCases); + const result = runRuleForItem(item); const messages = result.messages; @@ -860,6 +889,8 @@ class RuleTester { assert.fail("Invalid cases must have at least one error"); } + checkDuplicateTestCase(item, seenInvalidTestCases); + const ruleHasMetaMessages = hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages"); const friendlyIDList = ruleHasMetaMessages ? `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]` : null; diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 4d892c9fac2e..a2fb4319db23 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -2830,6 +2830,61 @@ describe("RuleTester", () => { }); + describe("duplicate test cases", () => { + describe("valid test cases", () => { + it("throws with duplicate string test cases", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: {}, + create() { + return {}; + } + }, { + valid: ["foo", "foo"], + invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }] + }); + }, "detected duplicate test case"); + }); + + it("throws with duplicate object test cases", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: {}, + create() { + return {}; + } + }, { + valid: [{ code: "foo" }, { code: "foo" }], + invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }] + }); + }, "detected duplicate test case"); + }); + }); + + describe("invalid test cases", () => { + it("throws with duplicate object test cases", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: {}, + create(context) { + return { + VariableDeclaration(node) { + context.report(node, "foo bar"); + } + }; + } + }, { + valid: ["foo"], + invalid: [ + { code: "const x = 123;", errors: [{ message: "foo bar" }] }, + { code: "const x = 123;", errors: [{ message: "foo bar" }] } + ] + }); + }, "detected duplicate test case"); + }); + }); + }); + describe("SourceCode forbidden methods", () => { [ diff --git a/tests/lib/rules/newline-after-var.js b/tests/lib/rules/newline-after-var.js index 2da5f20094d6..edc932d7d5a8 100644 --- a/tests/lib/rules/newline-after-var.js +++ b/tests/lib/rules/newline-after-var.js @@ -24,8 +24,8 @@ const NO_VAR = "console.log(greet);", FOR_LOOP_WITH_VAR = "for(var a = 1; a < 1; a++){\n break;\n}", FOR_IN_LOOP_WITH_LET = "for(let a in obj){\n break;\n}", FOR_IN_LOOP_WITH_VAR = "for(var a in obj){\n break;\n}", - FOR_OF_LOOP_WITH_LET = "for(let a in obj){\n break;\n}", - FOR_OF_LOOP_WITH_VAR = "for(var a in obj){\n break;\n}", + FOR_OF_LOOP_WITH_LET = "for(let a of obj){\n break;\n}", + FOR_OF_LOOP_WITH_VAR = "for(var a of obj){\n break;\n}", EXPORT_WITH_LET = "export let a = 1;\nexport let b = 2;", EXPORT_WITH_VAR = "export var a = 1;\nexport var b = 2;", EXPORT_WITH_CONST = "export const a = 1;\nexport const b = 2;",