test: share package builds before parallel test runs

[cpunion]

Summary

• remove the per-package child llgo test fan-out introduced by test: parallelize llgo test packages #2193
• restore one multi-root build.Do call so package loading, SSA construction, compilation, and linking share the union dependency graph
• collect all native test binaries after their sequential build/link phase, then run only those binaries concurrently according to -p
• preserve buffered per-package output, package result reporting, -failfast, JSON output, and sequential handling for shared profile/fuzz output files
• canonicalize byte/uint8 and rune/int32 generic ABI symbols, removing an order-dependent mismatch exposed by multi-root loading

Package compilation and test-main linking remain sequential. This PR does not implement parallel builds; it only parallelizes execution after every requested test binary has been built.

The multi-package runtime regression test verifies that four test roots share one build of the common testing package and then report all four test results. Focused tests cover the native runner success, exit-error, and start-error paths.

Validation

• exact 96-package primary macOS shard, cold LLGo build cache, -p=4: all links completed before parallel execution; all passed in 1077s
• crypto/ed25519, unique, and weak multi-root link and runtime regression passed
• go test -race ./cmd/internal/test ./internal/goflags
• go test -race ./internal/build -run '^TestRun(NativeTest|TestPrograms)' -count=1\n- go test ./cmd/internal/...\n- go test ./internal/build -count=1\n- go test ./ssa -count=1\n- focused ssa/cl tpnamed golden tests\n- go vet ./cmd/internal/test ./internal/build ./ssa/abi\n- go run ./cmd/llgo test -p=2 ./cl/_testgo/runextest/bar ./cl/_testgo/runextest/foo\n

[fennoai]

Review: share builds before parallel runs

Reworks llgo test to build all native test binaries once inside build.Do and then run them concurrently (runNativeTestPrograms/runTestPrograms), replacing the old fork-a-child-llgo-test-per-package model. The rewrite is clean: the worker pool, buffered result channel (sized to parallelism), fail-fast/skip accounting, and edge-case guards (empty slice, parallelism==0 → GOMAXPROCS, <1 → 1, clamp to len) all check out, and the removed LLGO_TEST_PARALLEL_WORKER / go list / child-arg machinery leaves no stragglers (verified by grep). TestRunSequential is wired only for the process-wide-output flags plus -fuzz, matching the prior guard, and the new unit tests cover concurrency limiting, fail-fast, and JSON output.

A couple of inline nits on comment wording. One non-blocking design note:

• Serial build, then parallel run. Compilation and linking in Do remain sequential; only the run phase is now parallelized. The win is deduping shared-dep builds (e.g. testing built once), but for compile-dominated suites the now-serial build phase becomes the bottleneck versus the old fully-concurrent per-package pipeline. Worth a benchmark on a multi-package suite; overlapping run-as-linked could recover throughput if the build phase dominates. Not a blocker.

No correctness, security, or regression issues found in the concurrency mechanics.

[fennoai]

The comment says these flags "name process-wide output files," but flags.TestFuzz (added on line 90) is not an output-file flag — fuzzing forces sequential execution for a different reason (a fuzz run drives a single target and can't be sharded across concurrently-running binaries). Consider adjusting the wording, e.g. "These flags name process-wide output files, or (for -fuzz) require a single active test binary."

[cpunion]

Addressed in 08bc69d: the comment now distinguishes shared output paths from flags such as -fuzz that require one active test binary.

[fennoai]

Same wording gap as testRunsMustBeSequential in cmd/internal/test/test.go: TestRunSequential is also set for -fuzz (see test.go:90), which does not "write to a shared process-wide output file." The stated rationale doesn't cover the fuzz trigger that actually sets this field.

[cpunion]

Addressed in 08bc69d: the comment now distinguishes shared output paths from flags such as -fuzz that require one active test binary.

[codecov]

Codecov Report

❌ Patch coverage is 94.38202% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/build/run.go	95.71%	3 Missing ⚠️
internal/build/build.go	85.71%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[cpunion]

The first CI run exposed an existing order-dependent generic ABI symbol issue after this PR began loading all roots together. go/types may retain either the alias spelling (byte/rune) or the underlying spelling (uint8/int32) for a cached generic instance, while LLGo previously included that spelling directly in the symbol name. This produced a weak.Pointer[byte].Value reference with a weak.Pointer[uint8].Value definition.

I pushed c2b9cda0a to canonicalize those basic aliases in generic ABI symbols, plus focused ABI/golden tests. I also added direct success/exit/start-error coverage for the native test runner in c0654d7de.

Local validation includes the exact 96-package primary macOS shard with a cold LLGo build cache (-p=4): all packages loaded/built together, all 96 links completed before test execution began, then tests ran concurrently; every package passed in 1077s. The previously failing crypto/ed25519 + unique + weak combination also passes both linking and execution. Race tests, focused ABI/golden tests, and vet pass. I will wait for both CI platforms and aggregated Codecov before evaluating the final result.

[cpunion]

All required checks are green on ca23d8043, including the final dual-platform Codecov aggregation (94.38% patch coverage). The exact 96-package cold-cache local run also passed, with every link completed before concurrent execution.

Latest Actions run llgo test timing versus #2193:

• Ubuntu primary: 41:40 -> 46:17 total; critical path 22:10 -> 26:19
• Ubuntu compatibility: 20:33 -> 20:07 total; critical path 11:24 -> 13:05
• macOS primary shard 0: 25:05 -> 34:07

The timings confirm the intended shared-build behavior but not a wall-time improvement for compile-heavy shards: build/link remain sequential and no longer overlap test execution. Parallel build work remains outside this PR.

@fennoai please review the final changes, including the generic basic-alias ABI canonicalization and added runner coverage.

[fennoai]

Review: shared build before parallel test runs

I checked out fennoai/pr-2234 (head ca23d8043) and reviewed the five PR commits (2ec8470e..ca23d804), which touch exactly the nine listed files. The test/goroot/* and setup-embed-deps entries visible in a raw main...HEAD diff come from already-merged PRs #2218/#2215 and are not part of this change.

Verdict: looks good. The refactor is well-structured, the ABI canonicalization is correct and minimal, and the new tests give strong coverage of the scheduling edges. No blocking issues found. A few observations below, all non-blocking.

Architecture — restoring the single multi-root build.Do

Collapsing the per-package llgo test fan-out (#2193) back into one build.Do call is the right call: package loading, SSA construction, and linking now share one union dependency graph, so common packages like testing are built once. build.go:601-704 cleanly separates the sequential build/link phase (appending to testPrograms) from a single post-build runNativeTestPrograms call. The CompileOnly, Target != "", profile/fuzz, and -failfast paths are all preserved. build_test.go's TestExtest now asserts testing is built exactly once via a ModuleHook counter — a good regression guard for the core claim.

Scheduler (run.go:94-148) — correct and race-clean

• Parallelism normalization (0 → GOMAXPROCS, clamp <1 → 1, clamp to len(programs)) is right.
• The prime-then-refill loop preserves the concurrency bound; TestRunTestProgramsLimitAndFailure verifies max concurrency never exceeds the cap under -race.
• -failfast stops launching new binaries once a failure is seen but lets in-flight binaries finish (they can't be cancelled), and skipped = len(programs) - next counts only the never-started ones. TestRunTestProgramsFailFast confirms skipped == 2 and only one run after the first failure.
• Per-package output is buffered into a bytes.Buffer and flushed whole in reportTestProgramResult, so JSON lines never interleave mid-record; TestRunTestProgramsJSONOutput confirms no plain-text ok line leaks into a -json run.

Behavior change worth noting (non-blocking)

In runNativeTestPrograms the run closure passes the same buffer as both stdout and stderr (return runNativeTest(program, conf, output, output)). Two consequences vs. the old streamed single-package path:

• Test-binary stderr is now folded into stdout (the buffered package output), rather than going to the real os.Stderr. The FAIL\t<pkg> / ok \t<pkg> summary markers still route to the real streams via reportTestProgramResult, so this matches how go test merges per-package output — just calling it out since it's a visible difference from test: parallelize llgo test packages #2193's streaming.
• With -x (PrintCommands), the command-trace line is written into that same per-package buffer instead of live stderr. This actually keeps the trace grouped with its package, which reads as an improvement.

Output is no longer streamed live during a package's run (it appears when the binary completes). That's the documented and intended trade-off of buffering for clean concurrent output.

ABI canonicalization (abi.go:245-257) — correct fix at the right layer

Canonicalizing byte→uint8 and rune→int32 inside typeArgString's *types.Basic case is the right chokepoint: it's the single mangling path for generic type arguments, so both spellings now produce identical ABI symbols regardless of which go/types cached the instance first. This directly removes the order-dependent mismatch that multi-root loading exposes. TestTypeArgs_CanonicalizesBasicAliases asserts the two spellings mangle identically, and the updated tpnamed/in.go golden ([0]byte → [0]uint8) plus FormatsCommonKinds (byte→uint8) confirm the observable output. Verified go test ./ssa/abi -run TestTypeArgs -count=1 passes locally and go vet ./ssa/abi is clean.

Minor: byte/rune are the only predeclared basic aliases in Go, so the two-case switch is complete; no other basic-kind spellings need the same treatment.

Verification

• Ran locally: go test ./ssa/abi -run TestTypeArgs -count=1 (pass), go vet ./ssa/abi (clean).
• Could not build internal/build or cmd/internal/test in this sandbox — they cgo-link against LLVM (llvm-c/Core.h headers unavailable here). The runner and scheduler tests, the -race runs, and the full-shard cold-cache validation reported in the PR description and your latest comment therefore rest on CI, which you note is green on ca23d8043 (94.38% patch coverage, all links completing before parallel execution).

On the timings

The reported per-shard regressions (e.g. macOS primary shard 0: 25:05 → 34:07) are consistent with the design: build/link are sequential and no longer overlap test execution, so compile-heavy shards lose the previous build/run overlap. The PR is explicit that it only parallelizes execution and defers parallel builds — nothing here contradicts that, and correctness (shared build, all results reported) is what this change is responsible for. Whether the wall-time cost on compile-heavy shards is acceptable ahead of the parallel-build follow-up is a maintainer call, not a code issue.