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

Allow CheckerTestCase to assertDoesNotAddMessages() to check that a specific message has not been added #9598

Open
StephenYu2018 opened this issue May 5, 2024 · 2 comments
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@StephenYu2018
Copy link

Current problem

I use CheckerTestCase in tests for linting scripts in a project. There are some tests where I want to assert that a specific message has not been added given a specific code snippet. While assertNoMessages() works for now, it is possible in the future that I want to assert that a different message occurs for the same code snippet. assertNoMessages() wouldn't work because we do want to check that the different message has been added, but replacing assertNoMessages() with assertAddsMessages(different_message, ...) would mean that we can no longer check that the original message is not being added.

Desired solution

If I could directly assert that a specific message has not been added, then I could have more control over which specific messages I'd want or would not want under various scenarios. For my given example, something like this:

assertDoesNotAddMessages(original_message, ...)
assertAddsMessages(different_message, ...)

Better represents which messages I want to occur and which messages I want to not occur.

Additional context

No response

@StephenYu2018 StephenYu2018 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label May 5, 2024
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels May 5, 2024
@Pierre-Sassoulas
Copy link
Member

What about adding one function like assertMessages(emitted:iterable[str] | None, not_emitted: iterable[str] | None) ? Emitted = None would means no message were emitted, not_emitted = None would means we don't care if unexpected messages are emitted.

@jacobtylerwalls
Copy link
Member

The original proposal is more like this from the stdlib:

self.assertLogs(...
self.assertNoLogs(...

If we add a new assertMessages, we have "two ways to do it" unless we deprecate the old way (no reason to).

@Pierre-Sassoulas Pierre-Sassoulas added Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation Good first issue Friendly and approachable by new contributors and removed Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

3 participants