fix: address whichtests review feedback

Author: ethanndickson

README.md

@@ -33,18 +33,22 @@ go run ./ \
   --out-matrix "$RUNNER_TEMP/flake-matrix.json"
 ```
 
+For `pull_request` events, checkout must use the PR head SHA, for example `github.event.pull_request.head.sha`. The default synthetic merge ref is rejected because the checked-out `HEAD` must match `pull_request.head.sha`.
+
+The matrix JSON contains `include` rows with `package`, `run_regex`, and `test_count`. `package` is normally one safe Go package pattern. If the matrix cap is hit, the final overflow row stores a space-separated list of safe package tokens in `package`, leaves `run_regex` empty, and sets `test_count` to `1`; this is the contract consumed by the current `flake-go` workflow.
+
 ## File layout
 
 The binary is a single `package main`, split into focused files:
 
 | File            | Responsibility                                                      |
 | --------------- | ------------------------------------------------------------------- |
-| `cli.go`        | `main`, flag parsing, command orchestration (`run`, `runCommand`).  |
+| `cli.go`        | `main`, flag parsing, command orchestration (`runCommand`).         |
 | `config.go`     | `config` / `commandConfig` types and defaults.                      |
 | `request.go`    | `runRequest`, `diffRange`, revision validation.                     |
 | `gitexec.go`    | `gitRunner` / `gitFetcher` types and the real `exec.Command` impl.  |
 | `diff.go`       | Reading and parsing `git diff`, change kinds, hunks, line ranges.   |
-| `snapshot.go`   | AST snapshot parsing, `fileSnapshot`, `sharedDecl`, fallbacks.      |
+| `snapshot.go`   | AST snapshot parsing, `fileSnapshot`, and `sharedDecl`.             |
 | `broadening.go` | Per-kind broadening rules (`broadeningScope`).                      |
 | `selection.go`  | Per-change selection logic (`selectChange`, broaden vs narrow).     |
 | `inventory.go`  | `inventoryCache` for package/directory test discovery.              |

broadening.go

@@ -9,24 +9,25 @@ const (
 )
 
 func broadeningScopeForOldHunk(decls []sharedDecl, candidate lineRange) broadeningScope {
+	scope := broadeningNone
 	for _, decl := range decls {
-		if decl.Range.overlaps(candidate) {
-			return decl.broadeningScope()
+		if !decl.Range.overlaps(candidate) {
+			continue
 		}
+		scope = max(scope, decl.broadeningScope())
 	}
-	return broadeningNone
+	return scope
 }
 
 func broadeningScopeForNewHunk(decls []sharedDecl, oldSnapshot *fileSnapshot, candidate lineRange) broadeningScope {
+	scope := broadeningNone
 	for _, decl := range decls {
 		if !decl.Range.overlaps(candidate) {
 			continue
 		}
-		if scope := decl.broadeningScopeOnNewSide(oldSnapshot); scope != broadeningNone {
-			return scope
-		}
+		scope = max(scope, decl.broadeningScopeOnNewSide(oldSnapshot))
 	}
-	return broadeningNone
+	return scope
 }
 
 func (decl sharedDecl) broadeningScope() broadeningScope {
@@ -43,6 +44,9 @@ func (decl sharedDecl) broadeningScope() broadeningScope {
 
 func (decl sharedDecl) broadeningScopeOnNewSide(oldSnapshot *fileSnapshot) broadeningScope {
 	switch decl.Kind {
+	// TODO: Decide whether new imports should narrow to tests that still
+	// reference package-local declarations. Today any import edit broadens
+	// the package.
 	case sharedDeclImport:
 		return broadeningPackage
 	case sharedDeclInit, sharedDeclTestMain:

broadening_test.go

@@ -0,0 +1,51 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestBroadeningScopeForOldHunkChoosesMaxOverlappingScope(t *testing.T) {
+	t.Parallel()
+
+	data := []byte(`package sample
+
+import "testing"
+
+func init() {
+	register()
+}
+
+func TestAlpha(t *testing.T) {}
+`)
+	snapshot, err := parseFileSnapshot(data)
+	require.NoError(t, err)
+	candidate := rangeSpan(
+		singleLineRange(t, string(data), `import "testing"`),
+		singleLineRange(t, string(data), "register()"),
+	)
+	require.Equal(t, broadeningDirectory, broadeningScopeForOldHunk(snapshot.shared, candidate))
+}
+
+func TestBroadeningScopeForNewHunkChoosesMaxOverlappingScope(t *testing.T) {
+	t.Parallel()
+
+	data := []byte(`package sample
+
+import "testing"
+
+func TestMain(m *testing.M) {
+	m.Run()
+}
+
+func TestAlpha(t *testing.T) {}
+`)
+	snapshot, err := parseFileSnapshot(data)
+	require.NoError(t, err)
+	candidate := rangeSpan(
+		singleLineRange(t, string(data), `import "testing"`),
+		singleLineRange(t, string(data), "m.Run()"),
+	)
+	require.Equal(t, broadeningDirectory, broadeningScopeForNewHunk(snapshot.shared, nil, candidate))
+}

cli.go

@@ -4,12 +4,11 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
 	"os"
-
-	"golang.org/x/xerrors"
 )
 
 func main() {
@@ -21,11 +20,6 @@ func main() {
 	flags.StringVar(&cfg.OutMatrix, "out-matrix", cfg.OutMatrix, "path to write workflow matrix JSON")
 	flags.StringVar(&cfg.OutSummary, "out-summary", cfg.OutSummary, "path to write Markdown summary, or - for stdout")
 	flags.BoolVar(&cfg.GitHubActions, "github-actions", cfg.GitHubActions, "read diff range and output paths from GitHub Actions environment")
-	flags.StringVar(&cfg.GitHubEventName, "github-event-name", cfg.GitHubEventName, "override GITHUB_EVENT_NAME")
-	flags.StringVar(&cfg.GitHubEventPath, "github-event-path", cfg.GitHubEventPath, "override GITHUB_EVENT_PATH")
-	flags.StringVar(&cfg.GitHubRepository, "github-repository", cfg.GitHubRepository, "override GITHUB_REPOSITORY")
-	flags.StringVar(&cfg.GitHubOutput, "github-output", cfg.GitHubOutput, "override GITHUB_OUTPUT")
-	flags.StringVar(&cfg.GitHubStepSummary, "github-step-summary", cfg.GitHubStepSummary, "override GITHUB_STEP_SUMMARY")
 	if err := flags.Parse(os.Args[1:]); err != nil {
 		_, _ = fmt.Fprintln(os.Stderr, err)
 		os.Exit(2)
@@ -36,14 +30,6 @@ func main() {
 	}
 }
 
-func run(ctx context.Context, cfg config, stdout, stderr io.Writer, git gitRunner) error {
-	req, err := explicitRunRequest(cfg)
-	if err != nil {
-		return err
-	}
-	return executeRunRequest(ctx, req, stdout, stderr, git, nil)
-}
-
 func runCommand(ctx context.Context, cfg commandConfig, stdout, stderr io.Writer, git gitRunner, fetch gitFetcher) error {
 	var (
 		req runRequest
@@ -63,15 +49,15 @@ func runCommand(ctx context.Context, cfg commandConfig, stdout, stderr io.Writer
 func explicitRunRequest(cfg config) (runRequest, error) {
 	cfg = cfg.withDefaults()
 	if cfg.BaseSHA == "" {
-		return runRequest{}, xerrors.New("--base-sha is required")
+		return runRequest{}, errors.New("--base-sha is required")
 	}
 	if cfg.OutMatrix == "" {
-		return runRequest{}, xerrors.New("--out-matrix is required")
+		return runRequest{}, errors.New("--out-matrix is required")
 	}
-	if err := validateRevision("--base-sha", cfg.BaseSHA); err != nil {
+	if err := validateRevisionArg("--base-sha", cfg.BaseSHA); err != nil {
 		return runRequest{}, err
 	}
-	if err := validateRevision("--head-sha", cfg.HeadSHA); err != nil {
+	if err := validateRevisionArg("--head-sha", cfg.HeadSHA); err != nil {
 		return runRequest{}, err
 	}
 	return runRequest{
@@ -101,7 +87,7 @@ func executeRunRequest(ctx context.Context, req runRequest, stdout, stderr io.Wr
 		return err
 	}
 	summary := renderSummary(changedFiles, result.Summary)
-	if err := publishPlan(req.Sinks, result.Matrix, summary, stdout, req.OutputSizeLimit); err != nil {
+	if err := publishPlan(req.Sinks, result.Matrix, summary, stdout); err != nil {
 		return err
 	}
 	_, _ = fmt.Fprintf(stderr, "selected %d package targets from %d changed test files\n", len(result.Matrix.Include), len(changedFiles))

cli_test.go

@@ -4,12 +4,12 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"golang.org/x/xerrors"
 )
 
 func TestRunValidationErrors(t *testing.T) {
@@ -18,22 +18,22 @@ func TestRunValidationErrors(t *testing.T) {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
 	neverGit := func(_ context.Context, _ string, _ ...string) (gitResult, error) {
-		return gitResult{}, xerrors.New("git should not be called")
+		return gitResult{}, errors.New("git should not be called")
 	}
 
-	err := run(t.Context(), config{OutMatrix: "matrix.json"}, &stdout, &stderr, neverGit)
+	err := runCommand(t.Context(), commandConfig{config: config{OutMatrix: "matrix.json"}}, &stdout, &stderr, neverGit, nil)
 	require.EqualError(t, err, "--base-sha is required")
 
-	err = run(t.Context(), config{BaseSHA: "base"}, &stdout, &stderr, neverGit)
+	err = runCommand(t.Context(), commandConfig{config: config{BaseSHA: "base"}}, &stdout, &stderr, neverGit, nil)
 	require.EqualError(t, err, "--out-matrix is required")
 
-	err = run(t.Context(), config{BaseSHA: "-bad", OutMatrix: "matrix.json"}, &stdout, &stderr, neverGit)
+	err = runCommand(t.Context(), commandConfig{config: config{BaseSHA: "-bad", OutMatrix: "matrix.json"}}, &stdout, &stderr, neverGit, nil)
 	require.ErrorContains(t, err, "must not start with '-'")
 
-	err = run(t.Context(), config{BaseSHA: "base:bad", OutMatrix: "matrix.json"}, &stdout, &stderr, neverGit)
+	err = runCommand(t.Context(), commandConfig{config: config{BaseSHA: "base:bad", OutMatrix: "matrix.json"}}, &stdout, &stderr, neverGit, nil)
 	require.ErrorContains(t, err, "must not contain ':'")
 
-	err = run(t.Context(), config{BaseSHA: "base\x00bad", OutMatrix: "matrix.json"}, &stdout, &stderr, neverGit)
+	err = runCommand(t.Context(), commandConfig{config: config{BaseSHA: "base\x00bad", OutMatrix: "matrix.json"}}, &stdout, &stderr, neverGit, nil)
 	require.ErrorContains(t, err, "must not contain NUL bytes")
 }
 
@@ -99,7 +99,7 @@ func TestShared(t *testing.T) {
 	summaryPath := filepath.Join(repoRoot, "summary.md")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 	require.Empty(t, stdout.String())
 	require.Contains(t, stderr.String(), "selected 2 package targets")
@@ -159,7 +159,7 @@ func TestAlpha(t *testing.T) {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: "-"}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: "-"}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 	require.Contains(t, stdout.String(), "## Go test flake detector selection")
 	require.Contains(t, stdout.String(), "### `./pkg`")
@@ -231,7 +231,7 @@ func TestMain(m *testing.M) {
 	summaryPath := filepath.Join(repoRoot, "summary.md")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -289,7 +289,7 @@ func TestRenamed(t *testing.T) {
 	summaryPath := filepath.Join(repoRoot, "summary.md")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -352,7 +352,7 @@ func TestHead(t *testing.T) {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -408,7 +408,7 @@ func TestHiddenIgnored(t *testing.T) {
 	summaryPath := filepath.Join(repoRoot, "summary.md")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -477,7 +477,7 @@ func TestPlatform(t *testing.T) {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -531,7 +531,7 @@ func TestAlpha(t *testing.T) {
 	summaryPath := filepath.Join(repoRoot, "summary.md")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath, OutSummary: summaryPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -600,7 +600,7 @@ func init() {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -660,7 +660,7 @@ func TestMoved(t *testing.T) {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput
@@ -731,7 +731,7 @@ func TestNewStable(t *testing.T) {
 	matrixPath := filepath.Join(repoRoot, "matrix.json")
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
-	err := run(t.Context(), config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}, &stdout, &stderr, repo.runner(t))
+	err := runCommand(t.Context(), commandConfig{config: config{RepoRoot: repoRoot, BaseSHA: "base", HeadSHA: "head", OutMatrix: matrixPath}}, &stdout, &stderr, repo.runner(t), nil)
 	require.NoError(t, err)
 
 	var matrix matrixOutput

config.go

@@ -5,11 +5,11 @@ import (
 )
 
 const (
-	defaultRepoRoot    = "."
-	defaultHeadSHA     = "HEAD"
-	defaultOutSummary  = "-"
-	defaultTargetCount = "10"
-	runOnceTargetCount = "1"
+	defaultRepoRoot   = "."
+	defaultHeadSHA    = "HEAD"
+	defaultOutSummary = "-"
+	defaultTestCount  = "10"
+	runOnceTestCount  = "1"
 
 	// Package-wide and matrix-wide caps keep the detector cheap by
 	// running broad fallback targets once instead of repeatedly.
@@ -44,13 +44,7 @@ func (cfg config) withDefaults() config {
 type commandConfig struct {
 	config
 
-	GitHubActions     bool
-	GitHubEventName   string
-	GitHubEventPath   string
-	GitHubRepository  string
-	GitHubOutput      string
-	GitHubStepSummary string
-	Env               map[string]string
+	GitHubActions bool
 }
 
 func defaultCommandConfig() commandConfig {