Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: rule tester ignore irrelevant test case properties #18235

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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() {} };",
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
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() {} };",
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -170,10 +170,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
3 changes: 0 additions & 3 deletions tests/lib/rules/no-object-constructor.js
Expand Up @@ -191,9 +191,6 @@ ruleTester.run("no-object-constructor", rule, {
})),

...[

// No semicolon required before `({})` because ASI does not occur
{ code: "Object()" },
{
DMartens marked this conversation as resolved.
Show resolved Hide resolved
code: `
{}
Expand Down
24 changes: 3 additions & 21 deletions tests/lib/rules/no-unused-vars.js
Expand Up @@ -1461,21 +1461,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") }]
DMartens marked this conversation as resolved.
Show resolved Hide resolved
},
{
code: `let myArray = [1,2,3,4].filter((x) => x == 0);
Expand All @@ -1488,24 +1483,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)/`,
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
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