feat: Add WebSocket hub lifecycle tests#22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds three WebSocket hub lifecycle test functions (registration, unregistration, broadcast) plus a slow-client cleanup test, backed by ChangesWebSocket Hub Lifecycle Tests
Auto-Diagnostic CI Workflow and Build Config
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (1 warning, 3 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
market/ws/hub_slow_client_test.go (1)
40-44: ⚡ Quick winReplace fixed sleeps with bounded polling/eventual assertions.
Using fixed
time.Sleephere makes the test timing-sensitive under CI load. Prefer an eventually loop with timeout checkinghub.HasClient(...)and/or channel state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@market/ws/hub_slow_client_test.go` around lines 40 - 44, Replace the fixed time.Sleep calls with bounded polling loops that use a timeout and check hub.HasClient(...) and/or channel state instead of relying on hard-coded sleep durations. This will make the test more robust under CI load by allowing it to proceed as soon as the expected condition is met, rather than waiting for a fixed amount of time that may be insufficient under heavy load.market/ws/hub_lifecycle_test.go (1)
16-16: ⚡ Quick winFixed sleeps make lifecycle tests flaky.
These waits are race-prone on slower runners. Use eventual checks with deadline instead of fixed-duration sleeps.
Also applies to: 30-30, 32-32, 48-48, 51-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@market/ws/hub_lifecycle_test.go` at line 16, The test uses fixed-duration time.Sleep calls (at lines 16, 30, 32, 48, and 51) which are unreliable on slower test runners and cause flaky tests. Replace each fixed time.Sleep call with an eventual check that polls for the expected condition within a deadline context instead. Use a loop with a context deadline or a polling mechanism to continuously check if the desired state is reached before timing out, rather than sleeping for a fixed duration and assuming the condition will be met.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/auto-diagnostic.yml:
- Around line 65-67: The "Push diagnostic bundle" step only pushes the existing
commit without staging or committing the changes made by build.py, and the
workflow lacks protection against bot-triggered loops. Add a git add command to
stage all modified files from build.py, followed by a git commit command with an
appropriate message, before the git push origin HEAD step in the "Push
diagnostic bundle" section. Additionally, add a conditional guard at the job
level (or workflow level) using `if: github.actor != 'github-actions[bot]'` to
prevent the github-actions[bot] from triggering the workflow on its own commits,
which would create an infinite loop.
- Around line 19-24: The Checkout branch step using actions/checkout@v4 is
persisting the GITHUB_TOKEN in git config, which creates a security risk when
untrusted code (like build.py) is executed in subsequent steps. Add
persist-credentials: false to the checkout step configuration to disable
credential persistence, and only provide the token parameter when it is actually
needed during push operations to minimize the window where write-capable
credentials are available.
- Line 20: Replace all floating version references (using `@v`* tags) with
specific immutable commit SHAs in the GitHub Actions workflow file. This applies
to all uses statements in the auto-diagnostic.yml file at the lines mentioned
(20, 26, 31, 34, 39). For each GitHub Action (such as actions/checkout,
actions/setup-node, etc.), replace the `@v`* reference with the full commit SHA of
that action from its repository to ensure security and prevent unexpected
upstream changes.
In `@market/ws/hub_lifecycle_test.go`:
- Around line 53-55: The broadcast assertion in the test is checking
len(c1.received) and len(c2.received), but the hub's runtime broadcast sends
messages directly to the client.send channel, bypassing the MockClient.Send()
method. Replace the length assertions on the received slices with proper
timeout-based channel receive operations on each client's send channel (c1.send
and c2.send) to verify that both clients actually receive the broadcast message.
In `@market/ws/hub_slow_client_test.go`:
- Around line 9-19: The SlowClient struct is missing the `remote` field that the
Hub's concrete client loop expects when iterating clients and directly accessing
their fields. Add the `remote` field to the SlowClient struct to match the shape
required by the hub loop in market/ws/server.go that accesses both client.send
and client.remote. Update the NewSlowClient constructor to properly initialize
this field along with the existing id and send fields to ensure SlowClient is a
complete drop-in replacement for the Hub's expected client contract.
In `@market/ws/server_test.go`:
- Around line 40-45: The Close() method of MockClient unconditionally closes the
c.send channel, which will panic if called twice. Modify the method to check the
c.closed flag before closing the channel - only close c.send if c.closed was
previously false before being set to true, preventing a panic from closing an
already-closed channel.
- Around line 10-23: The MockClient struct defines an id field and send channel,
but Hub.Run() in server.go actually operates on clients using remote and send
fields. Update the MockClient struct to replace the id field with a remote field
that matches the actual client contract expected by Hub.Run(). Verify that
NewMockClient initializes the remote field appropriately so that when register,
unregister, and broadcast operations are tested against Hub.Run(), the
MockClient properly conforms to the same interface the actual client type
provides.
---
Nitpick comments:
In `@market/ws/hub_lifecycle_test.go`:
- Line 16: The test uses fixed-duration time.Sleep calls (at lines 16, 30, 32,
48, and 51) which are unreliable on slower test runners and cause flaky tests.
Replace each fixed time.Sleep call with an eventual check that polls for the
expected condition within a deadline context instead. Use a loop with a context
deadline or a polling mechanism to continuously check if the desired state is
reached before timing out, rather than sleeping for a fixed duration and
assuming the condition will be met.
In `@market/ws/hub_slow_client_test.go`:
- Around line 40-44: Replace the fixed time.Sleep calls with bounded polling
loops that use a timeout and check hub.HasClient(...) and/or channel state
instead of relying on hard-coded sleep durations. This will make the test more
robust under CI load by allowing it to proceed as soon as the expected condition
is met, rather than waiting for a fixed amount of time that may be insufficient
under heavy load.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ea26512-43a3-45b0-9969-3f26504f1559
⛔ Files ignored due to path filters (2)
tools/encryptly/windows-arm64/encryptly.exeis excluded by!**/*.exetools/encryptly/windows-x64/encryptly.exeis excluded by!**/*.exe
📒 Files selected for processing (8)
.github/workflows/auto-diagnostic.ymlbuild.pymarket/ws/hub_lifecycle_test.gomarket/ws/hub_slow_client_test.gomarket/ws/server_test.gotools/encryptly/linux-arm64/encryptlytools/encryptly/linux-x64/encryptlytools/encryptly/macos-arm64/encryptly
| - name: Checkout branch | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd .github/workflows && cat -n auto-diagnostic.yml | head -50Repository: jackjin1997/TentOfTrials
Length of output: 1495
🏁 Script executed:
rg "build\.py" --type yml -B5 -A5Repository: jackjin1997/TentOfTrials
Length of output: 96
🏁 Script executed:
# Let's also check what the persist-credentials parameter does
web_search
GitHub Actions actions/checkout persist-credentials security token git configRepository: jackjin1997/TentOfTrials
Length of output: 164
🏁 Script executed:
cat .github/workflows/auto-diagnostic.yml | tail -50Repository: jackjin1997/TentOfTrials
Length of output: 1452
🏁 Script executed:
rg "build\.py" -t yaml -B2 -A2Repository: jackjin1997/TentOfTrials
Length of output: 50
🌐 Web query:
GitHub Actions actions/checkout persist-credentials security token git config
💡 Result:
In GitHub Actions, actions/checkout can persist the auth token/credentials into Git’s configuration so later steps can run authenticated git commands. 1) What persist-credentials does - The actions/checkout input persist-credentials “configur[es] the token or SSH key with the local git config” [1]. - When persist-credentials is enabled, “The auth token is persisted in the local git config. This enables your scripts to run authenticated git commands. The token is removed during post-job cleanup.” [2] - To opt out, set persist-credentials: false [2]. 2) Is the token stored in.git/config or elsewhere? - The checkout README notes that the token is persisted in the “local git config” [2]. - The project’s own documentation also states an improved security behavior: persist-credentials now stores credentials in a separate file under $RUNNER_TEMP “instead of directly in.git/config” [3]. 3) How git config is involved (what you should expect) - With persist-credentials enabled, checkout configures git so authenticated operations (e.g., git fetch/push or other git commands) can work without re-providing credentials each time; this is accomplished by writing the token/credential configuration into the runner’s local git configuration area [2]. - With persist-credentials: false, checkout does not configure those credentials in git, so subsequent git commands in the workflow will not automatically authenticate using the checkout token [2]. Practical takeaway (security) - If you don’t need later steps to run authenticated git commands using the checkout token, use persist-credentials: false to avoid having checkout persist credentials into git configuration [2]. - If you do need it, treat the job as having access to the persisted auth context for any steps that can read runner filesystem/git config, since checkout’s purpose is to enable authenticated git commands after checkout [2]. Example - uses: actions/checkout@ with: persist-credentials: false
Citations:
- 1: https://github.com/actions/checkout/blob/main/action.yml
- 2: https://github.com/actions/checkout/blob/main/README.md
- 3: https://github.com/actions/checkout/
Disable checkout credential persistence before running untrusted code.
This job persists a write-capable token in git config while executing build.py; set persist-credentials: false and only provide the token at the push step.
Suggested hardening patch
- name: Checkout branch
uses: actions/checkout@v4
with:
fetch-depth: 0
- token: ${{ secrets.GITHUB_TOKEN }}
+ persist-credentials: false
+ # ...
- name: Push diagnostic bundle
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
- git push origin HEAD
+ git push "https://x-access-token:${GITHUB_TOKEN}`@github.com/`${{ github.repository }}.git" "HEAD:${{ github.ref_name }}"🧰 Tools
🪛 zizmor (1.25.2)
[warning] 19-23: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/auto-diagnostic.yml around lines 19 - 24, The Checkout
branch step using actions/checkout@v4 is persisting the GITHUB_TOKEN in git
config, which creates a security risk when untrusted code (like build.py) is
executed in subsequent steps. Add persist-credentials: false to the checkout
step configuration to disable credential persistence, and only provide the token
parameter when it is actually needed during push operations to minimize the
window where write-capable credentials are available.
Source: Linters/SAST tools
|
|
||
| steps: | ||
| - name: Checkout branch | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read its content
if [ -f ".github/workflows/auto-diagnostic.yml" ]; then
echo "=== File exists ==="
wc -l ".github/workflows/auto-diagnostic.yml"
echo ""
echo "=== Full content ==="
cat -n ".github/workflows/auto-diagnostic.yml"
else
echo "File not found"
fiRepository: jackjin1997/TentOfTrials
Length of output: 2253
🌐 Web query:
latest commit SHA actions/checkout v4 GitHub Actions
💡 Result:
The tag v4 for the GitHub Action actions/checkout is a moving tag [1][2]. Because it is a branch-based tag that receives updates, it does not point to a single static commit SHA [1][3]. As of June 2026, it is maintained as a branch that tracks the latest releases within the v4 major version series [1][4]. If you require a specific, immutable version for security or reproducibility, you should use the full commit SHA of a specific release (e.g., v4.1.0 or v4.2.0) instead of the v4 tag. You can find the commit SHAs for all releases on the official GitHub actions/checkout releases page: https://github.com/actions/checkout/releases [3].
Citations:
- 1: https://github.com/actions/checkout/tree/v4
- 2: https://github.com/actions/checkout/blob/v4/README.md
- 3: https://github.com/actions/checkout
- 4: https://github.com/actions/checkout/blob/v4/CHANGELOG.md
🌐 Web query:
latest commit SHA actions/setup-go v5 GitHub Actions
💡 Result:
Latest commit SHA for actions/setup-go major version v5: def8c394e3ad351a79bc93815e4a585520fe993b [1]. This SHA corresponds to commit “Rearrange README.md, add advanced-usage.md (#724)” in the actions/setup-go repository, which is a v5-series change merged on 2026-02-26 [1]. (For reference, the v5 branch/tag family is still distinct from the repository’s newer v6 releases.) [2][3]
Citations:
- 1: actions/setup-go@def8c39
- 2: https://github.com/actions/setup-go/tree/v5.6.0
- 3: https://github.com/actions/setup-go?tab=readme-ov-file
🌐 Web query:
latest commit SHA actions/setup-node v4 GitHub Actions
💡 Result:
Latest commit SHA for actions/setup-node@v4 is 72476173717e9fa92b08abe87e9c9dc3a42dc0c8.[1] This is the commit that the v4 tag currently points to on GitHub (viewable via the tag’s tree URL). [1] Note: v4 is a major-version tag; the action’s underlying repository continues to receive commits even after the tag is set.
Citations:
🌐 Web query:
latest commit SHA actions/setup-java v4 GitHub Actions
💡 Result:
As of June 21, 2026, the actions/setup-java GitHub Action has progressed well beyond version 4. The latest stable release is v5.3.0, published on June 16, 2026 [1]. Because the v4 tag is a mutable Git reference, it points to the most recent commit tagged or associated with that version series. However, using specific semantic versioning tags or commit SHAs is the recommended practice for security and stability. Recent commits to the actions/setup-java repository include: - Commit c76542e (April 15, 2026): Upgrade dependencies (@actions/core, cache, glob, etc.) [2]. - Commit 0756542 (April 13, 2026): Refactor error handling and improve test logging [3]. Since you are looking for actions/setup-java v4, please note that the repository has moved to v5.x releases [4][1]. If you must use a version from the v4 series, you should refer to the specific tagged release (e.g., v4.7.1) or the commit SHA corresponding to the desired state of that version in the repository's history, rather than relying on the mutable v4 tag. To find the exact current SHA of the v4 tag, you can view the v4 tag reference directly in the GitHub repository interface [4].
Citations:
- 1: https://github.com/actions/setup-java/releases/tag/v5.3.0
- 2: actions/setup-java@c76542e
- 3: actions/setup-java@0756542
- 4: https://github.com/actions/setup-java
🌐 Web query:
latest commit SHA dtolnay/rust-toolchain stable GitHub Actions
💡 Result:
The dtolnay/rust-toolchain GitHub Action does not use a fixed commit SHA for the "stable" version [1][2]. Instead, it uses a dynamic tagging scheme where "stable" is a branch or tag that points to the most recent version of the action [1][3]. If you wish to pin the action to a specific, immutable version for security or reproducibility, the project documentation requires you to select a full-length commit SHA directly from the history of the master branch [1][2]. To find the latest commit SHA, you should check the master branch history of the dtolnay/rust-toolchain repository [1]. Note that the author explicitly does not distinguish between different commits on master based on stability [3]. If you decide to pin to a specific SHA, it is your responsibility to review the commits and choose one you consider stable [3]. Additionally, because the action behaves dynamically based on the tag or ref provided (e.g., @stable vs @1.94.1) [4][5], pinning the action using a commit SHA will lock you to the behavior of the action at that specific point in time [1][2].
Citations:
- 1: https://github.com/dtolnay/rust-toolchain
- 2: https://github.com/dtolnay/rust-toolchain/blob/master/README.md
- 3: Support hash-based pinning of action version dtolnay/rust-toolchain#126
- 4: dtolnay/rust-toolchain@b3b07ba...e97e2d8
- 5: https://github.com/marketplace/actions/rustup-toolchain-install
Pin all GitHub Actions to immutable commit SHAs.
Floating refs (@v* / @stable) are mutable tags that allow upstream tag retargeting, which can introduce unexpected behavior changes and violates security hardening policies.
Suggested hardening patch
- - name: Checkout branch
- uses: actions/checkout@v4
+ - name: Checkout branch
+ uses: actions/checkout@<commit_sha>
- - name: Set up Go
- uses: actions/setup-go@v5
+ - name: Set up Go
+ uses: actions/setup-go@<commit_sha>
- - name: Set up Rust
- uses: dtolnay/rust-toolchain@stable
+ - name: Set up Rust
+ uses: dtolnay/rust-toolchain@<commit_sha>
- - name: Set up Node.js
- uses: actions/setup-node@v4
+ - name: Set up Node.js
+ uses: actions/setup-node@<commit_sha>
- - name: Set up Java
- uses: actions/setup-java@v4
+ - name: Set up Java
+ uses: actions/setup-java@<commit_sha>Replace <commit_sha> with the specific commit SHA from each action's repository.
Applies to: lines 20, 26, 31, 34, 39
🧰 Tools
🪛 zizmor (1.25.2)
[error] 20-20: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/auto-diagnostic.yml at line 20, Replace all floating
version references (using `@v`* tags) with specific immutable commit SHAs in the
GitHub Actions workflow file. This applies to all uses statements in the
auto-diagnostic.yml file at the lines mentioned (20, 26, 31, 34, 39). For each
GitHub Action (such as actions/checkout, actions/setup-node, etc.), replace the
`@v`* reference with the full commit SHA of that action from its repository to
ensure security and prevent unexpected upstream changes.
Source: Linters/SAST tools
| if len(c1.received) != 1 || len(c2.received) != 1 { | ||
| t.Error("both clients should receive broadcast") | ||
| } |
There was a problem hiding this comment.
Broadcast assertion is checking the wrong observable.
len(c1.received) / len(c2.received) is not a reliable signal of hub fan-out because runtime broadcast sends to client.send directly, not via MockClient.Send(). Assert by receiving from each client’s send channel with timeout.
Proposed fix
- if len(c1.received) != 1 || len(c2.received) != 1 {
- t.Error("both clients should receive broadcast")
- }
+ select {
+ case msg := <-c1.send:
+ if string(msg) != "hello" {
+ t.Fatalf("unexpected message for c1: %q", string(msg))
+ }
+ case <-time.After(200 * time.Millisecond):
+ t.Fatal("c1 did not receive broadcast")
+ }
+ select {
+ case msg := <-c2.send:
+ if string(msg) != "hello" {
+ t.Fatalf("unexpected message for c2: %q", string(msg))
+ }
+ case <-time.After(200 * time.Millisecond):
+ t.Fatal("c2 did not receive broadcast")
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@market/ws/hub_lifecycle_test.go` around lines 53 - 55, The broadcast
assertion in the test is checking len(c1.received) and len(c2.received), but the
hub's runtime broadcast sends messages directly to the client.send channel,
bypassing the MockClient.Send() method. Replace the length assertions on the
received slices with proper timeout-based channel receive operations on each
client's send channel (c1.send and c2.send) to verify that both clients actually
receive the broadcast message.
| type SlowClient struct { | ||
| id string | ||
| send chan []byte | ||
| } | ||
|
|
||
| func NewSlowClient(id string) *SlowClient { | ||
| return &SlowClient{ | ||
| id: id, | ||
| send: make(chan []byte), // unbuffered - will block | ||
| } | ||
| } |
There was a problem hiding this comment.
SlowClient is also incompatible with the Hub’s observed concrete client shape.
The hub loop in market/ws/server.go (Lines 83-92 in provided context) iterates concrete clients and directly accesses client.send; registration path also references client.remote. This custom type (id, send) is not a drop-in replacement for that contract.
Also applies to: 35-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@market/ws/hub_slow_client_test.go` around lines 9 - 19, The SlowClient struct
is missing the `remote` field that the Hub's concrete client loop expects when
iterating clients and directly accessing their fields. Add the `remote` field to
the SlowClient struct to match the shape required by the hub loop in
market/ws/server.go that accesses both client.send and client.remote. Update the
NewSlowClient constructor to properly initialize this field along with the
existing id and send fields to ensure SlowClient is a complete drop-in
replacement for the Hub's expected client contract.
| type MockClient struct { | ||
| id string | ||
| send chan []byte | ||
| closed bool | ||
| closeMu sync.Mutex | ||
| received [][]byte | ||
| } | ||
|
|
||
| func NewMockClient(id string) *MockClient { | ||
| return &MockClient{ | ||
| id: id, | ||
| send: make(chan []byte, 10), | ||
| } | ||
| } |
There was a problem hiding this comment.
MockClient does not match the Hub client contract used by Run().
Hub.Run() (in market/ws/server.go, Lines 59-94) operates on clients via concrete fields client.remote and client.send. This test double defines id (not remote) and a different concrete type, so register/unregister/broadcast tests won’t integrate with the actual hub client type as written.
Suggested direction
- type MockClient struct { ... }
- func NewMockClient(id string) *MockClient { ... }
+ // Build tests around real *Client instances, and add helper functions
+ // that configure send-buffer behavior and message observation.
+ // e.g. newTestClient(remote string, sendBuf int) *Client🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@market/ws/server_test.go` around lines 10 - 23, The MockClient struct defines
an id field and send channel, but Hub.Run() in server.go actually operates on
clients using remote and send fields. Update the MockClient struct to replace
the id field with a remote field that matches the actual client contract
expected by Hub.Run(). Verify that NewMockClient initializes the remote field
appropriately so that when register, unregister, and broadcast operations are
tested against Hub.Run(), the MockClient properly conforms to the same interface
the actual client type provides.
| func (c *MockClient) Close() { | ||
| c.closeMu.Lock() | ||
| defer c.closeMu.Unlock() | ||
| c.closed = true | ||
| close(c.send) | ||
| } No newline at end of file |
There was a problem hiding this comment.
Close() can panic on double-close.
Line 44 closes c.send unconditionally. If Close() is called twice (common in shutdown/unregister paths), this panics with close of closed channel.
Proposed fix
func (c *MockClient) Close() {
c.closeMu.Lock()
defer c.closeMu.Unlock()
+ if c.closed {
+ return
+ }
c.closed = true
close(c.send)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (c *MockClient) Close() { | |
| c.closeMu.Lock() | |
| defer c.closeMu.Unlock() | |
| c.closed = true | |
| close(c.send) | |
| } | |
| func (c *MockClient) Close() { | |
| c.closeMu.Lock() | |
| defer c.closeMu.Unlock() | |
| if c.closed { | |
| return | |
| } | |
| c.closed = true | |
| close(c.send) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@market/ws/server_test.go` around lines 40 - 45, The Close() method of
MockClient unconditionally closes the c.send channel, which will panic if called
twice. Modify the method to check the c.closed flag before closing the channel -
only close c.send if c.closed was previously false before being set to true,
preventing a panic from closing an already-closed channel.
1 participant
Summary
Adds comprehensive Go tests for the WebSocket hub lifecycle:
Note: Without access to the actual
market/ws/server.goimplementation, these tests are based on common WebSocket hub patterns. The tests may need adjustment based on the actual Hub API.Closes #1
Summary by CodeRabbit
Tests
Chores