Repository navigation
test(e2e): arm the console-pty session before firing Terraria path A - #572
Merged
Merged
Conversation
Verify pod-attach stdout reader is live before the action's separate attach fires by sending a probe line and waiting for its TTY echo. Adds startPTYReader and awaitPTYMarker helpers to handle the unified frame channel across arming and marker wait phases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MnGTGHzHhFQCa22wRknUS Signed-off-by: valgulnecron <39313199+ValgulNecron@users.noreply.github.com>
ValgulNecron
added
area: e2e
|
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 5 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
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. Comment |
CI Report - 3c26739run 37343673804 - attempt 1 0 failed - 14 passed - 6 skipped - 30m42s elapsed Coverageno coverage/* commit statuses found on 3c26739 Changed
Slowest jobs
e2e buckets run
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the flaky
TestGameServer_TerrariaBot_Joinedfailure: "pod attach unreachable: console-pty WS read failed before<gs>-pathAappeared: … context deadline exceeded". It failed three times on 2026-10-05, on #561, #565 and #571, and passed each time on re-run.Root cause (triage, about 75% confidence):
runControlActionStdinPTYfired the action POST as soon as the console-pty WebSocket upgrade completed. The API starts its pod-attach session to the kubelet only afterwebsocket.Accept. The action writes through a second, separate attach, and attach output is live-tail only. A write that landed before the console-pty reader was attached was therefore lost, and the test waited out its 30-second read. Failing runs took about 329 s, against 313 s for a passing run.Fix (test only, in
test/e2e/gamebot_helpers_e2e_test.go):dialAuthedWS, the helper arms the session. It sends a probe stdin frame (<gs>-arm, in the same{"kind":"stdin","body":base64}format thatapi/internal/ws/attach.goexpects). It re-sends the probe every 2 s for up to 60 s, until the echo comes back on this same session. The action POST is fired only after that.startPTYReader) serves both phases. There are no per-Read timeouts, because coder/websocket closes the connection when a Read context expires. Cancellation closes the connection, so no goroutine or connection leaks.<gs>-pathAmarker, and the same 30 s deadline starting after the POST. Matching now uses a rolling tail, so a marker split across frames still matches.Changing this existing e2e helper was approved by the maintainer.
Follow-up (not in this PR): make the e2e Terraria spec use the module's console bridge, so Path A asserts the game's own output.
terraria_bot_e2e_test.go:106-114still describes the old expectation.Test plan
lint (test/e2e)(golangci-lint with--build-tags=e2e)e2e game bot (kind)green; re-run a few times to confirm the flake is goneNothing was built or run locally, at the maintainer's request; CI is the verification. Triaged and briefed at sonnet, implemented at haiku, reviewed at opus.
🤖 Generated with Claude Code
https://claude.ai/code/session_018MnGTGHzHhFQCa22wRknUS