Skip to content

Commit

Permalink
fix: rule tester ignore irrelevant test case properties (#18235)
Browse files Browse the repository at this point in the history
* fix: ignore irrelevant test case properties for the duplication check

* fix: only ignore top-level test case properties

* chore: test case merge fixes
  • Loading branch information
DMartens committed Apr 2, 2024
1 parent a129acb commit e508800
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 56 deletions.
20 changes: 18 additions & 2 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -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",
Expand Down Expand Up @@ -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<string>} seenTestCases set of serialized test cases we have seen so far (managed by this function)
* @returns {void}
* @private
Expand All @@ -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),
Expand Down
105 changes: 103 additions & 2 deletions tests/lib/rule-tester/rule-tester.js
Expand Up @@ -3148,7 +3148,7 @@ describe("RuleTester", () => {
}
}, {
valid: ["foo", "foo"],
invalid: [{ code: "foo", errors: [{ message: "foo bar" }] }]
invalid: []
});
}, "detected duplicate test case");
});
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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");
});
});
});

Expand Down
8 changes: 4 additions & 4 deletions tests/lib/rules/accessor-pairs.js
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 0 additions & 19 deletions tests/lib/rules/array-bracket-newline.js
Expand Up @@ -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];",
Expand Down
1 change: 0 additions & 1 deletion tests/lib/rules/array-callback-return.js
Expand Up @@ -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" } }] },
Expand Down
4 changes: 0 additions & 4 deletions tests/lib/rules/constructor-super.js
Expand Up @@ -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" }]
Expand Down
1 change: 0 additions & 1 deletion tests/lib/rules/no-object-constructor.js
Expand Up @@ -193,7 +193,6 @@ ruleTester.run("no-object-constructor", rule, {
...[

// No semicolon required before `({})` because ASI does not occur
{ code: "Object()" },
{
code: `
{}
Expand Down
24 changes: 3 additions & 21 deletions tests/lib/rules/no-unused-vars.js
Expand Up @@ -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);
Expand All @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/rules/no-useless-backreference.js
Expand Up @@ -162,9 +162,9 @@ ruleTester.run("no-useless-backreference", rule, {
}]
},
{
code: String.raw`/\k<foo>(?<foo>a)/`,
code: String.raw`/\k<foo>(?<foo>bar)/`,
errors: [{
message: String.raw`Backreference '\k<foo>' will be ignored. It references group '(?<foo>a)' which appears later in the pattern.`,
message: String.raw`Backreference '\k<foo>' will be ignored. It references group '(?<foo>bar)' which appears later in the pattern.`,
type: "Literal"
}]
},
Expand Down

0 comments on commit e508800

Please sign in to comment.