mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
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
104 lines
3.1 KiB
Go
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")
|
|
}
|
|
}
|