Skip to content

Commit

Permalink
Update: Allow testing Suggestions with data in RuleTester (fixes #12606)
Browse files Browse the repository at this point in the history
  • Loading branch information
mdjermanovic committed Dec 3, 2019
1 parent ea16de4 commit 6d5f6b2
Show file tree
Hide file tree
Showing 3 changed files with 374 additions and 22 deletions.
49 changes: 46 additions & 3 deletions docs/developer-guide/nodejs-api.md
Expand Up @@ -875,6 +875,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 @@ -896,12 +898,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 @@ -913,7 +939,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
59 changes: 45 additions & 14 deletions lib/rule-tester/rule-tester.js
Expand Up @@ -496,10 +496,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 @@ -537,12 +539,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 @@ -604,19 +604,50 @@ class RuleTester {

error.suggestions.forEach((expectedSuggestion, index) => {
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

0 comments on commit 6d5f6b2

Please sign in to comment.