Skip to content

Commit

Permalink
Breaking: Test with an unknown error property should fail in RuleTest…
Browse files Browse the repository at this point in the history
…er (#12096)

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

* Check suggestion properties
  • Loading branch information
mdjermanovic committed Feb 28, 2020
1 parent 9038a29 commit 4af06fc
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 3 deletions.
47 changes: 46 additions & 1 deletion lib/rule-tester/rule-tester.js
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
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
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

0 comments on commit 4af06fc

Please sign in to comment.