From d0e45a3d5634f4f15f0eb419125f7b4454dcbdfb Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 14:59:14 +0200 Subject: [PATCH 1/9] ci: shard Go race tests into a 3-way matrix (PLT-959) Race Detection in go-test.yml ran all ~420 packages in a single uci-default job (~18min), while the Makefile's test-group-N/NUM_SPLIT sharding sat unused by CI. Wire the race job to a dynamically-generated matrix (same pattern as eth_blocktests.yml) backed by `make split-test-packages`, with an aggregate job keeping the "Race Detection" check name so branch protection doesn't need to change. Also exclude sei-db/state_db from the Makefile's package list so `make test-group-N` matches the CI shards exactly. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/go-test.yml | 47 ++++++++++++++++++++++++++++++++--- AGENTS.md | 9 ++++--- Makefile | 6 ++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index f0364023f7..9b8add2ed5 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -18,11 +18,32 @@ env: # state_db tests run in their own workflow (sei-db-tests.yml); exclude # that subtree everywhere in this workflow to avoid double-running them. STATE_DB_PKG_PREFIX: github.com/sei-protocol/sei-chain/sei-db/state_db + # Number of race-detection shards. Matches `NUM_SPLIT` passed to + # `make test-group-N` so shards are locally reproducible. + NUM_SPLIT: 3 jobs: + shard-indexes: + name: Generate race shard indexes + runs-on: ubuntu-latest + outputs: + json: ${{ steps.generate-index-list.outputs.json }} + steps: + - id: generate-index-list + run: | + MAX_INDEX=$((${{ env.NUM_SPLIT }}-1)) + INDEX_LIST=$(seq 0 ${MAX_INDEX}) + INDEX_JSON=$(jq --null-input --compact-output '. |= [inputs]' <<< ${INDEX_LIST}) + echo "json=${INDEX_JSON}" >> "$GITHUB_OUTPUT" + test: - name: Race Detection + name: "Race Detection (shard ${{ matrix.shard }})" runs-on: uci-default + needs: shard-indexes + strategy: + fail-fast: false + matrix: + shard: ${{ fromJson(needs.shard-indexes.outputs.json) }} env: GOFLAGS: -race -tags=ledger,test_ledger_mock steps: @@ -52,9 +73,27 @@ jobs: - name: Go test run: | set -euo pipefail - PKGS=$(go list ./... | grep -v "^${STATE_DB_PKG_PREFIX}") - echo "$PKGS" | xargs go test \ - -timeout=${{ env.GO_TEST_TIMEOUT }} + make split-test-packages NUM_SPLIT=${{ env.NUM_SPLIT }} + SHARD_FILE=build/packages.txt.${{ matrix.shard }} + PARALLEL=() + if grep -qx "github.com/sei-protocol/sei-chain/occ_tests" "$SHARD_FILE"; then + echo "occ_tests present in this shard; forcing -parallel=1" + PARALLEL=(-parallel=1) + fi + xargs go test "${PARALLEL[@]}" -timeout=${{ env.GO_TEST_TIMEOUT }} < "$SHARD_FILE" + + test-check: + name: Race Detection + runs-on: ubuntu-latest + needs: test + if: always() + steps: + - name: Check shard results + run: | + if [[ "${{ needs.test.result }}" != "success" ]]; then + echo "One or more Race Detection shards failed" + exit 1 + fi coverage: name: Coverage diff --git a/AGENTS.md b/AGENTS.md index 758d5083f7..20cbd08f47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,12 +65,13 @@ make build # build the seid binary into ./build/seid make install # install seid into $GOBIN ``` -Tests run with the race detector and coverage. CI shards them into groups; while -iterating, run a single package directly: +Tests run with the race detector and coverage. `go-test.yml`'s Race Detection job +shards into `NUM_SPLIT` (currently 3) parallel matrix jobs; `make test-group-N` +reproduces a given shard locally with the same package split: ```bash -make test-group-0 # one CI test shard (race + coverage) -go test .//... # run a single package +NUM_SPLIT=3 make test-group-0 # reproduce CI race shard 0 locally +go test .//... # run a single package ``` CI mirrors these checks: `.github/workflows/golangci.yml` runs golangci-lint diff --git a/Makefile b/Makefile index 82448449f0..b97cb3c2f4 100644 --- a/Makefile +++ b/Makefile @@ -569,6 +569,10 @@ GO_TEST_FILES != find $(CURDIR) -name "*_test.go" # default to four splits by default NUM_SPLIT ?= 4 +# state_db tests run in their own workflow (sei-db-tests.yml); exclude that +# subtree here too so local shards match the CI shards exactly. +STATE_DB_PKG_PREFIX := github.com/sei-protocol/sei-chain/sei-db/state_db + $(BUILDDIR): mkdir -p $@ @@ -576,7 +580,7 @@ $(BUILDDIR): # Note we need to check for both in-package tests (.TestGoFiles) and # out-of-package tests (.XTestGoFiles). $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) - go list -f "{{ if (or .TestGoFiles .XTestGoFiles) }}{{ .ImportPath }}{{ end }}" ./... | sort > $@ + go list -f "{{ if (or .TestGoFiles .XTestGoFiles) }}{{ .ImportPath }}{{ end }}" ./... | grep -v "^$(STATE_DB_PKG_PREFIX)" | sort > $@ TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests From 8b9241460c2d6de27217755090d9ecc60075ad8f Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 15:06:58 +0200 Subject: [PATCH 2/9] ci: fix shard-file lookup to not assume split's suffix width split -d -n l/N doesn't necessarily narrow numeric suffixes to N's digit count (observed emitting packages.txt.00 instead of .0 on the CI runner), so both the go-test.yml shard step and the pre-existing `make test-group-N` target were reading a file that didn't exist. Pick the shard file by sorted position instead of assumed name. --- .github/workflows/go-test.yml | 9 ++++++++- Makefile | 9 +++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 9b8add2ed5..b6b2904798 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -74,7 +74,14 @@ jobs: run: | set -euo pipefail make split-test-packages NUM_SPLIT=${{ env.NUM_SPLIT }} - SHARD_FILE=build/packages.txt.${{ matrix.shard }} + # `split`'s numeric-suffix width isn't guaranteed to match NUM_SPLIT's + # digit count (e.g. it may emit packages.txt.00 instead of .0), so + # pick the shard file by sorted position rather than assumed name. + SHARD_FILE=$(ls build/packages.txt.* | sort | sed -n "$(( ${{ matrix.shard }} + 1 ))p") + if [[ -z "$SHARD_FILE" ]]; then + echo "no shard file found for index ${{ matrix.shard }}" >&2 + exit 1 + fi PARALLEL=() if grep -qx "github.com/sei-protocol/sei-chain/occ_tests" "$SHARD_FILE"; then echo "occ_tests present in this shard; forcing -parallel=1" diff --git a/Makefile b/Makefile index b97cb3c2f4..6dc7932fe2 100644 --- a/Makefile +++ b/Makefile @@ -588,11 +588,16 @@ split-test-packages:$(BUILDDIR)/packages.txt split -d -n l/$(NUM_SPLIT) $< $<. test-group-%:split-test-packages @echo "🔍 Checking for special package: $(TARGET_PACKAGE)" - @if grep -q "$(TARGET_PACKAGE)" $(BUILDDIR)/packages.txt.$*; then \ + @SHARD_FILE=$$(ls $(BUILDDIR)/packages.txt.* | sort | sed -n "$$(( $* + 1 ))p"); \ + if [ -z "$$SHARD_FILE" ]; then \ + echo "no shard file found for index $*" >&2; \ + exit 1; \ + fi; \ + if grep -q "$(TARGET_PACKAGE)" "$$SHARD_FILE"; then \ echo "🔒 Found $(TARGET_PACKAGE), running with -parallel=1"; \ PARALLEL="-parallel=1"; \ else \ echo "⚡ Not found, running with -parallel=4"; \ PARALLEL="-parallel=4"; \ fi; \ - cat $(BUILDDIR)/packages.txt.$* | xargs go test $$PARALLEL -mod=readonly -timeout=10m -race -coverprofile=$*.profile.out -covermode=atomic -coverpkg=./... + cat "$$SHARD_FILE" | xargs go test $$PARALLEL -mod=readonly -timeout=10m -race -coverprofile=$*.profile.out -covermode=atomic -coverpkg=./... From 34d246c483b91ba7510e6ec8d57a98ba7a913925 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 16:06:25 +0200 Subject: [PATCH 3/9] ci: balance race-detection shards by historical package duration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Count-based sharding (split -d -n l/N) split the ~420 test packages into equal-sized chunks with no awareness of runtime, so one shard absorbed a cluster of slow sei-ibc-go/sei-db packages and took ~17min while another finished in ~5min — wall-clock stayed pinned to the slowest shard, barely beating the pre-sharding baseline. Add .github/scripts/testsplit, a small dependency-free Go tool: - `plan` queries the GitHub API at run time for the last successful `main` run's timing artifact and bin-packs packages across shards by known duration (falling back to a deterministic round-robin split if the lookup fails, times out, or coverage is too thin — never a hard failure). - `record` parses `go test`'s own per-package summary lines to capture this shard's durations. - `merge` combines all shards' timings into one artifact for the next run's `plan` to consume. No cross-run cache/file persistence: each run publishes its own `package-timings` artifact and the next run finds it via the Actions API, scoped to the last successful `main` push so a noisy PR branch can never skew another PR's shard assignment. Also replaces the Makefile's `split -d -n l/N` with the same tool, removing the class of bug hit in the previous commit (GNU split's numeric-suffix width not matching NUM_SPLIT's digit count) by controlling the shard filenames directly instead of guessing them. --- .github/scripts/testsplit/main.go | 33 +++ .github/scripts/testsplit/merge.go | 40 +++ .github/scripts/testsplit/plan.go | 301 +++++++++++++++++++++++ .github/scripts/testsplit/plan_test.go | 116 +++++++++ .github/scripts/testsplit/record.go | 72 ++++++ .github/scripts/testsplit/record_test.go | 53 ++++ .github/workflows/go-test.yml | 81 +++++- AGENTS.md | 12 +- Makefile | 17 +- 9 files changed, 705 insertions(+), 20 deletions(-) create mode 100644 .github/scripts/testsplit/main.go create mode 100644 .github/scripts/testsplit/merge.go create mode 100644 .github/scripts/testsplit/plan.go create mode 100644 .github/scripts/testsplit/plan_test.go create mode 100644 .github/scripts/testsplit/record.go create mode 100644 .github/scripts/testsplit/record_test.go diff --git a/.github/scripts/testsplit/main.go b/.github/scripts/testsplit/main.go new file mode 100644 index 0000000000..582eff1184 --- /dev/null +++ b/.github/scripts/testsplit/main.go @@ -0,0 +1,33 @@ +// Command testsplit splits the Go package list for the Race Detection CI +// job across N shards, balancing by historical per-package test duration +// when available and falling back to round-robin when it isn't. +package main + +import ( + "fmt" + "os" +) + +func main() { + if len(os.Args) < 2 { + fmt.Fprintln(os.Stderr, "usage: testsplit [flags]") + os.Exit(2) + } + + var err error + switch os.Args[1] { + case "plan": + err = runPlan(os.Args[2:]) + case "record": + err = runRecord(os.Args[2:]) + case "merge": + err = runMerge(os.Args[2:]) + default: + fmt.Fprintf(os.Stderr, "unknown subcommand %q\n", os.Args[1]) + os.Exit(2) + } + if err != nil { + fmt.Fprintln(os.Stderr, "testsplit:", err) + os.Exit(1) + } +} diff --git a/.github/scripts/testsplit/merge.go b/.github/scripts/testsplit/merge.go new file mode 100644 index 0000000000..b0f1283389 --- /dev/null +++ b/.github/scripts/testsplit/merge.go @@ -0,0 +1,40 @@ +package main + +import ( + "encoding/json" + "flag" + "fmt" + "maps" + "os" +) + +// runMerge combines each shard's timings.shard-N.json into a single +// package_timings.json. Shards test disjoint package sets, so key +// collisions aren't expected; if one occurs, the last file wins. +func runMerge(args []string) error { + fs := flag.NewFlagSet("merge", flag.ExitOnError) + out := fs.String("out", "", "path to write the merged timing JSON to") + if err := fs.Parse(args); err != nil { + return err + } + inputs := fs.Args() + if *out == "" || len(inputs) == 0 { + return fmt.Errorf("--out and at least one input file are required") + } + + merged := map[string]float64{} + for _, path := range inputs { + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading %s: %w", path, err) + } + var timings map[string]float64 + if err := json.Unmarshal(data, &timings); err != nil { + return fmt.Errorf("decoding %s: %w", path, err) + } + maps.Copy(merged, timings) + } + + fmt.Fprintf(os.Stderr, "testsplit: merged %d package timings from %d shard files\n", len(merged), len(inputs)) + return writeTimings(*out, merged) +} diff --git a/.github/scripts/testsplit/plan.go b/.github/scripts/testsplit/plan.go new file mode 100644 index 0000000000..49a7aaa95f --- /dev/null +++ b/.github/scripts/testsplit/plan.go @@ -0,0 +1,301 @@ +package main + +import ( + "archive/zip" + "bufio" + "bytes" + "encoding/json" + "flag" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "sort" + "time" +) + +// minTimingCoverage is the fraction of packages that must have a known +// duration before we trust bin-packing over round-robin. Below this, too +// many durations would be guesses and bin-packing wouldn't out-perform a +// plain round-robin split. +const minTimingCoverage = 0.5 + +func runPlan(args []string) error { + fs := flag.NewFlagSet("plan", flag.ExitOnError) + numSplit := fs.Int("num-split", 0, "number of shards to split packages into") + outDir := fs.String("out-dir", "build", "directory to write packages.txt.N shard files into") + repo := fs.String("repo", os.Getenv("GITHUB_REPOSITORY"), "owner/repo to query for prior timing data") + workflow := fs.String("workflow", "go-test.yml", "workflow file name to query for the last successful run") + branch := fs.String("branch", "main", "branch to source prior timing data from") + if err := fs.Parse(args); err != nil { + return err + } + if *numSplit < 1 { + return fmt.Errorf("--num-split must be >= 1, got %d", *numSplit) + } + + packages, err := readPackages(os.Stdin) + if err != nil { + return fmt.Errorf("reading package list: %w", err) + } + if len(packages) == 0 { + return fmt.Errorf("no packages given on stdin") + } + + var shards [][]string + timings, err := fetchTimings(*repo, *workflow, *branch, os.Getenv("GITHUB_TOKEN")) + if err != nil { + fmt.Fprintf(os.Stderr, "testsplit: could not fetch prior timings, falling back to round-robin: %v\n", err) + shards = roundRobin(packages, *numSplit) + } else if coverage := timingCoverage(packages, timings); coverage < minTimingCoverage { + fmt.Fprintf(os.Stderr, "testsplit: only %.0f%% of packages have known timings, falling back to round-robin\n", coverage*100) + shards = roundRobin(packages, *numSplit) + } else { + fmt.Fprintf(os.Stderr, "testsplit: bin-packing %d packages across %d shards using historical timings\n", len(packages), *numSplit) + shards = binPack(packages, timings, *numSplit) + } + + if err := os.MkdirAll(*outDir, 0o755); err != nil { + return err + } + for i, shard := range shards { + path := filepath.Join(*outDir, fmt.Sprintf("packages.txt.%d", i)) + var buf bytes.Buffer + for _, pkg := range shard { + buf.WriteString(pkg) + buf.WriteByte('\n') + } + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + return fmt.Errorf("writing %s: %w", path, err) + } + } + return nil +} + +func readPackages(r io.Reader) ([]string, error) { + var packages []string + scanner := bufio.NewScanner(r) + for scanner.Scan() { + line := scanner.Text() + if line == "" { + continue + } + packages = append(packages, line) + } + return packages, scanner.Err() +} + +func timingCoverage(packages []string, timings map[string]float64) float64 { + if len(packages) == 0 { + return 0 + } + known := 0 + for _, pkg := range packages { + if _, ok := timings[pkg]; ok { + known++ + } + } + return float64(known) / float64(len(packages)) +} + +// roundRobin deterministically interleaves packages across shards. It +// requires no historical data and, unlike a contiguous chunk split, avoids +// dumping a whole cluster of alphabetically-adjacent (and often +// runtime-correlated, e.g. a module's many keeper packages) packages into +// a single shard. +func roundRobin(packages []string, numSplit int) [][]string { + shards := make([][]string, numSplit) + for i, pkg := range packages { + idx := i % numSplit + shards[idx] = append(shards[idx], pkg) + } + return shards +} + +// binPack assigns packages to shards using longest-processing-time-first +// greedy scheduling: process packages slowest-first, each time adding the +// package to whichever shard currently has the smallest total duration. +// Packages with no known duration are estimated at the mean of known +// durations so a handful of unknowns can't skew the packing. +func binPack(packages []string, timings map[string]float64, numSplit int) [][]string { + mean := meanDuration(timings) + + type pkgDuration struct { + pkg string + dur float64 + } + durations := make([]pkgDuration, 0, len(packages)) + for _, pkg := range packages { + dur, ok := timings[pkg] + if !ok { + dur = mean + } + durations = append(durations, pkgDuration{pkg: pkg, dur: dur}) + } + sort.SliceStable(durations, func(i, j int) bool { + return durations[i].dur > durations[j].dur + }) + + shards := make([][]string, numSplit) + totals := make([]float64, numSplit) + for _, pd := range durations { + lightest := 0 + for i := 1; i < numSplit; i++ { + if totals[i] < totals[lightest] { + lightest = i + } + } + shards[lightest] = append(shards[lightest], pd.pkg) + totals[lightest] += pd.dur + } + return shards +} + +func meanDuration(timings map[string]float64) float64 { + if len(timings) == 0 { + return 0 + } + var total float64 + for _, dur := range timings { + total += dur + } + return total / float64(len(timings)) +} + +// fetchTimings finds the most recent successful push-triggered run of +// `workflow` on `branch` and downloads the per-package timing data it +// recorded, if any. It intentionally only looks at `branch` runs so a +// noisy or unusually slow PR branch can never skew another PR's shard +// assignment. +func fetchTimings(repo, workflow, branch, token string) (map[string]float64, error) { + if repo == "" { + return nil, fmt.Errorf("repo is empty (set --repo or GITHUB_REPOSITORY)") + } + if token == "" { + return nil, fmt.Errorf("no GITHUB_TOKEN provided") + } + + client := &http.Client{Timeout: 30 * time.Second} + + runID, err := latestSuccessfulRun(client, repo, workflow, branch, token) + if err != nil { + return nil, err + } + + artifactURL, err := findTimingsArtifactURL(client, repo, runID, token) + if err != nil { + return nil, err + } + + return downloadTimingsArtifact(client, artifactURL, token) +} + +func apiGet(client *http.Client, url, token string, out any) error { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("User-Agent", "sei-chain-testsplit") + + resp, err := client.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + return fmt.Errorf("GET %s: unexpected status %s: %s", url, resp.Status, body) + } + return json.NewDecoder(resp.Body).Decode(out) +} + +func latestSuccessfulRun(client *http.Client, repo, workflow, branch, token string) (int64, error) { + url := fmt.Sprintf( + "https://api.github.com/repos/%s/actions/workflows/%s/runs?branch=%s&status=success&event=push&per_page=1", + repo, workflow, branch, + ) + var result struct { + WorkflowRuns []struct { + ID int64 `json:"id"` + } `json:"workflow_runs"` + } + if err := apiGet(client, url, token, &result); err != nil { + return 0, err + } + if len(result.WorkflowRuns) == 0 { + return 0, fmt.Errorf("no successful %s runs found on %s", workflow, branch) + } + return result.WorkflowRuns[0].ID, nil +} + +const timingsArtifactName = "package-timings" + +func findTimingsArtifactURL(client *http.Client, repo string, runID int64, token string) (string, error) { + url := fmt.Sprintf("https://api.github.com/repos/%s/actions/runs/%d/artifacts", repo, runID) + var result struct { + Artifacts []struct { + Name string `json:"name"` + ArchiveDownloadURL string `json:"archive_download_url"` + Expired bool `json:"expired"` + } `json:"artifacts"` + } + if err := apiGet(client, url, token, &result); err != nil { + return "", err + } + for _, a := range result.Artifacts { + if a.Name == timingsArtifactName && !a.Expired { + return a.ArchiveDownloadURL, nil + } + } + return "", fmt.Errorf("no %q artifact found on run %d", timingsArtifactName, runID) +} + +const timingsFileName = "package_timings.json" + +func downloadTimingsArtifact(client *http.Client, url, token string) (map[string]float64, error) { + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return nil, err + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("User-Agent", "sei-chain-testsplit") + + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("downloading artifact: unexpected status %s", resp.Status) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + zr, err := zip.NewReader(bytes.NewReader(body), int64(len(body))) + if err != nil { + return nil, fmt.Errorf("artifact is not a valid zip: %w", err) + } + for _, f := range zr.File { + if filepath.Base(f.Name) != timingsFileName { + continue + } + rc, err := f.Open() + if err != nil { + return nil, err + } + defer rc.Close() + var timings map[string]float64 + if err := json.NewDecoder(rc).Decode(&timings); err != nil { + return nil, fmt.Errorf("decoding %s: %w", timingsFileName, err) + } + return timings, nil + } + return nil, fmt.Errorf("%s not found in artifact zip", timingsFileName) +} diff --git a/.github/scripts/testsplit/plan_test.go b/.github/scripts/testsplit/plan_test.go new file mode 100644 index 0000000000..cade506b8b --- /dev/null +++ b/.github/scripts/testsplit/plan_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "math" + "testing" +) + +func TestRoundRobinInterleaves(t *testing.T) { + packages := []string{"a", "b", "c", "d", "e", "f", "g"} + shards := roundRobin(packages, 3) + + if got, want := len(shards), 3; got != want { + t.Fatalf("len(shards) = %d, want %d", got, want) + } + want := [][]string{{"a", "d", "g"}, {"b", "e"}, {"c", "f"}} + for i := range want { + if !equalSlices(shards[i], want[i]) { + t.Errorf("shard %d = %v, want %v", i, shards[i], want[i]) + } + } +} + +func TestBinPackBalancesKnownDurations(t *testing.T) { + packages := []string{"slow", "medium1", "medium2", "tiny1", "tiny2", "tiny3"} + timings := map[string]float64{ + "slow": 30, + "medium1": 25, + "medium2": 25, + "tiny1": 5, + "tiny2": 5, + "tiny3": 5, + } + + shards := binPack(packages, timings, 3) + if got, want := len(shards), 3; got != want { + t.Fatalf("len(shards) = %d, want %d", got, want) + } + + totals := make([]float64, 3) + seen := map[string]bool{} + for i, shard := range shards { + for _, pkg := range shard { + totals[i] += timings[pkg] + seen[pkg] = true + } + } + for _, pkg := range packages { + if !seen[pkg] { + t.Errorf("package %q missing from output", pkg) + } + } + + maxTotal, minTotal := totals[0], totals[0] + for _, tot := range totals { + maxTotal = math.Max(maxTotal, tot) + minTotal = math.Min(minTotal, tot) + } + if maxTotal-minTotal > 10 { + t.Errorf("shard totals too imbalanced: %v (spread %.0f)", totals, maxTotal-minTotal) + } +} + +func TestBinPackEstimatesUnknownAsMean(t *testing.T) { + packages := []string{"known1", "known2", "unknown"} + timings := map[string]float64{"known1": 10, "known2": 30} + // mean of known durations is 20, so "unknown" should behave like a 20s package. + + shards := binPack(packages, timings, 2) + totals := make([]float64, 2) + for i, shard := range shards { + for _, pkg := range shard { + dur, ok := timings[pkg] + if !ok { + dur = 20 + } + totals[i] += dur + } + } + if math.Abs(totals[0]-totals[1]) > 1e-9 { + t.Errorf("expected balanced totals treating unknown as mean, got %v", totals) + } +} + +func TestTimingCoverage(t *testing.T) { + packages := []string{"a", "b", "c", "d"} + timings := map[string]float64{"a": 1, "b": 2} + + if got, want := timingCoverage(packages, timings), 0.5; got != want { + t.Errorf("timingCoverage() = %v, want %v", got, want) + } + if got := timingCoverage(nil, timings); got != 0 { + t.Errorf("timingCoverage(nil, ...) = %v, want 0", got) + } +} + +func TestMeanDuration(t *testing.T) { + if got := meanDuration(nil); got != 0 { + t.Errorf("meanDuration(nil) = %v, want 0", got) + } + timings := map[string]float64{"a": 10, "b": 30} + if got, want := meanDuration(timings), 20.0; got != want { + t.Errorf("meanDuration() = %v, want %v", got, want) + } +} + +func equalSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/.github/scripts/testsplit/record.go b/.github/scripts/testsplit/record.go new file mode 100644 index 0000000000..eac113608d --- /dev/null +++ b/.github/scripts/testsplit/record.go @@ -0,0 +1,72 @@ +package main + +import ( + "bufio" + "encoding/json" + "flag" + "fmt" + "os" + "regexp" + "strconv" +) + +// resultLine matches Go's own test summary lines, e.g.: +// +// ok github.com/sei-protocol/sei-chain/x/evm 12.345s +// FAIL github.com/sei-protocol/sei-chain/x/evm 3.210s +// +// This is what `go test` already prints per package, so no extra flags +// (e.g. -json) are needed to recover per-package elapsed time. +var resultLine = regexp.MustCompile(`^(?:ok|FAIL)\s+(\S+)\s+([0-9]+(?:\.[0-9]+)?)s(?:\s|$)`) + +func runRecord(args []string) error { + fs := flag.NewFlagSet("record", flag.ExitOnError) + input := fs.String("input", "", "path to captured `go test` output") + out := fs.String("out", "", "path to write this shard's timing JSON to") + if err := fs.Parse(args); err != nil { + return err + } + if *input == "" || *out == "" { + return fmt.Errorf("--input and --out are required") + } + + f, err := os.Open(*input) + if err != nil { + return err + } + defer f.Close() + + timings := map[string]float64{} + scanner := bufio.NewScanner(f) + // Test output lines can be long (e.g. verbose failure dumps); grow the + // buffer rather than truncating/erroring on long lines. + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + for scanner.Scan() { + m := resultLine.FindStringSubmatch(scanner.Text()) + if m == nil { + continue + } + seconds, err := strconv.ParseFloat(m[2], 64) + if err != nil { + continue + } + timings[m[1]] = seconds + } + if err := scanner.Err(); err != nil { + return fmt.Errorf("reading %s: %w", *input, err) + } + + return writeTimings(*out, timings) +} + +func writeTimings(path string, timings map[string]float64) error { + f, err := os.Create(path) + if err != nil { + return err + } + defer f.Close() + + enc := json.NewEncoder(f) + enc.SetIndent("", " ") + return enc.Encode(timings) +} diff --git a/.github/scripts/testsplit/record_test.go b/.github/scripts/testsplit/record_test.go new file mode 100644 index 0000000000..53588d4b9d --- /dev/null +++ b/.github/scripts/testsplit/record_test.go @@ -0,0 +1,53 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestRunRecordParsesResultLines(t *testing.T) { + dir := t.TempDir() + input := filepath.Join(dir, "test-output.txt") + out := filepath.Join(dir, "timings.json") + + content := `=== RUN TestFoo +--- PASS: TestFoo (0.00s) +ok github.com/sei-protocol/sei-chain/x/evm 12.345s +FAIL github.com/sei-protocol/sei-chain/x/gov 3.5s +? github.com/sei-protocol/sei-chain/x/notest [no test files] +` + if err := os.WriteFile(input, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + if err := runRecord([]string{"--input=" + input, "--out=" + out}); err != nil { + t.Fatalf("runRecord() error = %v", err) + } + + data, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + var timings map[string]float64 + if err := json.Unmarshal(data, &timings); err != nil { + t.Fatal(err) + } + + want := map[string]float64{ + "github.com/sei-protocol/sei-chain/x/evm": 12.345, + "github.com/sei-protocol/sei-chain/x/gov": 3.5, + } + if len(timings) != len(want) { + t.Fatalf("timings = %v, want %v", timings, want) + } + for pkg, dur := range want { + if got := timings[pkg]; got != dur { + t.Errorf("timings[%q] = %v, want %v", pkg, got, dur) + } + } + if _, ok := timings["github.com/sei-protocol/sei-chain/x/notest"]; ok { + t.Error("expected packages with no test files to be excluded") + } +} diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index b6b2904798..863e0cb825 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -40,6 +40,11 @@ jobs: name: "Race Detection (shard ${{ matrix.shard }})" runs-on: uci-default needs: shard-indexes + permissions: + contents: read + # testsplit queries the Actions API for the last successful `main` + # run's timing artifact; this is the read-only scope that allows it. + actions: read strategy: fail-fast: false matrix: @@ -71,23 +76,42 @@ jobs: run: go mod download - name: Go test + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail make split-test-packages NUM_SPLIT=${{ env.NUM_SPLIT }} - # `split`'s numeric-suffix width isn't guaranteed to match NUM_SPLIT's - # digit count (e.g. it may emit packages.txt.00 instead of .0), so - # pick the shard file by sorted position rather than assumed name. - SHARD_FILE=$(ls build/packages.txt.* | sort | sed -n "$(( ${{ matrix.shard }} + 1 ))p") - if [[ -z "$SHARD_FILE" ]]; then - echo "no shard file found for index ${{ matrix.shard }}" >&2 - exit 1 - fi + SHARD_FILE=build/packages.txt.${{ matrix.shard }} PARALLEL=() if grep -qx "github.com/sei-protocol/sei-chain/occ_tests" "$SHARD_FILE"; then echo "occ_tests present in this shard; forcing -parallel=1" PARALLEL=(-parallel=1) fi - xargs go test "${PARALLEL[@]}" -timeout=${{ env.GO_TEST_TIMEOUT }} < "$SHARD_FILE" + set +e + xargs go test "${PARALLEL[@]}" -timeout=${{ env.GO_TEST_TIMEOUT }} < "$SHARD_FILE" 2>&1 | tee test-output.txt + TEST_EXIT=${PIPESTATUS[0]} + set -e + exit "$TEST_EXIT" + + - name: Record test timings + # Captures per-package duration even when tests fail, so a shard + # that fails partway through still contributes what it learned. + if: always() + run: | + set -euo pipefail + mkdir -p build + if [ -f test-output.txt ]; then + go run ./.github/scripts/testsplit record --input=test-output.txt --out=build/timings.shard-${{ matrix.shard }}.json + else + echo '{}' > build/timings.shard-${{ matrix.shard }}.json + fi + + - name: Upload shard timings + if: always() + uses: actions/upload-artifact@v5 + with: + name: race-timings-shard-${{ matrix.shard }} + path: build/timings.shard-${{ matrix.shard }}.json test-check: name: Race Detection @@ -102,6 +126,45 @@ jobs: exit 1 fi + record-timings: + name: Record package timings + runs-on: ubuntu-latest + needs: test + # Timings are only ever queried from the last successful `main` push + # run (see testsplit's fetchTimings), so there's no point recording + # them on PR/merge-group runs nobody will look up. + if: always() && github.event_name == 'push' + permissions: + contents: read + actions: read + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + with: + fetch-depth: 1 + + - uses: actions/setup-go@v6 + with: + go-version: ${{ env.GO_VERSION }} + cache: false + + - name: Download shard timings + uses: actions/download-artifact@v4 + with: + pattern: race-timings-shard-* + path: build/shard-timings + merge-multiple: true + + - name: Merge package timings + run: | + set -euo pipefail + go run ./.github/scripts/testsplit merge --out=build/package_timings.json build/shard-timings/*.json + + - name: Upload package timings + uses: actions/upload-artifact@v5 + with: + name: package-timings + path: build/package_timings.json + coverage: name: Coverage runs-on: ${{ github.event_name == 'merge_group' && 'ubuntu-latest' || 'uci-default' }} diff --git a/AGENTS.md b/AGENTS.md index 20cbd08f47..ab414a3acf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,11 +66,17 @@ make install # install seid into $GOBIN ``` Tests run with the race detector and coverage. `go-test.yml`'s Race Detection job -shards into `NUM_SPLIT` (currently 3) parallel matrix jobs; `make test-group-N` -reproduces a given shard locally with the same package split: +shards into `NUM_SPLIT` (currently 3) parallel matrix jobs, split by +[`.github/scripts/testsplit`](.github/scripts/testsplit): it bin-packs packages +by historical per-package duration (queried from the last successful `main` +run) when that data is available, falling back to a deterministic round-robin +split otherwise. `make test-group-N` runs the same tool locally — without a +`GITHUB_TOKEN` env var it always takes the round-robin path, so the local +split won't exactly match a given CI shard, but the package set and flags +(`-race`, `occ_tests` `-parallel=1`) are otherwise identical: ```bash -NUM_SPLIT=3 make test-group-0 # reproduce CI race shard 0 locally +NUM_SPLIT=3 make test-group-0 # run one local shard (round-robin without GITHUB_TOKEN) go test .//... # run a single package ``` diff --git a/Makefile b/Makefile index 6dc7932fe2..90a358d1f5 100644 --- a/Makefile +++ b/Makefile @@ -584,20 +584,21 @@ $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests +# testsplit balances shards by historical per-package test duration (queried +# from the last successful `main` run of go-test.yml) when available, and +# falls back to a deterministic round-robin split otherwise — see +# .github/scripts/testsplit. It writes packages.txt.0 .. packages.txt.N-1 +# next to packages.txt, replacing the old `split -d -n l/N` approach (whose +# numeric-suffix width isn't guaranteed to match NUM_SPLIT's digit count). split-test-packages:$(BUILDDIR)/packages.txt - split -d -n l/$(NUM_SPLIT) $< $<. + go run ./.github/scripts/testsplit plan --num-split=$(NUM_SPLIT) --out-dir=$(BUILDDIR) < $< test-group-%:split-test-packages @echo "🔍 Checking for special package: $(TARGET_PACKAGE)" - @SHARD_FILE=$$(ls $(BUILDDIR)/packages.txt.* | sort | sed -n "$$(( $* + 1 ))p"); \ - if [ -z "$$SHARD_FILE" ]; then \ - echo "no shard file found for index $*" >&2; \ - exit 1; \ - fi; \ - if grep -q "$(TARGET_PACKAGE)" "$$SHARD_FILE"; then \ + @if grep -q "$(TARGET_PACKAGE)" $(BUILDDIR)/packages.txt.$*; then \ echo "🔒 Found $(TARGET_PACKAGE), running with -parallel=1"; \ PARALLEL="-parallel=1"; \ else \ echo "⚡ Not found, running with -parallel=4"; \ PARALLEL="-parallel=4"; \ fi; \ - cat "$$SHARD_FILE" | xargs go test $$PARALLEL -mod=readonly -timeout=10m -race -coverprofile=$*.profile.out -covermode=atomic -coverpkg=./... + cat $(BUILDDIR)/packages.txt.$* | xargs go test $$PARALLEL -mod=readonly -timeout=10m -race -coverprofile=$*.profile.out -covermode=atomic -coverpkg=./... From ea6cf7c8fa309fbda7634628a600c547f8b56022 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 16:50:11 +0200 Subject: [PATCH 4/9] ci: prefer own-branch timing history over main-only lookup fetchTimings now tries this run's own branch first (via GITHUB_HEAD_REF/ GITHUB_REF_NAME, whichever Actions sets for the trigger) before falling back to main. A long-lived PR's shards then reflect exactly its own test changes rather than main's, while a brand-new branch with no history yet still gets useful data from main instead of dropping straight to round-robin. Requires record-timings to publish on pull_request runs too, not just push, so a PR's own history has something to accumulate across pushes. Adds httptest-backed coverage for the branch/base-branch fallback order. --- .github/scripts/testsplit/fetch_test.go | 150 ++++++++++++++++++++++++ .github/scripts/testsplit/plan.go | 74 +++++++++--- .github/workflows/go-test.yml | 10 +- AGENTS.md | 15 ++- 4 files changed, 225 insertions(+), 24 deletions(-) create mode 100644 .github/scripts/testsplit/fetch_test.go diff --git a/.github/scripts/testsplit/fetch_test.go b/.github/scripts/testsplit/fetch_test.go new file mode 100644 index 0000000000..2a9af20c91 --- /dev/null +++ b/.github/scripts/testsplit/fetch_test.go @@ -0,0 +1,150 @@ +package main + +import ( + "archive/zip" + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// fakeGitHub serves just enough of the Actions API for fetchTimings: a +// workflow-runs list (branch -> run ID) and an artifacts list + zip body +// for whichever branches have a "package-timings" artifact. +func fakeGitHub(t *testing.T, branchesWithHistory map[string]map[string]float64) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + + mux.HandleFunc("/repos/o/r/actions/workflows/go-test.yml/runs", func(w http.ResponseWriter, r *http.Request) { + branch := r.URL.Query().Get("branch") + if _, ok := branchesWithHistory[branch]; !ok { + fmt.Fprint(w, `{"workflow_runs": []}`) + return + } + fmt.Fprintf(w, `{"workflow_runs": [{"id": %d}]}`, branchRunID(branch)) + }) + + mux.HandleFunc("/repos/o/r/actions/runs/", func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/artifacts") { + fmt.Fprintf(w, `{"artifacts": [{"name": "package-timings", "archive_download_url": %q, "expired": false}]}`, + "http://"+r.Host+"/download/"+strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/repos/o/r/actions/runs/"), "/artifacts")) + return + } + http.NotFound(w, r) + }) + + mux.HandleFunc("/download/", func(w http.ResponseWriter, r *http.Request) { + runIDStr := strings.TrimPrefix(r.URL.Path, "/download/") + var branch string + for b := range branchesWithHistory { + if fmt.Sprint(branchRunID(b)) == runIDStr { + branch = b + } + } + timings := branchesWithHistory[branch] + data, err := json.Marshal(timings) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + f, err := zw.Create("package_timings.json") + if err != nil { + t.Fatal(err) + } + if _, err := f.Write(data); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + w.Write(buf.Bytes()) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func branchRunID(branch string) int { + sum := 0 + for _, c := range branch { + sum += int(c) + } + return sum +} + +func TestFetchTimingsPrefersOwnBranch(t *testing.T) { + srv := fakeGitHub(t, map[string]map[string]float64{ + "my-feature": {"pkgA": 1}, + "main": {"pkgA": 2}, + }) + restoreAPIBase := apiBase + apiBase = srv.URL + t.Cleanup(func() { apiBase = restoreAPIBase }) + + timings, source, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") + if err != nil { + t.Fatalf("fetchTimings() error = %v", err) + } + if source != "my-feature" { + t.Errorf("source = %q, want %q", source, "my-feature") + } + if timings["pkgA"] != 1 { + t.Errorf("timings[pkgA] = %v, want 1 (own-branch data, not main's)", timings["pkgA"]) + } +} + +func TestFetchTimingsFallsBackToBaseBranch(t *testing.T) { + srv := fakeGitHub(t, map[string]map[string]float64{ + // "my-feature" has no history yet (e.g. first push on this PR). + "main": {"pkgA": 2}, + }) + restoreAPIBase := apiBase + apiBase = srv.URL + t.Cleanup(func() { apiBase = restoreAPIBase }) + + timings, source, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") + if err != nil { + t.Fatalf("fetchTimings() error = %v", err) + } + if source != "main" { + t.Errorf("source = %q, want %q", source, "main") + } + if timings["pkgA"] != 2 { + t.Errorf("timings[pkgA] = %v, want 2 (base-branch data)", timings["pkgA"]) + } +} + +func TestFetchTimingsNoHistoryAnywhere(t *testing.T) { + srv := fakeGitHub(t, map[string]map[string]float64{}) + restoreAPIBase := apiBase + apiBase = srv.URL + t.Cleanup(func() { apiBase = restoreAPIBase }) + + _, _, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") + if err == nil { + t.Fatal("fetchTimings() expected an error when no branch has history, got nil") + } +} + +func TestFetchTimingsSameBranchAsBase(t *testing.T) { + // A push-triggered run on main: branch == baseBranch, must not double-query. + srv := fakeGitHub(t, map[string]map[string]float64{ + "main": {"pkgA": 2}, + }) + restoreAPIBase := apiBase + apiBase = srv.URL + t.Cleanup(func() { apiBase = restoreAPIBase }) + + timings, source, err := fetchTimings("o/r", "go-test.yml", "main", "main", "tok") + if err != nil { + t.Fatalf("fetchTimings() error = %v", err) + } + if source != "main" || timings["pkgA"] != 2 { + t.Errorf("got source=%q timings=%v, want source=main timings[pkgA]=2", source, timings) + } +} diff --git a/.github/scripts/testsplit/plan.go b/.github/scripts/testsplit/plan.go index 49a7aaa95f..e2ab1f10b1 100644 --- a/.github/scripts/testsplit/plan.go +++ b/.github/scripts/testsplit/plan.go @@ -5,6 +5,7 @@ import ( "bufio" "bytes" "encoding/json" + "errors" "flag" "fmt" "io" @@ -21,13 +22,17 @@ import ( // plain round-robin split. const minTimingCoverage = 0.5 +// apiBase is overridden in tests to point at an httptest server. +var apiBase = "https://api.github.com" + func runPlan(args []string) error { fs := flag.NewFlagSet("plan", flag.ExitOnError) numSplit := fs.Int("num-split", 0, "number of shards to split packages into") outDir := fs.String("out-dir", "build", "directory to write packages.txt.N shard files into") repo := fs.String("repo", os.Getenv("GITHUB_REPOSITORY"), "owner/repo to query for prior timing data") workflow := fs.String("workflow", "go-test.yml", "workflow file name to query for the last successful run") - branch := fs.String("branch", "main", "branch to source prior timing data from") + branch := fs.String("branch", defaultBranch(), "this run's own branch, tried before falling back to --base-branch") + baseBranch := fs.String("base-branch", "main", "branch to fall back to if --branch has no timing history yet") if err := fs.Parse(args); err != nil { return err } @@ -44,7 +49,7 @@ func runPlan(args []string) error { } var shards [][]string - timings, err := fetchTimings(*repo, *workflow, *branch, os.Getenv("GITHUB_TOKEN")) + timings, source, err := fetchTimings(*repo, *workflow, *branch, *baseBranch, os.Getenv("GITHUB_TOKEN")) if err != nil { fmt.Fprintf(os.Stderr, "testsplit: could not fetch prior timings, falling back to round-robin: %v\n", err) shards = roundRobin(packages, *numSplit) @@ -52,7 +57,7 @@ func runPlan(args []string) error { fmt.Fprintf(os.Stderr, "testsplit: only %.0f%% of packages have known timings, falling back to round-robin\n", coverage*100) shards = roundRobin(packages, *numSplit) } else { - fmt.Fprintf(os.Stderr, "testsplit: bin-packing %d packages across %d shards using historical timings\n", len(packages), *numSplit) + fmt.Fprintf(os.Stderr, "testsplit: bin-packing %d packages across %d shards using timings from %s\n", len(packages), *numSplit, source) shards = binPack(packages, timings, *numSplit) } @@ -163,21 +168,62 @@ func meanDuration(timings map[string]float64) float64 { return total / float64(len(timings)) } -// fetchTimings finds the most recent successful push-triggered run of -// `workflow` on `branch` and downloads the per-package timing data it -// recorded, if any. It intentionally only looks at `branch` runs so a -// noisy or unusually slow PR branch can never skew another PR's shard -// assignment. -func fetchTimings(repo, workflow, branch, token string) (map[string]float64, error) { +// defaultBranch reports this run's own branch from the environment GitHub +// Actions already sets: GITHUB_HEAD_REF for pull_request-triggered runs +// (the PR's head branch), falling back to GITHUB_REF_NAME otherwise (e.g. +// "main" or "release/v1.2" on a push-triggered run). +func defaultBranch() string { + if head := os.Getenv("GITHUB_HEAD_REF"); head != "" { + return head + } + return os.Getenv("GITHUB_REF_NAME") +} + +// fetchTimings looks for per-package timing data recorded by a prior run +// of `workflow`, trying `branch` (this run's own branch) first and falling +// back to `baseBranch` if `branch` has no successful run yet — e.g. a PR's +// first push, before it has any history of its own. Using each branch's +// own latest run first means a long-lived PR gets timings that reflect +// exactly its own test changes; falling back to `baseBranch` means a +// brand-new branch still benefits from steady-state history instead of +// dropping straight to round-robin. +func fetchTimings(repo, workflow, branch, baseBranch, token string) (map[string]float64, string, error) { if repo == "" { - return nil, fmt.Errorf("repo is empty (set --repo or GITHUB_REPOSITORY)") + return nil, "", fmt.Errorf("repo is empty (set --repo or GITHUB_REPOSITORY)") } if token == "" { - return nil, fmt.Errorf("no GITHUB_TOKEN provided") + return nil, "", fmt.Errorf("no GITHUB_TOKEN provided") } client := &http.Client{Timeout: 30 * time.Second} + var errs []error + for _, candidate := range dedupBranches(branch, baseBranch) { + timings, err := fetchTimingsFromBranch(client, repo, workflow, candidate, token) + if err == nil { + return timings, candidate, nil + } + errs = append(errs, fmt.Errorf("%s: %w", candidate, err)) + } + return nil, "", fmt.Errorf("no timing history on any of %v: %w", []string{branch, baseBranch}, errors.Join(errs...)) +} + +// dedupBranches returns the non-empty, order-preserved, de-duplicated +// branch candidates to try. +func dedupBranches(branches ...string) []string { + seen := map[string]bool{} + var out []string + for _, b := range branches { + if b == "" || seen[b] { + continue + } + seen[b] = true + out = append(out, b) + } + return out +} + +func fetchTimingsFromBranch(client *http.Client, repo, workflow, branch, token string) (map[string]float64, error) { runID, err := latestSuccessfulRun(client, repo, workflow, branch, token) if err != nil { return nil, err @@ -215,8 +261,8 @@ func apiGet(client *http.Client, url, token string, out any) error { func latestSuccessfulRun(client *http.Client, repo, workflow, branch, token string) (int64, error) { url := fmt.Sprintf( - "https://api.github.com/repos/%s/actions/workflows/%s/runs?branch=%s&status=success&event=push&per_page=1", - repo, workflow, branch, + "%s/repos/%s/actions/workflows/%s/runs?branch=%s&status=success&per_page=1", + apiBase, repo, workflow, branch, ) var result struct { WorkflowRuns []struct { @@ -235,7 +281,7 @@ func latestSuccessfulRun(client *http.Client, repo, workflow, branch, token stri const timingsArtifactName = "package-timings" func findTimingsArtifactURL(client *http.Client, repo string, runID int64, token string) (string, error) { - url := fmt.Sprintf("https://api.github.com/repos/%s/actions/runs/%d/artifacts", repo, runID) + url := fmt.Sprintf("%s/repos/%s/actions/runs/%d/artifacts", apiBase, repo, runID) var result struct { Artifacts []struct { Name string `json:"name"` diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 863e0cb825..06eeba933f 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -130,10 +130,12 @@ jobs: name: Record package timings runs-on: ubuntu-latest needs: test - # Timings are only ever queried from the last successful `main` push - # run (see testsplit's fetchTimings), so there's no point recording - # them on PR/merge-group runs nobody will look up. - if: always() && github.event_name == 'push' + # testsplit's fetchTimings checks this run's own branch first, then + # falls back to `main` — so PR runs need to publish their own history + # too, not just push runs on main. merge_group runs target a synthetic, + # ephemeral gh-readonly-queue/* ref that won't be queried again, so + # skip those. + if: always() && github.event_name != 'merge_group' permissions: contents: read actions: read diff --git a/AGENTS.md b/AGENTS.md index ab414a3acf..b5e92e6143 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,12 +68,15 @@ make install # install seid into $GOBIN Tests run with the race detector and coverage. `go-test.yml`'s Race Detection job shards into `NUM_SPLIT` (currently 3) parallel matrix jobs, split by [`.github/scripts/testsplit`](.github/scripts/testsplit): it bin-packs packages -by historical per-package duration (queried from the last successful `main` -run) when that data is available, falling back to a deterministic round-robin -split otherwise. `make test-group-N` runs the same tool locally — without a -`GITHUB_TOKEN` env var it always takes the round-robin path, so the local -split won't exactly match a given CI shard, but the package set and flags -(`-race`, `occ_tests` `-parallel=1`) are otherwise identical: +by historical per-package duration, querying the current branch's own last +successful run first (so a long-lived PR's shards reflect its own test +changes) and falling back to `main`'s last successful run if the branch has +no history yet (e.g. a PR's first push), then falling back further to a +deterministic round-robin split if neither has usable data. `make test-group-N` +runs the same tool locally — without a `GITHUB_TOKEN` env var it always takes +the round-robin path, so the local split won't exactly match a given CI +shard, but the package set and flags (`-race`, `occ_tests` `-parallel=1`) +are otherwise identical: ```bash NUM_SPLIT=3 make test-group-0 # run one local shard (round-robin without GITHUB_TOKEN) From ccafa3d116764a49b395caac1fd22553a67dbefe Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 17:05:12 +0200 Subject: [PATCH 5/9] ci: trigger a re-run to pick up this branch's timing artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No code change — checking whether testsplit's plan step now finds the package-timings artifact this branch's previous run published and bin-packs shards instead of falling back to round-robin. From d8b339656bf5f04a580b5cc6f212274cf1be2f33 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 18:09:40 +0200 Subject: [PATCH 6/9] ci: revert to plain round-robin sharding, drop testsplit tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dynamic API-lookup + bin-packing approach added real complexity (a Go tool, GITHUB_TOKEN + actions:read permissions, a record-timings job, cross-run artifacts) without a clean win to show for it: the one run where it found real timing data and bin-packed came in slower (24m40s/16m16s/16m27s) than the earlier round-robin run (12m17s max), apparently due to runner contention rather than a packing flaw — but that noise makes the whole approach hard to validate or trust. Back to a plain, dependency-free round-robin split (package i -> shard i % NUM_SPLIT) in the Makefile via a small awk one-liner. Round-robin still avoids the original bug (contiguous alphabetical chunks dumping a cluster of slow, runtime-correlated packages into one shard) that count-based `split -d -n l/N` had — it's just no longer trying to balance by measured duration. --- .github/scripts/testsplit/fetch_test.go | 150 ---------- .github/scripts/testsplit/main.go | 33 --- .github/scripts/testsplit/merge.go | 40 --- .github/scripts/testsplit/plan.go | 347 ----------------------- .github/scripts/testsplit/plan_test.go | 116 -------- .github/scripts/testsplit/record.go | 72 ----- .github/scripts/testsplit/record_test.go | 53 ---- .github/workflows/go-test.yml | 74 +---- AGENTS.md | 17 +- Makefile | 15 +- 10 files changed, 14 insertions(+), 903 deletions(-) delete mode 100644 .github/scripts/testsplit/fetch_test.go delete mode 100644 .github/scripts/testsplit/main.go delete mode 100644 .github/scripts/testsplit/merge.go delete mode 100644 .github/scripts/testsplit/plan.go delete mode 100644 .github/scripts/testsplit/plan_test.go delete mode 100644 .github/scripts/testsplit/record.go delete mode 100644 .github/scripts/testsplit/record_test.go diff --git a/.github/scripts/testsplit/fetch_test.go b/.github/scripts/testsplit/fetch_test.go deleted file mode 100644 index 2a9af20c91..0000000000 --- a/.github/scripts/testsplit/fetch_test.go +++ /dev/null @@ -1,150 +0,0 @@ -package main - -import ( - "archive/zip" - "bytes" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "strings" - "testing" -) - -// fakeGitHub serves just enough of the Actions API for fetchTimings: a -// workflow-runs list (branch -> run ID) and an artifacts list + zip body -// for whichever branches have a "package-timings" artifact. -func fakeGitHub(t *testing.T, branchesWithHistory map[string]map[string]float64) *httptest.Server { - t.Helper() - mux := http.NewServeMux() - - mux.HandleFunc("/repos/o/r/actions/workflows/go-test.yml/runs", func(w http.ResponseWriter, r *http.Request) { - branch := r.URL.Query().Get("branch") - if _, ok := branchesWithHistory[branch]; !ok { - fmt.Fprint(w, `{"workflow_runs": []}`) - return - } - fmt.Fprintf(w, `{"workflow_runs": [{"id": %d}]}`, branchRunID(branch)) - }) - - mux.HandleFunc("/repos/o/r/actions/runs/", func(w http.ResponseWriter, r *http.Request) { - if strings.HasSuffix(r.URL.Path, "/artifacts") { - fmt.Fprintf(w, `{"artifacts": [{"name": "package-timings", "archive_download_url": %q, "expired": false}]}`, - "http://"+r.Host+"/download/"+strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/repos/o/r/actions/runs/"), "/artifacts")) - return - } - http.NotFound(w, r) - }) - - mux.HandleFunc("/download/", func(w http.ResponseWriter, r *http.Request) { - runIDStr := strings.TrimPrefix(r.URL.Path, "/download/") - var branch string - for b := range branchesWithHistory { - if fmt.Sprint(branchRunID(b)) == runIDStr { - branch = b - } - } - timings := branchesWithHistory[branch] - data, err := json.Marshal(timings) - if err != nil { - t.Fatal(err) - } - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - f, err := zw.Create("package_timings.json") - if err != nil { - t.Fatal(err) - } - if _, err := f.Write(data); err != nil { - t.Fatal(err) - } - if err := zw.Close(); err != nil { - t.Fatal(err) - } - w.Write(buf.Bytes()) - }) - - srv := httptest.NewServer(mux) - t.Cleanup(srv.Close) - return srv -} - -func branchRunID(branch string) int { - sum := 0 - for _, c := range branch { - sum += int(c) - } - return sum -} - -func TestFetchTimingsPrefersOwnBranch(t *testing.T) { - srv := fakeGitHub(t, map[string]map[string]float64{ - "my-feature": {"pkgA": 1}, - "main": {"pkgA": 2}, - }) - restoreAPIBase := apiBase - apiBase = srv.URL - t.Cleanup(func() { apiBase = restoreAPIBase }) - - timings, source, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") - if err != nil { - t.Fatalf("fetchTimings() error = %v", err) - } - if source != "my-feature" { - t.Errorf("source = %q, want %q", source, "my-feature") - } - if timings["pkgA"] != 1 { - t.Errorf("timings[pkgA] = %v, want 1 (own-branch data, not main's)", timings["pkgA"]) - } -} - -func TestFetchTimingsFallsBackToBaseBranch(t *testing.T) { - srv := fakeGitHub(t, map[string]map[string]float64{ - // "my-feature" has no history yet (e.g. first push on this PR). - "main": {"pkgA": 2}, - }) - restoreAPIBase := apiBase - apiBase = srv.URL - t.Cleanup(func() { apiBase = restoreAPIBase }) - - timings, source, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") - if err != nil { - t.Fatalf("fetchTimings() error = %v", err) - } - if source != "main" { - t.Errorf("source = %q, want %q", source, "main") - } - if timings["pkgA"] != 2 { - t.Errorf("timings[pkgA] = %v, want 2 (base-branch data)", timings["pkgA"]) - } -} - -func TestFetchTimingsNoHistoryAnywhere(t *testing.T) { - srv := fakeGitHub(t, map[string]map[string]float64{}) - restoreAPIBase := apiBase - apiBase = srv.URL - t.Cleanup(func() { apiBase = restoreAPIBase }) - - _, _, err := fetchTimings("o/r", "go-test.yml", "my-feature", "main", "tok") - if err == nil { - t.Fatal("fetchTimings() expected an error when no branch has history, got nil") - } -} - -func TestFetchTimingsSameBranchAsBase(t *testing.T) { - // A push-triggered run on main: branch == baseBranch, must not double-query. - srv := fakeGitHub(t, map[string]map[string]float64{ - "main": {"pkgA": 2}, - }) - restoreAPIBase := apiBase - apiBase = srv.URL - t.Cleanup(func() { apiBase = restoreAPIBase }) - - timings, source, err := fetchTimings("o/r", "go-test.yml", "main", "main", "tok") - if err != nil { - t.Fatalf("fetchTimings() error = %v", err) - } - if source != "main" || timings["pkgA"] != 2 { - t.Errorf("got source=%q timings=%v, want source=main timings[pkgA]=2", source, timings) - } -} diff --git a/.github/scripts/testsplit/main.go b/.github/scripts/testsplit/main.go deleted file mode 100644 index 582eff1184..0000000000 --- a/.github/scripts/testsplit/main.go +++ /dev/null @@ -1,33 +0,0 @@ -// Command testsplit splits the Go package list for the Race Detection CI -// job across N shards, balancing by historical per-package test duration -// when available and falling back to round-robin when it isn't. -package main - -import ( - "fmt" - "os" -) - -func main() { - if len(os.Args) < 2 { - fmt.Fprintln(os.Stderr, "usage: testsplit [flags]") - os.Exit(2) - } - - var err error - switch os.Args[1] { - case "plan": - err = runPlan(os.Args[2:]) - case "record": - err = runRecord(os.Args[2:]) - case "merge": - err = runMerge(os.Args[2:]) - default: - fmt.Fprintf(os.Stderr, "unknown subcommand %q\n", os.Args[1]) - os.Exit(2) - } - if err != nil { - fmt.Fprintln(os.Stderr, "testsplit:", err) - os.Exit(1) - } -} diff --git a/.github/scripts/testsplit/merge.go b/.github/scripts/testsplit/merge.go deleted file mode 100644 index b0f1283389..0000000000 --- a/.github/scripts/testsplit/merge.go +++ /dev/null @@ -1,40 +0,0 @@ -package main - -import ( - "encoding/json" - "flag" - "fmt" - "maps" - "os" -) - -// runMerge combines each shard's timings.shard-N.json into a single -// package_timings.json. Shards test disjoint package sets, so key -// collisions aren't expected; if one occurs, the last file wins. -func runMerge(args []string) error { - fs := flag.NewFlagSet("merge", flag.ExitOnError) - out := fs.String("out", "", "path to write the merged timing JSON to") - if err := fs.Parse(args); err != nil { - return err - } - inputs := fs.Args() - if *out == "" || len(inputs) == 0 { - return fmt.Errorf("--out and at least one input file are required") - } - - merged := map[string]float64{} - for _, path := range inputs { - data, err := os.ReadFile(path) - if err != nil { - return fmt.Errorf("reading %s: %w", path, err) - } - var timings map[string]float64 - if err := json.Unmarshal(data, &timings); err != nil { - return fmt.Errorf("decoding %s: %w", path, err) - } - maps.Copy(merged, timings) - } - - fmt.Fprintf(os.Stderr, "testsplit: merged %d package timings from %d shard files\n", len(merged), len(inputs)) - return writeTimings(*out, merged) -} diff --git a/.github/scripts/testsplit/plan.go b/.github/scripts/testsplit/plan.go deleted file mode 100644 index e2ab1f10b1..0000000000 --- a/.github/scripts/testsplit/plan.go +++ /dev/null @@ -1,347 +0,0 @@ -package main - -import ( - "archive/zip" - "bufio" - "bytes" - "encoding/json" - "errors" - "flag" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "sort" - "time" -) - -// minTimingCoverage is the fraction of packages that must have a known -// duration before we trust bin-packing over round-robin. Below this, too -// many durations would be guesses and bin-packing wouldn't out-perform a -// plain round-robin split. -const minTimingCoverage = 0.5 - -// apiBase is overridden in tests to point at an httptest server. -var apiBase = "https://api.github.com" - -func runPlan(args []string) error { - fs := flag.NewFlagSet("plan", flag.ExitOnError) - numSplit := fs.Int("num-split", 0, "number of shards to split packages into") - outDir := fs.String("out-dir", "build", "directory to write packages.txt.N shard files into") - repo := fs.String("repo", os.Getenv("GITHUB_REPOSITORY"), "owner/repo to query for prior timing data") - workflow := fs.String("workflow", "go-test.yml", "workflow file name to query for the last successful run") - branch := fs.String("branch", defaultBranch(), "this run's own branch, tried before falling back to --base-branch") - baseBranch := fs.String("base-branch", "main", "branch to fall back to if --branch has no timing history yet") - if err := fs.Parse(args); err != nil { - return err - } - if *numSplit < 1 { - return fmt.Errorf("--num-split must be >= 1, got %d", *numSplit) - } - - packages, err := readPackages(os.Stdin) - if err != nil { - return fmt.Errorf("reading package list: %w", err) - } - if len(packages) == 0 { - return fmt.Errorf("no packages given on stdin") - } - - var shards [][]string - timings, source, err := fetchTimings(*repo, *workflow, *branch, *baseBranch, os.Getenv("GITHUB_TOKEN")) - if err != nil { - fmt.Fprintf(os.Stderr, "testsplit: could not fetch prior timings, falling back to round-robin: %v\n", err) - shards = roundRobin(packages, *numSplit) - } else if coverage := timingCoverage(packages, timings); coverage < minTimingCoverage { - fmt.Fprintf(os.Stderr, "testsplit: only %.0f%% of packages have known timings, falling back to round-robin\n", coverage*100) - shards = roundRobin(packages, *numSplit) - } else { - fmt.Fprintf(os.Stderr, "testsplit: bin-packing %d packages across %d shards using timings from %s\n", len(packages), *numSplit, source) - shards = binPack(packages, timings, *numSplit) - } - - if err := os.MkdirAll(*outDir, 0o755); err != nil { - return err - } - for i, shard := range shards { - path := filepath.Join(*outDir, fmt.Sprintf("packages.txt.%d", i)) - var buf bytes.Buffer - for _, pkg := range shard { - buf.WriteString(pkg) - buf.WriteByte('\n') - } - if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { - return fmt.Errorf("writing %s: %w", path, err) - } - } - return nil -} - -func readPackages(r io.Reader) ([]string, error) { - var packages []string - scanner := bufio.NewScanner(r) - for scanner.Scan() { - line := scanner.Text() - if line == "" { - continue - } - packages = append(packages, line) - } - return packages, scanner.Err() -} - -func timingCoverage(packages []string, timings map[string]float64) float64 { - if len(packages) == 0 { - return 0 - } - known := 0 - for _, pkg := range packages { - if _, ok := timings[pkg]; ok { - known++ - } - } - return float64(known) / float64(len(packages)) -} - -// roundRobin deterministically interleaves packages across shards. It -// requires no historical data and, unlike a contiguous chunk split, avoids -// dumping a whole cluster of alphabetically-adjacent (and often -// runtime-correlated, e.g. a module's many keeper packages) packages into -// a single shard. -func roundRobin(packages []string, numSplit int) [][]string { - shards := make([][]string, numSplit) - for i, pkg := range packages { - idx := i % numSplit - shards[idx] = append(shards[idx], pkg) - } - return shards -} - -// binPack assigns packages to shards using longest-processing-time-first -// greedy scheduling: process packages slowest-first, each time adding the -// package to whichever shard currently has the smallest total duration. -// Packages with no known duration are estimated at the mean of known -// durations so a handful of unknowns can't skew the packing. -func binPack(packages []string, timings map[string]float64, numSplit int) [][]string { - mean := meanDuration(timings) - - type pkgDuration struct { - pkg string - dur float64 - } - durations := make([]pkgDuration, 0, len(packages)) - for _, pkg := range packages { - dur, ok := timings[pkg] - if !ok { - dur = mean - } - durations = append(durations, pkgDuration{pkg: pkg, dur: dur}) - } - sort.SliceStable(durations, func(i, j int) bool { - return durations[i].dur > durations[j].dur - }) - - shards := make([][]string, numSplit) - totals := make([]float64, numSplit) - for _, pd := range durations { - lightest := 0 - for i := 1; i < numSplit; i++ { - if totals[i] < totals[lightest] { - lightest = i - } - } - shards[lightest] = append(shards[lightest], pd.pkg) - totals[lightest] += pd.dur - } - return shards -} - -func meanDuration(timings map[string]float64) float64 { - if len(timings) == 0 { - return 0 - } - var total float64 - for _, dur := range timings { - total += dur - } - return total / float64(len(timings)) -} - -// defaultBranch reports this run's own branch from the environment GitHub -// Actions already sets: GITHUB_HEAD_REF for pull_request-triggered runs -// (the PR's head branch), falling back to GITHUB_REF_NAME otherwise (e.g. -// "main" or "release/v1.2" on a push-triggered run). -func defaultBranch() string { - if head := os.Getenv("GITHUB_HEAD_REF"); head != "" { - return head - } - return os.Getenv("GITHUB_REF_NAME") -} - -// fetchTimings looks for per-package timing data recorded by a prior run -// of `workflow`, trying `branch` (this run's own branch) first and falling -// back to `baseBranch` if `branch` has no successful run yet — e.g. a PR's -// first push, before it has any history of its own. Using each branch's -// own latest run first means a long-lived PR gets timings that reflect -// exactly its own test changes; falling back to `baseBranch` means a -// brand-new branch still benefits from steady-state history instead of -// dropping straight to round-robin. -func fetchTimings(repo, workflow, branch, baseBranch, token string) (map[string]float64, string, error) { - if repo == "" { - return nil, "", fmt.Errorf("repo is empty (set --repo or GITHUB_REPOSITORY)") - } - if token == "" { - return nil, "", fmt.Errorf("no GITHUB_TOKEN provided") - } - - client := &http.Client{Timeout: 30 * time.Second} - - var errs []error - for _, candidate := range dedupBranches(branch, baseBranch) { - timings, err := fetchTimingsFromBranch(client, repo, workflow, candidate, token) - if err == nil { - return timings, candidate, nil - } - errs = append(errs, fmt.Errorf("%s: %w", candidate, err)) - } - return nil, "", fmt.Errorf("no timing history on any of %v: %w", []string{branch, baseBranch}, errors.Join(errs...)) -} - -// dedupBranches returns the non-empty, order-preserved, de-duplicated -// branch candidates to try. -func dedupBranches(branches ...string) []string { - seen := map[string]bool{} - var out []string - for _, b := range branches { - if b == "" || seen[b] { - continue - } - seen[b] = true - out = append(out, b) - } - return out -} - -func fetchTimingsFromBranch(client *http.Client, repo, workflow, branch, token string) (map[string]float64, error) { - runID, err := latestSuccessfulRun(client, repo, workflow, branch, token) - if err != nil { - return nil, err - } - - artifactURL, err := findTimingsArtifactURL(client, repo, runID, token) - if err != nil { - return nil, err - } - - return downloadTimingsArtifact(client, artifactURL, token) -} - -func apiGet(client *http.Client, url, token string, out any) error { - req, err := http.NewRequest(http.MethodGet, url, nil) - if err != nil { - return err - } - req.Header.Set("Authorization", "Bearer "+token) - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("User-Agent", "sei-chain-testsplit") - - resp, err := client.Do(req) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) - return fmt.Errorf("GET %s: unexpected status %s: %s", url, resp.Status, body) - } - return json.NewDecoder(resp.Body).Decode(out) -} - -func latestSuccessfulRun(client *http.Client, repo, workflow, branch, token string) (int64, error) { - url := fmt.Sprintf( - "%s/repos/%s/actions/workflows/%s/runs?branch=%s&status=success&per_page=1", - apiBase, repo, workflow, branch, - ) - var result struct { - WorkflowRuns []struct { - ID int64 `json:"id"` - } `json:"workflow_runs"` - } - if err := apiGet(client, url, token, &result); err != nil { - return 0, err - } - if len(result.WorkflowRuns) == 0 { - return 0, fmt.Errorf("no successful %s runs found on %s", workflow, branch) - } - return result.WorkflowRuns[0].ID, nil -} - -const timingsArtifactName = "package-timings" - -func findTimingsArtifactURL(client *http.Client, repo string, runID int64, token string) (string, error) { - url := fmt.Sprintf("%s/repos/%s/actions/runs/%d/artifacts", apiBase, repo, runID) - var result struct { - Artifacts []struct { - Name string `json:"name"` - ArchiveDownloadURL string `json:"archive_download_url"` - Expired bool `json:"expired"` - } `json:"artifacts"` - } - if err := apiGet(client, url, token, &result); err != nil { - return "", err - } - for _, a := range result.Artifacts { - if a.Name == timingsArtifactName && !a.Expired { - return a.ArchiveDownloadURL, nil - } - } - return "", fmt.Errorf("no %q artifact found on run %d", timingsArtifactName, runID) -} - -const timingsFileName = "package_timings.json" - -func downloadTimingsArtifact(client *http.Client, url, token string) (map[string]float64, error) { - req, err := http.NewRequest(http.MethodGet, url, nil) - if err != nil { - return nil, err - } - req.Header.Set("Authorization", "Bearer "+token) - req.Header.Set("User-Agent", "sei-chain-testsplit") - - resp, err := client.Do(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("downloading artifact: unexpected status %s", resp.Status) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - - zr, err := zip.NewReader(bytes.NewReader(body), int64(len(body))) - if err != nil { - return nil, fmt.Errorf("artifact is not a valid zip: %w", err) - } - for _, f := range zr.File { - if filepath.Base(f.Name) != timingsFileName { - continue - } - rc, err := f.Open() - if err != nil { - return nil, err - } - defer rc.Close() - var timings map[string]float64 - if err := json.NewDecoder(rc).Decode(&timings); err != nil { - return nil, fmt.Errorf("decoding %s: %w", timingsFileName, err) - } - return timings, nil - } - return nil, fmt.Errorf("%s not found in artifact zip", timingsFileName) -} diff --git a/.github/scripts/testsplit/plan_test.go b/.github/scripts/testsplit/plan_test.go deleted file mode 100644 index cade506b8b..0000000000 --- a/.github/scripts/testsplit/plan_test.go +++ /dev/null @@ -1,116 +0,0 @@ -package main - -import ( - "math" - "testing" -) - -func TestRoundRobinInterleaves(t *testing.T) { - packages := []string{"a", "b", "c", "d", "e", "f", "g"} - shards := roundRobin(packages, 3) - - if got, want := len(shards), 3; got != want { - t.Fatalf("len(shards) = %d, want %d", got, want) - } - want := [][]string{{"a", "d", "g"}, {"b", "e"}, {"c", "f"}} - for i := range want { - if !equalSlices(shards[i], want[i]) { - t.Errorf("shard %d = %v, want %v", i, shards[i], want[i]) - } - } -} - -func TestBinPackBalancesKnownDurations(t *testing.T) { - packages := []string{"slow", "medium1", "medium2", "tiny1", "tiny2", "tiny3"} - timings := map[string]float64{ - "slow": 30, - "medium1": 25, - "medium2": 25, - "tiny1": 5, - "tiny2": 5, - "tiny3": 5, - } - - shards := binPack(packages, timings, 3) - if got, want := len(shards), 3; got != want { - t.Fatalf("len(shards) = %d, want %d", got, want) - } - - totals := make([]float64, 3) - seen := map[string]bool{} - for i, shard := range shards { - for _, pkg := range shard { - totals[i] += timings[pkg] - seen[pkg] = true - } - } - for _, pkg := range packages { - if !seen[pkg] { - t.Errorf("package %q missing from output", pkg) - } - } - - maxTotal, minTotal := totals[0], totals[0] - for _, tot := range totals { - maxTotal = math.Max(maxTotal, tot) - minTotal = math.Min(minTotal, tot) - } - if maxTotal-minTotal > 10 { - t.Errorf("shard totals too imbalanced: %v (spread %.0f)", totals, maxTotal-minTotal) - } -} - -func TestBinPackEstimatesUnknownAsMean(t *testing.T) { - packages := []string{"known1", "known2", "unknown"} - timings := map[string]float64{"known1": 10, "known2": 30} - // mean of known durations is 20, so "unknown" should behave like a 20s package. - - shards := binPack(packages, timings, 2) - totals := make([]float64, 2) - for i, shard := range shards { - for _, pkg := range shard { - dur, ok := timings[pkg] - if !ok { - dur = 20 - } - totals[i] += dur - } - } - if math.Abs(totals[0]-totals[1]) > 1e-9 { - t.Errorf("expected balanced totals treating unknown as mean, got %v", totals) - } -} - -func TestTimingCoverage(t *testing.T) { - packages := []string{"a", "b", "c", "d"} - timings := map[string]float64{"a": 1, "b": 2} - - if got, want := timingCoverage(packages, timings), 0.5; got != want { - t.Errorf("timingCoverage() = %v, want %v", got, want) - } - if got := timingCoverage(nil, timings); got != 0 { - t.Errorf("timingCoverage(nil, ...) = %v, want 0", got) - } -} - -func TestMeanDuration(t *testing.T) { - if got := meanDuration(nil); got != 0 { - t.Errorf("meanDuration(nil) = %v, want 0", got) - } - timings := map[string]float64{"a": 10, "b": 30} - if got, want := meanDuration(timings), 20.0; got != want { - t.Errorf("meanDuration() = %v, want %v", got, want) - } -} - -func equalSlices(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if a[i] != b[i] { - return false - } - } - return true -} diff --git a/.github/scripts/testsplit/record.go b/.github/scripts/testsplit/record.go deleted file mode 100644 index eac113608d..0000000000 --- a/.github/scripts/testsplit/record.go +++ /dev/null @@ -1,72 +0,0 @@ -package main - -import ( - "bufio" - "encoding/json" - "flag" - "fmt" - "os" - "regexp" - "strconv" -) - -// resultLine matches Go's own test summary lines, e.g.: -// -// ok github.com/sei-protocol/sei-chain/x/evm 12.345s -// FAIL github.com/sei-protocol/sei-chain/x/evm 3.210s -// -// This is what `go test` already prints per package, so no extra flags -// (e.g. -json) are needed to recover per-package elapsed time. -var resultLine = regexp.MustCompile(`^(?:ok|FAIL)\s+(\S+)\s+([0-9]+(?:\.[0-9]+)?)s(?:\s|$)`) - -func runRecord(args []string) error { - fs := flag.NewFlagSet("record", flag.ExitOnError) - input := fs.String("input", "", "path to captured `go test` output") - out := fs.String("out", "", "path to write this shard's timing JSON to") - if err := fs.Parse(args); err != nil { - return err - } - if *input == "" || *out == "" { - return fmt.Errorf("--input and --out are required") - } - - f, err := os.Open(*input) - if err != nil { - return err - } - defer f.Close() - - timings := map[string]float64{} - scanner := bufio.NewScanner(f) - // Test output lines can be long (e.g. verbose failure dumps); grow the - // buffer rather than truncating/erroring on long lines. - scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) - for scanner.Scan() { - m := resultLine.FindStringSubmatch(scanner.Text()) - if m == nil { - continue - } - seconds, err := strconv.ParseFloat(m[2], 64) - if err != nil { - continue - } - timings[m[1]] = seconds - } - if err := scanner.Err(); err != nil { - return fmt.Errorf("reading %s: %w", *input, err) - } - - return writeTimings(*out, timings) -} - -func writeTimings(path string, timings map[string]float64) error { - f, err := os.Create(path) - if err != nil { - return err - } - defer f.Close() - - enc := json.NewEncoder(f) - enc.SetIndent("", " ") - return enc.Encode(timings) -} diff --git a/.github/scripts/testsplit/record_test.go b/.github/scripts/testsplit/record_test.go deleted file mode 100644 index 53588d4b9d..0000000000 --- a/.github/scripts/testsplit/record_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package main - -import ( - "encoding/json" - "os" - "path/filepath" - "testing" -) - -func TestRunRecordParsesResultLines(t *testing.T) { - dir := t.TempDir() - input := filepath.Join(dir, "test-output.txt") - out := filepath.Join(dir, "timings.json") - - content := `=== RUN TestFoo ---- PASS: TestFoo (0.00s) -ok github.com/sei-protocol/sei-chain/x/evm 12.345s -FAIL github.com/sei-protocol/sei-chain/x/gov 3.5s -? github.com/sei-protocol/sei-chain/x/notest [no test files] -` - if err := os.WriteFile(input, []byte(content), 0o644); err != nil { - t.Fatal(err) - } - - if err := runRecord([]string{"--input=" + input, "--out=" + out}); err != nil { - t.Fatalf("runRecord() error = %v", err) - } - - data, err := os.ReadFile(out) - if err != nil { - t.Fatal(err) - } - var timings map[string]float64 - if err := json.Unmarshal(data, &timings); err != nil { - t.Fatal(err) - } - - want := map[string]float64{ - "github.com/sei-protocol/sei-chain/x/evm": 12.345, - "github.com/sei-protocol/sei-chain/x/gov": 3.5, - } - if len(timings) != len(want) { - t.Fatalf("timings = %v, want %v", timings, want) - } - for pkg, dur := range want { - if got := timings[pkg]; got != dur { - t.Errorf("timings[%q] = %v, want %v", pkg, got, dur) - } - } - if _, ok := timings["github.com/sei-protocol/sei-chain/x/notest"]; ok { - t.Error("expected packages with no test files to be excluded") - } -} diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 06eeba933f..9b8add2ed5 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -40,11 +40,6 @@ jobs: name: "Race Detection (shard ${{ matrix.shard }})" runs-on: uci-default needs: shard-indexes - permissions: - contents: read - # testsplit queries the Actions API for the last successful `main` - # run's timing artifact; this is the read-only scope that allows it. - actions: read strategy: fail-fast: false matrix: @@ -76,8 +71,6 @@ jobs: run: go mod download - name: Go test - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | set -euo pipefail make split-test-packages NUM_SPLIT=${{ env.NUM_SPLIT }} @@ -87,31 +80,7 @@ jobs: echo "occ_tests present in this shard; forcing -parallel=1" PARALLEL=(-parallel=1) fi - set +e - xargs go test "${PARALLEL[@]}" -timeout=${{ env.GO_TEST_TIMEOUT }} < "$SHARD_FILE" 2>&1 | tee test-output.txt - TEST_EXIT=${PIPESTATUS[0]} - set -e - exit "$TEST_EXIT" - - - name: Record test timings - # Captures per-package duration even when tests fail, so a shard - # that fails partway through still contributes what it learned. - if: always() - run: | - set -euo pipefail - mkdir -p build - if [ -f test-output.txt ]; then - go run ./.github/scripts/testsplit record --input=test-output.txt --out=build/timings.shard-${{ matrix.shard }}.json - else - echo '{}' > build/timings.shard-${{ matrix.shard }}.json - fi - - - name: Upload shard timings - if: always() - uses: actions/upload-artifact@v5 - with: - name: race-timings-shard-${{ matrix.shard }} - path: build/timings.shard-${{ matrix.shard }}.json + xargs go test "${PARALLEL[@]}" -timeout=${{ env.GO_TEST_TIMEOUT }} < "$SHARD_FILE" test-check: name: Race Detection @@ -126,47 +95,6 @@ jobs: exit 1 fi - record-timings: - name: Record package timings - runs-on: ubuntu-latest - needs: test - # testsplit's fetchTimings checks this run's own branch first, then - # falls back to `main` — so PR runs need to publish their own history - # too, not just push runs on main. merge_group runs target a synthetic, - # ephemeral gh-readonly-queue/* ref that won't be queried again, so - # skip those. - if: always() && github.event_name != 'merge_group' - permissions: - contents: read - actions: read - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 - with: - fetch-depth: 1 - - - uses: actions/setup-go@v6 - with: - go-version: ${{ env.GO_VERSION }} - cache: false - - - name: Download shard timings - uses: actions/download-artifact@v4 - with: - pattern: race-timings-shard-* - path: build/shard-timings - merge-multiple: true - - - name: Merge package timings - run: | - set -euo pipefail - go run ./.github/scripts/testsplit merge --out=build/package_timings.json build/shard-timings/*.json - - - name: Upload package timings - uses: actions/upload-artifact@v5 - with: - name: package-timings - path: build/package_timings.json - coverage: name: Coverage runs-on: ${{ github.event_name == 'merge_group' && 'ubuntu-latest' || 'uci-default' }} diff --git a/AGENTS.md b/AGENTS.md index b5e92e6143..21f08ca51f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,20 +66,13 @@ make install # install seid into $GOBIN ``` Tests run with the race detector and coverage. `go-test.yml`'s Race Detection job -shards into `NUM_SPLIT` (currently 3) parallel matrix jobs, split by -[`.github/scripts/testsplit`](.github/scripts/testsplit): it bin-packs packages -by historical per-package duration, querying the current branch's own last -successful run first (so a long-lived PR's shards reflect its own test -changes) and falling back to `main`'s last successful run if the branch has -no history yet (e.g. a PR's first push), then falling back further to a -deterministic round-robin split if neither has usable data. `make test-group-N` -runs the same tool locally — without a `GITHUB_TOKEN` env var it always takes -the round-robin path, so the local split won't exactly match a given CI -shard, but the package set and flags (`-race`, `occ_tests` `-parallel=1`) -are otherwise identical: +shards into `NUM_SPLIT` (currently 3) parallel matrix jobs, round-robin split +(package `i` goes to shard `i % NUM_SPLIT`, not a contiguous chunk — see the +`split-test-packages` Makefile target). `make test-group-N` reproduces a given +shard locally with the same package split: ```bash -NUM_SPLIT=3 make test-group-0 # run one local shard (round-robin without GITHUB_TOKEN) +NUM_SPLIT=3 make test-group-0 # reproduce CI race shard 0 locally go test .//... # run a single package ``` diff --git a/Makefile b/Makefile index 90a358d1f5..89df2f113e 100644 --- a/Makefile +++ b/Makefile @@ -584,14 +584,15 @@ $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests -# testsplit balances shards by historical per-package test duration (queried -# from the last successful `main` run of go-test.yml) when available, and -# falls back to a deterministic round-robin split otherwise — see -# .github/scripts/testsplit. It writes packages.txt.0 .. packages.txt.N-1 -# next to packages.txt, replacing the old `split -d -n l/N` approach (whose -# numeric-suffix width isn't guaranteed to match NUM_SPLIT's digit count). +# Round-robin (not contiguous-chunk) split: package i goes to shard i%N. +# Interleaving avoids dumping a whole cluster of alphabetically-adjacent +# (and often runtime-correlated, e.g. a module's many keeper packages) +# packages into one shard, unlike a straight `split -d -n l/N` chunk split. +# Pre-touch all N files first so a shard with zero packages (NUM_SPLIT > +# package count) still gets an (empty) file instead of breaking test-group-%. split-test-packages:$(BUILDDIR)/packages.txt - go run ./.github/scripts/testsplit plan --num-split=$(NUM_SPLIT) --out-dir=$(BUILDDIR) < $< + @for i in $$(seq 0 $$(($(NUM_SPLIT)-1))); do : > $(BUILDDIR)/packages.txt.$$i; done + @awk -v n=$(NUM_SPLIT) -v dir=$(BUILDDIR) '{print > (dir "/packages.txt." (NR-1)%n)}' $< test-group-%:split-test-packages @echo "🔍 Checking for special package: $(TARGET_PACKAGE)" @if grep -q "$(TARGET_PACKAGE)" $(BUILDDIR)/packages.txt.$*; then \ From 9cb1953e657eb8fc418ab785bb087108264131c5 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Fri, 7 Aug 2026 18:44:17 +0200 Subject: [PATCH 7/9] fix: restore full package list for race sharding, not just test-having ones Wiring go-test.yml's Race Detection job through split-test-packages picked up a pre-existing filter (test-group-N's packages.txt only ever included packages with _test.go files, meant for local-dev use before this target was unused by CI). That silently narrowed the race job's scope from the original go list ./... to test-only packages, dropping compile coverage for packages with no tests under -race -tags=ledger,test_ledger_mock. `go test` on a package with no tests still compiles it, so go list ./... was doing double duty as a full-tree compile check. Drop the TestGoFiles/XTestGoFiles filter so both make test-group-N and CI's shards cover every package again, restoring that check while keeping local/CI parity intact. Flagged by the repo's automated AI review (Seidroid). --- Makefile | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 89df2f113e..1ba227b9a6 100644 --- a/Makefile +++ b/Makefile @@ -576,11 +576,13 @@ STATE_DB_PKG_PREFIX := github.com/sei-protocol/sei-chain/sei-db/state_db $(BUILDDIR): mkdir -p $@ -# The format statement filters out all packages that don't have tests. -# Note we need to check for both in-package tests (.TestGoFiles) and -# out-of-package tests (.XTestGoFiles). +# Includes every package, not just ones with test files: `go test` on a +# package with no tests still compiles it (reported as "no test files"), +# which is how go-test.yml's Race Detection job also acts as a compile +# check under -race -tags=ledger,test_ledger_mock for the whole tree. +# Filtering to test-only packages here would silently drop that coverage. $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) - go list -f "{{ if (or .TestGoFiles .XTestGoFiles) }}{{ .ImportPath }}{{ end }}" ./... | grep -v "^$(STATE_DB_PKG_PREFIX)" | sort > $@ + go list ./... | grep -v "^$(STATE_DB_PKG_PREFIX)" | sort > $@ TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests From cedd0094c2fb86d8ffd43f231a70a53f04c652a3 Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Mon, 10 Aug 2026 13:16:34 +0200 Subject: [PATCH 8/9] fix: make packages.txt generation fail on a broken go list go list ./... | grep ... | sort only checked sort's exit status, so a failing go list could silently produce a truncated packages.txt while the recipe still succeeded. Split go list onto its own line so its exit code aborts the build. Co-Authored-By: Claude Sonnet 5 --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1ba227b9a6..06f1733e24 100644 --- a/Makefile +++ b/Makefile @@ -582,7 +582,9 @@ $(BUILDDIR): # check under -race -tags=ledger,test_ledger_mock for the whole tree. # Filtering to test-only packages here would silently drop that coverage. $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) - go list ./... | grep -v "^$(STATE_DB_PKG_PREFIX)" | sort > $@ + go list ./... > $@.tmp + grep -v "^$(STATE_DB_PKG_PREFIX)" $@.tmp | sort > $@ + @rm -f $@.tmp TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests From b4793b4061078b9eea1b254278c9bc18e3a1848e Mon Sep 17 00:00:00 2001 From: Amir Deris Date: Mon, 10 Aug 2026 13:37:19 +0200 Subject: [PATCH 9/9] Split heavy tests in the shards first before other tests --- Makefile | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 06f1733e24..255e4508a3 100644 --- a/Makefile +++ b/Makefile @@ -588,15 +588,41 @@ $(BUILDDIR)/packages.txt:$(GO_TEST_FILES) $(BUILDDIR) TARGET_PACKAGE := github.com/sei-protocol/sei-chain/occ_tests -# Round-robin (not contiguous-chunk) split: package i goes to shard i%N. +# Packages whose test suite alone regularly runs 1-4+ minutes under -race. +# Plain i%N round-robin assigns these by their position in the full, +# alphabetically-sorted package list, so several of them can land on the +# same shard by coincidence. Splitting them into their own round-robin +# pass, ahead of the rest of the list, guarantees consecutive heavy +# packages rotate across shards instead of clustering. Re-derive this list +# occasionally from a race job's `ok s` log lines. +HEAVY_TEST_PACKAGES := \ + github.com/sei-protocol/sei-chain/sei-db/db_engine/litt/disktable \ + github.com/sei-protocol/sei-chain/sei-cosmos/storev2/rootmulti \ + github.com/sei-protocol/sei-chain/sei-db/db_engine/litt/test \ + github.com/sei-protocol/sei-chain/sei-ibc-go/modules/core/04-channel/keeper \ + github.com/sei-protocol/sei-chain/giga/tests \ + github.com/sei-protocol/sei-chain/sei-cosmos/x/staking/keeper \ + github.com/sei-protocol/sei-chain/evmrpc/tests \ + github.com/sei-protocol/sei-chain/sei-ibc-go/modules/core/03-connection/keeper \ + github.com/sei-protocol/sei-chain/sei-ibc-go/modules/apps/transfer/keeper \ + github.com/sei-protocol/sei-chain/sei-cosmos/x/bank/keeper + +# Round-robin split: package i goes to shard i%N. # Interleaving avoids dumping a whole cluster of alphabetically-adjacent # (and often runtime-correlated, e.g. a module's many keeper packages) # packages into one shard, unlike a straight `split -d -n l/N` chunk split. # Pre-touch all N files first so a shard with zero packages (NUM_SPLIT > # package count) still gets an (empty) file instead of breaking test-group-%. +# HEAVY_TEST_PACKAGES is round-robined separately, and first, so its own +# i%N indexing can't collide with the coincidental clustering above. split-test-packages:$(BUILDDIR)/packages.txt @for i in $$(seq 0 $$(($(NUM_SPLIT)-1))); do : > $(BUILDDIR)/packages.txt.$$i; done - @awk -v n=$(NUM_SPLIT) -v dir=$(BUILDDIR) '{print > (dir "/packages.txt." (NR-1)%n)}' $< + @printf '%s\n' $(HEAVY_TEST_PACKAGES) > $(BUILDDIR)/heavy-packages.txt + @grep -Fxf $(BUILDDIR)/heavy-packages.txt $< > $(BUILDDIR)/packages.txt.heavy || true + @grep -Fxvf $(BUILDDIR)/heavy-packages.txt $< > $(BUILDDIR)/packages.txt.rest || true + @awk -v n=$(NUM_SPLIT) -v dir=$(BUILDDIR) '{print >> (dir "/packages.txt." (NR-1)%n)}' $(BUILDDIR)/packages.txt.heavy + @awk -v n=$(NUM_SPLIT) -v dir=$(BUILDDIR) '{print >> (dir "/packages.txt." (NR-1)%n)}' $(BUILDDIR)/packages.txt.rest + @rm -f $(BUILDDIR)/heavy-packages.txt $(BUILDDIR)/packages.txt.heavy $(BUILDDIR)/packages.txt.rest test-group-%:split-test-packages @echo "🔍 Checking for special package: $(TARGET_PACKAGE)" @if grep -q "$(TARGET_PACKAGE)" $(BUILDDIR)/packages.txt.$*; then \