-
Notifications
You must be signed in to change notification settings - Fork 49
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
test[cartesian]: update hypothesis and fix the health check issues found in daily CI #1333
test[cartesian]: update hypothesis and fix the health check issues found in daily CI #1333
Conversation
Seems that the new version caused the health check for multiple executors to trigger on parametrized tests. From the tests in hypothesis itself it looked like this might not happen if parametrization is the outer decorator (applied after @given). |
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.
It looks good to me.
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.
We need to run the requirements update workflow before merging.
requirements-dev.in
Outdated
@@ -5,7 +5,7 @@ | |||
## for pkg in ['hypothesis', 'pytest']: | |||
## print(re.search(f"\n({pkg} *[=>~!].*)\n", versions)[1]) | |||
##]]] | |||
hypothesis>=6.0.0 |
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.
Whenever any requirement is modified, the tox workflow updating all config files need to be executed as explained here: https://github.com/GridTools/gt4py/blob/main/docs/development/tools/requirements.md
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 mypy version in pre-commit-config needs to be updated manually because the hook is defined in the mirror repository and it's not always up-to-date.
This has been now fixed so the PR is ready to be merged IMHO.
It looks good to me
2439c54
to
8c312cf
Compare
Description
The daily CI found that after updating to hypothesis 6.82.1 some tests in cartesian are failing hypothesis health checks. This PR tries to fix that.
Requirements