diff --git a/.gavel.yaml b/.gavel.yaml index 5f83f6df9..2f0d3a948 100644 --- a/.gavel.yaml +++ b/.gavel.yaml @@ -1,9 +1,46 @@ -commit: {} +commit: + gitignore: + - .bin/ + - .cache + - .claude + - .container-sandbox.yaml + - .DS_Store + - .DS_Store? + - .env + - .env.local + - .gavel/ + - .gingko/ + - .idea/ + - .jscpd.json + - .playwright-mcp/ + - .shell/ + - .Spotlight-V100 + - .task + - .tmp/ + - .Trashes + - .vitest/ + - .vscode/ + - '*.log' + - '*.test' + - node_modules/ + - dist/ + - '*.tmp' + - .metadata_never_index + - .tmp + - .agents/ + - .sandbox.yaml + - nohup.out + - skills-lock.json + linkedDeps: {} fixtures: {} lint: ignore: - file: .shell/checkout/8d512cac74b933d1f8d97629ab79966828d2e0789c66a463d8a78e7d40ccc42f/** -secrets: {} + - file: linters/betterleaks/testdata/betterleaks-output.json + source: betterleaks +secrets: + configs: + - .betterleaks.toml ssh: {} verify: checks: diff --git a/action.yml b/action.yml index b6265b9fc..6f3364c2e 100644 --- a/action.yml +++ b/action.yml @@ -122,10 +122,24 @@ runs: set +e rm -f "${JSON_FILE}" "${GAVEL_LOG_FILE}" + # --show-passed is only defined on `gavel test` / `gavel lint`. Other + # subcommands (e.g. `fixtures`) reject unknown flags and fail at parse + # time. Detect the first non-flag word in $GAVEL_ARGS and only append + # the test-specific flag for those subcommands. + first_subcmd="" + for tok in $GAVEL_ARGS; do + case "$tok" in + -*) ;; + *) first_subcmd="$tok"; break ;; + esac + done + extra_flags=("--no-progress" "--no-color" "--format" "json=${JSON_FILE}") + case "$first_subcmd" in + test|lint) extra_flags=("--show-passed" "${extra_flags[@]}") ;; + esac + # shellcheck disable=SC2086 - gavel $GAVEL_ARGS \ - --no-progress --no-color --show-passed \ - --format "json=${JSON_FILE}" \ + gavel $GAVEL_ARGS "${extra_flags[@]}" \ 2> >(tee "${GAVEL_LOG_FILE}" >&2) code=$? set -e diff --git a/cmd/gavel/summary.go b/cmd/gavel/summary.go index 5536ce53e..7fe66344d 100644 --- a/cmd/gavel/summary.go +++ b/cmd/gavel/summary.go @@ -231,9 +231,25 @@ func ensureSource(sources map[string]*sourceCounts, name string) *sourceCounts { return sc } +// writeCountsTable emits a markdown table containing only sources that have +// failures or skips. Sources with 100% passing tests are intentionally +// omitted — their pass count and duration are rolled into the totals row +// emitted by writeTotals so the PR comment stays short for the common +// "all green" case. When every source is clean, the table is omitted +// entirely and writeTotals carries the headline. func writeCountsTable(b *strings.Builder, sources map[string]*sourceCounts) { - rows := make([]*sourceCounts, 0, len(sources)) + var rows []*sourceCounts + var passingOnly, passingDuration int + var hiddenSources int + totalPassing := 0 for _, sc := range sources { + totalPassing += sc.passed + if sc.failed == 0 && sc.skipped == 0 { + passingOnly += sc.passed + passingDuration += int(sc.duration) + hiddenSources++ + continue + } rows = append(rows, sc) } sort.Slice(rows, func(i, j int) bool { @@ -245,12 +261,22 @@ func writeCountsTable(b *strings.Builder, sources map[string]*sourceCounts) { }) b.WriteString("## Gavel summary\n\n") + if len(rows) == 0 { + // Nothing to break out — totals row alone tells the story. + return + } b.WriteString("| Source | Pass | Fail | Skip | Duration |\n") b.WriteString("|---|---:|---:|---:|---:|\n") for _, sc := range rows { fmt.Fprintf(b, "| %s | %d | %d | %d | %s |\n", escapePipe(sc.name), sc.passed, sc.failed, sc.skipped, formatDuration(sc.duration)) } + if hiddenSources > 0 { + // One collapsed row covers every all-passing source so a reader + // can still see "we ran more than what's listed". + fmt.Fprintf(b, "| _%d more passing source(s)_ | %d | 0 | 0 | %s |\n", + hiddenSources, passingOnly, formatDuration(time.Duration(passingDuration))) + } b.WriteString("\n") } diff --git a/cmd/gavel/summary_test.go b/cmd/gavel/summary_test.go index 4996ec8c6..d921a22cd 100644 --- a/cmd/gavel/summary_test.go +++ b/cmd/gavel/summary_test.go @@ -84,16 +84,30 @@ func TestBuildCompactSummary(t *testing.T) { out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200}) - // Counts table by source — expect one row per test package AND one per linter. + // Counts table by source — only sources that have failures or skips + // should appear. The serve package has failures, the verify package + // has a skipped test, and golangci-lint reports a violation, so all + // three keep their row. Sources that are 100% passing (e.g. the + // clean gofmt linter below) are folded into the passing-only summary + // row + Totals line. if !strings.Contains(out, "github.com/flanksource/gavel/serve") { t.Errorf("expected serve package row in counts table, got:\n%s", out) } if !strings.Contains(out, "github.com/flanksource/gavel/verify") { - t.Errorf("expected verify package row in counts table, got:\n%s", out) + t.Errorf("expected verify package row (has a skipped test) in counts table, got:\n%s", out) } if !strings.Contains(out, "golangci-lint") { t.Errorf("expected golangci-lint row in counts table, got:\n%s", out) } + // gofmt is a passing-only linter — must not appear as its own row. + if strings.Contains(out, "| lint: gofmt |") { + t.Errorf("passing-only linter must be folded into the passing summary row, got:\n%s", out) + } + // The collapsed-passing row should mention the gofmt linter's pass count + // (1) and the "more passing source(s)" text. + if !strings.Contains(out, "more passing source(s)") { + t.Errorf("expected collapsed passing-source row, got:\n%s", out) + } // Passing rows present, but no per-test listing for passing tests in the detail section. if strings.Contains(out, "initializes a bare git repo") { @@ -280,8 +294,11 @@ func TestBuildCompactSummaryPrefersRealResultsOverCrashField(t *testing.T) { Error: "partial error that should be ignored", } out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200}) - if !strings.Contains(out, "pkg/x") { - t.Errorf("expected normal results to render, got:\n%s", out) + // pkg/x is 100% passing so its row is collapsed into the totals; we + // only need to confirm the normal counts path ran (Totals line) and + // that the crash-stub path did not render. + if !strings.Contains(out, "**Totals:** 1 passed") { + t.Errorf("expected normal Totals line for the single passing test, got:\n%s", out) } if strings.Contains(out, "Gavel crashed") { t.Errorf("normal results must not fall through to crash path, got:\n%s", out) @@ -329,6 +346,85 @@ func TestRunSummaryReadsJSONFile(t *testing.T) { } } +// TestBuildCompactSummaryCollapsesAllPassingSources exercises the +// "PR comment must stay short" behaviour: when every source is 100% +// passing, the per-source table disappears entirely and the Totals +// line carries the full headline (count + duration). Regressing to +// the old behaviour (one row per package) blows up PR comments on +// large monorepos. +func TestBuildCompactSummaryCollapsesAllPassingSources(t *testing.T) { + input := gavelResultJSON{ + Tests: []parsers.Test{ + {Package: "pkg/a", Children: parsers.Tests{ + {Package: "pkg/a", Name: "TestOne", Passed: true, Duration: 10 * time.Millisecond}, + {Package: "pkg/a", Name: "TestTwo", Passed: true, Duration: 20 * time.Millisecond}, + }}, + {Package: "pkg/b", Children: parsers.Tests{ + {Package: "pkg/b", Name: "TestThree", Passed: true, Duration: 30 * time.Millisecond}, + }}, + }, + } + out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200}) + + // No per-package rows when every source is clean. + if strings.Contains(out, "| pkg/a |") { + t.Errorf("all-passing pkg/a must not appear as its own row, got:\n%s", out) + } + if strings.Contains(out, "| pkg/b |") { + t.Errorf("all-passing pkg/b must not appear as its own row, got:\n%s", out) + } + // No table headers either — the table itself collapses. + if strings.Contains(out, "| Source |") { + t.Errorf("counts table must collapse when every source is clean, got:\n%s", out) + } + // Totals line still carries the headline. + if !strings.Contains(out, "**Totals:** 3 passed · 0 failed · 0 skipped") { + t.Errorf("expected Totals line with aggregate counts, got:\n%s", out) + } +} + +// TestBuildCompactSummaryCollapsesPassingAlongsideFailing checks the +// mixed case: failing sources keep their own row, but every all-passing +// source is folded into a single trailing summary row whose pass count +// matches the sum of the hidden sources. +func TestBuildCompactSummaryCollapsesPassingAlongsideFailing(t *testing.T) { + input := gavelResultJSON{ + Tests: []parsers.Test{ + {Package: "pkg/fail", Children: parsers.Tests{ + {Package: "pkg/fail", Name: "TestBoom", Failed: true, Message: "boom"}, + }}, + {Package: "pkg/clean1", Children: parsers.Tests{ + {Package: "pkg/clean1", Name: "TestOne", Passed: true, Duration: 100 * time.Millisecond}, + {Package: "pkg/clean1", Name: "TestTwo", Passed: true, Duration: 100 * time.Millisecond}, + }}, + {Package: "pkg/clean2", Children: parsers.Tests{ + {Package: "pkg/clean2", Name: "TestThree", Passed: true, Duration: 50 * time.Millisecond}, + }}, + }, + } + out := buildCompactSummary(input, compactSummaryBudget{maxFailures: 5, maxLinesPerFailure: 5, maxCharsPerLine: 200}) + + // Failing source keeps its dedicated row. + if !strings.Contains(out, "| pkg/fail |") { + t.Errorf("failing pkg/fail row must be present, got:\n%s", out) + } + // Clean sources do not appear by name… + if strings.Contains(out, "pkg/clean1") { + t.Errorf("clean pkg/clean1 must not appear by name, got:\n%s", out) + } + if strings.Contains(out, "pkg/clean2") { + t.Errorf("clean pkg/clean2 must not appear by name, got:\n%s", out) + } + // …but a single collapsed row reports the count of hidden sources + // AND the sum of their pass counts (2 + 1 = 3). + if !strings.Contains(out, "_2 more passing source(s)_") { + t.Errorf("expected '2 more passing source(s)' collapse row, got:\n%s", out) + } + if !strings.Contains(out, "| _2 more passing source(s)_ | 3 | 0 | 0 |") { + t.Errorf("expected collapsed row with summed pass count, got:\n%s", out) + } +} + func longMultilineStderr() string { var sb strings.Builder // 8 lines of 250 chars each — must be truncated to 5 lines of ≤200 chars. diff --git a/cmd/gavel/test.go b/cmd/gavel/test.go index f0a3d449e..9b453bbf9 100644 --- a/cmd/gavel/test.go +++ b/cmd/gavel/test.go @@ -544,7 +544,11 @@ var testCmd *cobra.Command func init() { testCmd = clicky.AddNamedCommand("test", rootCmd, testrunner.RunOptions{}, runTests) - testCmd.Flags().SetInterspersed(false) + // Allow flags and positional package paths to interleave so callers (e.g. the + // flanksource/gavel composite action) can append flags after user-supplied + // paths. Use `--` to terminate flag parsing when forwarding flags to the + // underlying runner via the `gavel test ./pkg -- --focus X` idiom. + testCmd.Flags().SetInterspersed(true) testCmd.Flags().BoolVar(&testDurationFlags.Detach, "detach", false, "With --ui, fork a detached UI server and exit. The child serves until --auto-stop (default 30m) or --idle-timeout (default 5m) fires.") testCmd.Flags().DurationVar(&testDurationFlags.AutoStop, "auto-stop", 0, diff --git a/cmd/gavel/test_framework_subcmds.go b/cmd/gavel/test_framework_subcmds.go index 61a9c7c4c..8c7494a2a 100644 --- a/cmd/gavel/test_framework_subcmds.go +++ b/cmd/gavel/test_framework_subcmds.go @@ -24,7 +24,7 @@ func registerTestFrameworkSubcommands() { return runTests(opts) }) sub.Short = fmt.Sprintf("Run only %s tests", fw) - sub.Flags().SetInterspersed(false) + sub.Flags().SetInterspersed(true) if err := sub.Flags().MarkHidden("framework"); err != nil { panic(fmt.Sprintf("hide --framework on %s subcommand: %v", name, err)) } diff --git a/cmd/gavel/ui_serve.go b/cmd/gavel/ui_serve.go index e041af2ee..15d38c81a 100644 --- a/cmd/gavel/ui_serve.go +++ b/cmd/gavel/ui_serve.go @@ -6,8 +6,10 @@ import ( "net" "net/http" "os" + "os/exec" "path/filepath" "strconv" + "strings" "time" "github.com/flanksource/clicky" @@ -94,7 +96,14 @@ func runUIServe(opts UIServeOptions) (any, error) { return nil, fmt.Errorf("load results %v: %w", opts.ResultsFiles, err) } } else { + root, err := resolveGavelRoot() + if err != nil { + listener.Close() //nolint:errcheck + return nil, err + } + srv.SetGavelDir(root) srv.MarkDone() + logger.Infof("Indexing snapshots from %s", filepath.Join(root, ".gavel")) } addr := listener.Addr().(*net.TCPAddr) @@ -315,6 +324,28 @@ func firstNonLoopbackIPv4() string { return "" } +// resolveGavelRoot returns the git root of the current working directory and +// validates that .gavel/ exists inside it. Used by `gavel ui serve` (no args) +// to discover the snapshot directory to index. +func resolveGavelRoot() (string, error) { + out, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + return "", fmt.Errorf("not inside a git repository: %w", err) + } + root := strings.TrimSpace(string(out)) + if root == "" { + return "", fmt.Errorf("git rev-parse --show-toplevel returned empty") + } + gavelDir := filepath.Join(root, ".gavel") + if _, err := os.Stat(gavelDir); err != nil { + if os.IsNotExist(err) { + return "", fmt.Errorf("no %s directory found in %s — run `gavel test` to populate it, or pass a snapshot path explicitly", ".gavel", root) + } + return "", err + } + return root, nil +} + // writeURLFile writes url to path atomically (write to tempfile in same dir, // fsync, rename). Atomicity matters because a wrapping shell script may be // polling the file concurrently. diff --git a/github/artifacts.go b/github/artifacts.go index 3ff5c5a81..21c25878e 100644 --- a/github/artifacts.go +++ b/github/artifacts.go @@ -4,6 +4,7 @@ import ( "archive/zip" "bytes" "context" + "encoding/json" "fmt" "io" "regexp" @@ -37,6 +38,10 @@ var artifactLinkPattern = regexp.MustCompile( // FindGavelArtifact scans PR comments for the gavel sticky comment and // extracts the artifact ID and URL. It returns the most recent match. +// +// Prefer FindGavelArtifacts (plural) for surfaces that can show multiple +// gavel runs per PR; this single-result helper exists for legacy callers +// that only need the headline artifact. func FindGavelArtifact(comments []PRComment) (artifactID int64, artifactURL string, found bool) { for i := len(comments) - 1; i >= 0; i-- { body := comments[i].Body @@ -57,10 +62,164 @@ func FindGavelArtifact(comments []PRComment) (artifactID int64, artifactURL stri return 0, "", false } -// DownloadArtifact downloads a GitHub Actions artifact ZIP and extracts -// gavel-results.json from it. The opts.Repo field must be set to the -// owner/repo that owns the artifact. +// GavelArtifact identifies a single uploaded artifact whose name starts with +// the "gavel" prefix on a PR's workflow runs. +type GavelArtifact struct { + // Name is the upload-artifact name (e.g. "gavel-results", "gavel-bench"). + Name string `json:"name"` + // ID is the GitHub artifact ID; pass to DownloadArtifact / DownloadArtifactFiles. + ID int64 `json:"id"` + // RunID is the workflow run that produced the artifact. Used to group + // artifacts coming from the same job. + RunID int64 `json:"runId"` + // URL is the human-facing artifact URL embedded in the PR comment / job summary. + URL string `json:"url"` + // SizeBytes is the compressed size reported by the API; useful for surfacing + // "too large to fetch" hints in the UI. + SizeBytes int64 `json:"sizeBytes,omitempty"` + // Expired is true when GitHub has aged out the artifact and the bytes are no + // longer downloadable. + Expired bool `json:"expired,omitempty"` +} + +// HTMLURL returns the github.com URL for browsing the artifact in the Actions UI. +func (a GavelArtifact) HTMLURL(repo string) string { + return fmt.Sprintf("https://github.com/%s/actions/runs/%d/artifacts/%d", repo, a.RunID, a.ID) +} + +type restArtifact struct { + ID int64 `json:"id"` + Name string `json:"name"` + SizeInBytes int64 `json:"size_in_bytes"` + ArchiveDownloadURL string `json:"archive_download_url"` + Expired bool `json:"expired"` + WorkflowRun struct { + ID int64 `json:"id"` + } `json:"workflow_run"` +} + +type restArtifactsResponse struct { + TotalCount int64 `json:"total_count"` + Artifacts []restArtifact `json:"artifacts"` +} + +// ListRunArtifacts returns every artifact attached to a single workflow run. +// The result is unfiltered — callers (e.g. FindGavelArtifacts) decide which +// names matter. +func ListRunArtifacts(opts Options, runID int64) ([]GavelArtifact, error) { + token, err := opts.token() + if err != nil { + return nil, fmt.Errorf("cannot list artifacts: %w", err) + } + repo, err := opts.resolveRepo() + if err != nil { + return nil, fmt.Errorf("cannot resolve repo for artifacts: %w", err) + } + + path := fmt.Sprintf("/repos/%s/actions/runs/%d/artifacts?per_page=100", repo, runID) + resp, err := cachedGet(context.Background(), token, path, nil) + if err != nil { + return nil, fmt.Errorf("list artifacts for run %d: %w", runID, err) + } + + var payload restArtifactsResponse + if err := json.Unmarshal(resp.Body, &payload); err != nil { + return nil, fmt.Errorf("parse artifacts list for run %d: %w", runID, err) + } + + out := make([]GavelArtifact, 0, len(payload.Artifacts)) + for _, a := range payload.Artifacts { + ga := GavelArtifact{ + Name: a.Name, + ID: a.ID, + RunID: a.WorkflowRun.ID, + SizeBytes: a.SizeInBytes, + Expired: a.Expired, + } + if ga.RunID == 0 { + ga.RunID = runID + } + ga.URL = ga.HTMLURL(repo) + out = append(out, ga) + } + return out, nil +} + +// FindGavelArtifacts walks every workflow run on the PR and returns artifacts +// whose name starts with the "gavel" prefix (case-insensitive). The result is +// de-duplicated by artifact ID. +// +// Discovery uses the Actions API rather than the sticky comment, so a PR with +// N parallel test jobs (matrix builds, multiple OSes, separate bench job…) +// surfaces N artifacts instead of just the most recently posted one. +func FindGavelArtifacts(opts Options, pr *PRInfo) ([]GavelArtifact, error) { + if pr == nil { + return nil, fmt.Errorf("nil PR") + } + + seenRun := make(map[int64]bool) + var runIDs []int64 + for _, check := range pr.StatusCheckRollup { + runID, err := ExtractRunID(check.DetailsURL) + if err != nil || seenRun[runID] { + continue + } + seenRun[runID] = true + runIDs = append(runIDs, runID) + } + + seenArtifact := make(map[int64]bool) + var out []GavelArtifact + for _, runID := range runIDs { + artifacts, err := ListRunArtifacts(opts, runID) + if err != nil { + logger.Warnf("list artifacts for run %d: %v", runID, err) + continue + } + for _, a := range artifacts { + if !hasGavelPrefix(a.Name) { + continue + } + if seenArtifact[a.ID] { + continue + } + seenArtifact[a.ID] = true + out = append(out, a) + } + } + return out, nil +} + +func hasGavelPrefix(name string) bool { + return strings.HasPrefix(strings.ToLower(name), "gavel") +} + +// DownloadArtifact downloads a GitHub Actions artifact ZIP and returns the +// first .json file found inside. Use DownloadArtifactFiles when an artifact +// can legitimately contain multiple JSON payloads that should be merged. func DownloadArtifact(opts Options, artifactID int64) ([]byte, error) { + files, err := DownloadArtifactFiles(opts, artifactID) + if err != nil { + return nil, err + } + for _, f := range files { + if strings.HasSuffix(strings.ToLower(f.Name), ".json") { + return f.Body, nil + } + } + return nil, fmt.Errorf("no .json file found in artifact zip") +} + +// ArtifactFile is one entry pulled from an artifact zip. +type ArtifactFile struct { + Name string + Body []byte +} + +// DownloadArtifactFiles downloads an artifact ZIP and returns every .json +// entry inside it. Useful when a single uploaded artifact bundles results +// from multiple test packages or jobs that should be merged. +func DownloadArtifactFiles(opts Options, artifactID int64) ([]ArtifactFile, error) { token, err := opts.token() if err != nil { return nil, fmt.Errorf("cannot download artifact: %w", err) @@ -76,16 +235,17 @@ func DownloadArtifact(opts Options, artifactID int64) ([]byte, error) { return nil, fmt.Errorf("download artifact %d: %w", artifactID, err) } - return extractJSONFromZip(result.Body) + return extractJSONFilesFromZip(result.Body) } -func extractJSONFromZip(data []byte) ([]byte, error) { +func extractJSONFilesFromZip(data []byte) ([]ArtifactFile, error) { r, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) if err != nil { return nil, fmt.Errorf("open artifact zip: %w", err) } + var out []ArtifactFile for _, f := range r.File { - if !strings.HasSuffix(f.Name, ".json") { + if !strings.HasSuffix(strings.ToLower(f.Name), ".json") { continue } rc, err := f.Open() @@ -93,12 +253,15 @@ func extractJSONFromZip(data []byte) ([]byte, error) { logger.Warnf("skip zip entry %s: %v", f.Name, err) continue } - defer rc.Close() - content, err := io.ReadAll(io.LimitReader(rc, 50<<20)) // 50 MB cap + content, err := io.ReadAll(io.LimitReader(rc, 50<<20)) // 50 MB cap per file + rc.Close() if err != nil { return nil, fmt.Errorf("read zip entry %s: %w", f.Name, err) } - return content, nil + out = append(out, ArtifactFile{Name: f.Name, Body: content}) } - return nil, fmt.Errorf("no .json file found in artifact zip") + if len(out) == 0 { + return nil, fmt.Errorf("no .json file found in artifact zip") + } + return out, nil } diff --git a/github/artifacts_test.go b/github/artifacts_test.go index bcb6e860e..4b115217e 100644 --- a/github/artifacts_test.go +++ b/github/artifacts_test.go @@ -1,6 +1,10 @@ package github import ( + "archive/zip" + "bytes" + "encoding/json" + "strings" "testing" ) @@ -154,3 +158,157 @@ func TestFindGavelArtifact(t *testing.T) { }) } } + +func buildZipFixture(t *testing.T, files map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for name, body := range files { + f, err := w.Create(name) + if err != nil { + t.Fatalf("zip create %s: %v", name, err) + } + if _, err := f.Write([]byte(body)); err != nil { + t.Fatalf("zip write %s: %v", name, err) + } + } + if err := w.Close(); err != nil { + t.Fatalf("zip close: %v", err) + } + return buf.Bytes() +} + +// TestExtractJSONFilesFromZipReturnsAllJSON proves the multi-file extractor +// keeps every .json entry — single-file artifacts that bundle multiple test +// shards (e.g. `gavel-results.json`, `gavel-bench.json`) need all of them +// merged downstream, not just the first one alphabetically. +func TestExtractJSONFilesFromZipReturnsAllJSON(t *testing.T) { + zipData := buildZipFixture(t, map[string]string{ + "gavel-results.json": `{"tests":[]}`, + "gavel-bench.json": `{"bench":{}}`, + "gavel.log": "log line\n", + "README.txt": "ignored", + }) + files, err := extractJSONFilesFromZip(zipData) + if err != nil { + t.Fatalf("extract: %v", err) + } + if len(files) != 2 { + t.Fatalf("expected 2 .json files, got %d (%v)", len(files), names(files)) + } + got := map[string]string{} + for _, f := range files { + got[f.Name] = string(f.Body) + } + if got["gavel-results.json"] != `{"tests":[]}` { + t.Errorf("gavel-results.json body = %q", got["gavel-results.json"]) + } + if got["gavel-bench.json"] != `{"bench":{}}` { + t.Errorf("gavel-bench.json body = %q", got["gavel-bench.json"]) + } +} + +func TestExtractJSONFilesFromZipNoJSON(t *testing.T) { + zipData := buildZipFixture(t, map[string]string{ + "gavel.log": "no json here", + "README.txt": "still nope", + }) + if _, err := extractJSONFilesFromZip(zipData); err == nil { + t.Fatal("expected error when zip has no .json file") + } +} + +func names(files []ArtifactFile) []string { + out := make([]string, len(files)) + for i, f := range files { + out[i] = f.Name + } + return out +} + +// TestHasGavelPrefixAcceptsExpectedNames pins down the set of artifact names +// that should match. Anything starting with "gavel" (case-insensitive) is in; +// names that contain "gavel" elsewhere or use other prefixes are out. +func TestHasGavelPrefixAcceptsExpectedNames(t *testing.T) { + for _, in := range []string{ + "gavel-results", "gavel-bench", "Gavel-Results", + "gavel", "gavel-self-test", "gavel-results-linux", + } { + if !hasGavelPrefix(in) { + t.Errorf("expected match for %q", in) + } + } + for _, out := range []string{ + "results", "test-results-gavel", "junit-gavel-results", "", + } { + if hasGavelPrefix(out) { + t.Errorf("unexpected match for %q", out) + } + } +} + +// TestRestArtifactsResponseUnmarshal sanity-checks the wire shape we parse +// out of /repos/{owner}/{name}/actions/runs/{id}/artifacts so a future GitHub +// payload tweak (snake_case key rename, nested workflow_run shape change) +// fails this test instead of silently returning empty results in production. +func TestRestArtifactsResponseUnmarshal(t *testing.T) { + const payload = `{ + "total_count": 2, + "artifacts": [ + { + "id": 111, + "name": "gavel-results", + "size_in_bytes": 4096, + "archive_download_url": "https://api.github.com/repos/o/r/actions/artifacts/111/zip", + "expired": false, + "workflow_run": {"id": 99} + }, + { + "id": 222, + "name": "gavel-bench", + "size_in_bytes": 2048, + "expired": true, + "workflow_run": {"id": 99} + } + ] + }` + var got restArtifactsResponse + if err := json.Unmarshal([]byte(payload), &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got.TotalCount != 2 { + t.Errorf("TotalCount = %d, want 2", got.TotalCount) + } + if len(got.Artifacts) != 2 { + t.Fatalf("artifacts = %d, want 2", len(got.Artifacts)) + } + if got.Artifacts[0].Name != "gavel-results" || got.Artifacts[0].WorkflowRun.ID != 99 { + t.Errorf("artifact 0 = %+v", got.Artifacts[0]) + } + if !got.Artifacts[1].Expired { + t.Errorf("artifact 1 should be Expired") + } +} + +// TestGavelArtifactHTMLURL pins down the artifact URL we hand the UI so +// in-app "open in Actions" links don't silently break if HTMLURL is +// refactored. +func TestGavelArtifactHTMLURL(t *testing.T) { + a := GavelArtifact{ID: 555, RunID: 999} + want := "https://github.com/owner/name/actions/runs/999/artifacts/555" + if got := a.HTMLURL("owner/name"); got != want { + t.Errorf("HTMLURL = %q, want %q", got, want) + } + // Confirm the URL is also recognised by the inverse parser — round-trip + // guards against a regex/fmt drift between the two helpers. + if !strings.Contains(want, "/runs/999/artifacts/555") { + t.Fatalf("test URL fixture is malformed: %q", want) + } + repo, runID, artID, err := ParseArtifactURL(want) + if err != nil { + t.Fatalf("ParseArtifactURL: %v", err) + } + if repo != "owner/name" || runID != 999 || artID != 555 { + t.Errorf("round-trip mismatch: repo=%q run=%d art=%d", repo, runID, artID) + } +} diff --git a/pr/ui/gavel_results.go b/pr/ui/gavel_results.go index d62ad7836..19a31d122 100644 --- a/pr/ui/gavel_results.go +++ b/pr/ui/gavel_results.go @@ -36,6 +36,37 @@ type GavelResultsSummary struct { TopFailures []TestFailure `json:"topFailures,omitempty"` // TopLintViolations lists the first 5 lint findings across all linters. TopLintViolations []LintViolation `json:"topLintViolations,omitempty"` + // Jobs is the per-job breakdown when the PR has more than one gavel + // artifact. Empty when only a single artifact was discovered. The + // outer counts/TopFailures fields are the sum across Jobs so PR + // sidebar badges and existing single-summary consumers keep working. + Jobs []GavelJobSummary `json:"jobs,omitempty"` +} + +// GavelJobSummary holds the merged results from one workflow run / job. +// When a single job uploads N gavel-* artifacts (matrix shards, separate +// bench json, etc.), all of them are downloaded and merged into one entry. +type GavelJobSummary struct { + // JobName is a human label — typically the artifact name (or a comma list + // when several artifacts were merged into the same run). + JobName string `json:"jobName"` + // RunID is the workflow run that produced these artifacts. + RunID int64 `json:"runId,omitempty"` + // ArtifactIDs lists every artifact ID merged into this entry. The first + // ID is treated as the canonical "open in UI" target. + ArtifactIDs []int64 `json:"artifactIds"` + ArtifactURL string `json:"artifactUrl,omitempty"` + TestsPassed int `json:"testsPassed"` + TestsFailed int `json:"testsFailed"` + TestsSkipped int `json:"testsSkipped"` + TestsTotal int `json:"testsTotal"` + LintViolations int `json:"lintViolations"` + LintLinters int `json:"lintLinters"` + HasBench bool `json:"hasBench"` + BenchRegressions int `json:"benchRegressions,omitempty"` + Error string `json:"error,omitempty"` + TopFailures []TestFailure `json:"topFailures,omitempty"` + TopLintViolations []LintViolation `json:"topLintViolations,omitempty"` } type TestFailure struct { @@ -82,6 +113,21 @@ func (g *gavelResultJSON) UnmarshalJSON(data []byte) error { return nil } +// mergeGavelResults concatenates tests/lint/bench from multiple JSON payloads +// belonging to the same job. The first non-nil bench wins (bench comparisons +// are aggregate-level; we don't try to merge two of them). +func mergeGavelResults(payloads ...gavelResultJSON) gavelResultJSON { + var merged gavelResultJSON + for _, p := range payloads { + merged.Tests = append(merged.Tests, p.Tests...) + merged.Lint = append(merged.Lint, p.Lint...) + if merged.Bench == nil { + merged.Bench = p.Bench + } + } + return merged +} + func computeGavelSummary(jsonBytes []byte, artifactID int64, artifactURL string) *GavelResultsSummary { var data gavelResultJSON if err := json.Unmarshal(jsonBytes, &data); err != nil { @@ -96,7 +142,14 @@ func computeGavelSummary(jsonBytes []byte, artifactID int64, artifactURL string) ArtifactID: artifactID, ArtifactURL: artifactURL, } + applyResultsToSummary(data, summary) + return summary +} +// applyResultsToSummary populates the count/top fields of a GavelResultsSummary +// from a (possibly-merged) gavelResultJSON payload. Used both by the +// single-artifact path and by the multi-artifact merge path. +func applyResultsToSummary(data gavelResultJSON, summary *GavelResultsSummary) { for _, root := range data.Tests { walkTestCounts(root, summary) } @@ -129,8 +182,149 @@ func computeGavelSummary(jsonBytes []byte, artifactID int64, artifactURL string) } } } +} - return summary +// computeGavelJobSummary downloads every artifact in `arts`, merges all of +// their .json payloads into one logical result set, and returns a single +// GavelJobSummary representing the job. +// +// `arts` MUST share a workflow run — typically the slice produced by grouping +// FindGavelArtifacts() output by RunID. Download errors on individual +// artifacts are recorded in the Error field rather than aborting the merge, +// so a partially-broken job still surfaces its successful files. +func computeGavelJobSummary(opts github.Options, arts []github.GavelArtifact) GavelJobSummary { + if len(arts) == 0 { + return GavelJobSummary{Error: "no artifacts"} + } + + job := GavelJobSummary{ + RunID: arts[0].RunID, + JobName: jobNameFromArtifacts(arts), + ArtifactURL: arts[0].URL, + } + job.ArtifactIDs = make([]int64, 0, len(arts)) + + var payloads []gavelResultJSON + var partialErrors []string + for _, a := range arts { + job.ArtifactIDs = append(job.ArtifactIDs, a.ID) + if a.Expired { + partialErrors = append(partialErrors, fmt.Sprintf("artifact %s (%d) expired", a.Name, a.ID)) + continue + } + files, err := github.DownloadArtifactFiles(opts, a.ID) + if err != nil { + partialErrors = append(partialErrors, fmt.Sprintf("download %s (%d): %v", a.Name, a.ID, err)) + continue + } + for _, f := range files { + var p gavelResultJSON + if err := json.Unmarshal(f.Body, &p); err != nil { + partialErrors = append(partialErrors, fmt.Sprintf("parse %s/%s: %v", a.Name, f.Name, err)) + continue + } + payloads = append(payloads, p) + } + } + + merged := mergeGavelResults(payloads...) + + // Reuse the summary-population logic via a transient summary, then copy + // the count fields onto the job struct. Keeps a single source of truth + // for "how do we count tests". + tmp := &GavelResultsSummary{} + applyResultsToSummary(merged, tmp) + + job.TestsPassed = tmp.TestsPassed + job.TestsFailed = tmp.TestsFailed + job.TestsSkipped = tmp.TestsSkipped + job.TestsTotal = tmp.TestsTotal + job.LintViolations = tmp.LintViolations + job.LintLinters = tmp.LintLinters + job.HasBench = tmp.HasBench + job.BenchRegressions = tmp.BenchRegressions + job.TopFailures = tmp.TopFailures + job.TopLintViolations = tmp.TopLintViolations + + if len(partialErrors) > 0 && len(payloads) == 0 { + // Total failure: surface the first error verbatim so the UI shows + // it instead of a misleading "0 tests" panel. + job.Error = partialErrors[0] + } else if len(partialErrors) > 0 { + job.Error = fmt.Sprintf("%d of %d artifact(s) failed to load: %s", + len(partialErrors), len(arts), strings.Join(partialErrors, "; ")) + } + return job +} + +// jobNameFromArtifacts produces a human label for a job entry. When all +// artifacts share a name we use that; when names differ we join them so the +// UI can show "gavel-results, gavel-bench". +func jobNameFromArtifacts(arts []github.GavelArtifact) string { + if len(arts) == 0 { + return "" + } + seen := make(map[string]bool, len(arts)) + var names []string + for _, a := range arts { + if seen[a.Name] { + continue + } + seen[a.Name] = true + names = append(names, a.Name) + } + return strings.Join(names, ", ") +} + +// summarizeGavelArtifacts groups the discovered artifacts by workflow run, +// downloads + merges each group, then folds the per-job results into a +// single PR-level summary. The PR-level summary's Jobs field carries the +// per-job breakdown for surfaces that want detail. +func summarizeGavelArtifacts(opts github.Options, arts []github.GavelArtifact) *GavelResultsSummary { + if len(arts) == 0 { + return nil + } + + byRun := make(map[int64][]github.GavelArtifact) + var runOrder []int64 + for _, a := range arts { + if _, ok := byRun[a.RunID]; !ok { + runOrder = append(runOrder, a.RunID) + } + byRun[a.RunID] = append(byRun[a.RunID], a) + } + + pr := &GavelResultsSummary{ + ArtifactID: arts[0].ID, + ArtifactURL: arts[0].URL, + } + for _, runID := range runOrder { + job := computeGavelJobSummary(opts, byRun[runID]) + pr.Jobs = append(pr.Jobs, job) + pr.TestsPassed += job.TestsPassed + pr.TestsFailed += job.TestsFailed + pr.TestsSkipped += job.TestsSkipped + pr.TestsTotal += job.TestsTotal + pr.LintViolations += job.LintViolations + pr.LintLinters += job.LintLinters + if job.HasBench { + pr.HasBench = true + } + pr.BenchRegressions += job.BenchRegressions + for _, f := range job.TopFailures { + if len(pr.TopFailures) >= 5 { + break + } + pr.TopFailures = append(pr.TopFailures, f) + } + for _, v := range job.TopLintViolations { + if len(pr.TopLintViolations) >= 5 { + break + } + pr.TopLintViolations = append(pr.TopLintViolations, v) + } + } + return pr } func walkTestCounts(t parsers.Test, s *GavelResultsSummary) { @@ -231,29 +425,39 @@ func (s *Server) getOrCreateArtifact(artifactID int64, repo string) (*artifactEn opts := s.ghOpts opts.Repo = repo - jsonBytes, err := github.DownloadArtifact(opts, artifactID) + files, err := github.DownloadArtifactFiles(opts, artifactID) if err != nil { return nil, err } - summary := computeGavelSummary(jsonBytes, artifactID, "") + // Merge every JSON entry in the zip into a single payload before + // computing summary / loading the snapshot. Single-file artifacts hit + // the same path with one payload, so behaviour is unchanged for them. + var payloads []gavelResultJSON + for _, f := range files { + var p gavelResultJSON + if err := json.Unmarshal(f.Body, &p); err != nil { + logger.Warnf("artifact %d: parse %s: %v", artifactID, f.Name, err) + continue + } + payloads = append(payloads, p) + } + if len(payloads) == 0 { + return nil, fmt.Errorf("artifact %d: no parseable .json file in zip", artifactID) + } + merged := mergeGavelResults(payloads...) + + summary := &GavelResultsSummary{ArtifactID: artifactID} + applyResultsToSummary(merged, summary) srv := testui.NewServer() - var snap testui.Snapshot - if err := json.Unmarshal(jsonBytes, &snap); err != nil { - logger.Warnf("artifact %d: unmarshal as snapshot: %v, trying legacy format", artifactID, err) - var data gavelResultJSON - if err := json.Unmarshal(jsonBytes, &data); err != nil { - return nil, fmt.Errorf("parse artifact %d: %w", artifactID, err) - } - snap = testui.Snapshot{ - Tests: data.Tests, - Lint: data.Lint, - Bench: data.Bench, - Status: testui.SnapshotStatus{ - LintRun: len(data.Lint) > 0, - }, - } + snap := testui.Snapshot{ + Tests: merged.Tests, + Lint: merged.Lint, + Bench: merged.Bench, + Status: testui.SnapshotStatus{ + LintRun: len(merged.Lint) > 0, + }, } srv.LoadSnapshot(snap) srv.MarkDone() diff --git a/pr/ui/gavel_results_test.go b/pr/ui/gavel_results_test.go index 80b669835..e0556705c 100644 --- a/pr/ui/gavel_results_test.go +++ b/pr/ui/gavel_results_test.go @@ -228,3 +228,127 @@ func TestSnapshotIncludesGavelResults(t *testing.T) { t.Errorf("marshaled snapshot missing gavelResults field: %s", b) } } + +// TestMergeGavelResultsConcatenates exercises the per-job merge: when one job +// uploads multiple gavel-* artifacts (e.g. matrix shards or a separate bench +// JSON), all of their tests/lint/bench data must aggregate into one logical +// payload. Pass + Fail + Skip counts must equal the sum across inputs. +func TestMergeGavelResultsConcatenates(t *testing.T) { + a := gavelResultJSON{} + if err := json.Unmarshal([]byte(`{"tests":[{"name":"A1","passed":true},{"name":"A2","failed":true}]}`), &a); err != nil { + t.Fatal(err) + } + b := gavelResultJSON{} + if err := json.Unmarshal([]byte(`{"tests":[{"name":"B1","passed":true}],"lint":[{"linter":"lint1","success":false,"violations":[{"file":"x.go","line":1,"message":"x"}]}]}`), &b); err != nil { + t.Fatal(err) + } + + merged := mergeGavelResults(a, b) + if len(merged.Tests) != 3 { + t.Errorf("merged tests = %d, want 3", len(merged.Tests)) + } + if len(merged.Lint) != 1 { + t.Errorf("merged lint = %d, want 1", len(merged.Lint)) + } + + summary := &GavelResultsSummary{} + applyResultsToSummary(merged, summary) + if summary.TestsPassed != 2 || summary.TestsFailed != 1 || summary.TestsTotal != 3 { + t.Errorf("summary counts wrong: passed=%d failed=%d total=%d", + summary.TestsPassed, summary.TestsFailed, summary.TestsTotal) + } + if summary.LintViolations != 1 { + t.Errorf("LintViolations = %d, want 1", summary.LintViolations) + } +} + +// TestMergeGavelResultsKeepsFirstBench documents that bench comparisons are +// not summed across artifacts — the first non-nil one wins. Two artifacts +// each carrying their own bench struct is rare, but if it happens we want a +// deterministic answer (no double-counting regressions, no panic). +func TestMergeGavelResultsKeepsFirstBench(t *testing.T) { + a := gavelResultJSON{} + b := gavelResultJSON{} + if err := json.Unmarshal([]byte(`{"bench":{"threshold":1.05}}`), &a); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal([]byte(`{"bench":{"threshold":2.0}}`), &b); err != nil { + t.Fatal(err) + } + merged := mergeGavelResults(a, b) + if merged.Bench == nil || merged.Bench.Threshold != 1.05 { + t.Errorf("expected first bench to win (threshold=1.05), got %+v", merged.Bench) + } +} + +// TestJobNameFromArtifactsDistinct asserts that distinct artifact names from +// the same job are joined into a comma-separated label so the UI can show +// "gavel-results, gavel-bench" instead of just "gavel-results". +func TestJobNameFromArtifactsDistinct(t *testing.T) { + got := jobNameFromArtifacts([]github.GavelArtifact{ + {Name: "gavel-results"}, + {Name: "gavel-bench"}, + {Name: "gavel-results"}, // duplicate must be deduped + }) + if got != "gavel-results, gavel-bench" { + t.Errorf("jobNameFromArtifacts = %q, want %q", got, "gavel-results, gavel-bench") + } +} + +// TestSummarizeGavelArtifactsGroupsByRun verifies that the PR-level rollup +// groups artifacts from the same workflow run into a single GavelJobSummary +// entry. We can't exercise the network-fetch path here (DownloadArtifactFiles +// would call out to GitHub), so we use an empty artifact set per run — the +// grouping/RunID handling is what matters. +func TestSummarizeGavelArtifactsGroupsByRun(t *testing.T) { + // Fake out the github layer by passing only metadata: the function will + // try to download and fail, recording an error on each job. That's + // enough to assert the grouping shape. + arts := []github.GavelArtifact{ + {Name: "gavel-results", ID: 1, RunID: 100, URL: "u1"}, + {Name: "gavel-bench", ID: 2, RunID: 100, URL: "u2"}, + {Name: "gavel-results", ID: 3, RunID: 200, URL: "u3"}, + } + // Without a token, DownloadArtifactFiles errors immediately at + // opts.token() — no network call. The merge logic still has to walk + // every group and produce the right number of Job entries. + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + pr := summarizeGavelArtifacts(github.Options{Repo: "o/r"}, arts) + if pr == nil { + t.Fatal("summarizeGavelArtifacts returned nil") + } + if len(pr.Jobs) != 2 { + t.Fatalf("Jobs = %d, want 2 (one per RunID)", len(pr.Jobs)) + } + // Job for RunID 100 must merge both artifact IDs. + var run100 *GavelJobSummary + for i := range pr.Jobs { + if pr.Jobs[i].RunID == 100 { + run100 = &pr.Jobs[i] + } + } + if run100 == nil { + t.Fatalf("missing RunID 100 in Jobs: %+v", pr.Jobs) + } + if len(run100.ArtifactIDs) != 2 { + t.Errorf("RunID 100 ArtifactIDs = %v, want 2 entries", run100.ArtifactIDs) + } + if run100.JobName != "gavel-results, gavel-bench" { + t.Errorf("RunID 100 JobName = %q", run100.JobName) + } + // Each job recorded an error (no token); the PR-level counts should + // therefore be zero rather than misleading partials. + if pr.TestsTotal != 0 { + t.Errorf("expected 0 tests (all downloads failed), got %d", pr.TestsTotal) + } +} + +// TestSummarizeGavelArtifactsEmpty asserts that an empty input returns nil +// so the caller can short-circuit emit("gavel", ...) and not show an empty +// "Gavel results" section in the UI. +func TestSummarizeGavelArtifactsEmpty(t *testing.T) { + if got := summarizeGavelArtifacts(github.Options{}, nil); got != nil { + t.Errorf("expected nil for empty input, got %+v", got) + } +} diff --git a/pr/ui/handler.go b/pr/ui/handler.go index 654d44abf..0441891a3 100644 --- a/pr/ui/handler.go +++ b/pr/ui/handler.go @@ -917,26 +917,42 @@ func (s *Server) handleDetail(w http.ResponseWriter, r *http.Request) { runIDs = append(runIDs, runID) } - // Start gavel results fetch in parallel + // Start gavel results fetch in parallel. Discovery first walks every + // workflow run on the PR for artifacts whose name starts with "gavel" + // (matrix shards, separate bench jobs, etc. all show up). Sticky + // comment is a fallback for cases where the runs API turns up empty — + // e.g. an artifact uploaded by a workflow whose run isn't in + // StatusCheckRollup. type gavelResult struct { summary *GavelResultsSummary } gavelCh := make(chan gavelResult, 1) allComments := append(pr.Comments, pr.ReviewThreads...) - artifactID, artifactURL, hasArtifact := github.FindGavelArtifact(allComments) - if hasArtifact { + artifacts, _ := github.FindGavelArtifacts(opts, pr) + hasArtifact := len(artifacts) > 0 + var stickyID int64 + var stickyURL string + var hasSticky bool + if !hasArtifact { + stickyID, stickyURL, hasSticky = github.FindGavelArtifact(allComments) + } + if hasArtifact || hasSticky { go func() { - jsonBytes, err := github.DownloadArtifact(opts, artifactID) + if hasArtifact { + gavelCh <- gavelResult{summarizeGavelArtifacts(opts, artifacts)} + return + } + jsonBytes, err := github.DownloadArtifact(opts, stickyID) if err != nil { - logger.Warnf("artifact %d download failed: %v", artifactID, err) + logger.Warnf("artifact %d download failed: %v", stickyID, err) gavelCh <- gavelResult{&GavelResultsSummary{ - ArtifactID: artifactID, - ArtifactURL: artifactURL, + ArtifactID: stickyID, + ArtifactURL: stickyURL, Error: err.Error(), }} - } else { - gavelCh <- gavelResult{computeGavelSummary(jsonBytes, artifactID, artifactURL)} + return } + gavelCh <- gavelResult{computeGavelSummary(jsonBytes, stickyID, stickyURL)} }() } @@ -967,7 +983,7 @@ func (s *Server) handleDetail(w http.ResponseWriter, r *http.Request) { // Wait for gavel results var gavelSummary *GavelResultsSummary - if hasArtifact { + if hasArtifact || hasSticky { gr := <-gavelCh gavelSummary = gr.summary emit("gavel", map[string]any{"gavelResults": gavelSummary}) @@ -1019,8 +1035,15 @@ func (s *Server) fetchPRDetail(repo string, number int) prDetail { result.Runs = runs result.Comments = prwatch.MergeAndFilter(pr.Comments, pr.ReviewThreads) - // Scan all comments (including general issue comments, not just review - // threads) for a gavel sticky comment with an artifact link. + // Discover gavel artifacts via the workflow runs API first, then fall + // back to the sticky-comment URL if nothing turned up. The runs API + // catches matrix shards / multiple gavel-* uploads that the single + // sticky-comment URL can't represent. + if artifacts, _ := github.FindGavelArtifacts(opts, pr); len(artifacts) > 0 { + result.GavelResults = summarizeGavelArtifacts(opts, artifacts) + s.setGavelSummary(repo, number, result.GavelResults) + return result + } allComments := append(pr.Comments, pr.ReviewThreads...) if artifactID, artifactURL, found := github.FindGavelArtifact(allComments); found { jsonBytes, err := github.DownloadArtifact(opts, artifactID) diff --git a/testrunner/path_filtering.go b/testrunner/path_filtering.go new file mode 100644 index 000000000..a12c25e28 --- /dev/null +++ b/testrunner/path_filtering.go @@ -0,0 +1,152 @@ +package testrunner + +import ( + "path/filepath" + "strings" +) + +// filterIgnoredGroups drops testGroups whose workDir is matched by an +// ignore pattern relative to baseWorkDir. This is needed because nested-go-mod +// expansion spawns independent groups (each with its own WorkDir) before +// per-package discovery happens, so package-level filtering would never see +// them. baseWorkDir is the original (top-level) workdir; patterns are +// resolved against it. +func filterIgnoredGroups(baseWorkDir string, groups []testGroup, ignore []string) []testGroup { + if len(ignore) == 0 || len(groups) == 0 { + return groups + } + normalized := make([]string, 0, len(ignore)) + for _, p := range ignore { + if n := normalizeIgnorePattern(p); n != "" { + normalized = append(normalized, n) + } + } + if len(normalized) == 0 { + return groups + } + baseAbs, err := filepath.Abs(baseWorkDir) + if err != nil { + return groups + } + out := groups[:0:0] + for _, g := range groups { + gAbs, err := filepath.Abs(g.workDir) + if err != nil { + out = append(out, g) + continue + } + rel, err := filepath.Rel(baseAbs, gAbs) + if err != nil || strings.HasPrefix(rel, "..") { + // Group lives outside the base workdir — leave it alone. + out = append(out, g) + continue + } + if rel == "." { + out = append(out, g) + continue + } + pkgLike := "./" + filepath.ToSlash(rel) + if !ignoreMatches(pkgLike, normalized) { + out = append(out, g) + } + } + return out +} + +// expandRecursiveWildcards translates Go-style recursive wildcards +// ("./...", "...") into the empty starting-path convention, which downstream +// code already interprets as "discover from WorkDir recursively". Any path +// of the form ".//..." is rewritten to "./" so the directory is +// preserved but the wildcard suffix is dropped (DiscoverPackages walks +// recursively by default). +// +// Returns a new slice; input is not mutated. An input that contains only +// "./..." (or equivalents) becomes nil. +func expandRecursiveWildcards(paths []string) []string { + if len(paths) == 0 { + return paths + } + out := make([]string, 0, len(paths)) + for _, p := range paths { + switch p { + case "./...", "...", "./", ".": + // Equivalent to "no starting path" — let the orchestrator + // discover from WorkDir. + continue + } + p = strings.TrimSuffix(p, "/...") + out = append(out, p) + } + if len(out) == 0 { + return nil + } + return out +} + +// applyIgnorePatterns returns the subset of pkgs that does not match any +// pattern in ignore. Patterns are repo-relative package paths. A bare +// directory pattern matches that directory and every package below it +// ("./bench" hides "./bench" and "./bench/sub"). Patterns may also use the +// Go-style "./bench/..." or "./bench/**" suffixes; both are treated as +// recursive directory matches. Empty ignore returns pkgs unchanged. +func applyIgnorePatterns(pkgs, ignore []string) []string { + if len(ignore) == 0 || len(pkgs) == 0 { + return pkgs + } + normalized := make([]string, 0, len(ignore)) + for _, p := range ignore { + if n := normalizeIgnorePattern(p); n != "" { + normalized = append(normalized, n) + } + } + if len(normalized) == 0 { + return pkgs + } + out := pkgs[:0:0] + for _, pkg := range pkgs { + if !ignoreMatches(pkg, normalized) { + out = append(out, pkg) + } + } + return out +} + +// normalizeIgnorePattern strips trailing /... and /** suffixes and collapses +// the path through filepath.Clean. The returned string is a directory +// prefix used as both an exact match and a path-prefix match. +func normalizeIgnorePattern(p string) string { + p = strings.TrimSpace(p) + if p == "" { + return "" + } + for _, suffix := range []string{"/...", "/**", "/*"} { + p = strings.TrimSuffix(p, suffix) + } + p = filepath.ToSlash(filepath.Clean(p)) + if p == "." { + return "" + } + if !strings.HasPrefix(p, "./") && !filepath.IsAbs(p) { + p = "./" + p + } + return p +} + +// ignoreMatches reports whether pkg matches any normalized ignore prefix. +// pkg is expected to look like "./foo/bar" (from getRelativePath). Match is +// either equality or a path-prefix check ("./bench" matches "./bench/sub"). +func ignoreMatches(pkg string, normalized []string) bool { + clean := filepath.ToSlash(filepath.Clean(pkg)) + if !strings.HasPrefix(clean, "./") && !filepath.IsAbs(clean) { + clean = "./" + clean + } + for _, pat := range normalized { + if clean == pat { + return true + } + if strings.HasPrefix(clean, pat+"/") { + return true + } + } + return false +} diff --git a/testrunner/path_filtering_unit_test.go b/testrunner/path_filtering_unit_test.go new file mode 100644 index 000000000..dbda92149 --- /dev/null +++ b/testrunner/path_filtering_unit_test.go @@ -0,0 +1,176 @@ +package testrunner + +import ( + "path/filepath" + "reflect" + "testing" +) + +func TestApplyIgnorePatterns(t *testing.T) { + tests := []struct { + name string + pkgs []string + ignore []string + want []string + }{ + { + name: "empty ignore returns input unchanged", + pkgs: []string{"./api", "./bench", "./bench/sub"}, + ignore: nil, + want: []string{"./api", "./bench", "./bench/sub"}, + }, + { + name: "bare directory ignore is recursive", + pkgs: []string{"./api", "./bench", "./bench/sub", "./bench/deep/inner"}, + ignore: []string{"./bench"}, + want: []string{"./api"}, + }, + { + name: "trailing /... is treated the same as bare dir", + pkgs: []string{"./api", "./hack", "./hack/foo"}, + ignore: []string{"./hack/..."}, + want: []string{"./api"}, + }, + { + name: "trailing /** is treated the same as bare dir", + pkgs: []string{"./api", "./hack", "./hack/foo"}, + ignore: []string{"./hack/**"}, + want: []string{"./api"}, + }, + { + name: "non-matching pattern is a no-op", + pkgs: []string{"./api", "./db"}, + ignore: []string{"./does-not-exist"}, + want: []string{"./api", "./db"}, + }, + { + name: "multiple patterns compose", + pkgs: []string{"./api", "./bench", "./hack/x", "./specs", "./tests/e2e", "./tests/unit"}, + ignore: []string{"./bench", "./hack", "./specs", "./tests/e2e"}, + want: []string{"./api", "./tests/unit"}, + }, + { + name: "exact-match without recursion does not over-match siblings", + pkgs: []string{"./bench", "./benchmarks"}, + ignore: []string{"./bench"}, + want: []string{"./benchmarks"}, + }, + { + name: "leading ./ is normalized when missing", + pkgs: []string{"./bench", "./api"}, + ignore: []string{"bench"}, + want: []string{"./api"}, + }, + { + name: "blank pattern is ignored", + pkgs: []string{"./api"}, + ignore: []string{" "}, + want: []string{"./api"}, + }, + { + name: "root pattern is rejected (would match everything)", + pkgs: []string{"./api", "./db"}, + ignore: []string{"."}, + want: []string{"./api", "./db"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := applyIgnorePatterns(tt.pkgs, tt.ignore) + // applyIgnorePatterns may return a nil slice for fully filtered input; + // normalize for comparison. + if got == nil { + got = []string{} + } + want := tt.want + if want == nil { + want = []string{} + } + if !reflect.DeepEqual(got, want) { + t.Errorf("applyIgnorePatterns(%v, %v) = %v, want %v", tt.pkgs, tt.ignore, got, want) + } + }) + } +} + +func TestExpandRecursiveWildcards(t *testing.T) { + tests := []struct { + name string + in []string + want []string + }{ + {name: "empty input", in: nil, want: nil}, + {name: "./... becomes empty (whole-workdir discovery)", in: []string{"./..."}, want: nil}, + {name: "... becomes empty", in: []string{"..."}, want: nil}, + {name: ". becomes empty", in: []string{"."}, want: nil}, + { + name: "./pkg/... is preserved without the suffix", + in: []string{"./pkg/..."}, + want: []string{"./pkg"}, + }, + { + name: "mixed wildcards and concrete paths", + in: []string{"./...", "./api", "./pkg/..."}, + want: []string{"./api", "./pkg"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := expandRecursiveWildcards(tt.in) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("expandRecursiveWildcards(%v) = %v, want %v", tt.in, got, tt.want) + } + }) + } +} + +func TestFilterIgnoredGroups(t *testing.T) { + base := filepath.FromSlash("/repo") + groups := []testGroup{ + {workDir: filepath.FromSlash("/repo")}, + {workDir: filepath.FromSlash("/repo/hack/migrate")}, + {workDir: filepath.FromSlash("/repo/hack/generate-schemas")}, + {workDir: filepath.FromSlash("/repo/sub")}, + } + + tests := []struct { + name string + ignore []string + wantDirs []string + }{ + { + name: "no ignore returns all", + ignore: nil, + wantDirs: []string{"/repo", "/repo/hack/migrate", "/repo/hack/generate-schemas", "/repo/sub"}, + }, + { + name: "bare dir ignore strips nested-module groups", + ignore: []string{"./hack"}, + wantDirs: []string{"/repo", "/repo/sub"}, + }, + { + name: "ignore that matches the base does not strip the base group", + ignore: []string{"./sub"}, + wantDirs: []string{"/repo", "/repo/hack/migrate", "/repo/hack/generate-schemas"}, + }, + { + name: "non-matching ignore is a no-op", + ignore: []string{"./does-not-exist"}, + wantDirs: []string{"/repo", "/repo/hack/migrate", "/repo/hack/generate-schemas", "/repo/sub"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filterIgnoredGroups(base, groups, tt.ignore) + gotDirs := make([]string, 0, len(got)) + for _, g := range got { + gotDirs = append(gotDirs, filepath.ToSlash(g.workDir)) + } + if !reflect.DeepEqual(gotDirs, tt.wantDirs) { + t.Errorf("filterIgnoredGroups dirs = %v, want %v", gotDirs, tt.wantDirs) + } + }) + } +} diff --git a/testrunner/runner.go b/testrunner/runner.go index d02fa46d9..fe84e6f12 100644 --- a/testrunner/runner.go +++ b/testrunner/runner.go @@ -105,6 +105,8 @@ type TestOrchestrator struct { type RunOptions struct { SyncTodos bool `json:"sync_todos,omitempty" flag:"sync-todos"` // Whether to sync test failures to TODOs StartingPaths []string `json:"starting_paths,omitempty" args:"true"` // Package paths to test (e.g., ["./pkg/testrunner"]). If empty, all packages are discovered. + Ignore []string `json:"ignore,omitempty" flag:"ignore"` // Package paths to exclude from discovery. Bare directories are recursive (e.g. ./bench excludes ./bench and ./bench/sub). Trailing /... is also accepted. + NoGitignore bool `json:"no_gitignore,omitempty" flag:"no-gitignore"` // Disable .gitignore-based pruning during package discovery. By default gitignored directories are skipped. ExtraArgs []string `json:"extra_args,omitempty" flag:"extra-args"` // Additional arguments to pass to test runners (e.g., ["--focus", "TestName"]) ShowPassed bool `json:"show_passed,omitempty" flag:"show-passed"` // Whether to show passed tests in output ShowStdout OutputMode `json:"show_stdout,omitempty" flag:"show-stdout" default:"OnFailure"` // When to show stdout: false|Never, OnFailure (default), true|Always @@ -147,6 +149,12 @@ func (opts RunOptions) Pretty() api.Text { if len(opts.StartingPaths) > 0 { text = text.Space().Append("StartingPaths: ", "text-muted").Append(clicky.CompactList(opts.StartingPaths), "text-blue-500") } + if len(opts.Ignore) > 0 { + text = text.Space().Append("Ignore: ", "text-muted").Append(clicky.CompactList(opts.Ignore), "text-blue-500") + } + if opts.NoGitignore { + text = text.Space().Append("NoGitignore: ", "text-muted").Append(icons.Check, "text-green-500") + } if len(opts.ExtraArgs) > 0 { text = text.Space().Append("ExtraArgs: ", "text-muted").Append(clicky.CompactList(opts.ExtraArgs), "text-blue-500") } @@ -245,6 +253,8 @@ type testGroup struct { func groupPathsByGitRoot(workDir string, startingPaths []string) ([]testGroup, error) { workDir, _ = filepath.Abs(workDir) + startingPaths = expandRecursiveWildcards(startingPaths) + if len(startingPaths) == 0 { return expandNestedModuleGroups([]testGroup{{workDir: workDir}}) } @@ -381,12 +391,23 @@ func Run(opts RunOptions) (any, error) { opts.WorkDir, _ = os.Getwd() } + // --no-gitignore: short-circuit gitignore-aware walking process-wide. The + // flag is package-global because the Runner interface does not thread + // per-call options through to discovery. Acceptable for a single-shot + // CLI process. + prevDisable := utils.SetGitignoreDisabled(opts.NoGitignore) + defer utils.SetGitignoreDisabled(prevDisable) + // Split starting paths by execution root so each group runs with the // correct WorkDir. Nested Go modules get their own groups. groups, err := groupPathsByGitRoot(opts.WorkDir, opts.StartingPaths) if err != nil { return nil, err } + groups = filterIgnoredGroups(opts.WorkDir, groups, opts.Ignore) + if len(groups) == 0 { + return parsers.TestSuiteResults{}, nil + } if len(groups) > 1 { return runMultiRoot(opts, groups) } @@ -662,6 +683,7 @@ func (o *TestOrchestrator) detectAndRun(frameworks []Framework, startingPaths [] packages, err = o.discoverPackagesInPaths(runner, startingPaths) } else { packages, err = runner.DiscoverPackages(o.WorkDir, o.Recursive) + packages = applyIgnorePatterns(packages, o.Ignore) } if err != nil { @@ -1213,7 +1235,7 @@ func (o *TestOrchestrator) discoverPackagesInPaths(runner runners.Runner, starti } - return lo.Uniq(allPackages), nil + return applyIgnorePatterns(lo.Uniq(allPackages), o.Ignore), nil } // parseTestResults parses test results from either stdout (go test) or report file (Ginkgo). diff --git a/testrunner/runners/buildtags.go b/testrunner/runners/buildtags.go new file mode 100644 index 000000000..c4440655e --- /dev/null +++ b/testrunner/runners/buildtags.go @@ -0,0 +1,27 @@ +package runners + +import ( + "go/build" + "path/filepath" +) + +// matchesBuildContext reports whether file (a single _test.go path) is +// compiled under the default Go build context (current GOOS/GOARCH and the +// caller's build tags). Files excluded by //go:build constraints return +// false; matching, malformed, or unreadable files return true so we keep +// the previous permissive behavior on edge cases. +// +// Used by package discovery so directories whose only test files are +// excluded by build constraints (e.g. //go:build never) are not surfaced +// as runnable packages. +func matchesBuildContext(file string) bool { + dir, name := filepath.Split(file) + if dir == "" { + dir = "." + } + matched, err := build.Default.MatchFile(dir, name) + if err != nil { + return true + } + return matched +} diff --git a/testrunner/runners/buildtags_test.go b/testrunner/runners/buildtags_test.go new file mode 100644 index 000000000..b8601f92b --- /dev/null +++ b/testrunner/runners/buildtags_test.go @@ -0,0 +1,59 @@ +package runners + +import ( + "os" + "path/filepath" + "testing" +) + +func TestMatchesBuildContext(t *testing.T) { + dir := t.TempDir() + + cases := []struct { + name string + filename string + body string + want bool + }{ + { + name: "plain test file matches", + filename: "plain_test.go", + body: "package x\n", + want: true, + }, + { + name: "//go:build never excludes the file", + filename: "never_test.go", + body: "//go:build never\n\npackage x\n", + want: false, + }, + { + name: "//go:build linux on darwin runners would not match — use a tag we do not pass", + filename: "tagged_test.go", + body: "//go:build buildtag_that_definitely_isnt_set\n\npackage x\n", + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(dir, tc.filename) + if err := os.WriteFile(path, []byte(tc.body), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + got := matchesBuildContext(path) + if got != tc.want { + t.Errorf("matchesBuildContext(%s) = %v, want %v", tc.filename, got, tc.want) + } + }) + } +} + +func TestMatchesBuildContextOnUnreadableReturnsTrue(t *testing.T) { + // Nonexistent file: MatchFile errors out. We treat error as "permissive" + // so the file isn't silently skipped on edge cases. + got := matchesBuildContext("/does/not/exist/foo_test.go") + if !got { + t.Errorf("matchesBuildContext on missing file = false, want true (permissive default)") + } +} diff --git a/testrunner/runners/ginkgo.go b/testrunner/runners/ginkgo.go index 8547abfe6..4c18a4b22 100644 --- a/testrunner/runners/ginkgo.go +++ b/testrunner/runners/ginkgo.go @@ -48,7 +48,7 @@ func (r *Ginkgo) Detect(workDir string) (bool, error) { if err != nil { return err } - if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") && hasGinkgoImports(path) { + if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") && matchesBuildContext(path) && hasGinkgoImports(path) { return errGinkgoDetected } return nil @@ -81,7 +81,7 @@ func (r *Ginkgo) DiscoverPackages(workDir string, recursive bool) ([]string, err return err } - if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") { + if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") && matchesBuildContext(path) { if hasGinkgoImports(path) { pkgDir := filepath.Dir(path) if !seen[pkgDir] { diff --git a/testrunner/runners/gotest.go b/testrunner/runners/gotest.go index 1e919c61e..4e623da27 100644 --- a/testrunner/runners/gotest.go +++ b/testrunner/runners/gotest.go @@ -56,7 +56,7 @@ func (r *GoTest) Detect(workDir string) (bool, error) { if d.IsDir() { return nil } - if strings.HasSuffix(d.Name(), "_test.go") { + if strings.HasSuffix(d.Name(), "_test.go") && matchesBuildContext(path) { return errGoTestDetected } return nil @@ -81,6 +81,9 @@ func (r *GoTest) packageHasNonGinkgoTests(pkgDir string) bool { for _, entry := range entries { if !entry.IsDir() && strings.HasSuffix(entry.Name(), "_test.go") { path := filepath.Join(pkgDir, entry.Name()) + if !matchesBuildContext(path) { + continue + } if !hasGinkgoImports(path) { return true } @@ -103,6 +106,9 @@ func (r *GoTest) inspectPackage(pkgDir string) (hasTests bool, hasBench bool) { continue } path := filepath.Join(pkgDir, entry.Name()) + if !matchesBuildContext(path) { + continue + } if hasGinkgoImports(path) { continue } @@ -162,6 +168,9 @@ func (r *GoTest) hasTestsBeyondTestMain(pkgDir string) bool { continue } path := filepath.Join(pkgDir, entry.Name()) + if !matchesBuildContext(path) { + continue + } if hasGinkgoImports(path) { continue } @@ -198,7 +207,7 @@ func (r *GoTest) DiscoverPackages(workDir string, recursive bool) ([]string, err return err } - if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") { + if !d.IsDir() && strings.HasSuffix(d.Name(), "_test.go") && matchesBuildContext(path) { pkgDir := filepath.Dir(path) if !seen[pkgDir] { seen[pkgDir] = true diff --git a/testrunner/ui/handler.go b/testrunner/ui/handler.go index 8652d4246..7a4b164c5 100644 --- a/testrunner/ui/handler.go +++ b/testrunner/ui/handler.go @@ -26,6 +26,7 @@ type Server struct { embeddedDiagnostics *DiagnosticsSnapshot updated chan struct{} gitRoot string + gavelDir string diag *DiagnosticsManager rerunMu sync.Mutex @@ -206,6 +207,8 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("/api/rerun/stream", s.handleRerunStream) mux.HandleFunc("/api/lint/ignore", s.handleLintIgnore) mux.HandleFunc("/api/benchmarks", s.handleBenchJSON) + mux.HandleFunc("/api/runs", s.handleRunsIndex) + mux.HandleFunc("/api/runs/", s.handleRunSnapshot) return mux } diff --git a/testrunner/ui/routes.go b/testrunner/ui/routes.go index 5c43fd761..bc18ba533 100644 --- a/testrunner/ui/routes.go +++ b/testrunner/ui/routes.go @@ -216,6 +216,23 @@ func parseRouteRequest(r *http.Request) (routeRequest, bool) { } segments := strings.Split(path, "/") + // `/run/` is a frontend-only prefix that selects which saved + // snapshot to load. Strip it so the rest of the route parses the same + // way as the bare `//...` form. + if segments[0] == "run" { + if len(segments) < 2 { + // `/run` with no name behaves like the index page. + req.TestFilters = parseTestFilters(r) + req.LintFilters = parseLintFilters(r) + return req, true + } + segments = segments[2:] + if len(segments) == 0 { + req.TestFilters = parseTestFilters(r) + req.LintFilters = parseLintFilters(r) + return req, true + } + } tabSeg := segments[0] pathFormat := "" if base, format := stripKnownFormat(tabSeg); format != "" { diff --git a/testrunner/ui/runs_index.go b/testrunner/ui/runs_index.go new file mode 100644 index 000000000..ed5ad9434 --- /dev/null +++ b/testrunner/ui/runs_index.go @@ -0,0 +1,263 @@ +package testui + +import ( + "encoding/json" + "fmt" + "net/http" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/flanksource/gavel/testrunner/parsers" +) + +// gavelDir is the conventional snapshot directory written by snapshots.Save. +// Duplicated here (rather than imported from the snapshots package) because +// snapshots already imports this package. +const gavelDir = ".gavel" + +// pointerJSON mirrors snapshots.Pointer's wire shape. Re-declared locally to +// avoid importing the snapshots package, which would create an import cycle. +type pointerJSON struct { + Path string `json:"path"` + SHA string `json:"sha"` + Uncommitted string `json:"uncommitted,omitempty"` +} + +type RunCounts struct { + Total int `json:"total"` + Passed int `json:"passed"` + Failed int `json:"failed"` + Skipped int `json:"skipped"` + Pending int `json:"pending"` +} + +type RunIndexEntry struct { + Name string `json:"name"` + Path string `json:"path"` + Pointer string `json:"pointer,omitempty"` + Modified time.Time `json:"modified"` + SHA string `json:"sha,omitempty"` + Started *time.Time `json:"started,omitempty"` + Ended *time.Time `json:"ended,omitempty"` + Counts *RunCounts `json:"counts,omitempty"` + Lint int `json:"lint,omitempty"` + Error string `json:"error,omitempty"` +} + +// SetGavelDir tells the server which directory holds previously-saved snapshots. +// When set, /api/runs scans this directory and /api/runs/{name} loads a single +// snapshot from it. Empty string disables both endpoints. +func (s *Server) SetGavelDir(root string) { + s.mu.Lock() + defer s.mu.Unlock() + if root == "" { + s.gavelDir = "" + return + } + s.gavelDir = filepath.Join(root, gavelDir) +} + +// GavelDir returns the directory backing /api/runs, or "" if no-arg index mode +// is disabled. +func (s *Server) GavelDir() string { + s.mu.RLock() + defer s.mu.RUnlock() + return s.gavelDir +} + +func (s *Server) handleRunsIndex(w http.ResponseWriter, _ *http.Request) { + dir := s.GavelDir() + if dir == "" { + http.NotFound(w, nil) + return + } + entries, err := buildRunIndex(dir) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(entries) //nolint:errcheck +} + +func (s *Server) handleRunSnapshot(w http.ResponseWriter, r *http.Request) { + dir := s.GavelDir() + if dir == "" { + http.NotFound(w, r) + return + } + name := strings.TrimPrefix(r.URL.Path, "/api/runs/") + if name == "" || strings.ContainsRune(name, '/') || strings.Contains(name, "..") { + http.Error(w, "invalid run name", http.StatusBadRequest) + return + } + if !strings.HasSuffix(name, ".json") { + name += ".json" + } + path := filepath.Join(dir, name) + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + http.NotFound(w, r) + return + } + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + // If this is a pointer, dereference once. + if resolved, ok := tryResolvePointer(dir, data); ok { + data = resolved + } + w.Header().Set("Content-Type", "application/json") + w.Write(data) //nolint:errcheck +} + +// tryResolvePointer detects pointer files (the small {path,sha} shape written +// by snapshots.Save) and returns the bytes of the snapshot they reference. +// Returns (nil, false) when raw is not a pointer or the pointed file cannot be +// read — caller falls back to the original bytes. +func tryResolvePointer(dir string, raw []byte) ([]byte, bool) { + var p pointerJSON + if err := json.Unmarshal(raw, &p); err != nil { + return nil, false + } + if p.Path == "" || p.SHA == "" { + return nil, false + } + target := p.Path + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(dir), target) + } + data, err := os.ReadFile(target) + if err != nil { + return nil, false + } + return data, true +} + +// buildRunIndex scans dir, returns one entry per .json file with summary +// metadata. Pointers are resolved (stats come from the underlying snapshot) +// and rendered as labeled rows pointing to the resolved snapshot. +func buildRunIndex(dir string) ([]RunIndexEntry, error) { + infos, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("read %s: %w", dir, err) + } + + pointers := make([]RunIndexEntry, 0) + snapshots := make([]RunIndexEntry, 0) + + for _, fi := range infos { + if fi.IsDir() { + continue + } + name := fi.Name() + if !strings.HasSuffix(name, ".json") { + continue + } + full := filepath.Join(dir, name) + stat, err := fi.Info() + if err != nil { + continue + } + entry := RunIndexEntry{ + Name: strings.TrimSuffix(name, ".json"), + Path: filepath.Join(gavelDir, name), + Modified: stat.ModTime(), + } + raw, err := os.ReadFile(full) + if err != nil { + entry.Error = err.Error() + snapshots = append(snapshots, entry) + continue + } + + if resolved, ok := tryResolvePointer(dir, raw); ok { + var ptr pointerJSON + _ = json.Unmarshal(raw, &ptr) + entry.Pointer = entry.Name + entry.SHA = shortSHA(ptr.SHA) + // Path points at the resolved snapshot so click-through deep links + // to the same URL as a direct snapshot row. + entry.Path = ptr.Path + fillFromSnapshot(&entry, resolved) + pointers = append(pointers, entry) + continue + } + + fillFromSnapshot(&entry, raw) + snapshots = append(snapshots, entry) + } + + sort.SliceStable(pointers, func(i, j int) bool { + return pointerOrder(pointers[i].Pointer) < pointerOrder(pointers[j].Pointer) || + (pointerOrder(pointers[i].Pointer) == pointerOrder(pointers[j].Pointer) && + pointers[i].Pointer < pointers[j].Pointer) + }) + sort.SliceStable(snapshots, func(i, j int) bool { + return snapshots[i].Modified.After(snapshots[j].Modified) + }) + + out := make([]RunIndexEntry, 0, len(pointers)+len(snapshots)) + out = append(out, pointers...) + out = append(out, snapshots...) + return out, nil +} + +func pointerOrder(name string) int { + switch name { + case "last": + return 0 + case "main", "master": + return 2 + default: + return 1 + } +} + +func fillFromSnapshot(entry *RunIndexEntry, raw []byte) { + var snap Snapshot + if err := json.Unmarshal(raw, &snap); err != nil { + entry.Error = err.Error() + return + } + if snap.Git != nil && entry.SHA == "" { + entry.SHA = shortSHA(snap.Git.SHA) + } + if snap.Metadata != nil { + if !snap.Metadata.Started.IsZero() { + t := snap.Metadata.Started + entry.Started = &t + } + if !snap.Metadata.Ended.IsZero() { + t := snap.Metadata.Ended + entry.Ended = &t + } + } + sum := parsers.Tests(snap.Tests).Sum() + if sum.Total > 0 || sum.Failed > 0 || sum.Skipped > 0 || sum.Pending > 0 { + entry.Counts = &RunCounts{ + Total: sum.Total, + Passed: sum.Passed, + Failed: sum.Failed, + Skipped: sum.Skipped, + Pending: sum.Pending, + } + } + for _, lr := range snap.Lint { + if lr == nil { + continue + } + entry.Lint += len(lr.Violations) + } +} + +func shortSHA(sha string) string { + if len(sha) > 7 { + return sha[:7] + } + return sha +} diff --git a/testrunner/ui/runs_index_test.go b/testrunner/ui/runs_index_test.go new file mode 100644 index 000000000..803ce6a77 --- /dev/null +++ b/testrunner/ui/runs_index_test.go @@ -0,0 +1,177 @@ +package testui_test + +import ( + "encoding/json" + "net/http" + "os" + "path/filepath" + "testing" + "time" + + "github.com/flanksource/gavel/testrunner/parsers" + testui "github.com/flanksource/gavel/testrunner/ui" +) + +func writeTestSnapshot(t *testing.T, path string, snap testui.Snapshot, mtime time.Time) { + t.Helper() + data, err := json.MarshalIndent(snap, "", " ") + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } + if err := os.Chtimes(path, mtime, mtime); err != nil { + t.Fatalf("chtimes %s: %v", path, err) + } +} + +func writeTestPointer(t *testing.T, path string, pointerPath, sha string) { + t.Helper() + body := map[string]string{"path": pointerPath, "sha": sha} + data, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatalf("write pointer %s: %v", path, err) + } +} + +func TestRunsIndexEndpointListsSnapshotsAndPointers(t *testing.T) { + root := t.TempDir() + gavelDir := filepath.Join(root, ".gavel") + if err := os.MkdirAll(gavelDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + older := time.Now().Add(-2 * time.Hour) + newer := time.Now().Add(-30 * time.Minute) + + snapOld := testui.Snapshot{ + Metadata: &testui.SnapshotMetadata{Started: older, Ended: older.Add(time.Minute)}, + Git: &testui.SnapshotGit{SHA: "abcdef1234567890"}, + Tests: []parsers.Test{ + {Name: "TestA", Passed: true}, + {Name: "TestB", Failed: true}, + }, + } + snapNew := testui.Snapshot{ + Metadata: &testui.SnapshotMetadata{Started: newer, Ended: newer.Add(2 * time.Minute)}, + Git: &testui.SnapshotGit{SHA: "1111111deadbeef"}, + Tests: []parsers.Test{ + {Name: "TestC", Passed: true}, + {Name: "TestD", Skipped: true}, + {Name: "TestE", Passed: true}, + }, + } + + oldPath := filepath.Join(gavelDir, "sha-abcdef1234567890.json") + newPath := filepath.Join(gavelDir, "sha-1111111deadbeef.json") + writeTestSnapshot(t, oldPath, snapOld, older) + writeTestSnapshot(t, newPath, snapNew, newer) + // Pointer that resolves to the new snapshot. + writeTestPointer(t, filepath.Join(gavelDir, "last.json"), + filepath.Join(".gavel", "sha-1111111deadbeef.json"), "1111111deadbeef") + + srv := testui.NewServer() + srv.SetGavelDir(root) + handler := srv.Handler() + + resp := doRequest(t, handler, http.MethodGet, "/api/runs", nil) + if resp.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", resp.Code, resp.Body.String()) + } + + var entries []testui.RunIndexEntry + if err := json.NewDecoder(resp.Body).Decode(&entries); err != nil { + t.Fatalf("decode: %v", err) + } + + if len(entries) != 3 { + t.Fatalf("got %d entries, want 3: %+v", len(entries), entries) + } + + // Pointer first. + if entries[0].Pointer != "last" { + t.Fatalf("entries[0].Pointer = %q, want last", entries[0].Pointer) + } + if entries[0].SHA != "1111111" { + t.Fatalf("entries[0].SHA = %q, want 1111111", entries[0].SHA) + } + if entries[0].Counts == nil || entries[0].Counts.Total != 3 { + t.Fatalf("entries[0].Counts = %+v, want Total=3", entries[0].Counts) + } + + // Snapshots after, newest first by mtime. + if entries[1].Pointer != "" { + t.Fatalf("entries[1].Pointer = %q, want empty", entries[1].Pointer) + } + if entries[1].Name != "sha-1111111deadbeef" { + t.Fatalf("entries[1].Name = %q, want sha-1111111deadbeef", entries[1].Name) + } + if entries[2].Name != "sha-abcdef1234567890" { + t.Fatalf("entries[2].Name = %q, want sha-abcdef1234567890", entries[2].Name) + } + if entries[2].Counts == nil || entries[2].Counts.Failed != 1 || entries[2].Counts.Passed != 1 { + t.Fatalf("entries[2].Counts = %+v, want Failed=1 Passed=1", entries[2].Counts) + } +} + +func TestRunsIndexEndpoint404WhenDirNotSet(t *testing.T) { + srv := testui.NewServer() + handler := srv.Handler() + resp := doRequest(t, handler, http.MethodGet, "/api/runs", nil) + if resp.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", resp.Code) + } +} + +func TestRunSnapshotEndpointResolvesPointer(t *testing.T) { + root := t.TempDir() + gavelDir := filepath.Join(root, ".gavel") + if err := os.MkdirAll(gavelDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + snap := testui.Snapshot{ + Tests: []parsers.Test{{Name: "TestX", Passed: true}}, + } + target := filepath.Join(gavelDir, "sha-deadbeef.json") + writeTestSnapshot(t, target, snap, time.Now()) + writeTestPointer(t, filepath.Join(gavelDir, "last.json"), + filepath.Join(".gavel", "sha-deadbeef.json"), "deadbeef") + + srv := testui.NewServer() + srv.SetGavelDir(root) + handler := srv.Handler() + + resp := doRequest(t, handler, http.MethodGet, "/api/runs/last.json", nil) + if resp.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", resp.Code, resp.Body.String()) + } + var got testui.Snapshot + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got.Tests) != 1 || got.Tests[0].Name != "TestX" { + t.Fatalf("got tests %+v, want [TestX]", got.Tests) + } +} + +func TestRunSnapshotEndpointRejectsTraversal(t *testing.T) { + root := t.TempDir() + gavelDir := filepath.Join(root, ".gavel") + if err := os.MkdirAll(gavelDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + srv := testui.NewServer() + srv.SetGavelDir(root) + handler := srv.Handler() + + for _, name := range []string{"..", "../etc/passwd", "foo/bar.json"} { + resp := doRequest(t, handler, http.MethodGet, "/api/runs/"+name, nil) + if resp.Code == http.StatusOK { + t.Fatalf("traversal %q returned 200; body=%s", name, resp.Body.String()) + } + } +} diff --git a/testrunner/ui/src/App.tsx b/testrunner/ui/src/App.tsx index 2ce375747..f249e3c6b 100644 --- a/testrunner/ui/src/App.tsx +++ b/testrunner/ui/src/App.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useRef, useMemo, useCallback } from 'preact/hooks'; -import type { Test, Snapshot, LinterResult, BenchComparison, DiagnosticsSnapshot, ProcessNode, ProcessDetails, RunMeta } from './types'; +import type { Test, Snapshot, LinterResult, BenchComparison, DiagnosticsSnapshot, ProcessNode, ProcessDetails, RunMeta, RunIndexEntry } from './types'; import { Summary } from './components/Summary'; import { TestNode } from './components/TestNode'; import { DetailPanel, type IgnoreRequest } from './components/DetailPanel'; @@ -10,6 +10,7 @@ import { LintFilterBar, type LintGrouping, type LintFilters } from './components import { LintView } from './components/LintView'; import { BenchView } from './components/BenchView'; import { RerunDialog } from './components/RerunDialog'; +import { RunIndex } from './components/RunIndex'; import { SplitPane } from './components/SplitPane'; import { copyCurrentViewForAgent, downloadCurrentView } from './export'; import { @@ -80,13 +81,15 @@ function applySnapshot( } function currentRouteState( + view: 'index' | 'run', + runName: string, tab: TabKey, selectedPath: string, filters: Filters, lintGrouping: LintGrouping, lintFilters: LintFilters, ): RouteState { - return { tab, selectedPath, filters, lintGrouping, lintFilters }; + return { view, runName, tab, selectedPath, filters, lintGrouping, lintFilters }; } function mergeProcessDetails(root: ProcessNode | undefined, details: ProcessDetails): ProcessNode | undefined { @@ -105,7 +108,9 @@ function mergeProcessDetails(root: ProcessNode | undefined, details: ProcessDeta } export function App() { - const initialRoute = typeof window !== 'undefined' ? parseRoute(window.location) : { + const initialRoute: RouteState = typeof window !== 'undefined' ? parseRoute(window.location) : { + view: 'run', + runName: '', tab: 'tests' as TabKey, selectedPath: '', filters: { status: defaultStatusFilter(), framework: new Map() }, @@ -124,6 +129,8 @@ export function App() { const [status, setStatus] = useState('Loading...'); const [expandAll, setExpandAll] = useState(null); const [filters, setFilters] = useState(initialRoute.filters); + const [activeView, setActiveView] = useState<'index' | 'run'>(initialRoute.view); + const [runName, setRunName] = useState(initialRoute.runName); const [activeTab, setActiveTab] = useState(initialRoute.tab); const [lintGrouping, setLintGrouping] = useState(initialRoute.lintGrouping); const [lintFilters, setLintFilters] = useState(initialRoute.lintFilters); @@ -142,11 +149,13 @@ export function App() { const copyResetTimer = useRef(null); const routeState = useMemo( - () => currentRouteState(activeTab, selectedPath, filters, lintGrouping, lintFilters), - [activeTab, selectedPath, filters, lintGrouping, lintFilters], + () => currentRouteState(activeView, runName, activeTab, selectedPath, filters, lintGrouping, lintFilters), + [activeView, runName, activeTab, selectedPath, filters, lintGrouping, lintFilters], ); const commitRoute = useCallback((next: RouteState, mode: 'push' | 'replace' = 'push') => { + setActiveView(next.view); + setRunName(next.runName); setActiveTab(next.tab); setSelectedPath(next.selectedPath); setFilters(next.filters); @@ -163,6 +172,8 @@ export function App() { useEffect(() => { const onPopState = () => { const next = parseRoute(window.location); + setActiveView(next.view); + setRunName(next.runName); setActiveTab(next.tab); setSelectedPath(next.selectedPath); setFilters(next.filters); @@ -183,6 +194,27 @@ export function App() { }, []); useEffect(() => { + if (activeView === 'index') return; + + // Static replay mode: fetch the named snapshot once, no SSE. + if (runName) { + fetch(apiUrl(`/api/runs/${encodeURIComponent(runName)}`)) + .then(async r => { + if (!r.ok) throw new Error(`HTTP ${r.status}: ${(await r.text()).trim()}`); + return r.json() as Promise; + }) + .then(snap => { + applySnapshot(snap, startTime, endTime, doneRef, setTests, setLint, setLintRun, setBench, setDiagnosticsAvailable, setDiagnostics, setRunMeta, setDone, setStatus); + doneRef.current = true; + setDone(true); + setStatus('Snapshot loaded'); + }) + .catch(e => { + setStatus(`Failed to load snapshot: ${e?.message || e}`); + }); + return; + } + if (streamToken === 0) { fetch(apiUrl('/api/tests')) .then(r => r.json()) @@ -217,7 +249,7 @@ export function App() { }, 1000); return () => { es.close(); clearInterval(timer); }; - }, [streamToken]); + }, [streamToken, activeView, runName]); const fetchDiagnostics = useCallback(async () => { const res = await fetch(apiUrl('/api/diagnostics')); @@ -323,6 +355,41 @@ export function App() { }); }, [routeState, commitRoute]); + const onIndexSelect = useCallback((entry: RunIndexEntry) => { + // Reset transient run-state so the new snapshot loads cleanly. + startTime.current = null; + endTime.current = null; + doneRef.current = false; + setTests([]); + setLint(undefined); + setLintRun(false); + setBench(undefined); + setDiagnosticsAvailable(false); + setDiagnostics(undefined); + setStatus('Loading snapshot...'); + commitRoute({ + view: 'run', + runName: entry.name, + tab: 'tests', + selectedPath: '', + filters: { status: defaultStatusFilter(), framework: new Map() }, + lintGrouping: 'linter-rule-file', + lintFilters: { severity: new Map(), linter: new Map() }, + }); + }, [commitRoute]); + + const onBackToIndex = useCallback(() => { + commitRoute({ + view: 'index', + runName: '', + tab: 'tests', + selectedPath: '', + filters: { status: defaultStatusFilter(), framework: new Map() }, + lintGrouping: 'linter-rule-file', + lintFilters: { severity: new Map(), linter: new Map() }, + }); + }, [commitRoute]); + const onSelect = useCallback((test: Test) => { const nextPath = test.route_path === selectedPath ? '' : (test.route_path || ''); commitRoute({ @@ -612,6 +679,22 @@ export function App() { const backTo = typeof window !== 'undefined' ? (window as any).__gavelBackTo as string | undefined : undefined; + if (activeView === 'index') { + return ( +
+ {backTo && ( +
+ + + Back to PR + +
+ )} + +
+ ); + } + return (
{backTo && ( @@ -622,6 +705,16 @@ export function App() {
)} + {runName && ( +
+ + / + {runName} +
+ )}
diff --git a/testrunner/ui/src/components/RunIndex.tsx b/testrunner/ui/src/components/RunIndex.tsx new file mode 100644 index 000000000..22344e783 --- /dev/null +++ b/testrunner/ui/src/components/RunIndex.tsx @@ -0,0 +1,180 @@ +import { useEffect, useState } from 'preact/hooks'; +import type { RunIndexEntry } from '../types'; +import { apiUrl } from '../config'; + +interface Props { + onSelect: (entry: RunIndexEntry) => void; +} + +function relativeTime(iso: string): string { + const t = Date.parse(iso); + if (Number.isNaN(t)) return ''; + const diff = Date.now() - t; + if (diff < 60_000) return 'just now'; + if (diff < 3_600_000) return `${Math.floor(diff / 60_000)} min ago`; + if (diff < 86_400_000) return `${Math.floor(diff / 3_600_000)} h ago`; + if (diff < 7 * 86_400_000) return `${Math.floor(diff / 86_400_000)} d ago`; + return new Date(t).toISOString().slice(0, 16).replace('T', ' '); +} + +function formatDuration(startedISO: string | undefined, endedISO: string | undefined): string { + if (!startedISO || !endedISO) return ''; + const start = Date.parse(startedISO); + const end = Date.parse(endedISO); + if (Number.isNaN(start) || Number.isNaN(end) || end < start) return ''; + const ms = end - start; + if (ms < 1000) return `${ms} ms`; + if (ms < 60_000) return `${(ms / 1000).toFixed(1)} s`; + const m = Math.floor(ms / 60_000); + const s = Math.floor((ms % 60_000) / 1000); + return `${m}m ${s}s`; +} + +function entryKey(entry: RunIndexEntry): string { + return entry.pointer ? `pointer:${entry.pointer}` : `name:${entry.name}`; +} + +export function RunIndex({ onSelect }: Props) { + const [entries, setEntries] = useState(null); + const [error, setError] = useState(''); + + useEffect(() => { + let cancelled = false; + fetch(apiUrl('/api/runs')) + .then(async r => { + if (!r.ok) throw new Error(`HTTP ${r.status}: ${(await r.text()).trim()}`); + return r.json() as Promise; + }) + .then(data => { if (!cancelled) setEntries(data); }) + .catch(e => { if (!cancelled) setError(e?.message || String(e)); }); + return () => { cancelled = true; }; + }, []); + + return ( +
+
+
+

+ + Saved Runs +

+ {entries && ( + + {entries.length} {entries.length === 1 ? 'snapshot' : 'snapshots'} in .gavel/ + + )} +
+ + {error && ( +
+ Failed to load run index: {error} +
+ )} + + {!entries && !error && ( +
+ +

Loading runs...

+
+ )} + + {entries && entries.length === 0 && ( +
+ No snapshots found in .gavel/. Run gavel test to populate it. +
+ )} + + {entries && entries.length > 0 && ( +
+ + + + + + + + + + + + {entries.map(entry => ( + + ))} + +
RunModifiedTestsLintDuration
+
+ )} +
+
+ ); +} + +interface RowProps { + entry: RunIndexEntry; + onSelect: (entry: RunIndexEntry) => void; +} + +function RunIndexRow({ entry, onSelect }: RowProps) { + const counts = entry.counts; + const failed = counts?.failed ?? 0; + const passed = counts?.passed ?? 0; + const skipped = counts?.skipped ?? 0; + const pending = counts?.pending ?? 0; + const total = counts?.total ?? 0; + const lint = entry.lint ?? 0; + + return ( + onSelect(entry)} + > + +
+ {entry.pointer ? ( + + + {entry.pointer} + + ) : ( + + )} + + {entry.pointer ? entry.name : entry.name} + + {entry.sha && ( + @{entry.sha} + )} +
+ {entry.error && ( +
{entry.error}
+ )} + + + {relativeTime(entry.modified)} + + + {total > 0 ? ( + + {failed > 0 && {failed} failed} + {failed > 0 && passed > 0 && ·} + {passed > 0 && {passed} passed} + {skipped > 0 && <>·{skipped} skipped} + {pending > 0 && <>·{pending} pending} + + ) : ( + — + )} + + + {lint > 0 ? ( + {lint} + ) : ( + — + )} + + + {formatDuration(entry.started, entry.ended) || —} + + + ); +} diff --git a/testrunner/ui/src/routes.test.ts b/testrunner/ui/src/routes.test.ts index 701423b6e..9844145a4 100644 --- a/testrunner/ui/src/routes.test.ts +++ b/testrunner/ui/src/routes.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest'; -import { buildRoute, defaultStatusFilter, parseRoute } from './routes'; + +// vitest runs in node by default; routes.ts -> config.ts touches `window` +// at import time. Stub it before importing. +(globalThis as any).window = (globalThis as any).window || {}; + +const { buildRoute, defaultStatusFilter, parseRoute } = await import('./routes'); describe('lint route grouping', () => { it('defaults lint routes to linter-rule-file grouping', () => { @@ -17,6 +22,8 @@ describe('lint route grouping', () => { it('omits the default grouping from lint URLs and keeps legacy grouping params', () => { const baseState = { + view: 'run' as const, + runName: '', tab: 'lint' as const, selectedPath: '', filters: { status: defaultStatusFilter(), framework: new Map() }, @@ -34,3 +41,58 @@ describe('lint route grouping', () => { })).toBe('/lint?grouping=linter-file'); }); }); + +describe('run index routes', () => { + it('parses / as the index view', () => { + const route = parseRoute(new URL('http://example.test/') as unknown as Location); + expect(route.view).toBe('index'); + expect(route.runName).toBe(''); + }); + + it('parses /run/ as the run view with default tab', () => { + const route = parseRoute(new URL('http://example.test/run/sha-abc123') as unknown as Location); + expect(route.view).toBe('run'); + expect(route.runName).toBe('sha-abc123'); + expect(route.tab).toBe('tests'); + }); + + it('parses /run//lint/?status=failed', () => { + const route = parseRoute(new URL('http://example.test/run/last/lint/golangci?status=failed') as unknown as Location); + expect(route.view).toBe('run'); + expect(route.runName).toBe('last'); + expect(route.tab).toBe('lint'); + expect(route.selectedPath).toBe('golangci'); + }); + + it('builds /run// for run-detail URLs', () => { + expect(buildRoute({ + view: 'run', + runName: 'sha-abc123', + tab: 'tests', + selectedPath: '', + filters: { status: defaultStatusFilter(), framework: new Map() }, + lintGrouping: 'linter-rule-file', + lintFilters: { severity: new Map(), linter: new Map() }, + })).toBe('/run/sha-abc123/tests'); + }); + + it('builds / for the index view regardless of other state', () => { + expect(buildRoute({ + view: 'index', + runName: '', + tab: 'tests', + selectedPath: '', + filters: { status: defaultStatusFilter(), framework: new Map() }, + lintGrouping: 'linter-rule-file', + lintFilters: { severity: new Map(), linter: new Map() }, + })).toBe('/'); + }); + + it('keeps backward-compat: /tests still parses as a run view with empty runName', () => { + const route = parseRoute(new URL('http://example.test/tests/foo') as unknown as Location); + expect(route.view).toBe('run'); + expect(route.runName).toBe(''); + expect(route.tab).toBe('tests'); + expect(route.selectedPath).toBe('foo'); + }); +}); diff --git a/testrunner/ui/src/routes.ts b/testrunner/ui/src/routes.ts index 216191b6f..051cba4c5 100644 --- a/testrunner/ui/src/routes.ts +++ b/testrunner/ui/src/routes.ts @@ -22,8 +22,14 @@ function isDefaultStatusFilter(state: FilterState): boolean { } export type TabKey = 'tests' | 'lint' | 'bench' | 'diagnostics'; +export type ViewKey = 'index' | 'run'; export interface RouteState { + view: ViewKey; + // runName identifies which .gavel/.json the detail view should fetch. + // Empty string means "use the in-memory snapshot" — the case when `gavel ui + // serve ` was launched with an explicit path. + runName: string; tab: TabKey; selectedPath: string; filters: Filters; @@ -45,12 +51,27 @@ export function parseRoute(location: Location): RouteState { } const trimmed = pathname.replace(/^\/+|\/+$/g, ''); const segments = trimmed ? trimmed.split('/').map(decodeURIComponent) : []; + let view: ViewKey = 'run'; + let runName = ''; let tab: TabKey = 'tests'; let selectedPath = ''; + let tabSegments = segments; + + if (segments.length === 0) { + view = 'index'; + tabSegments = []; + } else if (segments[0] === 'run') { + runName = segments[1] || ''; + if (!runName) { + // /run with no name behaves like the index — defensive against half-typed URLs. + view = 'index'; + } + tabSegments = segments.slice(2); + } - if (segments[0] === 'tests' || segments[0] === 'lint' || segments[0] === 'bench' || segments[0] === 'diagnostics') { - tab = segments[0]; - selectedPath = segments.slice(1).join('/'); + if (tabSegments[0] === 'tests' || tabSegments[0] === 'lint' || tabSegments[0] === 'bench' || tabSegments[0] === 'diagnostics') { + tab = tabSegments[0]; + selectedPath = tabSegments.slice(1).join('/'); } const params = new URLSearchParams(location.search); @@ -64,6 +85,8 @@ export function parseRoute(location: Location): RouteState { status = decodeFilterState(splitCSV(rawStatus)); } return { + view, + runName, tab, selectedPath, filters: { @@ -79,7 +102,13 @@ export function parseRoute(location: Location): RouteState { } export function buildRoute(state: RouteState): string { - const segments: string[] = [state.tab]; + if (state.view === 'index') { + return `${basePath}/`; + } + + const segments: string[] = []; + if (state.runName) segments.push('run', encodeURIComponent(state.runName)); + segments.push(state.tab); if (state.selectedPath) segments.push(...state.selectedPath.split('/').map(encodeURIComponent)); const params = new URLSearchParams(); diff --git a/testrunner/ui/src/types.ts b/testrunner/ui/src/types.ts index 2ed6cb33e..458794631 100644 --- a/testrunner/ui/src/types.ts +++ b/testrunner/ui/src/types.ts @@ -188,3 +188,24 @@ export interface FixtureContext { expected?: any; actual?: any; } + +export interface RunCounts { + total: number; + passed: number; + failed: number; + skipped: number; + pending: number; +} + +export interface RunIndexEntry { + name: string; + path: string; + pointer?: string; + modified: string; + sha?: string; + started?: string; + ended?: string; + counts?: RunCounts; + lint?: number; + error?: string; +} diff --git a/utils/walk.go b/utils/walk.go index b8c4eebb3..b18b540cd 100644 --- a/utils/walk.go +++ b/utils/walk.go @@ -13,6 +13,27 @@ import ( type walkStopFn func(root, path string, d fs.DirEntry) (bool, error) +// disableGitignoreWalk, when true, makes WalkGitIgnored / WalkGitIgnoredBounded +// behave like a plain filepath.WalkDir (still skipping .git and respecting the +// stopAtNestedProjectRoot bound, but ignoring .gitignore patterns). +// +// This is set at most once per CLI invocation from the testrunner orchestrator +// when the user passes --no-gitignore. It is process-global because the Runner +// interface threads no per-call options through to discovery, and propagating +// one would ripple through every runner implementation. Acceptable trade-off +// for a CLI binary. +var disableGitignoreWalk bool + +// SetGitignoreDisabled toggles the global gitignore-aware walking flag. +// Pass true to make subsequent WalkGitIgnored / WalkGitIgnoredBounded calls +// skip .gitignore filtering. Returns the previous value so callers can +// restore it (useful in tests). +func SetGitignoreDisabled(v bool) bool { + prev := disableGitignoreWalk + disableGitignoreWalk = v + return prev +} + func FindGitRoot(dir string) string { dir, _ = filepath.Abs(dir) for { @@ -135,7 +156,7 @@ func WalkGitIgnoredBounded(root string, fn fs.WalkDirFunc, allowList ...string) func walkGitIgnored(root string, fn fs.WalkDirFunc, stop walkStopFn, allowList ...string) error { root, _ = filepath.Abs(root) gitRoot := FindGitRoot(root) - if gitRoot == "" { + if gitRoot == "" || disableGitignoreWalk { return filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if err != nil { return fn(path, d, err) @@ -243,8 +264,12 @@ func stopAtNestedProjectRoot(root, path string, d fs.DirEntry) (bool, error) { // FilterGitIgnored returns the subset of absolute paths that are not matched // by .gitignore patterns. Uses go-git's ReadPatterns to recursively load all -// .gitignore files. If no git root is found, all paths are returned. +// .gitignore files. If no git root is found or the global disable flag is +// set (see SetGitignoreDisabled), all paths are returned. func FilterGitIgnored(paths []string, dir string) []string { + if disableGitignoreWalk { + return paths + } dir, _ = filepath.Abs(dir) gitRoot := FindGitRoot(dir) if gitRoot == "" { diff --git a/utils/walk_test.go b/utils/walk_test.go index c942d5e63..b504c33ab 100644 --- a/utils/walk_test.go +++ b/utils/walk_test.go @@ -381,3 +381,66 @@ var _ = Describe("FilterGitIgnored", func() { Expect(result).To(BeEmpty()) }) }) + +var _ = Describe("SetGitignoreDisabled", func() { + var root string + + BeforeEach(func() { + root = GinkgoT().TempDir() + }) + + AfterEach(func() { + // Always restore the default; test isolation depends on this. + SetGitignoreDisabled(false) + }) + + It("WalkGitIgnored honors gitignored paths when enabled (default)", func() { + setupGitRepo(root) + os.WriteFile(filepath.Join(root, ".gitignore"), []byte("ignored/\n"), 0644) + os.MkdirAll(filepath.Join(root, "ignored"), 0755) + os.WriteFile(filepath.Join(root, "ignored", "x.go"), nil, 0644) + os.WriteFile(filepath.Join(root, "main.go"), nil, 0644) + + paths, err := collectPaths(root) + Expect(err).NotTo(HaveOccurred()) + Expect(paths).To(ContainElement("main.go")) + Expect(paths).NotTo(ContainElement("ignored")) + }) + + It("WalkGitIgnored ignores .gitignore once disabled", func() { + setupGitRepo(root) + os.WriteFile(filepath.Join(root, ".gitignore"), []byte("ignored/\n"), 0644) + os.MkdirAll(filepath.Join(root, "ignored"), 0755) + os.WriteFile(filepath.Join(root, "ignored", "x.go"), nil, 0644) + os.WriteFile(filepath.Join(root, "main.go"), nil, 0644) + + SetGitignoreDisabled(true) + + paths, err := collectPaths(root) + Expect(err).NotTo(HaveOccurred()) + Expect(paths).To(ContainElement("main.go")) + Expect(paths).To(ContainElement("ignored")) + Expect(paths).To(ContainElement("ignored/x.go")) + }) + + It("FilterGitIgnored is bypassed once disabled", func() { + setupGitRepo(root) + os.WriteFile(filepath.Join(root, ".gitignore"), []byte("*.log\n"), 0644) + + paths := []string{ + filepath.Join(root, "main.go"), + filepath.Join(root, "debug.log"), + } + + SetGitignoreDisabled(true) + result := FilterGitIgnored(paths, root) + Expect(result).To(ConsistOf(paths)) + }) + + It("returns the previous value so callers can restore it", func() { + prev := SetGitignoreDisabled(true) + Expect(prev).To(BeFalse()) + prev2 := SetGitignoreDisabled(false) + Expect(prev2).To(BeTrue()) + }) +})