Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/actions/setup-go/action.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: "Setup Go"
description: |
Sets up the Go environment for tests, builds, etc.
inputs:
version:
description: "The Go version to use."
default: "1.26.2"
use-cache:
description: "Whether to use the cache."
default: "true"
runs:
using: "composite"
steps:
- name: Setup Go
uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0
with:
go-version: ${{ inputs.version }}
cache: ${{ inputs.use-cache }}

# It isn't necessary that we ever do this, but it helps separate the "setup"
# from the "run" times.
- name: go mod download
shell: bash
run: go mod download -x
68 changes: 68 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
name: quality

on:
push:
branches:
- main
pull_request:
workflow_dispatch:

permissions:
contents: read

# Cancel in-progress runs for pull requests when developers push additional
# changes.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

jobs:
fmt:
name: fmt
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
persist-credentials: false

- name: make fmt
run: make fmt

- name: Check unstaged
run: |
if [[ -n $(git ls-files --other --modified --exclude-standard) ]]; then
echo "Unexpected difference in directories after formatting. Run 'make fmt' and include the output in the commit."
exit 1
fi

lint:
name: lint
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
persist-credentials: false

- name: Setup Go
uses: ./.github/actions/setup-go

- name: make lint
run: make lint

test:
name: test
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
persist-credentials: false

- name: Setup Go
uses: ./.github/actions/setup-go

- name: make test
run: make test
34 changes: 34 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Binaries for programs and plugins
*.exe
*.exe~
*.dll
*.so
*.dylib

# Test binary, built with `go test -c`
*.test

# Code coverage profiles and other test artifacts
*.out
coverage.*
*.coverprofile
profile.cov

# Go workspace file
go.work
go.work.sum

# env file
.env

# Editor/IDE
.idea/
.vscode/

# Key files
*.key
*.pub
*.pem

# Output directory
build/
28 changes: 28 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
version: "2"

linters:
exclusions:
rules:
- path: _test\.go
linters:
- gosec
text: "G304: Potential file inclusion via variable"
enable:
- goconst
- gocritic
- gosec
- misspell
- nakedret
- revive
- unconvert
- unparam
settings:
govet:
enable:
- shadow
misspell:
locale: US
revive:
rules:
- name: package-comments
disabled: true
14 changes: 14 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"printWidth": 120,
"semi": false,
"trailingComma": "all",
"overrides": [
{
"files": ["./*.md", "./**/*.md"],
"options": {
"printWidth": 80,
"proseWrap": "always"
}
}
]
}
27 changes: 27 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
FIND_EXCLUSIONS= \
-not \( \( -path '*/.git/*' -o -path './build/*' -o -path './vendor/*' -o -path '*/.terraform/*' \) -prune \)
GO_SRC_FILES := $(shell find . $(FIND_EXCLUSIONS) -type f -name '*.go' -not -name '*_test.go')
GO_FMT_FILES := $(shell find . $(FIND_EXCLUSIONS) -type f -name '*.go' -print0 | xargs -0 grep -E --null -L '^// Code generated .* DO NOT EDIT\.$$' | tr '\0' ' ')

default: build

build/whichtests: $(GO_SRC_FILES) go.mod go.sum
mkdir -p ./build
go build -o ./build/whichtests .

build: build/whichtests
.PHONY: build

fmt:
go mod tidy
go run golang.org/x/tools/cmd/goimports@v0.35.0 -w $(GO_FMT_FILES)
go run mvdan.cc/gofumpt@v0.8.0 -w -l $(GO_FMT_FILES)
.PHONY: fmt

lint:
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.4.0 run ./...
.PHONY: lint

test:
go test -test.v -timeout 30s -cover ./...
.PHONY: test
63 changes: 63 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# whichtests

`whichtests` is the Go test-plan generator that drives the `flake-go` CI
workflow in `coder/coder`. Given a base/head git revision pair (or a
GitHub Actions event), it walks the diff, parses each changed test
file, picks the smallest set of tests to rerun, and emits a workflow
matrix plus a human-readable Markdown summary.

## Building and running

```sh
go build ./
./whichtests --help
```

Typical invocation against the local working tree:

```sh
./whichtests \
--repo-root . \
--base-sha origin/main \
--head-sha HEAD \
--out-matrix ./flake-matrix.json \
--out-summary -
```

In GitHub Actions:

```sh
go run ./ \
--repo-root . \
--github-actions \
--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 (`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`, 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. |
| `plan.go` | Plan construction, matrix and summary rendering (`buildExecutionPlan`, `selectTestPlan`). |
| `githubactions.go` | GitHub Actions request builder and history preparation. |
| `publish.go` | Single sink for matrix and summary outputs. |

## Testing

```sh
go test ./...
```
60 changes: 60 additions & 0 deletions broadening.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package main

type broadeningScope uint8

const (
broadeningNone broadeningScope = iota
broadeningPackage
broadeningDirectory
)

func broadeningScopeForOldHunk(decls []sharedDecl, candidate lineRange) broadeningScope {
scope := broadeningNone
for _, decl := range decls {
Comment thread
ethanndickson marked this conversation as resolved.
if !decl.Range.overlaps(candidate) {
continue
}
scope = max(scope, decl.broadeningScope())
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit Small cleanups across the rewritten code, none blocking. (Style Reviewer, Modernization Reviewer, Duplication Checker, Go Architect)

  • broadening.go:33,45 — paired sharedDecl methods are not named in parallel: broadeningScope() vs broadeningScopeOnNewSide(...). Renaming to broadeningScopeOnOldSide mirrors the existing broadeningScopeForOldHunk / broadeningScopeForNewHunk pair.
  • snapshot.go:275hasSharedKey(keys []string) reads as a single-key lookup; semantics are "true if any key is shared." Rename hasAnySharedKey.
  • selection.go:28-31parsedFileSnapshot uses lowercase fields (key, snapshot) while every other struct in the file uses Title-case. Either align to siblings or spell out Key / Snapshot.
  • selection.go:160,167,199 — three switch scope := broadeningScopeFor*Hunk(...); scope { ... } blocks declare scope but never reference it; idiomatic form is switch broadeningScopeFor*Hunk(...) {.
  • diff.go:37-53pathspecs() can collapse via cmp.Or (already imported, and displayPath two lines above uses it): oldPath := cmp.Or(change.OldPath, change.NewPath) / newPath := cmp.Or(change.NewPath, change.OldPath). Removes the unreachable empty-string branches that fall out of the cross-fill.
  • inventory.go:72-75 — same cmp.Or idiom: pathspec := cmp.Or(cleanDir, ".").
  • selection.go:250-257needsOldSnapshot is the exact shape of slices.ContainsFunc(hunks, func(h diffHunk) bool { return h.Old.hasLines() }). Adding "slices" to the imports is the only friction.
  • diff.go:14hunkHeaderPattern breaks the *RE naming used by safeTestNameRE, safePackagePatternRE, repoFullNameRE. Rename to hunkHeaderRE.
  • snapshot.go:178-196isTopLevelTestFunc / isTopLevelFuzzFunc differ only in literals ("Test"/"T" vs "Fuzz"/"F"); same for the downstream hasParamSelectorName / hasParamIdentName and pointerSelectorName / pointerIdentName pairs. Six functions encode what two parameterized helpers could.
  • cli.go:23-26 — with flag.NewFlagSet(_, flag.ExitOnError), flags.Parse calls os.Exit(2) itself; the manual error block is unreachable. Either switch to ContinueOnError (so the handler does real work and tests can probe it) or delete the dead branch.

return scope
}

func broadeningScopeForNewHunk(decls []sharedDecl, oldSnapshot *fileSnapshot, candidate lineRange) broadeningScope {
scope := broadeningNone
for _, decl := range decls {
if !decl.Range.overlaps(candidate) {
continue
}
scope = max(scope, decl.broadeningScopeOnNewSide(oldSnapshot))
}
return scope
}

func (decl sharedDecl) broadeningScope() broadeningScope {
switch decl.Kind {
case sharedDeclInit, sharedDeclTestMain:
// Go builds package and package_test files into one test binary.
// Init and TestMain changes can affect every test in the directory.
return broadeningDirectory
case sharedDeclImport, sharedDeclVar, sharedDeclConst, sharedDeclType, sharedDeclHelper:
return broadeningPackage
}
return broadeningNone
}

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
Comment thread
ethanndickson marked this conversation as resolved.
case sharedDeclInit, sharedDeclTestMain:
return broadeningDirectory
case sharedDeclVar, sharedDeclConst, sharedDeclType, sharedDeclHelper:
if oldSnapshot != nil && oldSnapshot.hasSharedKey(decl.Keys) {
return broadeningPackage
}
}
return broadeningNone
}
51 changes: 51 additions & 0 deletions broadening_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
Loading
Loading