Go: tag test-only deps with EnvTesting so --include-unused-deps works#1714
Draft
zlav wants to merge 2 commits into
Draft
Go: tag test-only deps with EnvTesting so --include-unused-deps works#1714zlav wants to merge 2 commits into
zlav wants to merge 2 commits into
Conversation
Test dependencies reached via TestImports of a main-module package are now traversed and labeled EnvTesting. They remain filtered from uploads by default via shouldPublishDep, but can now be surfaced with --include-unused-deps. Previously they were dropped from the graph entirely and --include-unused-deps had no effect for Go projects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Overview
The
--include-unused-depsflag is documented as opting in to "all deps found, instead of filtering non-production deps." On Go projects today, this flag has no effect — test dependencies are dropped at the graph-building stage inStrategy.Go.GoListPackagesand never carry any environment label, so there is nothing left to filter back in.This PR changes
buildGraphto also walk theTestImportsof each main-module package, tagging those packages (and their transitivepackageDeps) withEnvTesting. ExistingshouldPublishDepfiltering inSrclib.Converteralready stripsEnvTesting-only deps from the upload, so default behavior is preserved;--include-unused-depsnow lets these deps through, matching the flag's documented behavior on other ecosystems.Production traversal still runs first, so any module reachable from both production and test code keeps its
EnvProductionlabel (and full prod subtree) and ends up published.Acceptance criteria
fossa analyzeon a Go project produces the same upload as before — test-only deps are excluded by default.fossa analyze --include-unused-depson a Go project now includes test-only deps in the upload.Testing plan
Automated:
cabal test unit-tests --test-options='--match="Graphing deps with go list -json -deps all"'— the existingtestPackagesfixture already declared atestModtest dep that was being silently dropped. The expected graph now asserts it appears as a directEnvTestingdependency, and that the transitive test-of-a-test (transitiveTestMod) is still excluded (we don't recurse into non-main packages'TestImports).Manual sanity check a reviewer can run:
cdinto any Go project that has a non-trivial test-only dependency (e.g. anything pulling ingithub.com/stretchr/testifyonly from_test.go).fossa analyze --outputand confirm test deps are NOT in the source unit.fossa analyze --include-unused-deps --outputand confirm test deps ARE now present with the testing environment.Risks
EnvTestinglabel appended (becauselabelruns outside theunless existsAlreadyguard), but the test-subtree below it is not re-walked. Reviewers should sanity-check that this is OK: in practice, if the production traversal already added the vertex, every transitive dep below it has already been added with the right labels too, so re-walking would produce no new edges.TestImportsof non-main packages. Go does not pull a dependency's test imports into your test binary, so they shouldn't appear in your bill of materials. Worth a second pair of eyes on that judgement call.GoListPackagesis the modern, primary path; the older tactics already had their own (different) handling of test scope and are out of scope here.Metrics
Not easily tracked from the CLI side. The right place to notice an impact is the FOSSA UI surfacing the additional test-environment deps on uploads that pass
--include-unused-deps.References
--include-unused-depson a Go project does not surface test deps.bb5e4d23("ANE-891 exclude test deps").Checklist
docs/. — Behavior of--include-unused-depsis documented in the existing subcommands reference; the flag's documented intent already matches the new behavior, so no docs change is required.docs/README.msand gave consideration to how discoverable or not my documentation is. — N/AChangelog.md. If this PR did not mark a release, I added my changes into an## Unreleasedsection at the top. — Added under## 3.17.6..fossa.ymlorfossa-deps.{json.yml}, I updateddocs/references/files/*.schema.jsonAND I have updated example files used byfossa initcommand. — N/Adocs/references/subcommands/<subcommand>.md. — N/A; no flag surface change.