Skip to content

12313 Fix test_manhole.py on Python 3.13rc2#12314

Merged
itamarst merged 4 commits intotrunkfrom
12313-test_manholepy-is-failing-with-latest-313-pre-release
Sep 16, 2024
Merged

12313 Fix test_manhole.py on Python 3.13rc2#12314
itamarst merged 4 commits intotrunkfrom
12313-test_manholepy-is-failing-with-latest-313-pre-release

Conversation

@itamarst
Copy link
Contributor

Scope and purpose

Fixes #12313

I am not sure why this is necessary but it solves it.

@itamarst itamarst linked an issue Sep 10, 2024 that may be closed by this pull request
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 10, 2024

CodSpeed Performance Report

Merging #12314 will not alter performance

Comparing 12313-test_manholepy-is-failing-with-latest-313-pre-release (5a8d3d5) with trunk (f0afc5e)

Summary

✅ 23 untouched benchmarks

@itamarst itamarst marked this pull request as ready for review September 10, 2024 18:11
@chevah-robot chevah-robot requested a review from a team September 10, 2024 18:11
@alyssais
Copy link

I've been seeing what I think is the same failure since updating to Python 3.12.6.

@itamarst
Copy link
Contributor Author

It's possible this is a regression in CPython. I'll try to reproduce; perhaps a better way to implement it is to inspect the possibly-skipped frame and see if it should be skipped, given it's in a released version. And I guess file a bug report.

@adiroiban
Copy link
Member

adiroiban commented Sep 15, 2024

Thanks for the PR

I see that on trunk, these tests were not failing with 3.13.0rc1 (main, Aug 29 2024, 14:16:22) [GCC 11.4.0]

https://github.com/twisted/twisted/actions/runs/10774970649/job/29878322696#step:10:1079


I did a re-run to get latest 3.13.0rc2 (main, Sep 9 2024, 03:11:37) [GCC 11.4.0] and I can see that trunk tests are failing

https://github.com/twisted/twisted/actions/runs/10774970649/job/30162695096#step:10:1081

Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

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

Thanks for the changes and the fix.

I think that we can merge this, so that we fix trunk

It would help to create an upstream bug report and add the link as an inline code comment

@cjwatson
Copy link
Contributor

There's a similar failure with Debian's Python 3.12.6, so maybe "less than 3.13" isn't quite the right test?

@adiroiban
Copy link
Member

adiroiban commented Sep 16, 2024

Thanks for the feedback

It looks like via actions/setup-python@v5 we only get CPython (3.12.5)


Maybe we need to wait a bit more. I see that 3.12.6 is available via actions/python-versions and with a definition of 3.12 , actions/setup-python should have picked up the latest release ... so 3.12.6

https://github.com/actions/python-versions/releases/tag/3.12.6-10765725458


The strange thing, is that on the actions/setup-python CI, I can see that 3.12 is picked up as 3.12.6
https://github.com/actions/setup-python/actions/runs/10795247111/job/29941593380#step:3:3

@cjwatson
Copy link
Contributor

I bisected Python v3.12.5..v3.12.6 and got python/cpython@8fe586d, i.e. python/cpython#122478. Does that help with working out the correct fix?

@adiroiban
Copy link
Member

adiroiban commented Sep 16, 2024

To replicate this error on 3.12.6 we need

Run actions/setup-python@v5
  with:
    python-version: 3.12
-    check-latest: false
+    check-latest: true

I have pushed a change on this PR to reproduce the error in 3.12.6

@itamarst itamarst enabled auto-merge September 16, 2024 14:56
@itamarst
Copy link
Contributor Author

I updated the check to be tied to the symptoms, rather than version specific.

@itamarst itamarst merged commit 3422f79 into trunk Sep 16, 2024
@itamarst itamarst deleted the 12313-test_manholepy-is-failing-with-latest-313-pre-release branch September 16, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_manhole.py is failing with latest 3.13 pre-release (rc2)

6 participants