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.

Sign up for GitHub

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

Jump to bottom

Breaking: Test with an unknown error property should fail in RuleTester #12096

Merged
merged 2 commits into from Feb 28, 2020
Merged

Breaking: Test with an unknown error property should fail in RuleTester #12096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19a5737
Breaking: Test with an unknown error property should fail in RuleTester
mdjermanovic Feb 22, 2020
da8810d
Check suggestion properties
mdjermanovic Feb 25, 2020
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
47 changes: 46 additions & 1 deletion lib/rule-tester/rule-tester.js
View file
Open in desktop
Expand Up @@ -119,6 +119,32 @@ const RuleTesterParameters = [
"output"
];

/*
* All allowed property names in error objects.
*/
const errorObjectParameters = new Set([
"message",
"messageId",
"data",
"type",
"line",
"column",
"endLine",
"endColumn",
"suggestions"
]);
const friendlyErrorObjectParameterList = `[${[...errorObjectParameters].map(key => `'${key}'`).join(", ")}]`;

/*
* All allowed property names in suggestion objects.
*/
const suggestionObjectParameters = new Set([
"desc",
"messageId",
"output"
]);
const friendlySuggestionObjectParameterList = `[${[...suggestionObjectParameters].map(key => `'${key}'`).join(", ")}]`;

const hasOwnProperty = Function.call.bind(Object.hasOwnProperty);

/**
Expand Down Expand Up @@ -573,13 +599,21 @@ class RuleTester {

// Just an error message.
assertMessageMatches(message.message, error);
} else if (typeof error === "object") {
} else if (typeof error === "object" && error !== null) {

/*
* Error object.
* This may have a message, messageId, data, node type, line, and/or
* column.
*/

Object.keys(error).forEach(propertyName => {
assert.ok(
errorObjectParameters.has(propertyName),
`Invalid error property name '${propertyName}'. Expected one of ${friendlyErrorObjectParameterList}.`
);
});

if (hasOwnProperty(error, "message")) {
assert.ok(!hasOwnProperty(error, "messageId"), "Error should not specify both 'message' and a 'messageId'.");
assert.ok(!hasOwnProperty(error, "data"), "Error should not specify both 'data' and 'message'.");
Expand Down Expand Up @@ -654,6 +688,17 @@ class RuleTester {
assert.strictEqual(message.suggestions.length, error.suggestions.length, `Error should have ${error.suggestions.length} suggestions. Instead found ${message.suggestions.length} suggestions`);

error.suggestions.forEach((expectedSuggestion, index) => {
assert.ok(
typeof expectedSuggestion === "object" && expectedSuggestion !== null,
"Test suggestion in 'suggestions' array must be an object."
);
Object.keys(expectedSuggestion).forEach(propertyName => {
assert.ok(
suggestionObjectParameters.has(propertyName),
`Invalid suggestion property name '${propertyName}'. Expected one of ${friendlySuggestionObjectParameterList}.`
);
});

const actualSuggestion = message.suggestions[index];

/**
Expand Down
117 changes: 117 additions & 0 deletions tests/lib/rule-tester/rule-tester.js
View file
Open in desktop
Expand Up @@ -182,6 +182,21 @@ describe("RuleTester", () => {
}, /Error should be a string, object, or RegExp/u);
});

it("should throw an error when any of the errors is not a supported type", () => {
assert.throws(() => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {

// Only the invalid test matters here
valid: [
"bar = baz;"
],
invalid: [
{ code: "var foo = bar; var baz = quux", errors: [{ type: "VariableDeclaration" }, null] }
]
});
}, /Error should be a string, object, or RegExp/u);
});

it("should throw an error when the error is a string and it does not match error message", () => {
assert.throws(() => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
Expand Down Expand Up @@ -232,6 +247,38 @@ describe("RuleTester", () => {
});
});

it("should throw an error when the error is an object with an unknown property name", () => {
assert.throws(() => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
valid: [
"bar = baz;"
],
invalid: [
{ code: "var foo = bar;", errors: [{ Message: "Bad var." }] }
]
});
}, /Invalid error property name 'Message'/u);
});

it("should throw an error when any of the errors is an object with an unknown property name", () => {
assert.throws(() => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
valid: [
"bar = baz;"
],
invalid: [
{
code: "var foo = bar; var baz = quux",
errors: [
{ message: "Bad var.", type: "VariableDeclaration" },
{ message: "Bad var.", typo: "VariableDeclaration" }
]
}
]
});
}, /Invalid error property name 'typo'/u);
});

it("should not throw an error when the error is a regex in an object and it matches error message", () => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
valid: [],
Expand Down Expand Up @@ -1116,6 +1163,76 @@ describe("RuleTester", () => {
});
}, "Expected the applied suggestion fix to match the test suggestion output");
});

it("should fail when specified suggestion isn't an object", () => {
assert.throws(() => {
ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, {
valid: [],
invalid: [{
code: "var foo;",
errors: [{
suggestions: [null]
}]
}]
});
}, "Test suggestion in 'suggestions' array must be an object.");

assert.throws(() => {
ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, {
valid: [],
invalid: [{
code: "var foo;",
errors: [{
suggestions: [
{
messageId: "renameFoo",
output: "var bar;"
},
"Rename identifier 'foo' to 'baz'"
]
}]
}]
});
}, "Test suggestion in 'suggestions' array must be an object.");
});

it("should fail when the suggestion is an object with an unknown property name", () => {
assert.throws(() => {
ruleTester.run("suggestions-basic", require("../../fixtures/testers/rule-tester/suggestions").basic, {
valid: [
"var boo;"
],
invalid: [{
code: "var foo;",
errors: [{
suggestions: [{
message: "Rename identifier 'foo' to 'bar'"
}]
}]
}]
});
}, /Invalid suggestion property name 'message'/u);
});

it("should fail when any of the suggestions is an object with an unknown property name", () => {
assert.throws(() => {
ruleTester.run("suggestions-messageIds", require("../../fixtures/testers/rule-tester/suggestions").withMessageIds, {
valid: [],
invalid: [{
code: "var foo;",
errors: [{
suggestions: [{
messageId: "renameFoo",
output: "var bar;"
}, {
messageId: "renameFoo",
outpt: "var baz;"
}]
}]
}]
});
}, /Invalid suggestion property name 'outpt'/u);
});
});

describe("naming test cases", () => {
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/rules/multiline-comment-style.js
View file
Open in desktop
Expand Up @@ -1289,7 +1289,7 @@ ${" "}
// bar${" "}
`,
options: ["separate-lines"],
errors: [{ messageid: "expectedlines", line: 2 }]
errors: [{ messageId: "expectedLines", line: 2 }]
},
{
code: `
Expand All @@ -1303,7 +1303,7 @@ ${" "}
// bar${" "}
`,
options: ["separate-lines"],
errors: [{ messageid: "expectedlines", line: 2 }]
errors: [{ messageId: "expectedLines", line: 2 }]
},
{
code: `
Expand Down