-
Notifications
You must be signed in to change notification settings - Fork 9
Change hypothesis check to be version based #96
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
Change hypothesis check to be version based #96
Conversation
|
I triggered CI and it looks like tests are failing: https://github.com/Quansight-Labs/pytest-run-parallel/actions/runs/16329573950/job/46129022108?pr=96#step:7:67 Since hypothesis itself is using pytest-run-parallel to debug this stuff, there probably isn't much point in adding hypothesis uses in the tests here, but it looks like you need to update the test that we have now. |
|
tox is pinning hypothesis to 6.113.0 since we dropped support for python 3.8 in 6.114.0 (https://hypothesis.readthedocs.io/en/latest/changelog.html#v6-114-0), I'll remove that job? or would you prefer to keep it and conditionally run the hypothesis test on 3.9+? |
I think we can drop 3.8 support in the next release that adds 3.14 support but let’s wait for @lysnikolaou to confirm. |
Sounds good to me! We should probably do a release before all of this gets merged and then do a proper minor release with all of this after that also drops 3.8 support. |
|
OK cool, let's drop Python 3.8 support here and add 3.14 support in #84. Should the AST parsing issue for |
|
Test failure looks a little like a pypy-specific thread safety issue in hypothesis. |
|
Thanks, I think I see how this could happen in general: HypothesisWorks/hypothesis#4476. Self-note:
|
|
pypy failure is a We can easily work around this here with |
|
@lysnikolaou and I chatted and I think we'd like to add a new configuration knob that people can use to disable running hypothesis tests. We'll default to running hypothesis tests. The knob is just in case turning on hypothesis tests leads to new test failures in buggy code or tests that were previously masked by skipping hypothesis tests. That will let people quickly update to the newest pytest-run-parallel version but then fix their code and tests at their leisure. I'll be adding a similar feature when I finish up #84, so you can crib off of me once that's done, hopefully in the next few days. |
|
That makes sense to me, I wouldn't want a hypothesis bug to block downstream from updating pytest-run-parallel 👍 |
|
I triggered the tests - looks like everything is OK. I noticed some oddness on the 3.14 CI which I missed when we added it, so I opened #99 to fix that. I'm not sure why github is showing the 3.8 tests as being required to pass. Maybe if you rebase on main once #99 is merged that will clear it? I'm not sure if protected branch settings require that to be fixed or if we can just merge. |
ngoldbaum
left a comment
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.
Just one small docs suggestion.
|
LGTM, @lysnikolaou did you want to look at this again before merging? |
I'm not sure yeah...maybe github doesn't like a non-member modifying CI workflows. Worst case you can try PRing this |
lysnikolaou
left a comment
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.
LGTM too! Let's merge it! Thanks @tybug!
That's because those were required checks according our branch protection rules. I've removed them, so all good now. Thanks again! |
|
Thanks Nathan and Lysandros! I'll add a |
|
I'd like to quickly get a fix in for #100 today, but other than that I think we're ready for a 0.6.0 release. |
|
Hey @tybug, I tried to release 0.6.0 today but it seems that 3.14 CI is failing because of hypothesis. Could you give that a look? Here's the test report: https://github.com/Quansight-Labs/pytest-run-parallel/actions/runs/16649513802/job/47118238943#step:5:204 |
|
looking into it now |
|
initial reaction: I don't know if this the fault of hypothesis, or a bug in 3.14t which happens to be induced by hypothesis. The base error looks pathlib-internal: |
|
It looks like pathlib did have some changes in the 3.14 release cycle. Maybe we need to make a CPython bug report. Ping @barneygale in case you're interested in a possible pathlib bug in 3.14 found by hypothesis. |
|
Rerunning the tests with |
|
I can't reproduce this locally on macos |
|
I cannot reproduce this locally either and rerunning the tests on CI made them pass. It probably looks like a thread safety issue in |
Hypothesis is ~basically thread safe as of 6.135.32 (see HypothesisWorks/hypothesis#4451). This changes the check for hypothesis to be version-based instead of unconditional. Probably shouldn't be merged until Hypothesis is tested on a few more repositories in the wild.
I won't be offended if you close or don't use this PR if it's less effort for you that way, I just had it locally and figured I would PR.