From 19a57379c717b61843bf9edab88d1cf6b1f41e65 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Sat, 22 Feb 2020 01:52:04 +0100 Subject: [PATCH 1/2] Breaking: Test with an unknown error property should fail in RuleTester --- lib/rule-tester/rule-tester.js | 26 +++++++++++- tests/lib/rule-tester/rule-tester.js | 47 ++++++++++++++++++++++ tests/lib/rules/multiline-comment-style.js | 4 +- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index 2ac26534fcc..c819c4b5753 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -119,6 +119,22 @@ 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(", ")}]`; + const hasOwnProperty = Function.call.bind(Object.hasOwnProperty); /** @@ -573,13 +589,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'."); diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 7f2b6de67f0..51a73d739b2 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -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"), { @@ -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: [], diff --git a/tests/lib/rules/multiline-comment-style.js b/tests/lib/rules/multiline-comment-style.js index 456494b7bd5..6c8d4698e06 100644 --- a/tests/lib/rules/multiline-comment-style.js +++ b/tests/lib/rules/multiline-comment-style.js @@ -1289,7 +1289,7 @@ ${" "} // bar${" "} `, options: ["separate-lines"], - errors: [{ messageid: "expectedlines", line: 2 }] + errors: [{ messageId: "expectedLines", line: 2 }] }, { code: ` @@ -1303,7 +1303,7 @@ ${" "} // bar${" "} `, options: ["separate-lines"], - errors: [{ messageid: "expectedlines", line: 2 }] + errors: [{ messageId: "expectedLines", line: 2 }] }, { code: ` From da8810d12a81fda68c6434985b59bbd9e364ce46 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 25 Feb 2020 22:21:47 +0100 Subject: [PATCH 2/2] Check suggestion properties --- lib/rule-tester/rule-tester.js | 21 +++++++++ tests/lib/rule-tester/rule-tester.js | 70 ++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js index c819c4b5753..5180f87a316 100644 --- a/lib/rule-tester/rule-tester.js +++ b/lib/rule-tester/rule-tester.js @@ -135,6 +135,16 @@ const errorObjectParameters = new Set([ ]); 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); /** @@ -678,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]; /** diff --git a/tests/lib/rule-tester/rule-tester.js b/tests/lib/rule-tester/rule-tester.js index 51a73d739b2..a8bf356fed1 100644 --- a/tests/lib/rule-tester/rule-tester.js +++ b/tests/lib/rule-tester/rule-tester.js @@ -1163,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", () => {