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

Breaking: Rule Tester strictly checks messageId and data. (fixes #9890) #9902

Closed

Conversation

betaorbust
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

This PR:

  1. Addresses the solution discussed in the comments on messageId messages with placeholders fail during tests. #9890
  2. Adds associated tests.
  3. Adds message formatting data to the reported problem object. (Related to 1, but also very useful for deeper checking, and future localization work based off of messageId
  4. Fixes some unrelated tests that were either throwing whole nodes into the data field, just to pull off name or were relying on a hard-coded problem object shape.

Is there anything you'd like reviewers to focus on?

  • This is technically a breaking change, as there are some stricter assertions around message, messageId, and data usage in rule-tester.
  • Unfortunately, the data property is somewhat generically named, but I left it that way to keep parity with what's actually being set during context.report. Any suggestions on that would be helpful, but I think this is probably the right name, if unfortunately a little ambiguous.

@betaorbust betaorbust changed the title Add reworked messageId logic. Breaking: Make rule-tester strictly check messageId and data. (fixes #9890) Jan 28, 2018
@betaorbust betaorbust force-pushed the update-message-id-rule-tester--jf branch from bae2e9d to c783b09 Compare January 28, 2018 07:03
@betaorbust betaorbust changed the title Breaking: Make rule-tester strictly check messageId and data. (fixes #9890) Breaking: Rule Tester strictly checks messageId and data. (fixes #9890) Jan 28, 2018
@platinumazure
Copy link
Member

Hi @betaorbust, should this be closed? Thanks!

@platinumazure platinumazure added breaking This change is backwards-incompatible do not merge This pull request should not be merged yet labels Feb 2, 2018
@betaorbust
Copy link
Contributor Author

Closing in favor of #9908

@betaorbust betaorbust closed this Feb 26, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Aug 27, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible do not merge This pull request should not be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants