diff --git a/internal/collect/collect_test.go b/internal/collect/collect_test.go index 12cb26e..004a75c 100644 --- a/internal/collect/collect_test.go +++ b/internal/collect/collect_test.go @@ -459,6 +459,11 @@ func TestStatePersistsDedupAcrossProcesses(t *testing.T) { if got := rec1.CountExact("dedup state save failed"); got != 0 { t.Errorf("successful state save emitted %d save-failure warnings, want 0", got) } + // Nor the marshal-failure warning: the whole scan is read through its log + // stream, so a healthy save has to be silent about both halves. + if got := rec1.CountExact("dedup state marshal failed"); got != 0 { + t.Errorf("successful state save emitted %d marshal-failure warnings, want 0", got) + } c2, rec2 := mk() // fresh "process", same state file c2.Scan(t.Context()) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9348b20..5615dcb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -21,11 +21,16 @@ func TestLoadDefaults(t *testing.T) { t.Setenv("LOG_LEVEL", "") cfg := Load() - if cfg.ScanInterval != DefaultScanInterval { - t.Errorf("ScanInterval = %v, want %v", cfg.ScanInterval, DefaultScanInterval) + + // Expected values are written out rather than read back from the + // constants they check: an assertion against DefaultScanInterval moves + // with any edit to it and so pins nothing. 15m and 72h are the cadence + // and window the compose contract and the bundled dashboard assume. + if cfg.ScanInterval != 15*time.Minute { + t.Errorf("ScanInterval = %v, want 15m0s", cfg.ScanInterval) } - if cfg.Lookback != DefaultLookbackHours*time.Hour { - t.Errorf("Lookback = %v, want %v", cfg.Lookback, DefaultLookbackHours*time.Hour) + if cfg.Lookback != 72*time.Hour { + t.Errorf("Lookback = %v, want 72h0m0s", cfg.Lookback) } if cfg.LogLevel != slog.LevelInfo { t.Errorf("LogLevel = %v, want Info", cfg.LogLevel) @@ -82,8 +87,8 @@ func TestScanIntervalSentinelsFallBackToDefault(t *testing.T) { t.Run(v, func(t *testing.T) { rec := captureDefaultSlog(t) t.Setenv("SCAN_INTERVAL", v) - if got := Load().ScanInterval; got != DefaultScanInterval { - t.Errorf("SCAN_INTERVAL=%q ScanInterval = %v, want default %v", v, got, DefaultScanInterval) + if got := Load().ScanInterval; got != 15*time.Minute { + t.Errorf("SCAN_INTERVAL=%q ScanInterval = %v, want default 15m0s", v, got) } if n := rec.CountExact("invalid SCAN_INTERVAL, using default"); n != 1 { t.Errorf("SCAN_INTERVAL=%q warned %d times, want exactly 1", v, n) @@ -99,6 +104,10 @@ func TestScanIntervalParsesDuration(t *testing.T) { } } +// TestClampingAndFallbacks pins every out-of-range and unparseable input to the +// duration it actually yields. Each want is a literal, never the constant the +// clamp reads: a want written as `maxScanInterval` holds however that constant +// is edited, so the bound it claims to check is unpinned. func TestClampingAndFallbacks(t *testing.T) { tests := []struct { selector func(Config) time.Duration @@ -107,14 +116,14 @@ func TestClampingAndFallbacks(t *testing.T) { val string want time.Duration }{ - {name: "scan negative falls back to default", key: "SCAN_INTERVAL", val: "-5m", want: DefaultScanInterval, selector: func(c Config) time.Duration { return c.ScanInterval }}, - {name: "scan garbage falls back to default", key: "SCAN_INTERVAL", val: "abc", want: DefaultScanInterval, selector: func(c Config) time.Duration { return c.ScanInterval }}, - {name: "scan over max is clamped", key: "SCAN_INTERVAL", val: "10000h", want: maxScanInterval, selector: func(c Config) time.Duration { return c.ScanInterval }}, + {name: "scan negative falls back to default", key: "SCAN_INTERVAL", val: "-5m", want: 15 * time.Minute, selector: func(c Config) time.Duration { return c.ScanInterval }}, + {name: "scan garbage falls back to default", key: "SCAN_INTERVAL", val: "abc", want: 15 * time.Minute, selector: func(c Config) time.Duration { return c.ScanInterval }}, + {name: "scan over max is clamped", key: "SCAN_INTERVAL", val: "10000h", want: 8760 * time.Hour, selector: func(c Config) time.Duration { return c.ScanInterval }}, // 365 days {name: "lookback zero floors to lo=1", key: "LOOKBACK_HOURS", val: "0", want: 1 * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, {name: "lookback at lo boundary is kept", key: "LOOKBACK_HOURS", val: "1", want: 1 * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, - {name: "lookback negative falls back to default", key: "LOOKBACK_HOURS", val: "-1", want: DefaultLookbackHours * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, - {name: "lookback at hi boundary is kept", key: "LOOKBACK_HOURS", val: "720", want: maxLookbackHours * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, - {name: "lookback over max is clamped", key: "LOOKBACK_HOURS", val: "100000", want: maxLookbackHours * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, + {name: "lookback negative falls back to default", key: "LOOKBACK_HOURS", val: "-1", want: 72 * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, + {name: "lookback at hi boundary is kept", key: "LOOKBACK_HOURS", val: "720", want: 720 * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, // 30 days + {name: "lookback over max is clamped", key: "LOOKBACK_HOURS", val: "100000", want: 720 * time.Hour, selector: func(c Config) time.Duration { return c.Lookback }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -131,7 +140,7 @@ func TestClampingAndFallbacks(t *testing.T) { // remaining conditional is `clamped != v`, which gates this warning. Asserting // the warning fires when (and only when) the value is clamped down makes that // guard's mutants (==, removal) killable — the return-value table tests alone -// cannot see a log-only branch. Guards against the L163 "living mutant". +// cannot see a log-only branch. func TestClampedIntWarnsOverMax(t *testing.T) { capture := func(val string) string { var buf bytes.Buffer @@ -184,11 +193,15 @@ func TestExcludeQueriesDefaultWhenUnset(t *testing.T) { cfg := Load() - if cfg.PRExclude != DefaultPRExclude { - t.Errorf("PRExclude = %q, want default %q", cfg.PRExclude, DefaultPRExclude) + const ( + wantPR = "-author:app/renovate" + wantIssue = "-author:app/renovate -label:renovate -label:auto-generated" + ) + if cfg.PRExclude != wantPR { + t.Errorf("PRExclude = %q, want default %q", cfg.PRExclude, wantPR) } - if cfg.IssueExclude != DefaultIssueExclude { - t.Errorf("IssueExclude = %q, want default %q", cfg.IssueExclude, DefaultIssueExclude) + if cfg.IssueExclude != wantIssue { + t.Errorf("IssueExclude = %q, want default %q", cfg.IssueExclude, wantIssue) } } @@ -236,11 +249,11 @@ func TestLookbackAtMaxIsAcceptedWithoutWarning(t *testing.T) { // (a v >= hi mutant would warn spuriously at the legal maximum). rec := captureDefaultSlog(t) - t.Setenv("LOOKBACK_HOURS", "720") // == maxLookbackHours + t.Setenv("LOOKBACK_HOURS", "720") // the maximum, 30 days cfg := Load() - if cfg.Lookback != maxLookbackHours*time.Hour { - t.Errorf("Lookback = %v, want %v (max accepted as-is)", cfg.Lookback, maxLookbackHours*time.Hour) + if cfg.Lookback != 720*time.Hour { + t.Errorf("Lookback = %v, want 720h0m0s (max accepted as-is)", cfg.Lookback) } if n := rec.CountExact("env value clamped"); n != 0 { t.Errorf("value at exactly the max should not warn; got %d clamp warnings", n) @@ -255,8 +268,8 @@ func TestLookbackAboveMaxIsClampedWithWarning(t *testing.T) { t.Setenv("LOOKBACK_HOURS", "721") cfg := Load() - if cfg.Lookback != maxLookbackHours*time.Hour { - t.Errorf("Lookback = %v, want clamped to %v", cfg.Lookback, maxLookbackHours*time.Hour) + if cfg.Lookback != 720*time.Hour { + t.Errorf("Lookback = %v, want clamped to 720h0m0s", cfg.Lookback) } if n := rec.CountExact("env value clamped"); n != 1 { t.Errorf("value over the max should warn once; got %d clamp warnings", n) @@ -274,20 +287,22 @@ func captureDefaultSlog(t *testing.T) *capture.Recorder { } // TestScanIntervalBelowMinimumClamped pins the minScanInterval floor: a positive -// sub-minute SCAN_INTERVAL is clamped up to minScanInterval (1m) so a too-frequent -// scan of a multi-repo account can't exhaust GitHub's 5000 req/hour budget. The +// sub-minute SCAN_INTERVAL is clamped up to 1m so a too-frequent scan of a +// multi-repo account can't exhaust GitHub's 5000 req/hour budget. The // existing TestClampingAndFallbacks covers negative/garbage/over-max but no // sub-minute case. The "exactly 1m" row is kept via the default branch, pinning // the strict `<` lower edge; "2m" confirms an above-floor value passes through. +// The floor is written as a literal, not as minScanInterval, so the row states +// what the floor IS rather than restating whatever it has become. func TestScanIntervalBelowMinimumClamped(t *testing.T) { tests := []struct { name string val string want time.Duration }{ - {"sub-minute clamps up to the floor", "30s", minScanInterval}, - {"one second below the floor clamps", "59s", minScanInterval}, - {"exactly at the floor is kept", "1m", minScanInterval}, + {"sub-minute clamps up to the floor", "30s", time.Minute}, + {"one second below the floor clamps", "59s", time.Minute}, + {"exactly at the floor is kept", "1m", time.Minute}, {"above the floor is kept", "2m", 2 * time.Minute}, } for _, tt := range tests { diff --git a/internal/github/client_conditional_test.go b/internal/github/client_conditional_test.go index 8e911bd..cbfa54f 100644 --- a/internal/github/client_conditional_test.go +++ b/internal/github/client_conditional_test.go @@ -8,12 +8,14 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "sync/atomic" "testing" "time" "github.com/cplieger/github-scout/internal/ghsignal" "github.com/cplieger/httpx/v5" + "github.com/cplieger/slogx/capture" ) // condServer is an httptest server that serves body with an ETag and answers @@ -322,3 +324,124 @@ func TestCondCache_persistEvictsOldestWhenOverBound(t *testing.T) { t.Error("newest entry was evicted; eviction must drop oldest-first") } } + +// TestCondCache_lastModifiedOnlyIsRevalidatable pins that a representation +// validated only by Last-Modified is cached like an ETag'd one. Several GitHub +// endpoints answer without an ETag, and treating those as unrevalidatable +// would spend a full-price GET on every scan forever. +func TestCondCache_lastModifiedOnlyIsRevalidatable(t *testing.T) { + const ( + reqURL = "https://example.test/user/repos" + lastModified = "Fri, 21 Aug 2026 15:35:00 GMT" + ) + c := newCondCache("", slog.Default()) + c.store(reqURL, httpx.Validators{LastModified: lastModified}, []string{"keep"}) + + got := c.validators(reqURL) + if got.LastModified != lastModified { + t.Errorf("validators(%q).LastModified = %q, want %q", reqURL, got.LastModified, lastModified) + } + if got.ETag != "" { + t.Errorf("validators(%q).ETag = %q, want empty", reqURL, got.ETag) + } + var items []string + if !c.decodeInto(reqURL, &items) { + t.Fatalf("decodeInto(%q) = false, want the stored items re-servable on a 304", reqURL) + } + if len(items) != 1 || items[0] != "keep" { + t.Errorf("decodeInto(%q) items = %v, want [keep]", reqURL, items) + } +} + +// TestCondCache_successfulSaveIsSilent pins that a healthy save emits nothing. +// The watcher is read entirely through its log stream, so a line on the +// success path would report a broken cache on every single scan. +func TestCondCache_successfulSaveIsSilent(t *testing.T) { + logger, rec := capture.New() + c := newCondCache(filepath.Join(t.TempDir(), "cond-cache.json"), logger) + c.store("https://example.test/user/repos", httpx.Validators{ETag: `W/"v1"`}, []string{"keep"}) + + if rec.Len() != 0 { + t.Errorf("a successful cache save logged %d line(s) %v, want none", rec.Len(), rec.Messages()) + } +} + +// TestCondCache_saveKeepsAnotherProcessesEntries pins the merge under the +// slot's flock: an entry written by a concurrent process after this cache +// loaded — the scheduled daemon racing a hand-exec'd trigger — must survive +// this process's save instead of being overwritten away. +func TestCondCache_saveKeepsAnotherProcessesEntries(t *testing.T) { + const ( + mine = "https://example.test/mine" + other = "https://example.test/other" + ) + path := filepath.Join(t.TempDir(), "cond-cache.json") + c := newCondCache(path, slog.Default()) // loads cold: memory holds nothing + + // The other process writes its entry after this cache loaded, so the only + // copy of it is the one on disk. + persisted, err := json.Marshal(map[string]cacheEntry{ + other: {ETag: `W/"other"`, Items: json.RawMessage(`["o"]`), UsedAt: time.Now().Add(-time.Hour)}, + }) + if err != nil { + t.Fatalf("setup: marshal the other process's entry: %v", err) + } + if err := os.WriteFile(path, persisted, 0o600); err != nil { + t.Fatalf("setup: %v", err) + } + + c.store(mine, httpx.Validators{ETag: `W/"mine"`}, []string{"m"}) + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read the cache file back: %v", err) + } + var merged map[string]cacheEntry + if err := json.Unmarshal(data, &merged); err != nil { + t.Fatalf("cache file does not parse after the save: %v (%s)", err, data) + } + if _, ok := merged[other]; !ok { + t.Errorf("save dropped the concurrently written entry %q; file holds %d entry/entries", other, len(merged)) + } + if _, ok := merged[mine]; !ok { + t.Errorf("save did not persist this process's entry %q", mine) + } +} + +// TestCondCache_payloadExactlyAtTheBoundIsKeptWhole pins the inclusive edge of +// the persisted-size cap: a payload marshaling to exactly the bound still +// reads back whole, so evicting from it would discard a usable validator for +// no gain. +func TestCondCache_payloadExactlyAtTheBoundIsKeptWhole(t *testing.T) { + // Sized so the marshaled map lands exactly on the cap. UsedAt is fixed + // and carries no trailing-zero nanoseconds, so its encoding is a stable + // width; the guard below fails loudly if the entry shape ever changes. + const padding = 61339 + const reqURL = "https://example.test/p" + entries := map[string]cacheEntry{ + reqURL: { + ETag: `W/"x"`, + Items: json.RawMessage(`["` + strings.Repeat("a", padding) + `"]`), + UsedAt: time.Date(2026, 8, 21, 15, 35, 0, 123456789, time.UTC), + }, + } + raw, err := json.Marshal(entries) + if err != nil { + t.Fatalf("setup: marshal fixture: %v", err) + } + if len(raw) != condCacheMaxBytes { + t.Fatalf("fixture drifted: entry marshals to %d bytes, want exactly %d (adjust padding by %d)", + len(raw), condCacheMaxBytes, condCacheMaxBytes-len(raw)) + } + + data, err := marshalBounded(entries) + if err != nil { + t.Fatalf("marshalBounded: %v", err) + } + if _, ok := entries[reqURL]; !ok { + t.Errorf("a payload of exactly %d bytes was evicted, want it retained", condCacheMaxBytes) + } + if len(data) != condCacheMaxBytes { + t.Errorf("marshalBounded returned %d bytes, want the whole %d-byte payload", len(data), condCacheMaxBytes) + } +} diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 892fef1..49f209a 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -14,13 +14,21 @@ import ( "github.com/cplieger/github-scout/internal/ghsignal" "github.com/cplieger/httpx/v5" + "github.com/cplieger/slogx/capture" ) // newTestClient wires a Client at the test server's URL with a short-timeout // http.Client so tests never hang. func newTestClient(t *testing.T, srv *httptest.Server) *Client { t.Helper() - c := NewClient(Options{HTTP: httpx.NewClient(5 * time.Second), Token: "test-token", Logger: slog.Default()}) + return newTestClientWithLogger(t, srv, slog.Default()) +} + +// newTestClientWithLogger is newTestClient with a caller-supplied logger, for +// the paths whose only observable output is the log line they emit. +func newTestClientWithLogger(t *testing.T, srv *httptest.Server, logger *slog.Logger) *Client { + t.Helper() + c := NewClient(Options{HTTP: httpx.NewClient(5 * time.Second), Token: "test-token", Logger: logger}) c.baseURL = srv.URL return c } @@ -31,8 +39,8 @@ func TestListReposFiltersOwnerAndArchived(t *testing.T) { if got := r.Header.Get("Authorization"); got != "Bearer test-token" { t.Errorf("Authorization = %q, want Bearer test-token", got) } - if got := r.Header.Get("X-GitHub-Api-Version"); got != apiVersion { - t.Errorf("api version header = %q, want %q", got, apiVersion) + if got := r.Header.Get("X-GitHub-Api-Version"); got != "2022-11-28" { + t.Errorf("api version header = %q, want %q", got, "2022-11-28") } w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`[ @@ -479,7 +487,8 @@ func TestListReposStopsAtMaxPages(t *testing.T) { `{"name":"r%d","owner":{"login":"cplieger"}}`)) defer srv.Close() - repos, err := newTestClient(t, srv).ListRepos(t.Context(), "cplieger") + logger, rec := capture.New() + repos, err := newTestClientWithLogger(t, srv, logger).ListRepos(t.Context(), "cplieger") if err != nil { t.Fatalf("ListRepos: %v", err) } @@ -489,6 +498,16 @@ func TestListReposStopsAtMaxPages(t *testing.T) { if len(repos) != maxPages*perPage { t.Errorf("got %d repos, want %d", len(repos), maxPages*perPage) } + // The warning is the only signal an operator gets that the scan universe + // was capped, so it must name the ceiling actually scanned: 5 pages of + // 100 repos. + const warning = "repo listing hit pagination bound; scan universe may be truncated" + if got := rec.CountExact(warning); got != 1 { + t.Errorf("ListRepos over the page cap emitted %d truncation warnings, want 1", got) + } + if got, ok := rec.AttrValueExact(warning, "repo_cap"); !ok || got != "500" { + t.Errorf("truncation warning repo_cap = %q (present=%v), want %q", got, ok, "500") + } } func TestListRunsStopsAtMaxPages(t *testing.T) { @@ -518,7 +537,8 @@ func TestSearchStopsAtMaxPages(t *testing.T) { `{"number":%d,"repository_url":"https://api.github.com/repos/cplieger/a","user":{"login":"cplieger"}}`)) defer srv.Close() - prs, err := newTestClient(t, srv).SearchOpenPRs(t.Context(), "cplieger", "") + logger, rec := capture.New() + prs, err := newTestClientWithLogger(t, srv, logger).SearchOpenPRs(t.Context(), "cplieger", "") if err != nil { t.Fatalf("SearchOpenPRs: %v", err) } @@ -528,6 +548,15 @@ func TestSearchStopsAtMaxPages(t *testing.T) { if len(prs) != maxPages*perPage { t.Errorf("got %d PRs, want %d", len(prs), maxPages*perPage) } + // A partial snapshot is only distinguishable from a complete one by this + // warning, so it must name the ceiling actually searched: 5 pages of 100. + const warning = "search hit pagination bound; snapshot may be truncated" + if got := rec.CountExact(warning); got != 1 { + t.Errorf("search over the page cap emitted %d truncation warnings, want 1", got) + } + if got, ok := rec.AttrValueExact(warning, "item_cap"); !ok || got != "500" { + t.Errorf("truncation warning item_cap = %q (present=%v), want %q", got, ok, "500") + } } func TestListCodeScanningAlertsStopsAtMaxPages(t *testing.T) {