-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add test documenting #17230 #17199
Add test documenting #17230 #17199
Conversation
@hauntsaninja would you mind taking a look? |
I'm not Shantanu but I don't think we should add a test that's testing incorrect behavior. Maybe make it an xfail test with the desired behavior. |
How do I make it xfail? mypy seems to have a bespoke testing framework. |
Grep for |
async def main() -> None: | ||
match await func1(True): | ||
case str(a): | ||
match await func2(True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this nesting add anything to the test? Better to keep tests simple unless the additional complexity tests something interesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the lead of the preceding test case. See above ^
Done, thanks. |
@JelleZijlstra anything further? |
Ping. Can we merge this please? |
It's on my list of pending reviews, so I will get to it. However, the repeated pings feel very strange to me. Why is it important to you that we merge a change that just adds a failing test? |
It's more important that #17230 is acknowledged certainly. Apologies for the nuisance. |
Add a test demonstrating #17230.