From 43de945c72e353b8aebe0aa97a2d51f228089399 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 27 May 2026 20:27:24 -0700 Subject: [PATCH] tests: tighten coverage by retiring unreachable defensive code (wave 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetchOnce: http.NewRequestWithContext only fails on invalid method/URL, both compile-time constants here — drop the error branch with a comment so future readers don't add the check back. Add two test seams: - initialJitterMax (var, was const-arg) — lets tests shrink the first-fetch jitter to 0 so timer.C fires immediately. - httpClientForRun (var, was inline literal) — lets tests inject a failing http.Client so the slog.Warn-on-fetch-fail branch is also exercised. New test TestRun_TimerFires drives the full Run loop through one timer.C → fetchOnce → slog.Warn → timer.Reset cycle deterministically, without depending on the real network. Coverage: 90.9% → 94.8%. Run climbs from 66.7% → 100%, fetchOnce from 94.7% → 100%. The remaining gap is the embedded-JSON-malformed init() fallback and rand.Int failure in jitter — both real defensive paths. --- runtime.go | 22 ++++++++++++++++------ zz_fetch_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/runtime.go b/runtime.go index aab3120..296f754 100644 --- a/runtime.go +++ b/runtime.go @@ -27,13 +27,24 @@ const ( // happen to share a process (e.g. tests). var fetchMu sync.Mutex +// initialJitterMax controls the first-fetch jitter window. Exposed as a +// var (not a const) so tests can shorten it to 0 and deterministically +// drive Run through one full timer.C → fetchOnce → timer.Reset cycle. +var initialJitterMax = 30 * time.Second + +// httpClientForRun is the http.Client Run uses. Exposed as a var so +// tests can inject a transport that returns a deterministic error, +// covering the slog.Warn-on-fetch-fail branch without depending on the +// real network. +var httpClientForRun = func() *http.Client { return &http.Client{Timeout: 30 * time.Second} } + // Run polls the canonical URL on a timer, replacing the active list // whenever a new one is fetched. Blocks until ctx is cancelled. The // first fetch is delayed 0–30s so a fleet rebooting at the same time // doesn't thunder the URL. func Run(ctx context.Context) { - client := &http.Client{Timeout: 30 * time.Second} - timer := time.NewTimer(jitter(30 * time.Second)) + client := httpClientForRun() + timer := time.NewTimer(jitter(initialJitterMax)) defer timer.Stop() for { select { @@ -51,10 +62,9 @@ func Run(ctx context.Context) { func fetchOnce(ctx context.Context, client *http.Client) error { fetchMu.Lock() defer fetchMu.Unlock() - req, err := http.NewRequestWithContext(ctx, "GET", defaultURL, nil) - if err != nil { - return err - } + // http.NewRequestWithContext only fails on an invalid method or URL — + // both are compile-time constants here, so the error is unreachable. + req, _ := http.NewRequestWithContext(ctx, "GET", defaultURL, nil) req.Header.Set("User-Agent", "pilot-daemon/trustedagents") resp, err := client.Do(req) if err != nil { diff --git a/zz_fetch_test.go b/zz_fetch_test.go index cc523e5..e8caac5 100644 --- a/zz_fetch_test.go +++ b/zz_fetch_test.go @@ -310,6 +310,41 @@ func TestRun_FullIteration(t *testing.T) { } } +// TestRun_TimerFires drives Run through the timer.C → fetchOnce → +// timer.Reset arm. We shrink initialJitterMax to 0 so the first timer +// fires immediately, and inject a failing http client so fetchOnce +// returns an error and the slog.Warn branch is also covered. +func TestRun_TimerFires(t *testing.T) { + // Mutates package state — no t.Parallel. + prevJ := initialJitterMax + initialJitterMax = 0 + t.Cleanup(func() { initialJitterMax = prevJ }) + + prevClient := httpClientForRun + httpClientForRun = func() *http.Client { + return &http.Client{Transport: &errTransport{err: errors.New("injected: no network")}} + } + t.Cleanup(func() { httpClientForRun = prevClient }) + + restore := SetForTest(nil) + t.Cleanup(restore) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + Run(ctx) + close(done) + }() + // Give Run enough time to fire the timer once + log + reset. + time.Sleep(150 * time.Millisecond) + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Run did not return after timer-fired path + cancel") + } +} + // TestRun_FetchPath drives Run through the timer.C -> fetchOnce -> // timer.Reset arm. We can't shrink fetchInterval (const), so we can't // drive a second iteration in test time. But the first iteration —