plugin: filter testLocalPackages to packages reachable from build-closure test imports#126
Merged
aldoborrero merged 1 commit intomainfrom Apr 16, 2026
Merged
Conversation
…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
Benchmark Regression Check
Baseline: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#113 (
ad06490) changed the-testpass 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 resolvesTestEmbedFilesfor 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'sgo.sum; the classify step silently drops those imports (in neitherthird_party_pathsnortest_only_paths), so the package lands intestLocalPackageswith an incomplete import set, gets a compile drv, and fails:This bites any project where a sibling replace target has packages with disjoint third-party deps from the building module.
Fix: capture
{Test,XTest}Importsfrom each bare local entry (added to-json=andGoPackage), then BFS from build-closure locals' test imports throughraw_test_locals'Imports+{Test,XTest}Imports. Filterraw_test_localsto the reachable set before classification. The fixpoint over reached test-only-locals' own test imports preserves the testrunner contract (everything inlocalArchivesis tested).Subtractive only —
testLocalPackagesshrinks, no schema change.Test plan
tests/fixtures/sibling-testonly/sib/unreachable/importsrsc.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.test_parse_reachability_drops_unreachable_sibling; three pre-existing tests updated to seed viaTestImports(they relied on the over-scoped behaviour)test-golden-vs-gobuildall IDENTICAL; canary regenerated (plugin source change)cargo test(56),cargo clippy -D warnings,go test ./...,go vet ./...,nix flake check