From 347457def1d1d0b94a6a6fe5fda8257c0fc610e4 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:19:24 +0100 Subject: [PATCH 1/9] chore: capture the ubuntu check leg cancelled at its 30-minute ceiling PR #1003's check (ubuntu-latest) was cancelled at the cap twice and PR #1008's first attempt and PR #976's merge-queue entry once each, so the cancel policy of 2026-09-28 opens a speed lane for the ubuntu leg. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- ...162972-ubuntu-check-leg-cancelled-at-its-30.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .abcd/work/issues/open/iss-2610150500162972-ubuntu-check-leg-cancelled-at-its-30.md diff --git a/.abcd/work/issues/open/iss-2610150500162972-ubuntu-check-leg-cancelled-at-its-30.md b/.abcd/work/issues/open/iss-2610150500162972-ubuntu-check-leg-cancelled-at-its-30.md new file mode 100644 index 0000000000..800e9402b2 --- /dev/null +++ b/.abcd/work/issues/open/iss-2610150500162972-ubuntu-check-leg-cancelled-at-its-30.md @@ -0,0 +1,15 @@ +--- +schema_version: 1 +id: "iss-2610150500162972" +slug: "ubuntu-check-leg-cancelled-at-its-30" +severity: "major" +category: "process" +source: "agent-observation" +found_during: "the cancel policy's speed lane for the ubuntu check leg, 2026-10-15" +origin: researcher-authored +production_mode: hand-written +found_at: ".github/workflows/ci.yml" +remedy: "Cut the ubuntu check job's wall time by at least five minutes without raising its timeout or changing a required check or the step split: name the slowest packages first in both test steps, share the live-repository fixtures the heaviest race tests rebuild, and make the run lock's retry delay injectable for the take-back test, then confirm from the job's step times on the merge queue." +--- + +The ubuntu leg of ci.yml's check job (timeout-minutes 30) now runs at its limit for every pull request, and the cancel policy of 2026-09-28 has fired: a check cancelled at the cap twice for the same reason stops that pull request and opens a speed lane. PR #1003 (feat/memory-recall-notes) was cancelled at 30 minutes twice: run 38676280619 attempt 2 spent 03:58:37-04:06:22 in Test (about 8 minutes) and was still in Test (race, internal) from 04:06:22 when it was cancelled at 04:27:48. PR #1008 passed in 25m40 only on a rerun after its first attempt was cancelled at 30m05, and the merge-queue entry for PR #976 (run 38680018438) was cancelled at 30 minutes too, with internal/core/implement/loop still running. On passing runs the race step took 16-18 minutes and the plain step 6-7; the race step's package times sum to about 3,500-3,750 seconds on the four-core runner, so it is bound by total CPU rather than by one package, and its heaviest packages are internal/core/lifeboat (578-594s), internal/core/reading (547-557s), internal/core/launch (357-365s) and internal/surface/cli (333-367s). iss-2610142326511964 covers the macOS leg's slowdown; this is the ubuntu leg. From 6290c82c4f00509a445cc5880fb8f2a0753b390e Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:19:35 +0100 Subject: [PATCH 2/9] test: scope the two live-tree lint tests to the rule each counts TestForbiddenSynonymsRealGlossary and TestColdReadingDefinitionsSatisfyTheAgentContract ran the whole record-lint configuration over this repository to count one rule's findings. Each now keeps the committed roots and arms only the rule it counts (GL002, and agent_contract), which reads only its own configuration, so the count is the same; the record-lint step and make preflight still run the full set over the live tree. A planted "epic" in the brief and a cold-reading agent without prompt_version each still fail their test. Under -race (back to back on one machine): 74.4s to 3.5s and 76.0s to 1.5s; plain: 5.0s to 0.2s and 5.1s to 0.1s. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- internal/core/lint/forbidden_synonyms_test.go | 7 +++++++ internal/core/reading/definitions_test.go | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/core/lint/forbidden_synonyms_test.go b/internal/core/lint/forbidden_synonyms_test.go index d4cd1d66fd..a2c87a8730 100644 --- a/internal/core/lint/forbidden_synonyms_test.go +++ b/internal/core/lint/forbidden_synonyms_test.go @@ -218,6 +218,13 @@ func TestForbiddenSynonymsRealGlossary(t *testing.T) { if !found { t.Fatal("record-lint.json forbidden_synonyms.enforce must include 'epic' (itd-43)") } + // The committed roots with GL002 alone: the rule reads only its own + // configuration and the glossary, so every other family the full config + // arms would change nothing this test counts. The record-lint step and + // make preflight run the full set over the live tree; under -race the full + // set cost about 110s here, against seconds for GL002. + cfg.BannedTokens = nil + cfg.Rules = map[string]RuleConfig{"forbidden_synonyms": rc} fs, err := Lint(cfg, repoRoot) if err != nil { t.Fatalf("Lint: %v", err) diff --git a/internal/core/reading/definitions_test.go b/internal/core/reading/definitions_test.go index f6426b05f2..a1f1436a15 100644 --- a/internal/core/reading/definitions_test.go +++ b/internal/core/reading/definitions_test.go @@ -404,9 +404,17 @@ func TestColdReadingDefinitionsSatisfyTheAgentContract(t *testing.T) { } // Proof of life: a disabled rule reports nothing, and a test that reads // nothing from a rule that ran nowhere passes for the wrong reason. - if rule, ok := cfg.Rules["agent_contract"]; !ok || !rule.Enabled { + rule, ok := cfg.Rules["agent_contract"] + if !ok || !rule.Enabled { t.Fatalf("%s does not enable the agent_contract rule, so this case asserts nothing", LintConfigPath) } + // The committed roots with agent_contract alone: the rule reads only its + // own configuration and the agents directory, so every other family the + // full config arms would change nothing this test reads. The record-lint + // step and make preflight run the full set over the live tree; under -race + // the full set cost about 87s here. + cfg.BannedTokens = nil + cfg.Rules = map[string]lint.RuleConfig{"agent_contract": rule} findings, err := lint.Lint(cfg, root) if err != nil { t.Fatalf("lint: %v", err) From c8f360f68a7ef6575a6b7b5520b706e29759e2dd Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:38:44 +0100 Subject: [PATCH 3/9] test: share one plan of this repository across the lifeboat tests Five tests probed this repository from scratch, and the pack test probed it and then planned it, which probes again. Every probe of this record costs about 40s under -race, nearly all of it the open-questions marker scan over every file. The tests that only read the report now share one Plan of this repository (planThisRepo), whose Coverage is the probe Plan packed from, unchanged, and the pack test reads that plan's own coverage rather than a second probe, so each brief file is checked against the report it was written from. TestEverySourcePartialCarriesAReason still runs every source over its own context. The five tests, back to back on one machine: 254.7s to 92.1s under -race, 22.5s to 11.9s plain. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- internal/core/lifeboat/grounding_test.go | 5 +-- internal/core/lifeboat/mapping_test.go | 33 +++++++++++++++++++ internal/core/lifeboat/partial_reason_test.go | 29 ++++++++++------ internal/core/lifeboat/probe_test.go | 5 +-- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/internal/core/lifeboat/grounding_test.go b/internal/core/lifeboat/grounding_test.go index f47a0b66a4..6f12c09820 100644 --- a/internal/core/lifeboat/grounding_test.go +++ b/internal/core/lifeboat/grounding_test.go @@ -67,10 +67,7 @@ func findSection(t *testing.T, cov Coverage, name Section) SectionCoverage { // sections are grounded from the tier that owns them. With the adapter stubs // this fails (every section blank); it passes once the tiered adapters land. func TestProbeGroundsRichRepoFromEveryTier(t *testing.T) { - cov, err := Probe(repoRoot(t)) - if err != nil { - t.Fatal(err) - } + cov := probeThisRepo(t) if cov.Summary.Grounded < 5 { t.Errorf("rich repo grounded only %d sections; the record should ground many", cov.Summary.Grounded) diff --git a/internal/core/lifeboat/mapping_test.go b/internal/core/lifeboat/mapping_test.go index 35ab9a72e4..2a512252b6 100644 --- a/internal/core/lifeboat/mapping_test.go +++ b/internal/core/lifeboat/mapping_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "strings" + "sync" "testing" ) @@ -31,6 +32,38 @@ func repoRoot(t *testing.T) string { } } +// thisRepoPlan is one plan of this repository, shared by the tests that only +// read it or the coverage it was packed from. Plan probes the tree and packs +// from that report, unchanged, so its Coverage is exactly what Probe returns; +// both are pure functions of the tree, and no test in this package writes to +// this repository, so every one of those tests saw the same report. Under +// -race each probe of this record cost about 40 seconds, nearly all of it the +// open-questions marker scan over every file. A test that changes what it was +// handed must plan or probe for itself. +var thisRepoPlan struct { + once sync.Once + lb Lifeboat + err error +} + +// planThisRepo returns that shared plan of this repository. +func planThisRepo(t *testing.T) Lifeboat { + t.Helper() + root := repoRoot(t) + thisRepoPlan.once.Do(func() { thisRepoPlan.lb, thisRepoPlan.err = Plan(root) }) + if thisRepoPlan.err != nil { + t.Fatal(thisRepoPlan.err) + } + return thisRepoPlan.lb +} + +// probeThisRepo returns the coverage of that shared plan: this repository's +// probe. +func probeThisRepo(t *testing.T) Coverage { + t.Helper() + return planThisRepo(t).Coverage +} + // TestBriefCarriesTheRenderedMappingTable is the anti-drift detector: the brief // document and the Go table are one contract, and the code is its source of // truth. Editing either alone fails here. diff --git a/internal/core/lifeboat/partial_reason_test.go b/internal/core/lifeboat/partial_reason_test.go index a9ed3229c5..81d072b821 100644 --- a/internal/core/lifeboat/partial_reason_test.go +++ b/internal/core/lifeboat/partial_reason_test.go @@ -29,9 +29,14 @@ func partialReasonFixtures(t *testing.T) map[string]string { func TestProbeEveryPartialCarriesAReason(t *testing.T) { sawPartial := false for name, dir := range partialReasonFixtures(t) { - cov, err := Probe(dir) - if err != nil { - t.Fatalf("%s: %v", name, err) + var cov Coverage + if name == "repo" { + cov = probeThisRepo(t) + } else { + var err error + if cov, err = Probe(dir); err != nil { + t.Fatalf("%s: %v", name, err) + } } for _, s := range cov.Sections { if s.Status != StatusPartial { @@ -116,14 +121,18 @@ func TestPartialIsBuiltOnlyThroughTheConstructor(t *testing.T) { func TestPackSectionFileStatesWhyPartial(t *testing.T) { sawPartial := false for name, dir := range partialReasonFixtures(t) { - cov, err := Probe(dir) - if err != nil { - t.Fatalf("%s: %v", name, err) - } - lb, err := Plan(dir) - if err != nil { - t.Fatalf("%s: plan: %v", name, err) + var lb Lifeboat + if name == "repo" { + lb = planThisRepo(t) + } else { + var err error + if lb, err = Plan(dir); err != nil { + t.Fatalf("%s: plan: %v", name, err) + } } + // The coverage the plan packed from, rather than a second probe of + // the same tree: it is the report each brief file was written against. + cov := lb.Coverage for _, s := range cov.Sections { leaf, ok := briefLeaf(s.Name) if !ok || s.Status == StatusBlank { diff --git a/internal/core/lifeboat/probe_test.go b/internal/core/lifeboat/probe_test.go index 5a47e248f6..4cafa13be6 100644 --- a/internal/core/lifeboat/probe_test.go +++ b/internal/core/lifeboat/probe_test.go @@ -390,10 +390,7 @@ func fileHashes(t *testing.T, root string) map[string]string { // accounts for all of them. A section silently dropped from the report would be // a gap the coverage experiment could not see. func TestProbeReportsEverySection(t *testing.T) { - cov, err := Probe(repoRoot(t)) - if err != nil { - t.Fatal(err) - } + cov := probeThisRepo(t) if cov.SchemaVersion != SchemaVersion { t.Errorf("schema_version = %d, want %d", cov.SchemaVersion, SchemaVersion) } From 478e1b0978e51536707fc78cad98a44e6b43726d Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:38:47 +0100 Subject: [PATCH 4/9] test: share one render and scan of this repository's payload TestPayloadTreeImplementationsResolveIdentically and TestRenderPayloadLeavesSourceTreeUnversioned each rendered this repository's payload, and TestRepoPayloadBinariesScanClean scanned it again; each pass is the payload's secret scan, about 50s under -race. The tests now share one render (renderThisRepo), which reads the committed manifests before it renders, so the unversioned test still compares them against the bytes that stood before the render ran, and keeps the scan its pre-check made of ResolveBundle(root, nil), the bundle the scan test read. scanBundle becomes a variable so the shared render can keep that scan; it refuses unless the render scanned exactly once. The three tests, back to back on one machine: 166.8s to 54.3s under -race, 9.7s to 3.2s plain. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- internal/core/launch/dryrun.go | 4 +- internal/core/launch/installsurface_test.go | 10 +-- internal/core/launch/render_test.go | 19 +--- internal/core/launch/scan_coverage_test.go | 9 +- internal/core/launch/thisrepo_render_test.go | 91 ++++++++++++++++++++ 5 files changed, 100 insertions(+), 33 deletions(-) create mode 100644 internal/core/launch/thisrepo_render_test.go diff --git a/internal/core/launch/dryrun.go b/internal/core/launch/dryrun.go index e39d46b999..482fc682d8 100644 --- a/internal/core/launch/dryrun.go +++ b/internal/core/launch/dryrun.go @@ -205,7 +205,9 @@ func DryRun(req DryRunRequest) (DryRunReport, error) { } // scanBundle adapts the bundle's Included files to the scanner and runs it. -func scanBundle(repoRoot string, bundle Bundle) scanner.ScanResult { +// It is a variable so a test can keep the scan a render of this repository +// made, rather than scan the same payload a second time. +var scanBundle = func(repoRoot string, bundle Bundle) scanner.ScanResult { sc, err := scanner.New(repoRoot) if err != nil { return scanner.ScanResult{Unavailable: true, UnavailableReason: err.Error()} diff --git a/internal/core/launch/installsurface_test.go b/internal/core/launch/installsurface_test.go index 17e858a5fa..1ec0cd45f3 100644 --- a/internal/core/launch/installsurface_test.go +++ b/internal/core/launch/installsurface_test.go @@ -1,10 +1,8 @@ package launch import ( - "path/filepath" "reflect" "testing" - "time" ) // bundleTreeFor resolves root's payload the way a ship would and returns the @@ -263,13 +261,7 @@ func TestResolveInstallSurfaceCommittedPayload(t *testing.T) { // prevent. func TestPayloadTreeImplementationsResolveIdentically(t *testing.T) { root := repoRootForTest(t) - dest := filepath.Join(t.TempDir(), "payload") - if _, err := RenderPayload(PayloadRenderRequest{ - RepoRoot: root, Dest: dest, Version: "9.9.9", - Entry: ChangelogEntry{Tier: "patch", Reason: "seam parity", Date: time.Now(), SourceSHA: "deadbeef"}, Dirty: DirtySkip, - }); err != nil { - t.Fatalf("render the payload: %v", err) - } + dest, _ := renderThisRepo(t) fromBundle, err := ResolveInstallSurface(bundleTreeFor(t, root)) if err != nil { diff --git a/internal/core/launch/render_test.go b/internal/core/launch/render_test.go index 52b41a77c8..87e90503f1 100644 --- a/internal/core/launch/render_test.go +++ b/internal/core/launch/render_test.go @@ -93,24 +93,9 @@ func TestRenderPayloadSatisfiesPublicLockstep(t *testing.T) { // working tree would pass every other test here and silently break the premise. func TestRenderPayloadLeavesSourceTreeUnversioned(t *testing.T) { root := repoRootForTest(t) - manifests := []string{".claude-plugin/plugin.json", ".claude-plugin/marketplace.json"} - before := make(map[string][]byte, len(manifests)) - for _, rel := range manifests { - data, err := os.ReadFile(filepath.Join(root, rel)) - if err != nil { - t.Fatalf("read %s: %v", rel, err) - } - before[rel] = data - } - - dest := filepath.Join(t.TempDir(), "payload") - if _, err := RenderPayload(PayloadRenderRequest{ - RepoRoot: root, Dest: dest, Version: "9.9.9", Entry: sampleEntry(), Dirty: DirtySkip, - }); err != nil { - t.Fatalf("render this repository's payload: %v", err) - } + _, before := renderThisRepo(t) - for _, rel := range manifests { + for _, rel := range thisRepoManifests { after, err := os.ReadFile(filepath.Join(root, rel)) if err != nil { t.Fatalf("re-read %s: %v", rel, err) diff --git a/internal/core/launch/scan_coverage_test.go b/internal/core/launch/scan_coverage_test.go index 1e3ba4d5d0..9fe85e7b26 100644 --- a/internal/core/launch/scan_coverage_test.go +++ b/internal/core/launch/scan_coverage_test.go @@ -139,12 +139,9 @@ func TestBinaryPayloadSecretRefuses(t *testing.T) { // deliberately not asserted here — an environmental identity match on a text // file (iss-2608291444328326) is a different rule and a different file class. func TestRepoPayloadBinariesScanClean(t *testing.T) { - root := repoRootForTest(t) - bundle, err := ResolveBundle(root, nil) - if err != nil { - t.Fatalf("resolve the payload bundle: %v", err) - } - scan := scanBundle(root, bundle) + // The scan the render of this repository's payload made: the same scanner + // over the same bundle (ResolveBundle(root, nil)) a direct call reads. + scan := scanThisRepo(t) if scan.Unavailable { t.Fatalf("scanner unavailable on the repo's own payload: %s", scan.UnavailableReason) } diff --git a/internal/core/launch/thisrepo_render_test.go b/internal/core/launch/thisrepo_render_test.go new file mode 100644 index 0000000000..7a8967b81b --- /dev/null +++ b/internal/core/launch/thisrepo_render_test.go @@ -0,0 +1,91 @@ +package launch + +import ( + "fmt" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/intentdriven/abcd/internal/adapter/scanner" +) + +// thisRepoRender is one render of this repository's own payload, shared by the +// tests that only read what it wrote. The render is a pure function of the +// tree and its request, and no test in this package writes to this +// repository, so each of those tests saw the same payload; under -race every +// render of this repository cost about 45 seconds, nearly all of it the +// payload's secret scan, so the scan the render's pre-check made of the +// payload is kept as well, for the test that reads the scan itself. The +// committed manifests are read before the render, +// so a test that holds the render to leaving them unchanged compares against +// the bytes as they stood before it ran, whichever test ran it. +var thisRepoRender struct { + once sync.Once + dir string // the scratch directory holding the payload; TestMain removes it + dest string + before map[string][]byte + scan scanner.ScanResult + err error +} + +// thisRepoManifests are the committed manifests adr-19 keeps version-absent. +var thisRepoManifests = []string{".claude-plugin/plugin.json", ".claude-plugin/marketplace.json"} + +// renderThisRepo returns that shared render's payload directory and the +// committed manifests' bytes read before it was made. +func renderThisRepo(t *testing.T) (dest string, before map[string][]byte) { + t.Helper() + root := repoRootForTest(t) + r := &thisRepoRender + r.once.Do(func() { + r.before = make(map[string][]byte, len(thisRepoManifests)) + for _, rel := range thisRepoManifests { + data, err := os.ReadFile(filepath.Join(root, rel)) + if err != nil { + r.err = fmt.Errorf("read %s: %w", rel, err) + return + } + r.before[rel] = data + } + if r.dir, r.err = os.MkdirTemp("", "abcd-launch-render-"); r.err != nil { + return + } + r.dest = filepath.Join(r.dir, "payload") + // The render scans exactly once: the pre-check's scan of the bundle + // ResolveBundle(root, nil) returns, the input the scan test reads. + scans, prev := 0, scanBundle + scanBundle = func(repoRoot string, bundle Bundle) scanner.ScanResult { + scans++ + r.scan = prev(repoRoot, bundle) + return r.scan + } + defer func() { scanBundle = prev }() + _, r.err = RenderPayload(PayloadRenderRequest{ + RepoRoot: root, Dest: r.dest, Version: "9.9.9", Entry: sampleEntry(), Dirty: DirtySkip, + }) + if r.err == nil && scans != 1 { + r.err = fmt.Errorf("the render scanned its payload %d times, want once", scans) + } + }) + if r.err != nil { + t.Fatalf("render this repository's payload: %v", r.err) + } + return r.dest, r.before +} + +// scanThisRepo returns the scan that shared render's pre-check made of this +// repository's payload. +func scanThisRepo(t *testing.T) scanner.ScanResult { + t.Helper() + renderThisRepo(t) + return thisRepoRender.scan +} + +func TestMain(m *testing.M) { + code := m.Run() + if thisRepoRender.dir != "" { + _ = os.RemoveAll(thisRepoRender.dir) + } + os.Exit(code) +} From 40974489ae19c01b3942d9b76f082e5900ad9e26 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:38:49 +0100 Subject: [PATCH 5/9] test: skip the bound-only linearity test under -race TestTheHiddenHTMLReadersStayLinear asserts two CPU bounds and nothing else, and both are asserted in the plain lane only (raceEnabled), while the two readers it times run on the caller's goroutine. Under -race it built two inputs at the size cap, asserted nothing and gave the detector nothing to watch. It now skips there and runs unchanged in the plain lane, which the ubuntu leg runs on every event. Under -race, back to back: 67.5s to a skip. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- internal/core/reading/project_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/core/reading/project_test.go b/internal/core/reading/project_test.go index 72b3caeb0f..7781383dd1 100644 --- a/internal/core/reading/project_test.go +++ b/internal/core/reading/project_test.go @@ -191,7 +191,15 @@ func TestAnElementABrowserDoesNotRenderIsReadWithoutItsContent(t *testing.T) { // searched the whole remainder for `-->` once per unclosed ` From a0b4d1c9d1b5cc5351d897fee7e22142bc2e946d Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:43:02 +0100 Subject: [PATCH 9/9] test: run the interview stub from a build without the race detector The interview tests play the harness with their own test binary, re-run as claude, once or more per turn. Under -race every start of that binary pays the detector's set-up, about a second on a developer Mac, so the race lane spent most of the package's time starting stubs: 2.3s a case against 0.37s plain. Under -race stubBinary now builds the same package's test binary without the detector, once per run, in the environment the run started with, and puts that on PATH; it runs the same TestMain, so the stub behaves the same, and a build that fails leaves the instrumented binary in its place. The plain lane is unchanged. The package under -race, back to back on one machine: 180.4s to 35.6s, including the build. Refs: iss-2610150500162972 Assisted-by: Claude:claude-opus-5-5 --- internal/core/interview/main_test.go | 67 ++++++++++++++++++++++-- internal/core/interview/race_off_test.go | 10 ++++ internal/core/interview/race_on_test.go | 7 +++ 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 internal/core/interview/race_off_test.go create mode 100644 internal/core/interview/race_on_test.go diff --git a/internal/core/interview/main_test.go b/internal/core/interview/main_test.go index 9738e8bdbb..1a340dd658 100644 --- a/internal/core/interview/main_test.go +++ b/internal/core/interview/main_test.go @@ -22,10 +22,12 @@ import ( "errors" "fmt" "os" + "os/exec" "path/filepath" "slices" "strconv" "strings" + "sync" "testing" "time" ) @@ -49,7 +51,65 @@ func TestMain(m *testing.M) { if dir := os.Getenv(stubScriptEnv); dir != "" && slices.Contains(os.Args, "--print") { os.Exit(stubRunner(dir)) } - os.Exit(m.Run()) + plainStub.env = os.Environ() + plainStub.goCmd, _ = exec.LookPath("go") + code := m.Run() + if plainStub.dir != "" { + _ = os.RemoveAll(plainStub.dir) + } + os.Exit(code) +} + +// plainStub is this package's test binary built without the race detector, +// made once per run, for stubBinary under -race; TestMain removes it. It is +// built in the environment the run started with, never one a test has set +// (a sandboxed HOME would point the go command at an empty cache). +var plainStub struct { + once sync.Once + env []string + goCmd string + dir string + path string + err error +} + +// stubBinary returns the binary that plays the harness: this test binary, or +// under -race the same package's test binary built without the detector. Both +// run the same TestMain, so the stub's behaviour is the same; only its start +// is cheaper, which every turn of every stubbed interview pays. A build that +// fails leaves the instrumented binary in its place, which costs time and +// changes nothing else. +func stubBinary(t *testing.T) string { + t.Helper() + self, err := os.Executable() + if err != nil { + t.Fatal(err) + } + if !raceEnabled { + return self + } + plainStub.once.Do(func() { + if plainStub.goCmd == "" { + plainStub.err = errors.New("no go command on the PATH the run started with") + return + } + if plainStub.dir, plainStub.err = os.MkdirTemp("", "abcd-interview-stub-"); plainStub.err != nil { + return + } + out := filepath.Join(plainStub.dir, "interview.test") + cmd := exec.Command(plainStub.goCmd, "test", "-c", "-o", out, ".") + cmd.Env = plainStub.env + if b, err := cmd.CombinedOutput(); err != nil { + plainStub.err = fmt.Errorf("go test -c: %v: %s", err, b) + return + } + plainStub.path = out + }) + if plainStub.err != nil { + t.Logf("the stub runs instrumented: %v", plainStub.err) + return self + } + return plainStub.path } func stubRunner(dir string) int { @@ -132,10 +192,7 @@ func stubRunner(dir string) int { // script's directory. func stubOnPath(t *testing.T, receipts ...string) string { t.Helper() - self, err := os.Executable() - if err != nil { - t.Fatal(err) - } + self := stubBinary(t) bin, script := t.TempDir(), t.TempDir() if err := os.Symlink(self, filepath.Join(bin, "claude")); err != nil { t.Fatal(err) diff --git a/internal/core/interview/race_off_test.go b/internal/core/interview/race_off_test.go new file mode 100644 index 0000000000..c8879b4ba8 --- /dev/null +++ b/internal/core/interview/race_off_test.go @@ -0,0 +1,10 @@ +//go:build !race + +package interview + +// raceEnabled reports whether the race detector is instrumenting this build. +// stubBinary reads it: under -race the stub runner is this package's test +// binary built without the detector, since every start of an instrumented +// binary pays the detector's set-up (about a second on a developer Mac) and +// the stub is the harness's stand-in, not the code under test. +const raceEnabled = false diff --git a/internal/core/interview/race_on_test.go b/internal/core/interview/race_on_test.go new file mode 100644 index 0000000000..440b850e38 --- /dev/null +++ b/internal/core/interview/race_on_test.go @@ -0,0 +1,7 @@ +//go:build race + +package interview + +// raceEnabled reports whether the race detector is instrumenting this build. +// See the sibling file for what reads it. +const raceEnabled = true