Skip to content

plugin: filter testLocalPackages to packages reachable from build-closure test imports#126

Merged
aldoborrero merged 1 commit intomainfrom
aldo/fix-testlocals-reachability
Apr 16, 2026
Merged

plugin: filter testLocalPackages to packages reachable from build-closure test imports#126
aldoborrero merged 1 commit intomainfrom
aldo/fix-testlocals-reachability

Conversation

@aldoborrero
Copy link
Copy Markdown
Member

Summary

#113 (ad06490) changed the -test pass to use ./... <sibling>/... patterns instead of one argv per build-closure local — bounding argv length and making every local a pattern match so cmd/go resolves TestEmbedFiles for test-only-locals (load/test.go:132). Both wins preserved.

But <sibling>/... enumerates every package in sibling modules, not only those reachable from the building module's tests. An unreachable sibling package may import third-party modules outside the building module's go.sum; the classify step silently drops those imports (in neither third_party_paths nor test_only_paths), so the package lands in testLocalPackages with an incomplete import set, gets a compile drv, and fails:

could not import X (open : no such file or directory)

This bites any project where a sibling replace target has packages with disjoint third-party deps from the building module.

Fix: capture {Test,XTest}Imports from each bare local entry (added to -json= and GoPackage), then BFS from build-closure locals' test imports through raw_test_locals' Imports + {Test,XTest}Imports. Filter raw_test_locals to the reachable set before classification. The fixpoint over reached test-only-locals' own test imports preserves the testrunner contract (everything in localArchives is tested).

Subtractive only — testLocalPackages shrinks, no schema change.

Test plan

  • Regression tests/fixtures/sibling-testonly/sib/unreachable/ imports rsc.io/quote (not in any go.sum). Without fix: could not import rsc.io/quote → exit 100. With fix: filtered out, fixture builds, testLocalPackages = ["example.com/sib/testutil"] only.
  • Unit test_parse_reachability_drops_unreachable_sibling; three pre-existing tests updated to seed via TestImports (they relied on the over-scoped behaviour)
  • test-golden-vs-gobuild all IDENTICAL; canary regenerated (plugin source change)
  • cargo test (56), cargo clippy -D warnings, go test ./..., go vet ./..., nix flake check

…sure test imports

#113 (ad06490) changed run_go_list_test to pass `./... <sibling>/...`
patterns instead of one argv per build-closure local. That bounds argv
length and makes every local a pattern match (so cmd/go resolves
TestEmbedFiles for test-only-locals — load/test.go:132). Both wins
preserved.

But the `<sibling>/...` patterns enumerate every package in sibling
modules, not just those reachable from the building module's tests. An
unreachable sibling package may import third-party modules outside the
building module's go.sum; the classify step silently drops those imports
(they're in neither third_party_paths nor test_only_paths), so the package
lands in testLocalPackages with an incomplete import set, gets a compile
drv, and fails at build time:
  could not import X (open : no such file or directory)

Fix: capture {Test,XTest}Imports from each bare local entry, then BFS from
build-closure locals' test imports through raw_test_locals' Imports +
{Test,XTest}Imports. Filter raw_test_locals to the reachable set before
classification. The fixpoint over reached test-only-locals' own test
imports preserves the existing testrunner contract (everything in
localArchives is tested).

Subtractive only — testLocalPackages shrinks, no schema change.

Regression: tests/fixtures/sibling-testonly/sib/unreachable/ imports
rsc.io/quote (not in any go.sum). Without the fix, its compile drv fails;
with the fix, it's filtered out and the fixture builds.
test_parse_reachability_drops_unreachable_sibling covers the unit level.

:house: Remote-Dev: homespace
@aldoborrero aldoborrero enabled auto-merge April 16, 2026 11:35
@aldoborrero aldoborrero added this pull request to the merge queue Apr 16, 2026
Merged via the queue into main with commit 1839db2 Apr 16, 2026
3 checks passed
@aldoborrero aldoborrero deleted the aldo/fix-testlocals-reachability branch April 16, 2026 11:38
@github-actions
Copy link
Copy Markdown

Benchmark Regression Check

Scenario Tool Base (s) Current (s) Change Drvs (base) Drvs (curr) Status
no_change nix-ca-nocgo 0.71 0.71 +0.9% 0 0 ok
no_change nix-nocgo 0.70 0.71 +1.2% 0 0 ok
leaf-private nix-ca-nocgo 7.00 1.90 -72.9% 2 2 ok
leaf-private nix-nocgo 1.52 1.45 -4.6% 2 2 ok
mid-private nix-ca-nocgo 2.13 1.88 -11.4% 2 2 ok
mid-private nix-nocgo 1.63 1.52 -7.1% 4 4 ok
deep-private nix-ca-nocgo 2.34 2.05 -12.3% 3 3 ok
deep-private nix-nocgo 1.84 1.70 -7.8% 8 8 ok

Baseline: main | Current: 967be422bdf7e9c98bb25ceb123d0080d2bf46cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant