#10343 - run tests w/ python 3.11 #11734

eevelweezel · 2022-10-28T02:34:08Z

Scope and purpose

Add support for running tests w/ Python 3.11 on Windows, MacOS, and Linux.

eevelweezel · 2022-10-28T05:51:00Z

I'd appreciate some guidance. This enables tests in Python 3.11, but there are multiple places where the test suite fails under that version.

adiroiban · 2022-10-28T08:32:17Z

Many thanks for creating the PR and your help with 3.11

Yes. I think that is best to create separate tickets and try to fix each error type/class in separate PRs.

I think that we will also need to wait for @glyph to do a new release for automat and then have a PR in twisted that uses the newer version.

glyph · 2022-10-29T07:04:28Z

New automat release is out.

adiroiban · 2022-10-31T10:32:04Z

I thinks is best to start small and don't use 3.11 as default.

I see pydoctor failing...and we might have other development-only dependencies failing.

The first step is to get at least a subset of Twisted test on 3.11

We don't have to move all the build tools to 3.11, yet.

adiroiban · 2022-10-31T11:16:16Z

I see twisted/persisted/aot.py and twisted.spread failing.

If they need only 2 lines of code to fix 3.11 support, we can have the fix in this PR.

Otherwise, I think is best to fix the changes in separate tickets.

eevelweezel · 2022-11-03T02:37:14Z

I see twisted/persisted/aot.py and twisted.spread failing.

If they need only 2 lines of code to fix 3.11 support, we can have the fix in this PR.

Otherwise, I think is best to fix the changes in separate tickets.

I've reverted the commit making 3.11 the default python & fixed the tests for twisted.spread and twisted.persisted.

glyph · 2022-11-03T10:40:14Z

Looks like the test fixes have not worked?

adiroiban · 2022-11-03T11:54:17Z

FAILED (skips=514, failures=3, successes=10598)

Not bad. This is good progress. Thanks!

eevelweezel · 2022-11-04T04:20:38Z

I looked into the 3 remaining failures. Those also needed only 1-2 lines to fix, so I addressed those here as well.

eevelweezel · 2022-11-04T04:41:33Z

please review

glyph · 2022-11-04T04:53:14Z

@eevelweezel Thank you so much for doing this critical maintenance!

glyph

LGTM

I'd really like to move that .misc to a feature so if anyone has time to do that and land that would be great.

src/twisted/newsfragments/10343.misc

src/twisted/web/test/test_flatten.py

adiroiban · 2022-11-04T11:38:32Z

.github/workflows/test.yaml

@@ -122,11 +122,19 @@ jobs:
          # Just Python 3.9 with default settings.
          - python-version: '3.9'

+          # Just Python 3.11 with default settings.
+          - python-version: '3.11'
+
          # Newest macOS and newest Python supported versions.


Please update this comment... 3.10 is no longer the newes Python.

The idea with this comment want to highlight that we only run the tests on a single macOS environemnt.

And that environment is newest macos supported by GitHub + newest Python supported by Twisted.

In the past we used to run tests on macOS-11 ...but the GitHub environmet was broken , so we stopped running those tests.

Maybe, we can also just remote these tests, and only run python 3.11 on macOS.

I don't think that we need 2 macOS jobs.

.github/workflows/test.yaml

adiroiban

Great job. Thanks.

I am requesting changes due to:

release notes
test_flatten

Not adding new jobs for macOS and Windows, would be nice.
we already have a big list of jobs.

glyph · 2022-11-04T19:25:03Z

I left off the comments about "newest" just because this is a general cleanup the file needs and nothing to do with 3.11 support. Anywhere we say something like "newest" or "currently supported", we should really be saying "as of" and a date, but I don't want to put the burden of that kind of a fix onto this PR which is really doing something else. I realize that this makes things even more out of date but each PR should fix one thing at a time and "3.11 support" and "none of the comments refer to the current accurate state of our CI" are not really related :).

adiroiban · 2022-11-04T23:09:01Z

.github/workflows/test.yaml

@@ -122,6 +122,9 @@ jobs:
          # Just Python 3.9 with default settings.
          - python-version: '3.9'

+          # Just Python 3.11 with default settings.
+          - python-version: '3.11'
+
          # Newest macOS and newest Python supported versions.
          - python-version: '3.10'


My suggestion was to use 3.11 here

and also use 3.11 for Windows on line 144

Suggested change

- python-version: '3.10'

- python-version: '3.11'

In this way, we run Windows and macOS python trial tests with latest Python version

Ah, makes sense. Updated.

src/twisted/web/test/test_flatten.py

adiroiban · 2022-11-04T23:14:46Z

src/twisted/web/test/test_flatten.py

                    """
                ),
                flags=re.MULTILINE,
            ),
        )
+        self.assertTrue("RuntimeError: example" in str(failure.value))


I don't think that this is ok.

The previous check already has builtins.RuntimeError: example>> which should be the same thing.

So I don't see any extra value / info / assertion here.

I am not 100% what is the expected output, but as mentioned by Glyph ... getting the same expected regex on 3.11 and other python version would be hard.

As it looks like in 3.11 the default expception rending was changed.

Why, exactly, is this not OK?

What we are trying to test for here is that the exception type is relayed somewhere in the message to the user and we have not, in our custom traceback construction logic, accidentally elided or changed it. The previous check does not have RuntimeError in it any more, it was removed, due to the traceback formatting changes. Ostensibly we could check it with an individual assertion, sure, but is it worth any additional effort to figure out exactly what precise logic in Python's traceback formatting changed to offset this slightly differently than it was before? We're testing Twisted's traceback construction, not Python's traceback formatting.

I guess as long as we're spending so long discussing this line, I could point out that the new assertion loses information, so it would be better to change it to use the unittest idiom for this type of assertion rather than assertTrue (which is almost always the wrong choice and gives no information upon failure)

Suggested change

self.assertTrue("RuntimeError: example" in str(failure.value))

self.assertIn("RuntimeError: example", str(failure.value))

The check is not ok as I think that the previous check already does the same thing.

The assertion can be left, it there is no extra coverage for this assertion.

Read the diff more carefully. The previous check does not check that any more.

Specifically, from the diff view:

I agree that this should be assertIn vs assertTrue, updated.

adiroiban

Thanks. This can be merged.

I think that assertIn is not really needed as assertRegex already does the same job.

@glyph feel free to merge

Thanks you for your help.

Happy to see 3.11 up and running :)

src/twisted/web/test/test_flatten.py

https://build.opensuse.org/request/show/1057659 by user dgarcia + dimstar_suse - Add py311-tests-compat.patch to fix tests with python 3.11 gh#twisted/twisted#11734 gh#twisted/twisted#11733

eevelweezel changed the title ~~running tests on 3.11~~ #10343 - run tests w/ python 3.11 Oct 28, 2022

eevelweezel force-pushed the 10343 branch from b83838d to d5207e1 Compare October 31, 2022 04:33

attempt tests on 3.11

1958686

eevelweezel force-pushed the 10343 branch from d5207e1 to 1958686 Compare November 2, 2022 23:25

eevel added 2 commits November 2, 2022 20:35

fix twisted.persisted tests

4f6d7fb

fix tests for twisted.spread

5253771

eevel added 4 commits November 3, 2022 21:12

fix test for twisted.web

afcc224

fix persisted tests in twisted.test

4b5ab38

fix twisted.trial tests

f8f56d4

make test_flatten backwards-compatible

d91675a

add newsfragment

603c619

chevah-robot added the needs-review label Nov 4, 2022

chevah-robot requested a review from a team November 4, 2022 04:41

glyph approved these changes Nov 4, 2022

View reviewed changes

src/twisted/newsfragments/10343.misc Outdated Show resolved Hide resolved

src/twisted/web/test/test_flatten.py Show resolved Hide resolved

chevah-robot added needs-merge and removed needs-review labels Nov 4, 2022

adiroiban reviewed Nov 4, 2022

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

adiroiban requested changes Nov 4, 2022

View reviewed changes

chevah-robot added needs-changes and removed needs-merge labels Nov 4, 2022

glyph requested review from glyph and removed request for glyph November 4, 2022 19:22

eevel added 2 commits November 4, 2022 15:16

PR feedback

800f9e9

run linter

6b069e1

eevelweezel force-pushed the 10343 branch from b5b6158 to 6b069e1 Compare November 4, 2022 20:37

eevelweezel requested review from adiroiban and removed request for glyph November 4, 2022 22:11

adiroiban reviewed Nov 4, 2022

View reviewed changes

src/twisted/web/test/test_flatten.py Show resolved Hide resolved

adiroiban reviewed Nov 4, 2022

View reviewed changes

eevel added 2 commits November 4, 2022 19:10

use assertIn vs assertTrue

fe8e956

run mac & windows tests w/ 3.11

4c9bd3c

adiroiban approved these changes Nov 5, 2022

View reviewed changes

src/twisted/web/test/test_flatten.py Show resolved Hide resolved

chevah-robot added needs-merge and removed needs-changes labels Nov 5, 2022

glyph merged commit a604afd into twisted:trunk Nov 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#10343 - run tests w/ python 3.11 #11734

#10343 - run tests w/ python 3.11 #11734

eevelweezel commented Oct 28, 2022 •

edited

eevelweezel commented Oct 28, 2022

adiroiban commented Oct 28, 2022

glyph commented Oct 29, 2022

adiroiban commented Oct 31, 2022

adiroiban commented Oct 31, 2022

eevelweezel commented Nov 3, 2022

glyph commented Nov 3, 2022

adiroiban commented Nov 3, 2022

eevelweezel commented Nov 4, 2022

eevelweezel commented Nov 4, 2022

glyph commented Nov 4, 2022

glyph left a comment

adiroiban Nov 4, 2022

adiroiban left a comment

glyph commented Nov 4, 2022

adiroiban Nov 4, 2022 •

edited

eevelweezel Nov 5, 2022

adiroiban Nov 4, 2022 •

edited

glyph Nov 4, 2022 •

edited

glyph Nov 4, 2022

adiroiban Nov 4, 2022

glyph Nov 4, 2022

glyph Nov 4, 2022

eevelweezel Nov 5, 2022

adiroiban left a comment

	self.assertTrue("RuntimeError: example" in str(failure.value))
	self.assertIn("RuntimeError: example", str(failure.value))

#10343 - run tests w/ python 3.11 #11734

#10343 - run tests w/ python 3.11 #11734

Conversation

eevelweezel commented Oct 28, 2022 • edited

Scope and purpose

eevelweezel commented Oct 28, 2022

adiroiban commented Oct 28, 2022

glyph commented Oct 29, 2022

adiroiban commented Oct 31, 2022

adiroiban commented Oct 31, 2022

eevelweezel commented Nov 3, 2022

glyph commented Nov 3, 2022

adiroiban commented Nov 3, 2022

eevelweezel commented Nov 4, 2022

eevelweezel commented Nov 4, 2022

glyph commented Nov 4, 2022

glyph left a comment

Choose a reason for hiding this comment

adiroiban Nov 4, 2022

Choose a reason for hiding this comment

adiroiban left a comment

Choose a reason for hiding this comment

glyph commented Nov 4, 2022

adiroiban Nov 4, 2022 • edited

Choose a reason for hiding this comment

eevelweezel Nov 5, 2022

Choose a reason for hiding this comment

adiroiban Nov 4, 2022 • edited

Choose a reason for hiding this comment

glyph Nov 4, 2022 • edited

Choose a reason for hiding this comment

glyph Nov 4, 2022

Choose a reason for hiding this comment

adiroiban Nov 4, 2022

Choose a reason for hiding this comment

glyph Nov 4, 2022

Choose a reason for hiding this comment

glyph Nov 4, 2022

Choose a reason for hiding this comment

eevelweezel Nov 5, 2022

Choose a reason for hiding this comment

adiroiban left a comment

Choose a reason for hiding this comment

eevelweezel commented Oct 28, 2022 •

edited

adiroiban Nov 4, 2022 •

edited

adiroiban Nov 4, 2022 •

edited

glyph Nov 4, 2022 •

edited