fix(ci): shard the focused test job instead of one long-lived run

The focused path tested every impacted package in a single job with a
plain `go test -race`, so internal/app ran inside one long-lived process
alongside all of its reverse dependencies. That is exactly the shape
scripts/ci/run-app-race-tests.sh exists to avoid: a single app test
process retains every constructed command tree in framework registries,
so the run grows to 900s and the job stays alive long enough to be
reclaimed by the runner. Recent focused runs failed with SIGTERM after
9-10 minutes without a single test failure, and one earlier run failed
at `internal/app 902.651s`, 2.65s past the package timeout.

Fan the same package plan across the shard matrix test-race already
uses, and run each shard the way test-race runs it: internal/app through
the process-isolating helper, cli/smoke with their wider package budget,
release-scripts without race and with archive tooling.

changed-test-packages.sh gains `list-shard`, which intersects the
impacted set with scripts/ci/test-packages.sh shard membership so shard
definitions stay single-sourced — and so an unknown shard name aborts
there rather than reporting an empty selection, which would let a
mistyped shard skip every test while reporting success.

release-scripts is in the matrix on purpose: its dedicated job only runs
at full-suite or release-sensitive scope, so omitting it here would stop
testing test/scripts changes altogether. A test pins that the shard
selections partition the impacted set exactly, so shard-plan drift
cannot silently shrink focused coverage.
This commit is contained in:
chichuan
2026-08-15 22:10:28 +08:00
parent 8560830d3e
commit ecce09b355
3 changed files with 357 additions and 18 deletions
+72 -9
View File
@@ -513,11 +513,28 @@ jobs:
run: node .github/reviewer-routing.test.js
test-focused:
name: Test (changed packages)
name: "Test (focused: ${{ matrix.shard }})"
needs: lint
if: ${{ needs.lint.outputs.changelog_only != 'true' && needs.lint.outputs.docs_only != 'true' && needs.lint.outputs.full_suite != 'true' }}
runs-on: ubuntu-latest
# Each shard owns one bounded slice of the impacted set, so no single job
# carries internal/app together with every reverse dependency. The shard
# list and per-shard execution below mirror test-race, which runs the same
# shards at full-suite scope; release-scripts is included because its
# dedicated job only runs at full-suite or release-sensitive scope, and
# dropping it here would stop testing test/scripts changes entirely.
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
shard:
- app
- generators
- helpers
- cli
- smoke
- remaining
- release-scripts
steps:
- name: Check out repository
uses: actions/checkout@v4
@@ -546,23 +563,69 @@ jobs:
with:
go-version-file: go.mod
- name: Test changed packages and reverse dependencies
- name: Select impacted packages for shard
id: select
shell: bash
env:
DWS_PACKAGE_VERSION: 0.0.0-test
TEST_SHARD: ${{ matrix.shard }}
run: |
set -euo pipefail
package_output="$(
./scripts/ci/changed-test-packages.sh \
list "$TEST_BASE_REF" "$TEST_HEAD_REF"
list-shard "$TEST_SHARD" "$TEST_BASE_REF" "$TEST_HEAD_REF"
)"
if [ -z "$package_output" ]; then
echo "No buildable Go package is affected by this revision." \
echo "No buildable Go package in shard $TEST_SHARD is affected by this revision." \
>> "$GITHUB_STEP_SUMMARY"
exit 0
fi
mapfile -t packages <<< "$package_output"
go test -v -race -count=1 -timeout=15m "${packages[@]}"
{
echo "packages<<SHARD_PACKAGES_EOF"
printf '%s\n' "$package_output"
echo "SHARD_PACKAGES_EOF"
} >> "$GITHUB_OUTPUT"
- name: Build
if: ${{ matrix.shard == 'remaining' && steps.select.outputs.packages != '' }}
run: make build
- name: Install archive tooling
if: ${{ matrix.shard == 'release-scripts' && steps.select.outputs.packages != '' }}
run: sudo apt-get update && sudo apt-get install -y zip unzip
- name: Test shard with Race Detection
if: ${{ steps.select.outputs.packages != '' }}
shell: bash
env:
DWS_PACKAGE_VERSION: 0.0.0-test
TEST_SHARD: ${{ matrix.shard }}
SHARD_PACKAGES: ${{ steps.select.outputs.packages }}
run: |
set -euo pipefail
mapfile -t packages <<< "$SHARD_PACKAGES"
test "${#packages[@]}" -gt 0
if [ "$TEST_SHARD" = app ]; then
# A single long-lived app test process retains every constructed
# command tree in framework registries. Isolate Schema assembly and
# bounded name ranges so each process releases that state on exit.
test "${#packages[@]}" -eq 1
./scripts/ci/run-app-race-tests.sh run "${packages[0]}"
exit 0
fi
if [ "$TEST_SHARD" = release-scripts ]; then
# Mirror the dedicated release-contract job: these suites shell out
# to archive tooling and are not race-instrumented there.
go test -v -count=1 -timeout=10m "${packages[@]}"
exit 0
fi
# cli/smoke own heavy NewRootCommand / Schema assembly under -race;
# give them a dedicated package timeout on slower hosted runners.
timeout_budget=12m
if [ "$TEST_SHARD" = cli ] ||
[ "$TEST_SHARD" = smoke ]; then
timeout_budget=15m
fi
go test -v -race -count=1 -timeout="$timeout_budget" "${packages[@]}"
test-race:
name: "Test (race: ${{ matrix.shard }})"
@@ -715,7 +778,7 @@ jobs:
failed=0
if [ "$CHANGELOG_ONLY" = true ] || [ "$DOCS_ONLY" = true ]; then
for shard in \
"changed packages:$FOCUSED_RESULT" \
"focused shards:$FOCUSED_RESULT" \
"race shards:$RACE_RESULT" \
"release scripts:$RELEASE_SCRIPTS_RESULT" \
"cross-platform compile:$CROSS_PLATFORM_RESULT" \
@@ -744,7 +807,7 @@ jobs:
release_expected=success
fi
for shard in \
"changed packages:$FOCUSED_RESULT:$focused_expected" \
"focused shards:$FOCUSED_RESULT:$focused_expected" \
"race shards:$RACE_RESULT:$race_expected" \
"release scripts:$RELEASE_SCRIPTS_RESULT:$release_expected" \
"cross-platform compile:$CROSS_PLATFORM_RESULT:success"
+46 -9
View File
@@ -7,16 +7,32 @@ set -eu
ROOT="$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd)"
usage() {
printf '%s\n' "usage: $0 <changed|list> BASE_REF HEAD_REF" >&2
printf '%s\n' \
"usage: $0 <changed|list> BASE_REF HEAD_REF" \
" $0 list-shard SHARD BASE_REF HEAD_REF" >&2
exit 2
}
[ "${1:-}" = changed ] || [ "${1:-}" = list ] || usage
[ "$#" -eq 3 ] || usage
MODE="$1"
BASE_REF="$2"
HEAD_REF="$3"
# list-shard narrows the impacted set to one test shard so focused CI runs can
# fan the same package plan across the shard matrix instead of testing every
# impacted package in a single long-lived job.
SHARD=""
case "${1:-}" in
changed|list)
[ "$#" -eq 3 ] || usage
MODE="$1"
BASE_REF="$2"
HEAD_REF="$3"
;;
list-shard)
[ "$#" -eq 4 ] || usage
MODE="$1"
SHARD="$2"
BASE_REF="$3"
HEAD_REF="$4"
;;
*) usage ;;
esac
cd "$ROOT"
git rev-parse --verify --quiet "${BASE_REF}^{commit}" >/dev/null || {
@@ -116,9 +132,29 @@ while IFS= read -r file; do
[ "$embedded_owner_found" = true ] || continue
done < "$files"
# emit_selection prints the planned packages, restricted to SHARD when the
# caller asked for one. Restriction reuses scripts/ci/test-packages.sh as the
# single source of shard membership, which also means an unknown shard name
# aborts there instead of being reported as an empty selection: a silently empty
# shard would let a mistyped CI shard name skip every test and still succeed.
emit_selection() {
selection="$1"
if [ -z "$SHARD" ]; then
cat "$selection"
return 0
fi
shard_packages="$workdir/shard-packages"
"$ROOT/scripts/ci/test-packages.sh" list "$SHARD" > "$shard_packages"
LC_ALL=C sort -u "$shard_packages" -o "$shard_packages"
LC_ALL=C sort -u "$selection" -o "$selection"
# An empty intersection is a normal outcome for a shard no change touched, so
# comm's silence must not be treated as an error.
LC_ALL=C comm -12 "$selection" "$shard_packages"
}
LC_ALL=C sort -u "$changed_packages" -o "$changed_packages"
if [ "$MODE" = changed ] || [ ! -s "$changed_packages" ]; then
cat "$changed_packages"
emit_selection "$changed_packages"
exit 0
fi
@@ -133,4 +169,5 @@ while IFS= read -r package; do
done < "$all_packages" > "$impacted_packages"
cat "$changed_packages" >> "$impacted_packages"
LC_ALL=C sort -u "$impacted_packages"
LC_ALL=C sort -u "$impacted_packages" -o "$impacted_packages"
emit_selection "$impacted_packages"
+239
View File
@@ -129,6 +129,245 @@ func Value() string {
}
}
// shardChangedPackagesShards mirrors the shard names scripts/ci/test-packages.sh
// asserts complete coverage for in its verify mode. Keeping the list here in
// full is deliberate: the union assertion below fails if a shard is added
// upstream without being selected, and an unknown-shard error fails if one is
// removed, so shard-plan drift cannot silently shrink focused-test coverage.
var shardChangedPackagesShards = []string{
"app",
"generators",
"helpers",
"cli",
"smoke",
"remaining",
"release-scripts",
}
func TestChangedTestPackagesShardSelectionCoversEveryImpactedPackageExactlyOnce(t *testing.T) {
repository := newShardChangedPackagesRepository(t)
baseRef := commitChangedTestPackagesFixture(t, repository, "initial shard fixture")
writeChangedTestPackagesFixture(t, repository, "core/core.go", `package core
func Value() string {
return "changed"
}
`)
headRef := commitChangedTestPackagesFixture(t, repository, "change core")
impacted := runChangedTestPackages(t, repository, "list", baseRef, headRef)
if len(impacted) == 0 {
t.Fatal("shard fixture produced no impacted packages")
}
owner := map[string]string{}
union := make([]string, 0, len(impacted))
for _, shard := range shardChangedPackagesShards {
for _, pkg := range runChangedTestPackagesShard(t, repository, shard, baseRef, headRef) {
if previous, ok := owner[pkg]; ok {
t.Fatalf("package %q selected by both shard %q and %q", pkg, previous, shard)
}
owner[pkg] = shard
union = append(union, pkg)
}
}
slices.Sort(union)
if !slices.Equal(union, impacted) {
t.Fatalf("shard selection union = %q, want every impacted package %q", union, impacted)
}
}
func TestChangedTestPackagesShardSelectionFailsClosedOnUnknownShard(t *testing.T) {
repository := newShardChangedPackagesRepository(t)
baseRef := commitChangedTestPackagesFixture(t, repository, "initial shard fixture")
writeChangedTestPackagesFixture(t, repository, "core/core.go", `package core
func Value() string {
return "changed"
}
`)
headRef := commitChangedTestPackagesFixture(t, repository, "change core")
script := filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh")
command := exec.Command(script, "list-shard", "nonexistent", baseRef, headRef)
command.Dir = repository
output, err := command.CombinedOutput()
if err == nil {
t.Fatalf("unknown shard unexpectedly succeeded:\n%s", output)
}
// An unknown shard must abort rather than report an empty selection: a
// silently empty shard would let a mistyped CI shard name skip every test
// while still reporting success.
if !strings.Contains(string(output), "unknown test package shard") {
t.Fatalf("unknown shard failure = %q, want fail-closed diagnostic", output)
}
if strings.TrimSpace(string(output)) == "" {
t.Fatal("unknown shard produced empty output instead of a diagnostic")
}
}
func TestChangedTestPackagesShardSelectionIsEmptyForUnaffectedShard(t *testing.T) {
repository := newShardChangedPackagesRepository(t)
baseRef := commitChangedTestPackagesFixture(t, repository, "initial shard fixture")
writeChangedTestPackagesFixture(t, repository, "internal/helpers/helpers.go", `package helpers
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " helpers changed"
}
`)
headRef := commitChangedTestPackagesFixture(t, repository, "change helpers only")
if selected := runChangedTestPackagesShard(t, repository, "helpers", baseRef, headRef); len(selected) == 0 {
t.Fatal("helpers shard selected nothing for a helpers-only change")
}
// An unaffected shard is a normal outcome and must exit zero with no
// output so the CI step can no-op instead of failing the job.
if selected := runChangedTestPackagesShard(t, repository, "app", baseRef, headRef); len(selected) != 0 {
t.Fatalf("app shard = %q, want empty for a helpers-only change", selected)
}
}
func TestChangedTestPackagesShardSelectionRejectsMissingShardArgument(t *testing.T) {
repository := newShardChangedPackagesRepository(t)
baseRef := commitChangedTestPackagesFixture(t, repository, "initial shard fixture")
script := filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh")
command := exec.Command(script, "list-shard", baseRef, baseRef)
command.Dir = repository
output, err := command.CombinedOutput()
if err == nil {
t.Fatalf("list-shard without a shard argument unexpectedly succeeded:\n%s", output)
}
if !strings.Contains(string(output), "usage:") {
t.Fatalf("missing shard argument failure = %q, want usage diagnostic", output)
}
}
func runChangedTestPackagesShard(t *testing.T, repository, shard, baseRef, headRef string) []string {
t.Helper()
output := runChangedTestPackagesCommand(
t,
repository,
filepath.Join(repository, "scripts", "ci", "changed-test-packages.sh"),
"list-shard",
shard,
baseRef,
headRef,
)
return strings.Fields(output)
}
// newShardChangedPackagesRepository builds a fixture whose directory layout
// matches every shard scripts/ci/test-packages.sh knows about, so shard
// selection can be exercised without depending on the real repository's working
// tree state. Every package imports core, letting a single core edit mark the
// whole module impacted.
func newShardChangedPackagesRepository(t *testing.T) string {
t.Helper()
repository := t.TempDir()
for path, contents := range map[string]string{
"go.mod": `module example.com/shardfixture
go 1.22
`,
"core/core.go": `package core
func Value() string {
return "core"
}
`,
"internal/app/app.go": `package app
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " app"
}
`,
"internal/generator/gen/gen.go": `package gen
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " gen"
}
`,
"internal/helpers/helpers.go": `package helpers
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " helpers"
}
`,
"internal/cli/cli.go": `package cli
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " cli"
}
`,
"test/smoke/smoke.go": `package smoke
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " smoke"
}
`,
"test/scripts/scripts.go": `package scripts
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " scripts"
}
`,
"pkg/extra/extra.go": `package extra
import "example.com/shardfixture/core"
func Value() string {
return core.Value() + " extra"
}
`,
} {
writeChangedTestPackagesFixture(t, repository, path, contents)
}
projectRoot, err := filepath.Abs(filepath.Join("..", ".."))
if err != nil {
t.Fatalf("resolve project root: %v", err)
}
for _, name := range []string{"changed-test-packages.sh", "test-packages.sh"} {
script, err := os.ReadFile(filepath.Join(projectRoot, "scripts", "ci", name))
if err != nil {
t.Fatalf("read %s: %v", name, err)
}
scriptPath := filepath.Join(repository, "scripts", "ci", name)
if err := os.MkdirAll(filepath.Dir(scriptPath), 0o755); err != nil {
t.Fatalf("create script directory: %v", err)
}
if err := os.WriteFile(scriptPath, script, 0o755); err != nil {
t.Fatalf("copy %s: %v", name, err)
}
}
runChangedTestPackagesCommand(t, repository, "git", "init", "-q", "-b", "main")
runChangedTestPackagesCommand(t, repository, "git", "config", "user.name", "DWS CI")
runChangedTestPackagesCommand(t, repository, "git", "config", "user.email", "dws-ci@example.invalid")
return repository
}
func newChangedTestPackagesRepository(t *testing.T) string {
t.Helper()