Skip to content

Conversation

@matthiasgoergens
Copy link
Contributor

@matthiasgoergensmatthiasgoergens commented Sep 27, 2022

The tests are expected to fail, and are marked as such.

@bedevere-botbedevere-bot added awaiting review tests Tests in the Lib/test dir labels Sep 27, 2022
@matthiasgoergensmatthiasgoergens changed the title gh-97588: Some tests to demonstrate the issuegh-97588: Failing tests to demonstrate the issueSep 30, 2022
@matthiasgoergens
Copy link
ContributorAuthor

@mdickinson I marked the tests as failing, so we cant potentially get them into main as a basis for further work.

Copy link
Contributor

@nanjekyejoannahnanjekyejoannah left a 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.

@matthiasgoergens
Copy link
ContributorAuthor

@nanjekyejoannah See #97702 for the fix.

Copy link
Contributor

@nanjekyejoannahnanjekyejoannah left a 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?

@matthiasgoergens
Copy link
ContributorAuthor

@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?

Copy link
Contributor

@nanjekyejoannahnanjekyejoannah left a 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?

@matthiasgoergens
Copy link
ContributorAuthor

@nanjekyejoannah Feel free to review #97702

@encukou
Copy link
Member

These were merged with #97702. Thank you!

@encukouencukou closed this Aug 26, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviewtestsTests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@matthiasgoergens@encukou@nanjekyejoannah@bedevere-bot