Skip to content

gh-136297: Fix hypothesis and subTest usage in test_zoneinfo_property.py #136384

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 8, 2025
Merged

gh-136297: Fix hypothesis and subTest usage in test_zoneinfo_property.py #136384

merged 5 commits into from
Jul 8, 2025

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jul 7, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Jul 7, 2025
Closed
@bedevere-app bedevere-app bot added the tests Tests in the Lib/test dir label Jul 7, 2025
@sobolevn
Copy link
Member Author

sobolevn commented Jul 7, 2025

It is interesting why CI for #136298 passed. We explicitly have Hypothesis job. Why it didn't fail?

CC @vstinner and @serhiy-storchaka

@kulikjak
Copy link
Contributor

kulikjak commented Jul 7, 2025

Thanks! I tested the patch and the test passes now.

serhiy-storchaka
serhiy-storchaka approved these changes Jul 7, 2025
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this depend on the hypothesis version?

@sobolevn
Copy link
Member Author

sobolevn commented Jul 7, 2025 •
edited
Loading

@kulikjak what version do you use?
We have hypothesis==6.111.2 on main, I also tried 6.135.26 and it worked locally with both subTest and without it.

@kulikjak
Copy link
Contributor

kulikjak commented Jul 7, 2025

Oh, that might be the issue. I have 6.100.1 installed - I will test it with the latest one.

@kulikjak
Copy link
Contributor

kulikjak commented Jul 7, 2025

I just tried it with 6.111.2 and 6.135.26 and the subTest is still failing :(.

I checked the hypothesis sources and this warning/error was added with HypothesisWorks/hypothesis#1072 and is still there. And the comment there indeed suggest that @given and subTest do not work nicely together.

@sobolevn
Copy link
Member Author

sobolevn commented Jul 7, 2025

Aha! Adding -We indeed helps to fail the test case! I will try adding -We to the job.

@sobolevn
Copy link
Member Author

sobolevn commented Jul 7, 2025

I added -We as an experiment. I also kept subTest in one place. Now CI must fail.

kulikjak
kulikjak approved these changes Jul 8, 2025
View reviewed changes
Copy link
Contributor

@kulikjak kulikjak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I retested the changes and the test passes.

serhiy-storchaka
serhiy-storchaka reviewed Jul 8, 2025
View reviewed changes
.github/workflows/build.yml Outdated
@@ -426,7 +426,7 @@ jobs:
#
# (GH-104097) test_sysconfig is skipped because it has tests that are
# failing when executed from inside a virtual environment.
"${VENV_PYTHON}" -m test \
"${VENV_PYTHON}" -We -m test \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related change. It is better to do it separately, and first examine the history, because it can break tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will open a new issue about it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done #136409

@sobolevn sobolevn enabled auto-merge (squash) July 8, 2025 07:28
@sobolevn sobolevn added the needs backport to 3.13 bugs and security fixes label Jul 8, 2025
@sobolevn sobolevn added the needs backport to 3.14 bugs and security fixes label Jul 8, 2025
@sobolevn
Copy link
Member Author

sobolevn commented Jul 8, 2025

Thanks everyone!

@sobolevn sobolevn merged commit db699db into python:main Jul 8, 2025
43 checks passed
@miss-islington-app
Copy link

Thanks @sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 8, 2025
@sobolevn @miss-islington
pythongh-136297: Fix hypothesis and subTest usage in `test_zonein…
e424a53 
…fo_property.py` (pythonGH-136384)

(cherry picked from commit db699db)

Co-authored-by: sobolevn <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-app bot commented Jul 8, 2025

GH-136407 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 8, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 8, 2025
@sobolevn @miss-islington
pythongh-136297: Fix hypothesis and subTest usage in `test_zonein…
12f1172 
…fo_property.py` (pythonGH-136384)

(cherry picked from commit db699db)

Co-authored-by: sobolevn <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-app bot commented Jul 8, 2025

GH-136408 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 8, 2025
sobolevn added a commit that referenced this pull request Jul 8, 2025
@miss-islington @sobolevn
[3.13] gh-136297: Fix hypothesis and subTest usage in `test_zonei…
4a889d8 
…nfo_property.py` (GH-136384) (#136408)

gh-136297: Fix `hypothesis` and `subTest` usage in `test_zoneinfo_property.py` (GH-136384)
(cherry picked from commit db699db)

Co-authored-by: sobolevn <mail@sobolevn.me>
@kulikjak
Copy link
Contributor

kulikjak commented Jul 8, 2025

Thanks for the fix!

sobolevn added a commit that referenced this pull request Jul 8, 2025
@miss-islington @sobolevn
[3.14] gh-136297: Fix hypothesis and subTest usage in `test_zonei…
145c086 
…nfo_property.py` (GH-136384) (#136407)

gh-136297: Fix `hypothesis` and `subTest` usage in `test_zoneinfo_property.py` (GH-136384)
(cherry picked from commit db699db)

Co-authored-by: sobolevn <mail@sobolevn.me>
@sobolevn sobolevn mentioned this pull request Jul 8, 2025
Open
AndPuQing pushed a commit to AndPuQing/cpython that referenced this pull request Jul 11, 2025
@sobolevn @AndPuQing
pythongh-136297: Fix hypothesis and subTest usage in `test_zonein…
1b348c2 
…fo_property.py` (python#136384)
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@sobolevn @Pranjal095
pythongh-136297: Fix hypothesis and subTest usage in `test_zonein…
67149e5 
…fo_property.py` (python#136384)
picnixz pushed a commit to picnixz/cpython that referenced this pull request Jul 13, 2025
@sobolevn @picnixz
pythongh-136297: Fix hypothesis and subTest usage in `test_zonein…
6f403d1 
…fo_property.py` (python#136384)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@kulikjak kulikjak kulikjak approved these changes

@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti

@hugovk hugovk Awaiting requested review from hugovk

@AA-Turner AA-Turner Awaiting requested review from AA-Turner

Assignees
No one assigned
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sobolevn @kulikjak @serhiy-storchaka