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

Update: Allow testing Suggestions with data in RuleTester (fixes #12606) #12635

Merged
merged 2 commits into from Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
49 changes: 46 additions & 3 deletions docs/developer-guide/nodejs-api.md
Expand Up @@ -876,6 +876,8 @@ In addition to the properties above, invalid test cases can also have the follow

* `errors` (number or array, required): Asserts some properties of the errors that the rule is expected to produce when run on this code. If this is a number, asserts the number of errors produced. Otherwise, this should be a list of objects, each containing information about a single reported error. The following properties can be used for an error (all are optional):
* `message` (string/regexp): The message for the error
* `messageId` (string): The Id for the error. See [testing errors with messageId](#testing-errors-with-messageid) for details
* `data` (object): Placeholder data which can be used in combination with `messageId`
* `type` (string): The type of the reported AST node
* `line` (number): The 1-based line number of the reported location
* `column` (number): The 1-based column number of the reported location
Expand All @@ -897,12 +899,36 @@ Any additional properties of a test case will be passed directly to the linter a

If a valid test case only uses the `code` property, it can optionally be provided as a string containing the code, rather than an object with a `code` key.

#### Testing errors with `messageId`

If the rule under test uses `messageId`s, you can use `messageId` property in a test case to assert reported error's `messageId` instead of its `message`.

```js
{
code: "let foo;",
errors: [{ messageId: "unexpected" }]
}
```

For messages with placeholders, a test case can also use `data` property to additionally assert reported error's `message`.

```js
{
code: "let foo;",
errors: [{ messageId: "unexpected", data: { name: "foo" } }]
}
```

Please note that `data` in a test case does not assert `data` passed to `context.report`. Instead, it is used to form the expected message text which is then compared with the received `message`.

#### Testing Suggestions

Suggestions can be tested by defining a `suggestions` key on an errors object. The options to check for the suggestions are the following (all are optional):
* `desc` (string): The suggestion `desc` value
* `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s
* `output` (string): A code string representing the result of applying the suggestion fix to the input code

* `desc` (string): The suggestion `desc` value
* `messageId` (string): The suggestion `messageId` value for suggestions that use `messageId`s
* `data` (object): Placeholder data which can be used in combination with `messageId`
* `output` (string): A code string representing the result of applying the suggestion fix to the input code

Example:

Expand All @@ -914,7 +940,24 @@ ruleTester.run("my-rule-for-no-foo", rule, {
errors: [{
suggestions: [{
desc: "Rename identifier 'foo' to 'bar'",
output: "var bar;"
}]
}]
}]
})
```

`messageId` and `data` properties in suggestion test objects work the same way as in error test objects. See [testing errors with messageId](#testing-errors-with-messageid) for details.

```js
ruleTester.run("my-rule-for-no-foo", rule, {
valid: [],
invalid: [{
code: "var foo;",
errors: [{
suggestions: [{
messageId: "renameFoo",
data: { newName: "bar" },
output: "var bar;"
}]
}]
Expand Down
60 changes: 46 additions & 14 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -141,6 +141,7 @@ const friendlyErrorObjectParameterList = `[${[...errorObjectParameters].map(key
const suggestionObjectParameters = new Set([
"desc",
"messageId",
"data",
"output"
]);
const friendlySuggestionObjectParameterList = `[${[...suggestionObjectParameters].map(key => `'${key}'`).join(", ")}]`;
Expand Down Expand Up @@ -571,10 +572,12 @@ class RuleTester {
assert.ok(item.errors || item.errors === 0,
`Did not specify errors for an invalid test of ${ruleName}`);

const ruleHasMetaMessages = hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages");
const friendlyIDList = ruleHasMetaMessages ? `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]` : null;

const result = runRuleForItem(item);
const messages = result.messages;


if (typeof item.errors === "number") {
assert.strictEqual(messages.length, item.errors, util.format("Should have %d error%s but had %d: %s",
item.errors, item.errors === 1 ? "" : "s", messages.length, util.inspect(messages)));
Expand Down Expand Up @@ -620,12 +623,10 @@ class RuleTester {
assertMessageMatches(message.message, error.message);
} else if (hasOwnProperty(error, "messageId")) {
assert.ok(
hasOwnProperty(rule, "meta") && hasOwnProperty(rule.meta, "messages"),
ruleHasMetaMessages,
"Error can not use 'messageId' if rule under test doesn't define 'meta.messages'."
);
if (!hasOwnProperty(rule.meta.messages, error.messageId)) {
const friendlyIDList = `[${Object.keys(rule.meta.messages).map(key => `'${key}'`).join(", ")}]`;

assert(false, `Invalid messageId '${error.messageId}'. Expected one of ${friendlyIDList}.`);
}
assert.strictEqual(
Expand Down Expand Up @@ -700,19 +701,50 @@ class RuleTester {
});

const actualSuggestion = message.suggestions[index];
const suggestionPrefix = `Error Suggestion at index ${index} :`;

if (hasOwnProperty(expectedSuggestion, "desc")) {
assert.ok(
!hasOwnProperty(expectedSuggestion, "data"),
`${suggestionPrefix} Test should not specify both 'desc' and 'data'.`
);
assert.strictEqual(
actualSuggestion.desc,
expectedSuggestion.desc,
`${suggestionPrefix} desc should be "${expectedSuggestion.desc}" but got "${actualSuggestion.desc}" instead.`
);
}

/**
* Tests equality of a suggestion key if that key is defined in the expected output.
* @param {string} key Key to validate from the suggestion object
* @returns {void}
*/
function assertSuggestionKeyEquals(key) {
if (hasOwnProperty(expectedSuggestion, key)) {
assert.deepStrictEqual(actualSuggestion[key], expectedSuggestion[key], `Error suggestion at index: ${index} should have desc of: "${actualSuggestion[key]}"`);
if (hasOwnProperty(expectedSuggestion, "messageId")) {
assert.ok(
ruleHasMetaMessages,
`${suggestionPrefix} Test can not use 'messageId' if rule under test doesn't define 'meta.messages'.`
);
assert.ok(
hasOwnProperty(rule.meta.messages, expectedSuggestion.messageId),
`${suggestionPrefix} Test has invalid messageId '${expectedSuggestion.messageId}', the rule under test allows only one of ${friendlyIDList}.`
);
assert.strictEqual(
actualSuggestion.messageId,
expectedSuggestion.messageId,
`${suggestionPrefix} messageId should be '${expectedSuggestion.messageId}' but got '${actualSuggestion.messageId}' instead.`
);
if (hasOwnProperty(expectedSuggestion, "data")) {
const unformattedMetaMessage = rule.meta.messages[expectedSuggestion.messageId];
const rehydratedDesc = interpolate(unformattedMetaMessage, expectedSuggestion.data);

assert.strictEqual(
actualSuggestion.desc,
rehydratedDesc,
`${suggestionPrefix} Hydrated test desc "${rehydratedDesc}" does not match received desc "${actualSuggestion.desc}".`
);
}
} else {
assert.ok(
!hasOwnProperty(expectedSuggestion, "data"),
`${suggestionPrefix} Test must specify 'messageId' if 'data' is used.`
);
}
assertSuggestionKeyEquals("desc");
assertSuggestionKeyEquals("messageId");

if (hasOwnProperty(expectedSuggestion, "output")) {
const codeWithAppliedSuggestion = SourceCodeFixer.applyFixes(item.code, [actualSuggestion]).output;
Expand Down