From e508800658d0a71356ccc8b94a30e06140fc8858 Mon Sep 17 00:00:00 2001 From: fnx <966276+DMartens@users.noreply.github.com> Date: Tue, 2 Apr 2024 23:20:56 +0200 Subject: [PATCH] fix: rule tester ignore irrelevant test case properties (#18235) * fix: ignore irrelevant test case properties for the duplication check * fix: only ignore top-level test case properties * chore: test case merge fixes --- lib/rule-tester/rule-tester.js | 20 +++- tests/lib/rule-tester/rule-tester.js | 105 +++++++++++++++++++- tests/lib/rules/accessor-pairs.js | 8 +- tests/lib/rules/array-bracket-newline.js | 19 ---- tests/lib/rules/array-callback-return.js | 1 - tests/lib/rules/constructor-super.js | 4 - tests/lib/rules/no-object-constructor.js | 1 - tests/lib/rules/no-unused-vars.js | 24 +---- tests/lib/rules/no-useless-backreference.js | 4 +- 9 files changed, 130 insertions(+), 56 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 261a1bb73bf..f965fbbe472 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -136,6 +136,15 @@ const suggestionObjectParameters = new Set([ ]); const friendlySuggestionObjectParameterList = `[${[...suggestionObjectParameters].map(key => `'${key}'`).join(", ")}]`; +/* + * Ignored test case properties when checking for test case duplicates. + */ +const duplicationIgnoredParameters = new Set([ + "name", + "errors", + "output" +]); + const forbiddenMethods = [ "applyInlineConfig", "applyLanguageOptions", @@ -848,7 +857,7 @@ class RuleTester { /** * Check if this test case is a duplicate of one we have seen before. - * @param {Object} item test case object + * @param {string|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 @@ -863,7 +872,14 @@ class RuleTester { return; } - const serializedTestCase = stringify(item); + const normalizedItem = typeof item === "string" ? { code: item } : item; + const serializedTestCase = stringify(normalizedItem, { + replacer(key, value) { + + // "this" is the currently stringified object --> only ignore top-level properties + return (normalizedItem !== this || !duplicationIgnoredParameters.has(key)) ? value : void 0; + } + }); assert( !seenTestCases.has(serializedTestCase), diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 28860af6f7f..4cb34b96dfe 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -3148,7 +3148,7 @@ describe("RuleTester", () => { } }, { valid: ["foo", "foo"], - invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }] + invalid: [] }); }, "detected duplicate test case"); }); @@ -3162,10 +3162,69 @@ describe("RuleTester", () => { } }, { valid: [{ code: "foo" }, { code: "foo" }], - invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }] + invalid: [] + }); + }, "detected duplicate test case"); + }); + + it("throws with string and object test cases", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: {}, + create() { + return {}; + } + }, { + valid: ["foo", { code: "foo" }], + invalid: [] + }); + }, "detected duplicate test case"); + }); + + it("ignores the name property", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: {}, + create() { + return {}; + } + }, { + valid: [{ code: "foo" }, { name: "bar", code: "foo" }], + invalid: [] }); }, "detected duplicate test case"); }); + + it("does not ignore top level test case properties nested in other test case properties", () => { + ruleTester.run("foo", { + meta: { schema: [{ type: "object" }] }, + create() { + return {}; + } + }, { + valid: [{ options: [{ name: "foo" }], name: "foo", code: "same" }, { options: [{ name: "bar" }], name: "bar", code: "same" }], + invalid: [] + }); + }); + + it("does not throw an error for defining the same test case in different run calls", () => { + const rule = { + meta: {}, + create() { + return {}; + } + }; + + ruleTester.run("foo", rule, { + valid: ["foo"], + invalid: [] + }); + + ruleTester.run("foo", rule, { + valid: ["foo"], + invalid: [] + }); + }); }); describe("invalid test cases", () => { @@ -3328,6 +3387,48 @@ describe("RuleTester", () => { ] }); }); + + it("detects duplicate test cases even if the error matchers differ", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: { schema: false }, + create(context) { + return { + VariableDeclaration(node) { + context.report(node, "foo bar"); + } + }; + } + }, { + valid: [], + invalid: [ + { code: "const x = 123;", errors: [{ message: "foo bar" }] }, + { code: "const x = 123;", errors: 1 } + ] + }); + }, "detected duplicate test case"); + }); + + it("detects duplicate test cases even if the presence of the output property differs", () => { + assert.throws(() => { + ruleTester.run("foo", { + meta: { schema: false }, + create(context) { + return { + VariableDeclaration(node) { + context.report(node, "foo bar"); + } + }; + } + }, { + valid: [], + invalid: [ + { code: "const x = 123;", errors: 1 }, + { code: "const x = 123;", errors: 1, output: null } + ] + }); + }, "detected duplicate test case"); + }); }); }); diff --git a/tests/lib/rules/accessor-pairs.js b/tests/lib/rules/accessor-pairs.js index c183bbacdd7..91a902d87f6 100644 --- a/tests/lib/rules/accessor-pairs.js +++ b/tests/lib/rules/accessor-pairs.js @@ -1047,10 +1047,10 @@ ruleTester.run("accessor-pairs", rule, { // Full location tests { - code: "var o = { get a() {} };", + code: "var o = { get b() {} };", options: [{ setWithoutGet: true, getWithoutSet: true }], errors: [{ - message: "Setter is not present for getter 'a'.", + message: "Setter is not present for getter 'b'.", type: "Property", line: 1, column: 11, @@ -1831,11 +1831,11 @@ ruleTester.run("accessor-pairs", rule, { }] }, { - code: "class A { static get a() {} };", + code: "class A { static get b() {} };", options: [{ setWithoutGet: true, getWithoutSet: true, enforceForClassMembers: true }], languageOptions: { ecmaVersion: 6 }, errors: [{ - message: "Setter is not present for class static getter 'a'.", + message: "Setter is not present for class static getter 'b'.", type: "MethodDefinition", line: 1, column: 11, diff --git a/tests/lib/rules/array-bracket-newline.js b/tests/lib/rules/array-bracket-newline.js index b4d8fd9198c..9b6a9e1ce9f 100644 --- a/tests/lib/rules/array-bracket-newline.js +++ b/tests/lib/rules/array-bracket-newline.js @@ -761,25 +761,6 @@ ruleTester.run("array-bracket-newline", rule, { } ] }, - { - code: "var foo = [\n1\n];", - output: "var foo = [1];", - options: ["never"], - errors: [ - { - messageId: "unexpectedOpeningLinebreak", - type: "ArrayExpression", - line: 1, - column: 11 - }, - { - messageId: "unexpectedClosingLinebreak", - type: "ArrayExpression", - line: 3, - column: 1 - } - ] - }, { code: "var foo = [ /* any comment */\n1, 2\n];", output: "var foo = [ /* any comment */\n1, 2];", diff --git a/tests/lib/rules/array-callback-return.js b/tests/lib/rules/array-callback-return.js index 938f73f7a2d..9bb9334391e 100644 --- a/tests/lib/rules/array-callback-return.js +++ b/tests/lib/rules/array-callback-return.js @@ -273,7 +273,6 @@ ruleTester.run("array-callback-return", rule, { { code: "foo.forEach(function(x) { if (a == b) {return x;}})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "foo.forEach(function(x) { if (a == b) {return undefined;}})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "foo.forEach(function bar(x) { return x;})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function 'bar'", arrayMethodName: "Array.prototype.forEach" } }] }, - { code: "foo.forEach(function bar(x) { return x;})", options: checkForEachOptions, errors: ["Array.prototype.forEach() expects no useless return value from function 'bar'."] }, { code: "foo.bar().forEach(function bar(x) { return x;})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function 'bar'", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "[\"foo\",\"bar\"].forEach(function bar(x) { return x;})", options: checkForEachOptions, errors: [{ messageId: "expectedNoReturnValue", data: { name: "function 'bar'", arrayMethodName: "Array.prototype.forEach" } }] }, { code: "foo.forEach((x) => { return x;})", options: checkForEachOptions, languageOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedNoReturnValue", data: { name: "arrow function", arrayMethodName: "Array.prototype.forEach" } }] }, diff --git a/tests/lib/rules/constructor-super.js b/tests/lib/rules/constructor-super.js index 9268a13fb7d..ed61640e81f 100644 --- a/tests/lib/rules/constructor-super.js +++ b/tests/lib/rules/constructor-super.js @@ -225,10 +225,6 @@ ruleTester.run("constructor-super", rule, { }, // nested execution scope. - { - code: "class A extends B { constructor() { class C extends D { constructor() { super(); } } } }", - errors: [{ messageId: "missingAll", type: "MethodDefinition" }] - }, { code: "class A extends B { constructor() { var c = class extends D { constructor() { super(); } } } }", errors: [{ messageId: "missingAll", type: "MethodDefinition" }] diff --git a/tests/lib/rules/no-object-constructor.js b/tests/lib/rules/no-object-constructor.js index 5b94a12ffea..c865f3e146c 100644 --- a/tests/lib/rules/no-object-constructor.js +++ b/tests/lib/rules/no-object-constructor.js @@ -193,7 +193,6 @@ ruleTester.run("no-object-constructor", rule, { ...[ // No semicolon required before `({})` because ASI does not occur - { code: "Object()" }, { code: ` {} diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js index b25e718f41b..2f2d4f2125c 100644 --- a/tests/lib/rules/no-unused-vars.js +++ b/tests/lib/rules/no-unused-vars.js @@ -1512,21 +1512,16 @@ ruleTester.run("no-unused-vars", rule, { // https://github.com/eslint/eslint/issues/10982 { code: "var a = function() { a(); };", - errors: [assignedError("a")] + errors: [{ ...assignedError("a"), line: 1, column: 5 }] }, { code: "var a = function(){ return function() { a(); } };", - errors: [assignedError("a")] - }, - { - code: "const a = () => { a(); };", - languageOptions: { ecmaVersion: 2015 }, - errors: [assignedError("a")] + errors: [{ ...assignedError("a"), line: 1, column: 5 }] }, { code: "const a = () => () => { a(); };", languageOptions: { ecmaVersion: 2015 }, - errors: [assignedError("a")] + errors: [{ ...assignedError("a"), line: 1, column: 7 }] }, { code: `let myArray = [1,2,3,4].filter((x) => x == 0); @@ -1539,24 +1534,11 @@ ruleTester.run("no-unused-vars", rule, { languageOptions: { ecmaVersion: 2015 }, errors: [{ ...assignedError("a"), line: 1, column: 14 }] }, - { - code: "var a = function() { a(); };", - errors: [{ ...assignedError("a"), line: 1, column: 5 }] - }, - { - code: "var a = function(){ return function() { a(); } };", - errors: [{ ...assignedError("a"), line: 1, column: 5 }] - }, { code: "const a = () => { a(); };", languageOptions: { ecmaVersion: 2015 }, errors: [{ ...assignedError("a"), line: 1, column: 7 }] }, - { - code: "const a = () => () => { a(); };", - languageOptions: { ecmaVersion: 2015 }, - errors: [{ ...assignedError("a"), line: 1, column: 7 }] - }, // https://github.com/eslint/eslint/issues/14324 { diff --git a/tests/lib/rules/no-useless-backreference.js b/tests/lib/rules/no-useless-backreference.js index 666ec0a1742..c868184d06a 100644 --- a/tests/lib/rules/no-useless-backreference.js +++ b/tests/lib/rules/no-useless-backreference.js @@ -162,9 +162,9 @@ ruleTester.run("no-useless-backreference", rule, { }] }, { - code: String.raw`/\k(?a)/`, + code: String.raw`/\k(?bar)/`, errors: [{ - message: String.raw`Backreference '\k' will be ignored. It references group '(?a)' which appears later in the pattern.`, + message: String.raw`Backreference '\k' will be ignored. It references group '(?bar)' which appears later in the pattern.`, type: "Literal" }] },