12084 PyPy stack depth check change for 7.3.14 #12083
Conversation
|
There was a timeout in the 3.8-macos-12-default-tests run, but this seems to be happening in other PRs. |
|
please review |
|
Thanks Matti for the PR. So we had #12077 in which we updated the code to work with pypy3.10-v7.3.13 I am not sure what is our policy for pypy support, but I guess that with our limited resourcers we should try to support the latest pypy version. With that in mind, I think that we can consider this PR as something like I will push a commit on this branch to run our tests with 7.3.14 |
There was a problem hiding this comment.
Many thanks for the fix. Much appreciated.
I only have minor comments.
I left some inline suggestion. I hope @glyph can review and we can have this merged soon.
I can see PyPy test running with Successfully set up PyPy 7.3.14 with Python (3.10.13)
. So the changes in this PR ar tested.
Co-authored-by: Adi Roiban <adiroiban@gmail.com>
Please feel free to ping me directly in this repo whenever PyPy problems crop up. I appreciate the efforts you all do to support PyPy, and will try to make it a priority to help out where I can. |
MacOS tests are flaky. We need to fix that soon as they can be a big annoyance and confusion for contributors. |
There was a problem hiding this comment.
All good. Thanks, Matti.
We now run Twisted tests on CPython 3.10 with whatever GitHub Action thinks it's the latest PyPy release.
So we should be able to observe any future changes and we will get in touch.
We don't run tests with unreleased PyPy versions.
For reference, if someone would like to run Twisted tests with main PyPy dev version , there is subset of Twisted tests that can be executed and which should provide good enough coverage
https://github.com/twisted/twisted/actions/runs/7518115938/job/20465098025?pr=12083#step:10:689
On our GitHub Actions, the test run takes 1 minutes...but it mostly tox venv setup. The tests run is about 10 seconds.
|
@glyph looks like codecov.io is flaky again. Note that we have this PR for review in which the diff coverage is generated using only GitHub Actions. Please double check that we are ok with the coverage and feel free to merge. Thanks! I have flagged PR so that we will not forget to merge this before the next release. |
|
I am going to make a few minor tweaks here (getting the additional arithmetic / comparison out of a very hot loop, moving to a single constant, and re-pinning pypy to .14; we should figure out another way to get notified of pypy upgrades that doesn't break CI for unrelated issues) Thanks a ton @mattip for figuring this out; great to work with you on it :). |
This is a real coverage issue. If we drop .13 from the matrix entirely, then we don't cover the old branch. I'm going to run .13 with coverage, and .14 without, since we already have 2 pypy jobs, and that should make codecov happy. |
glyph
changed the title
glyph
changed the title
Scope and purpose
Fixes #12084
PyPy 7.3.14 made stack crawling more compatible with CPython, this broke tests in twisted. Since this is a testing-only fix, does it need a release note?
Contributor Checklist:
This process applies to all pull requests - no matter how small.
Have a look at our developer documentation before submitting your Pull Request.
Below is a non-exhaustive list (as a reminder):
please review.Our bot will trigger the review process, by applying the pending review label
and requesting a review from the Twisted dev team.