-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-97588: Failing tests to demonstrate the issue #97589
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
Conversation
@mdickinson I marked the tests as failing, so we cant potentially get them into |
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.
Shouldn't we be patching a fix instead? Unless we are accepting this as desired behaviour.
@nanjekyejoannah See #97702 for the fix. |
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.
Then we close this to merge the fix instead, no?
@nanjekyejoannah I'm not completely confident in the fix, yet. But I am confident that these test cases are good. Not sure we have a general policy against contributing failing test cases? |
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.
The tests test the failing caes. But with the fix, these tests will change i.e not failing, I would prefer reviewing a PR with a fix and updated tests, since there is a fix. What do you think?
@nanjekyejoannah Feel free to review #97702 |
2acd7a9
to
be065d5
Compare
These were merged with #97702. Thank you! |
The tests are expected to fail, and are marked as such.