Repository navigation
test(py): quarantine the TCP close test and plan its fix - #8359
Conversation
Under load the reader got Unroutable after the endpoint closed, which the test's accepted shutdown errors leave out. Skip it and plan the fix under the test-flakes-2 epic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 11 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
pytest.mark.skipquarantine:py/moq-rs/tests/test_socket.py'stest_tcp_rst_and_endpoint_close_are_errorsis skipped, with a comment and reason naming its follow-up quest; the rest of the module is untouched.- Follow-up quest:
quest/m0/test-flakes-2/tcp-close-unroutable.mdis a new[S]child that records the observedUnroutableaftera.close()and asks whether the socket contract should promise it, or the cause should be fixed soCancelled/Closedwins. - Epic update:
quest/m0/test-flakes-2/README.mdlogs the quarantine under the 2026-11-09 entries and lists the child in## Required, matching the quest's title and path.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
|
|
||
| # Quarantined: under load the reader once got `Unroutable` after `a.close()`, which the | ||
| # accepted shutdown errors leave out. See quest/m0/test-flakes-2/tcp-close-unroutable.md. | ||
| @pytest.mark.skip(reason="flaky under load: quest/m0/test-flakes-2/tcp-close-unroutable.md") |
There was a problem hiding this comment.
Only the endpoint-close half of this test flakes. The reset assertions just below (abort(17) reaching the reader as Protocol with code 64 + 17) are deterministic, and no other Python test asserts the app code after a stream reset. Splitting that half into its own test would keep it in the gate while the close half stays quarantined.
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed 135249b. No actionable findings in this quarantine-only change. The skip targets the reported combined TCP reset/endpoint-close test, and both the skip reason and epic link point to the new follow-up. That follow-up preserves the required reset application-code and non-clean-EOF assertions rather than accepting arbitrary errors. The underlying shutdown error remains unresolved, and this test's coverage is disabled until the follow-up restores it.
Verification limits: GitHub-only static diff review; no Python tests or shutdown-race reproduction performed.
Summary
Quarantines
py/moq-rs/tests/test_socket.py'stest_tcp_rst_and_endpoint_close_are_errorsunder the repository's flaky-test rule and plans its fix as a child ofquest/m0/test-flakes-2.It failed once in moq-dev/moq#8293's loaded
just check. Aftera.close(), the reader gotUnroutable, but the test accepts onlyCancelledorClosed. Run alone, it then passed 30 of 30 times. The test uses an in-process origin with no lite-07 session, so the change in #8293 can't reach it.pytest.mark.skipon the test, with a comment that points to the quest.quest/m0/test-flakes-2/tcp-close-unroutable.md[S]: decide whetherUnroutableis a promised error after an endpoint close, or fix the cause soCancelledorClosedwins.Impact
Test and quest only. No change to the public API or the wire format.
Review
Not reviewed: this change only skips a test and adds a quest.
🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with high effort)