Skip to content
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

#11771 attempt a fix for cfreactor flakiness #12150

Draft
wants to merge 65 commits into
base: trunk
Choose a base branch
from
Draft

Conversation

glyph
Copy link
Member

@glyph glyph commented May 1, 2024

Scope and purpose

Fixes #11771

@glyph
Copy link
Member Author

glyph commented May 1, 2024

The fact that twisted.internet.test.test_fdset.ReactorFDSetTestsBuilder_CFReactorTests.test_connectionLostOnShutdown failed is interesting. That test does nothing but start up, add a couple of file descriptors which never do anything, then shut itself down. The fact that it's hanging suggests that there's an error in the logic that sets up _scheduleSimulate to always run.

@glyph
Copy link
Member Author

glyph commented May 2, 2024

OK, same test still flaky. The fact that the reactor always says that it couldn't stop a reactor that is already running suggests that somehow the application's call to reactor.stop() is happening, but it's not happening until the reactor wakes up for the framework's timeout stop().

I guess I will try to do some print statement debugging in CI to validate this theory.

@glyph
Copy link
Member Author

glyph commented May 2, 2024

dangit fail

@glyph
Copy link
Member Author

glyph commented May 3, 2024

The failure on try 2 is this

===============================================================================
[FAIL]
Traceback (most recent call last):
  File "/Users/runner/work/twisted/twisted/.tox/macos-withcov-alldeps/lib/python3.12/site-packages/twisted/internet/test/test_udp.py", line 103, in test_connectionLostLogMessage
    self.assertEqual((expectedMessage,), loggedMessages[0]["message"])
  File "/Users/runner/work/twisted/twisted/.tox/macos-withcov-alldeps/lib/python3.12/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
    super().assertEqual(first, second, msg)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 885, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 1102, in assertTupleEqual
    self.assertSequenceEqual(tuple1, tuple2, msg, seq_type=tuple)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 1073, in assertSequenceEqual
    self.fail(msg)
twisted.trial.unittest.FailTest: Tuples differ: ('(UDP Port 63035 Closed)',) != ()

First tuple contains 1 additional elements.
First extra element 0:
'(UDP Port 63035 Closed)'

- ('(UDP Port 63035 Closed)',)
+ ()

twisted.internet.test.test_udp.UDPFDServerTestsBuilder_CFReactorTests.test_connectionLostLogMessage

which… coincidentally is on the CF reactor, but seems to be unrelated to the core issue here

@glyph
Copy link
Member Author

glyph commented May 3, 2024

The failure on try 3 is similar:

Traceback (most recent call last):
  File "/Users/runner/work/twisted/twisted/.tox/macos-withcov-alldeps/lib/python3.12/site-packages/twisted/internet/test/test_udp.py", line 103, in test_connectionLostLogMessage
    self.assertEqual((expectedMessage,), loggedMessages[0]["message"])
  File "/Users/runner/work/twisted/twisted/.tox/macos-withcov-alldeps/lib/python3.12/site-packages/twisted/trial/_synctest.py", line 444, in assertEqual
    super().assertEqual(first, second, msg)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 885, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 1102, in assertTupleEqual
    self.assertSequenceEqual(tuple1, tuple2, msg, seq_type=tuple)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 1073, in assertSequenceEqual
    self.fail(msg)
twisted.trial.unittest.FailTest: Tuples differ: ('(UDP Port 57724 Closed)',) != ()

First tuple contains 1 additional elements.
First extra element 0:
'(UDP Port 57724 Closed)'

- ('(UDP Port 57724 Closed)',)
+ ()

twisted.internet.test.test_udp.UDPFDServerTestsBuilder_CFReactorTests.test_connectionLostLogMessage

… still the CF reactor, again, which I don't love. Will investigate but I think it's just a different flake

@glyph glyph marked this pull request as draft May 3, 2024 18:16
@glyph
Copy link
Member Author

glyph commented May 3, 2024

Converting this to a draft PR because I am pretty sure #12160 fixed the actual issue here, but this branch contains some useful code (particularly the disttrial debug-print stuff, which should be spun out into a PR addressing #12163 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants