please review

Thansk for the PR.

Do you have time to help reviewing this PR first #12059 ?

In that PR, python 3.13 automated tests are introduced, so that in future PR we can not only apply changes, but make sure they work on 3.13.

Thanks!

Thanks Heather for the PR.

I will give it try.

As part of this PT, we will also need to update the set of tests we run on Python 3.13

I see there is a FIXME for this Issue to enable more tests

It looks like with this change, we should be able to run all the internet tests.

            # FIXME:https://github.com/twisted/twisted/issues/12061
            # Add full twisted.internet

https://github.com/twisted/twisted/blob/trunk/.github/workflows/test.yaml#L146-L150

Please call me Weezel. :)
I added this, but there's some environmental difference between my local machine (3.13.a.3 venv on Debian) and 3.13-alpha.3-nodeps-nocov-3.13. Specifically, I see that .github/workflows/test.yaml has coverage disabled, but the output of os.environ includes a variable for COVERAGE.
twisted.trial.unittest.FailTest: b"[('[763 chars] ('CONDA', '/usr/share/miniconda'), ('COVERAGE[6422 chars]4')]" != b"[('[763 chars] ('COLUMNS', '80'), ('CONDA', '/usr/share/mini[6458 chars]4')]"

Thanks Weezel for the update and your help with Python 3.13 support.

Which test is failing on your local dev system?

Do you use tox or you run trial directly?

For the GitHub Actions job, we use tox, as a way of "standardised" run environment.

It's important to have the test pass on local dev environments, but some test are harder to write to account for all the local variations.

needs-review

I am not sure what is going on here... Not sure if this is a Twisted bug for 3.13 ...or Python 3.13... or something else.

i can see the full stack for non-generators

11:32 $ cat demo.py 
def erroring():
    raise Exception("Error Marker")

def calling3():
    erroring()

def calling2():
    calling3()

def calling1():
    calling2()

calling1()

(venv-3.13) ✔  ~/chevah/twisted [12061-py3.13 L|✚ 1…2⚑ 3] 
11:33 $ python demo.py 
Traceback (most recent call last):
  File "/home/adi/chevah/twisted/demo.py", line 14, in <module>
    calling1()
  File "/home/adi/chevah/twisted/demo.py", line 12, in calling1
    calling2()
  File "/home/adi/chevah/twisted/demo.py", line 9, in calling2
    calling3()
  File "/home/adi/chevah/twisted/demo.py", line 6, in calling3
    erroring()
  File "/home/adi/chevah/twisted/demo.py", line 3, in erroring
    raise Exception("Error Marker")
Exception: Error Marker

There is also another issue. On 3.12, all tests are skipped

12:04 $ python --version
Python 3.12.1

12:04 $ trial twisted.internet.test.test_inlinecb.ForwardTraceBackTests
twisted.internet.test.test_inlinecb
  ForwardTraceBackTests
    test_forwardLotsOfTracebacks_312 ...                              [SKIPPED]
    test_forwardLotsOfTracebacks_313 ...                              [SKIPPED]
    test_forwardTracebacks_312 ...                                    [SKIPPED]
    test_forwardTracebacks_313 ...                                    [SKIPPED]

===============================================================================
[SKIPPED]
Needs Python 3.12 or older

twisted.internet.test.test_inlinecb.ForwardTraceBackTests.test_forwardLotsOfTracebacks_312
twisted.internet.test.test_inlinecb.ForwardTraceBackTests.test_forwardTracebacks_312
===============================================================================
[SKIPPED]
Needs Python 3.13 or newer

twisted.internet.test.test_inlinecb.ForwardTraceBackTests.test_forwardLotsOfTracebacks_313
twisted.internet.test.test_inlinecb.ForwardTraceBackTests.test_forwardTracebacks_313
-------------------------------------------------------------------------------
Ran 4 tests in 0.003s

PASSED (skips=4)

we need something like this ... so that we have the negation of the exact same condition.

diff --git a/src/twisted/internet/test/test_inlinecb.py b/src/twisted/internet/test/test_inlinecb.py
index e730e3daf7..36d4b620cf 100644
--- a/src/twisted/internet/test/test_inlinecb.py
+++ b/src/twisted/internet/test/test_inlinecb.py
@@ -127,7 +127,7 @@ class NonLocalExitTests(TestCase):
 
 
 class ForwardTraceBackTests(SynchronousTestCase):
-    @skipIf(sys.version_info > (3, 12), "Needs Python 3.12 or older")
+    @skipIf(not sys.version_info < (3, 13), "Needs Python 3.12 or older")
     def test_forwardTracebacks_312(self):
         """
         Chained inlineCallbacks are forwarding the traceback information
@@ -177,7 +177,7 @@ class ForwardTraceBackTests(SynchronousTestCase):
         self.assertIn("in calling", tb)
         self.assertIn("Error Marker", tb)
 
-    @skipIf(sys.version_info > (3, 12), "Needs Python 3.12 or older")
+    @skipIf(not sys.version_info < (3, 13), "Needs Python 3.12 or older")
     def test_forwardLotsOfTracebacks_312(self):

Something even better

+
+HAVE_PY3_12_OR_OLDER = sys.version_info < (3, 13)
 class ForwardTraceBackTests(SynchronousTestCase):
-    @skipIf(sys.version_info > (3, 12), "Needs Python 3.12 or older")
+    @skipIf(not HAVE_PY3_12_OR_OLDER, "Needs Python 3.12 or older")
     def test_forwardTracebacks_312(self):
         """
         Chained inlineCallbacks are forwarding the traceback information
@@ -152,7 +154,7 @@ class ForwardTraceBackTests(SynchronousTestCase):
         self.assertIn("in calling", tb)
         self.assertIn("Error Marker", tb)
 
-    @skipIf(sys.version_info < (3, 13), "Needs Python 3.13 or newer")
+    @skipIf(HAVE_PY3_12_OR_OLDER, "Needs Python 3.13 or newer")
     def test_forwardTracebacks_313(self):
         """
         Chained inlineCallbacks are forwarding the traceback information
@@ -177,7 +179,7 @@ class ForwardTraceBackTests(SynchronousTestCase):
         self.assertIn("in calling", tb)
         self.assertIn("Error Marker", tb)
 
-    @skipIf(sys.version_info > (3, 12), "Needs Python 3.12 or older")
+    @skipIf(not HAVE_PY3_12_OR_OLDER, "Needs Python 3.12 or older")
     def test_forwardLotsOfTracebacks_312(self):
         """
         Several Chained inlineCallbacks gives information about all generators.
@@ -225,7 +227,7 @@ class ForwardTraceBackTests(SynchronousTestCase):
         self.assertIn("Error Marker", tb)
         self.assertIn("in erroring", f.getTraceback())
 
-    @skipIf(sys.version_info < (3, 13), "Needs Python 3.13 or newer")
+    @skipIf(HAVE_PY3_12_OR_OLDER, "Needs Python 3.13 or newer")

@eevelweezel Thanks for working on this! Is it ready for review again?

trial twisted.internet segfaults when I run it locally with 3.13alpha4, specifically on twisted.internet.test.test_core.SystemEventTestsBuilder_AsyncioSelectorReactorTests.test_shutdownDisconnectsCleanly. 😢 So I'm going to upgrade the version in GitHub Actions and see what happens when tests run there. If it still segfaults I guess that (1) I will file an upstream bug and (2) trial twisted.internet can't be enabled yet in GHA.

Please call me Weezel. :) I added this, but there's some environmental difference between my local machine (3.13.a.3 venv on Debian) and 3.13-alpha.3-nodeps-nocov-3.13. Specifically, I see that .github/workflows/test.yaml has coverage disabled, but the output of os.environ includes a variable for COVERAGE. twisted.trial.unittest.FailTest: b"[('[763 chars] ('CONDA', '/usr/share/miniconda'), ('COVERAGE[6422 chars]4')]" != b"[('[763 chars] ('COLUMNS', '80'), ('CONDA', '/usr/share/mini[6458 chars]4')]"

I just tried to test this branch in Gentoo with Python 3.13.0b2 and I see similar failure for twisted.internet.test.test_process.ProcessTestsBuilder_*.test_environmentForkEnvNone tests like you mentioned few comments above. However, LINES env variable is appended to the list in my case, not COLUMNS. see:

[FAIL]
Traceback (most recent call last):
  File "/var/tmp/portage/dev-python/twisted-9999/work/twisted-9999-python3_13/install/usr/lib/python3.13/site-packages/twisted/internet/test/test_process.py", line 1103, in test_environmentForkEnvNone
    return self.checkSpawnProcessEnvironmentWithFork({"env": None}, os.environ)
  File "/var/tmp/portage/dev-python/twisted-9999/work/twisted-9999-python3_13/install/usr/lib/python3.13/site-packages/twisted/internet/test/test_process.py", line 1059, in checkSpawnProcessEnvironmentWithFork
    return self.checkSpawnProcessEnvironment(
  File "/var/tmp/portage/dev-python/twisted-9999/work/twisted-9999-python3_13/install/usr/lib/python3.13/site-packages/twisted/internet/test/test_process.py", line 1048, in checkSpawnProcessEnvironment
    self.assertEqual(
  File "/var/tmp/portage/dev-python/twisted-9999/work/twisted-9999-python3_13/install/usr/lib/python3.13/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
    super().assertEqual(first, second, msg)
  File "/usr/lib/python3.13/unittest/case.py", line 907, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.13/unittest/case.py", line 900, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: b'[(\[6112 chars] (\'LLVM_SLOT\', \'\'), (\'LLVM_TARGETS\', \'\[12503 chars]\')]' != b'[(\[6112 chars] (\'LINES\', \'24\'), (\'LLVM_SLOT\', \'\'), ([12524 chars]\')]'

twisted.internet.test.test_process.ProcessTestsBuilder_AsyncioSelectorReactorTests.test_environmentForkEnvNone
twisted.internet.test.test_process.ProcessTestsBuilder_EPollReactorTests.test_environmentForkEnvNone
twisted.internet.test.test_process.ProcessTestsBuilder_PollReactorTests.test_environmentForkEnvNone
twisted.internet.test.test_process.ProcessTestsBuilder_SelectReactorTests.test_environmentForkEnvNone

This does not happen with Python 3.12.

For completeness, the twisted.test.test_failure.ExtendedGeneratorTests.test_findFailureInGenerator test also fails for me.

I have pushed an update to get the test pass.

I think that is OK to ignore COLUMNS and LINES for these tests.

We still need to check the missing coverage

I think that this is ready for review.

As I have also made a few changes to this PR, @itamarst can you please double-check the changes and see if we can merge them.

This enables the tests for the whole twisted.internet

needs-review

needs-review

Before we commit this, worth seeing how things do in 3.13b3. It's possible the issues will go away.

I'm inclined to agree with @itamarst, I'm still having difficulty believing the changes to expected opcodes are deliberate.

Which issue will go away ?

The LINES/COLUMNS environment variables or the formating of the traceback ?

Are there any upstream reports for these issues?

I did a quick search on https://github.com/python/cpython/issues?q=is%3Aissue and I was not able to identify any upstream report.

Thanks

Going to merge this forward and see if I can get it in finished state.

I am unhappy with the 3.13 alternative tests; it's not showing erroring(), which is the most important part of the traceback. failure.py is extremely confusing in this area so I'm having issues figuring out why this is happening, but will keep poking at it.

The following code in _findFailure in failure.py appears to be why the tests fails, it returns on 3.13 but not earlier versions. If I comment it out it fixes the 3.13 failures (but adds a bunch of other failures that match across 3.13 and 3.11):

        if (not lastFrame.f_code.co_code) or lastFrame.f_code.co_code[
            lastTb.tb_lasti
        ] != cls._yieldOpcode:
            return

I think I've come up with a fix to failure.py hat just makes the current tests pass, unchanged. I can't tell if it's actually a correct fix or it's hardcoded to the specific test. Basically it looks like Python 3.13 has different pattern of bytecode for returns. Maybe?

This whole approach in failure.py is going to be a problem as we switch to a JIT-based world, since bytecode not might even be a thing...

Anyway I'm going to close this because the correct solution is almost certainly to fix failure.py instead of changing the tests. Sorry this took so much of your time 😢