Files
OwnCord/Server/ws/topic_rate_limiter_test.go
Claude 33a6b23cde fix: resolve golangci-lint failures and correct audit-doc inaccuracies
A pre-merge review caught things my local verification missed, because two CI
gates could not run in the sandbox and I mis-read a third.

golangci-lint (BLOCKER — would have turned CI red on both server matrix legs).
My local binary was built for Go 1.25 against a repo targeting 1.26, so I could
not run it. Installed 2.11.3 with the repo's own toolchain: 5 issues, all in
files this PR adds, base clean. Now 0 issues:
- bodyclose x3 in api/livekit_proxy_ws_test.go — websocket.Dial's *http.Response
  was discarded; adopt the repo's existing pattern from ws/ws_integration_test.go
- gocritic stringXbytes — string(got) != string(payload) -> !bytes.Equal
- staticcheck SA4000 in ws/topic_rate_limiter_test.go — `!Allow() || !Allow()`.
  This was a real defect, not just a lint: || short-circuits, so a failing first
  call skipped the second, left a token unspent, and the next assertion would
  have reported the wrong thing. Split into two statements.

Playwright: I reported this suite as passing. It does not. I read the exit code
of `tail` through a pipeline instead of playwright's own. Re-run properly: 229
of 255 web tests fail, all cascading from the shared login helper
(navigateToMainPage never sees [data-testid='app-layout']). It reproduces on a
clean b3caceb worktree, so it is pre-existing on main and unrelated to this
diff — but it was never true that I had verified it. Recorded as new finding
T-2026-07-25-21 and promoted to backlog #2; the client-e2e job stays
continue-on-error and now carries timeout-minutes so a red suite cannot burn
unbounded Actions minutes. rust-tests gets a timeout too.

Audit-doc corrections (all confirmed by re-measurement):
- screenShare.ts was listed as "untouched by this pass" at 61.1% when this PR
  takes it to 100%; T-12's wording made it the exception when it is the best
- IsEitherBlocked was NOT zero-coverage — 83.3% at base via message_test.go
- excluded LOC 2,229 -> 1,827
- "40 Playwright spec files" -> 44 (33 web + 11 native), 255 web tests
- HandleLiveKitHealthForTest callers: eight -> seven
- admin coverage: 71.4% is with only the T-01 fix; 77.9% with this PR's tests

Verified after the fixes: golangci-lint 0 issues, go vet, go test -race,
go test -tags deadlock, vitest 94.87%, cargo test --lib 74/74, tsc, prettier.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AEETs3Vh6sAHHb1jMBL75g
2026-07-28 12:25:10 +00:00

104 lines
3.1 KiB
Go

package ws
import (
"testing"
"time"
)
// TopicRateLimiter.Cleanup is the only thing bounding the bucket map: a busy
// server sees a bucket per topic, and topics include per-channel and per-DM
// values, so without the sweep the map grows for the life of the process.
// It had no coverage. This test lives in package ws so it can read the
// unexported bucket map directly rather than inferring size from behaviour.
func (trl *TopicRateLimiter) bucketCount() int {
trl.mu.Lock()
defer trl.mu.Unlock()
return len(trl.buckets)
}
func TestTopicRateLimiter_Cleanup_RemovesStaleBuckets(t *testing.T) {
trl := NewTopicRateLimiter(10, time.Second)
trl.Allow(Topic("channel:1"))
trl.Allow(Topic("channel:2"))
if got := trl.bucketCount(); got != 2 {
t.Fatalf("bucketCount = %d after two topics, want 2", got)
}
// Backdate one bucket so it falls outside the max age.
trl.mu.Lock()
trl.buckets[Topic("channel:1")].lastReset = time.Now().Add(-time.Hour)
trl.mu.Unlock()
trl.Cleanup(30 * time.Minute)
if got := trl.bucketCount(); got != 1 {
t.Fatalf("bucketCount = %d after cleanup, want 1", got)
}
trl.mu.Lock()
_, staleSurvives := trl.buckets[Topic("channel:1")]
_, freshSurvives := trl.buckets[Topic("channel:2")]
trl.mu.Unlock()
if staleSurvives {
t.Error("the stale bucket survived Cleanup")
}
if !freshSurvives {
t.Error("Cleanup removed a bucket that was still within maxAge")
}
}
func TestTopicRateLimiter_Cleanup_KeepsFreshBuckets(t *testing.T) {
trl := NewTopicRateLimiter(10, time.Second)
trl.Allow(Topic("channel:1"))
trl.Cleanup(time.Hour)
if got := trl.bucketCount(); got != 1 {
t.Errorf("bucketCount = %d, want the fresh bucket to survive", got)
}
}
func TestTopicRateLimiter_Cleanup_EmptyMap(t *testing.T) {
trl := NewTopicRateLimiter(10, time.Second)
trl.Cleanup(time.Minute) // must not panic on an empty map
if got := trl.bucketCount(); got != 0 {
t.Errorf("bucketCount = %d, want 0", got)
}
}
func TestTopicRateLimiter_Allow_EnforcesQuotaThenRefills(t *testing.T) {
trl := NewTopicRateLimiter(2, 50*time.Millisecond)
topic := Topic("channel:1")
// Kept as two statements rather than `a || b`: `||` short-circuits, so a
// failing first call would skip the second and leave a token unspent —
// the third Allow below would then be within quota and the test would
// report the wrong thing.
if !trl.Allow(topic) {
t.Fatal("the first message was rejected despite a quota of 2")
}
if !trl.Allow(topic) {
t.Fatal("the second message was rejected despite a quota of 2")
}
if trl.Allow(topic) {
t.Error("a third message was allowed within the same window")
}
// A separate topic has its own bucket — one busy channel must not starve
// the others, which is the whole point of the per-topic limiter.
if !trl.Allow(Topic("channel:2")) {
t.Error("a different topic was rate limited by channel:1's usage")
}
// After the window elapses the bucket refills.
trl.mu.Lock()
trl.buckets[topic].lastReset = time.Now().Add(-time.Second)
trl.mu.Unlock()
if !trl.Allow(topic) {
t.Error("the bucket did not refill after its window elapsed")
}
}