From 5dd6822d807e3352fe4452a3b071e043d958a020 Mon Sep 17 00:00:00 2001 From: zarazaex69 Date: Mon, 1 Jun 2026 15:44:59 +0300 Subject: [PATCH] refactor(jitsi): clean up linter suppressions and test helpers --- internal/engine/jitsi/jitsi.go | 10 +--- internal/engine/jitsi/jitsi_test.go | 4 +- internal/engine/jitsi/keepalive_test.go | 2 +- internal/engine/jitsi/paired_chaos_test.go | 65 ++++++++++++---------- 4 files changed, 43 insertions(+), 38 deletions(-) diff --git a/internal/engine/jitsi/jitsi.go b/internal/engine/jitsi/jitsi.go index bbaedde..6eb2695 100644 --- a/internal/engine/jitsi/jitsi.go +++ b/internal/engine/jitsi/jitsi.go @@ -55,11 +55,7 @@ const ( // Without this, the peer's own recovery (which produces a fresh epoch) // drives us into an infinite reconnect loop. reconnectGrace = 20 * time.Second - // stableUptime is how long the bridge must stay healthy before the - // reconnectCount is reset. Without this, healthy reconnects accumulated - // over hours of operation eventually cross maxReconnects and the engine - // gives up on a perfectly recoverable failure. - stableUptime = 60 * time.Second + // xmppKeepaliveInterval keeps the underlying XMPP transport alive while // we wait for a peer. BOSH has no built-in stream management; without // any application traffic Prosody closes the BOSH session after roughly @@ -666,7 +662,7 @@ func (s *Session) negotiatePC(ctx context.Context, jSess *j.Session, sctpBridge // after the default 1-minute inactivity timeout, which causes JVB to // shut down the DTLS session and emit close_notify. s.wg.Add(1) - go s.rtcpKeepalive(pcCtx, pc) + go s.rtcpKeepalive(pcCtx, pc) //nolint:contextcheck // pcCtx intentionally derives from s.runCtx to outlive this call return nil } @@ -1297,7 +1293,7 @@ func (s *Session) inReconnectGrace() bool { // magic. A non-olcrtc participant in the same MUC (a regular Jitsi web // client, an unrelated bot, etc.) gets filtered out before we ever // get here. -func (s *Session) peerLatchAccepts(from string) bool { +func (s *Session) peerLatchAccepts(from string) bool { //nolint:unparam // filter contract; always-true is policy if cur := s.peerEndpoint.Load(); cur != nil { if *cur == from { return true diff --git a/internal/engine/jitsi/jitsi_test.go b/internal/engine/jitsi/jitsi_test.go index ff52e72..8b961c0 100644 --- a/internal/engine/jitsi/jitsi_test.go +++ b/internal/engine/jitsi/jitsi_test.go @@ -180,11 +180,11 @@ func TestSanitiseNick(t *testing.T) { raw string want string }{ - {"alice", "alice"}, + {nameAlice, nameAlice}, {"Alice Smith", "Alice-Smith"}, {"Конрад Олег", "Konrad-Oleg"}, {"olcrtc-bot42", "olcrtc-bot42"}, - {" bob ", "bob"}, + {" bob ", nameBob}, {"$$$ %%%", ""}, {"verylongnicknamethatexceedslimit", "verylongnicknamet"[:16]}, } diff --git a/internal/engine/jitsi/keepalive_test.go b/internal/engine/jitsi/keepalive_test.go index 1ef969e..c3155b2 100644 --- a/internal/engine/jitsi/keepalive_test.go +++ b/internal/engine/jitsi/keepalive_test.go @@ -282,7 +282,7 @@ func TestRequestReconnectIdempotent(t *testing.T) { js.SetShouldReconnect(func() bool { return true }) var wg sync.WaitGroup - for i := 0; i < 10; i++ { + for range 10 { wg.Add(1) go func() { defer wg.Done() diff --git a/internal/engine/jitsi/paired_chaos_test.go b/internal/engine/jitsi/paired_chaos_test.go index 6615c12..a9543a8 100644 --- a/internal/engine/jitsi/paired_chaos_test.go +++ b/internal/engine/jitsi/paired_chaos_test.go @@ -48,12 +48,12 @@ package jitsi import ( "context" + "errors" "fmt" "math/rand" "os" "strings" "sync" - "sync/atomic" "testing" "time" @@ -67,6 +67,16 @@ const ( envPairedIdle = "OLCRTC_JITSI_PAIRED_IDLE" envPairedChaosInterval = "OLCRTC_JITSI_PAIRED_CHAOS_INTERVAL" envPairedVerbose = "OLCRTC_JITSI_PAIRED_VERBOSE" + + nameAlice = "alice" + nameBob = "bob" +) + +var ( + errCastFailed = errors.New("cast to *Session failed") + errBridgeTimeout = errors.New("bridge not ready before deadline") + errRoundtripTimeout = errors.New("roundtrip timeout") + errReceiveTimeout = errors.New("receive timeout") ) type pairedConfig struct { @@ -84,7 +94,7 @@ func (c *pairedConfig) durationLabel() string { return c.duration.String() } -func readPairedConfig(t *testing.T) *pairedConfig { +func readPairedConfig(t *testing.T) *pairedConfig { //nolint:cyclop // config parsing is naturally branchy t.Helper() host := strings.TrimSpace(os.Getenv(envPairedHost)) if host == "" { @@ -153,10 +163,10 @@ func (p *pairedInstance) note(b []byte) { p.mu.Unlock() } -func (p *pairedInstance) snapshot() (count int64, lastAt time.Time) { +func (p *pairedInstance) snapshot() int64 { p.mu.Lock() defer p.mu.Unlock() - return p.receivedFromOther, p.lastReceiveAt + return p.receivedFromOther } // startInstance spins up one Session at a time so the second one is @@ -178,7 +188,7 @@ func startInstance(ctx context.Context, t *testing.T, cfg *pairedConfig, name st js, ok := sess.(*Session) if !ok { _ = sess.Close() - return nil, fmt.Errorf("%s: cast to *Session failed", name) + return nil, fmt.Errorf("%s: cast to *Session failed: %w", name, errCastFailed) } js.SetShouldReconnect(func() bool { return ctx.Err() == nil }) inst.js = js @@ -201,11 +211,11 @@ func waitForBridge(ctx context.Context, inst *pairedInstance, deadline time.Time } select { case <-ctx.Done(): - return ctx.Err() + return fmt.Errorf("%s: wait for bridge: %w", inst.name, ctx.Err()) case <-time.After(time.Second): } } - return fmt.Errorf("%s: bridge not ready before deadline", inst.name) + return fmt.Errorf("%s: bridge not ready before deadline: %w", inst.name, errBridgeTimeout) } // pumpLoop sends a small heartbeat payload from `from` to the other @@ -229,8 +239,8 @@ func pumpLoop(ctx context.Context, t *testing.T, from *pairedInstance, interval seq++ buf := append([]byte(nil), payload...) if len(buf) >= 8 { - for i := 0; i < 8; i++ { - buf[i] = byte(seq >> (8 * i)) + for i := range 8 { + buf[i] = byte(seq >> (8 * i)) //nolint:gosec // intentional truncation to byte } } if err := from.js.Send(buf); err != nil { @@ -245,7 +255,6 @@ type pairedStats struct { cycles int64 chaosKicks int64 wedgesPair int64 // periods where neither side received any data - maxObservedRttMs atomic.Int64 startedAt time.Time lastChaosAt time.Time bothSidesReceived bool @@ -268,7 +277,7 @@ type pairedStats struct { // - Either side hits ErrSessionClosed at the engine level // (the closed flag is the canonical "we gave up" signal). // -//nolint:cyclop // chaos cycle structure naturally branches on phase + side +//nolint:cyclop,gocognit // chaos cycle structure naturally branches on phase + side func TestJitsiPairedChaosStress(t *testing.T) { cfg := readPairedConfig(t) infinite := cfg.duration == 0 @@ -289,7 +298,7 @@ func TestJitsiPairedChaosStress(t *testing.T) { // Spin up Alice first so she's already in the room when Bob arrives — // this guarantees min-participants triggers session-initiate. - alice, err := startInstance(ctx, t, cfg, "alice") + alice, err := startInstance(ctx, t, cfg, nameAlice) if err != nil { t.Fatalf("alice: %v", err) } @@ -298,7 +307,7 @@ func TestJitsiPairedChaosStress(t *testing.T) { // Brief settle so Alice is fully in the MUC before Bob joins. time.Sleep(2 * time.Second) - bob, err := startInstance(ctx, t, cfg, "bob") + bob, err := startInstance(ctx, t, cfg, nameBob) if err != nil { t.Fatalf("bob: %v", err) } @@ -374,10 +383,10 @@ func TestJitsiPairedChaosStress(t *testing.T) { // === Phase B: pick a victim and chaos === victim := alice - victimName := "alice" + victimName := nameAlice if rng.Intn(2) == 1 { victim = bob - victimName = "bob" + victimName = nameBob } if cfg.verbose { t.Logf("[paired][%d] CHAOS victim=%s — teardownPC + requestReconnect", stats.cycles, victimName) @@ -393,10 +402,10 @@ func TestJitsiPairedChaosStress(t *testing.T) { // engine has wedged. recoveryBudget := cfg.idle + 60*time.Second survivor := alice - survivorName := "alice" + survivorName := nameAlice if victim == alice { survivor = bob - survivorName = "bob" + survivorName = nameBob } if err := waitFreshReceive(ctx, survivor, recoveryBudget); err != nil { stats.wedgesPair++ @@ -425,21 +434,21 @@ func TestJitsiPairedChaosStress(t *testing.T) { func waitFirstReceive(ctx context.Context, a, b *pairedInstance, budget time.Duration) error { deadline := time.Now().Add(budget) for time.Now().Before(deadline) { - ac, _ := a.snapshot() - bc, _ := b.snapshot() + ac := a.snapshot() + bc := b.snapshot() if ac > 0 && bc > 0 { return nil } select { case <-ctx.Done(): - return ctx.Err() + return fmt.Errorf("wait first receive: %w", ctx.Err()) case <-time.After(500 * time.Millisecond): } } - ac, _ := a.snapshot() - bc, _ := b.snapshot() - return fmt.Errorf("did not see bidirectional roundtrip in %s (alice=%d bob=%d)", - budget, ac, bc) + ac := a.snapshot() + bc := b.snapshot() + return fmt.Errorf("did not see bidirectional roundtrip in %s (alice=%d bob=%d): %w", + budget, ac, bc, errRoundtripTimeout) } // waitFreshReceive blocks until target sees a NEW receive after this call, @@ -447,20 +456,20 @@ func waitFirstReceive(ctx context.Context, a, b *pairedInstance, budget time.Dur // bridge fully recovered: bytes are arriving from the (forced-to-reconnect) // peer. func waitFreshReceive(ctx context.Context, target *pairedInstance, budget time.Duration) error { - startCount, _ := target.snapshot() + startCount := target.snapshot() deadline := time.Now().Add(budget) for time.Now().Before(deadline) { - c, _ := target.snapshot() + c := target.snapshot() if c > startCount { return nil } select { case <-ctx.Done(): - return ctx.Err() + return fmt.Errorf("wait fresh receive: %w", ctx.Err()) case <-time.After(500 * time.Millisecond): } } - return fmt.Errorf("no new receive in %s (count stuck at %d)", budget, startCount) + return fmt.Errorf("no new receive in %s (count stuck at %d): %w", budget, startCount, errReceiveTimeout) } func reportPairedStats(t *testing.T, s *pairedStats, cfg *pairedConfig) {