Chore: "Error type should be" assertion in rule-tester (fixes 6106) #7550
Conversation
|
LGTM |
|
@Fransz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kaicataldo, @gyandeeps and @nzakas to be potential reviewers. |
| @@ -446,7 +446,7 @@ RuleTester.prototype = { | |||
| } | |||
|
|
|||
| if (item.errors[i].type) { | |||
| assert.equal(messages[i].nodeType, item.errors[i].type, `Error type should be ${item.errors[i].type}`); | |||
There was a problem hiding this comment.
I'm not sure that removing this message is the right way to go about this change; I think "Error type should be CallExpression" provides useful context about the issue, whereas "CallExpression != SomethingElse" is sort of confusing.
Instead, maybe the message should say something along the lines of: "Expected an error type of CallExpression but found Indentifier". That keeps the helpful "Error type should be" context, while also providing the additional information in #6106.
There was a problem hiding this comment.
i've changed the message as requested.
not-an-aardvark
added
enhancement
|
LGTM |
There was a problem hiding this comment.
LGTM, just waiting for @not-an-aardvark to re-review.
Also, this should be an "Update:" because it's making a change that's not related to a bug.
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[X ] Other, please explain: Change in rule-tester.
issue 6106
What changes did you make? (Give an overview)
Removed the overwriting message in the assert.equal statement.
Is there anything you'd like reviewers to focus on?