diff --git a/go/extractor/extractor.go b/go/extractor/extractor.go index c9ac7c6088a..158f0029704 100644 --- a/go/extractor/extractor.go +++ b/go/extractor/extractor.go @@ -65,9 +65,6 @@ func init() { func isExactTestPackage(pkg *packages.Package) bool { // Test packages have IDs in the format: "pkgpath [pkgpath.test]" // or for nested test dependencies: "pkgpath [pkgpath/nested.test]" - if !strings.Contains(pkg.ID, " [") { - return false - } expectedTestID := pkg.PkgPath + " [" + pkg.PkgPath + ".test]" return pkg.ID == expectedTestID } @@ -97,6 +94,28 @@ func isBetterPackage(pkg, current *packages.Package) bool { return len(pkg.ID) > len(current.ID) } +// selectBestPackages builds a map from package paths to their best package variants. +// In the context of a `go test -c` compilation, we see the same package more than +// once, with IDs like "abc.com/pkgname [abc.com/pkgname.test]" to distinguish the version +// that contains and is used by test code. +// We prefer the version with the most complete test coverage, which is typically: +// 1. The exact test package (e.g., "pkg [pkg.test]") over nested test dependencies +// 2. The package with the most Syntax nodes (most files to extract) +// 3. The longest ID string as a tiebreaker +func selectBestPackages(pkgs []*packages.Package) map[string]*packages.Package { + bestPackageIds := make(map[string]*packages.Package) + packages.Visit(pkgs, nil, func(pkg *packages.Package) { + if bestSoFar, present := bestPackageIds[pkg.PkgPath]; present { + if isBetterPackage(pkg, bestSoFar) { + bestPackageIds[pkg.PkgPath] = pkg + } + } else { + bestPackageIds[pkg.PkgPath] = pkg + } + }) + return bestPackageIds +} + // ExtractWithFlags extracts the packages specified by the given patterns and build flags func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool, sourceRoot string) error { startTime := time.Now() @@ -191,24 +210,8 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool, pkgsNotFound := make([]string, 0, len(pkgs)) - // Build a map from package paths to their best IDs-- - // in the context of a `go test -c` compilation, we will see the same package more than - // once, with IDs like "abc.com/pkgname [abc.com/pkgname.test]" to distinguish the version - // that contains and is used by test code. - // We prefer the version with the most complete test coverage, which is typically: - // 1. The exact test package (e.g., "pkg [pkg.test]") over nested test dependencies - // 2. The package with the most Syntax nodes (most files to extract) - // 3. The longest ID string as a tiebreaker - bestPackageIds := make(map[string]*packages.Package) - packages.Visit(pkgs, nil, func(pkg *packages.Package) { - if bestSoFar, present := bestPackageIds[pkg.PkgPath]; present { - if isBetterPackage(pkg, bestSoFar) { - bestPackageIds[pkg.PkgPath] = pkg - } - } else { - bestPackageIds[pkg.PkgPath] = pkg - } - }) + // Build a map from package paths to their best IDs + bestPackageIds := selectBestPackages(pkgs) // Do a post-order traversal and extract the package scope of each package packages.Visit(pkgs, nil, func(pkg *packages.Package) { diff --git a/go/extractor/extractor_test.go b/go/extractor/extractor_test.go index 54f9dac1c12..2b585ec7fa1 100644 --- a/go/extractor/extractor_test.go +++ b/go/extractor/extractor_test.go @@ -190,6 +190,103 @@ func TestIsBetterPackage(t *testing.T) { } } +// TestSelectBestPackages tests the selectBestPackages function +func TestSelectBestPackages(t *testing.T) { + // Helper to create a package with specified properties + makePkg := func(id, path string, syntaxCount int) *packages.Package { + syntax := make([]*ast.File, syntaxCount) + return &packages.Package{ + ID: id, + PkgPath: path, + Syntax: syntax, + } + } + + tests := []struct { + name string + pkgs []*packages.Package + expectedPkgIDs map[string]string // pkgPath -> expected selected ID + }{ + { + name: "single package", + pkgs: []*packages.Package{ + makePkg("example.com/pkg", "example.com/pkg", 10), + }, + expectedPkgIDs: map[string]string{ + "example.com/pkg": "example.com/pkg", + }, + }, + { + name: "test package preferred over production", + pkgs: []*packages.Package{ + makePkg("example.com/pkg", "example.com/pkg", 10), + makePkg("example.com/pkg [example.com/pkg.test]", "example.com/pkg", 15), + }, + expectedPkgIDs: map[string]string{ + "example.com/pkg": "example.com/pkg [example.com/pkg.test]", + }, + }, + { + name: "exact test preferred over nested test", + pkgs: []*packages.Package{ + makePkg("example.com/pkg [example.com/pkg.test]", "example.com/pkg", 20), + makePkg("example.com/pkg [example.com/pkg/nested.test]", "example.com/pkg", 15), + }, + expectedPkgIDs: map[string]string{ + "example.com/pkg": "example.com/pkg [example.com/pkg.test]", + }, + }, + { + name: "multiple packages with different paths", + pkgs: []*packages.Package{ + makePkg("example.com/pkg1", "example.com/pkg1", 10), + makePkg("example.com/pkg1 [example.com/pkg1.test]", "example.com/pkg1", 15), + makePkg("example.com/pkg2", "example.com/pkg2", 8), + makePkg("example.com/pkg2 [example.com/pkg2.test]", "example.com/pkg2", 12), + }, + expectedPkgIDs: map[string]string{ + "example.com/pkg1": "example.com/pkg1 [example.com/pkg1.test]", + "example.com/pkg2": "example.com/pkg2 [example.com/pkg2.test]", + }, + }, + { + name: "more syntax nodes wins among nested tests", + pkgs: []*packages.Package{ + makePkg("example.com/pkg [example.com/pkg/a.test]", "example.com/pkg", 10), + makePkg("example.com/pkg [example.com/pkg/b.test]", "example.com/pkg", 20), + }, + expectedPkgIDs: map[string]string{ + "example.com/pkg": "example.com/pkg [example.com/pkg/b.test]", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := selectBestPackages(tt.pkgs) + + // Check that all expected packages are present + for pkgPath, expectedID := range tt.expectedPkgIDs { + selected, found := result[pkgPath] + if !found { + t.Errorf("Expected package path %q not found in result", pkgPath) + continue + } + if selected.ID != expectedID { + t.Errorf("For package path %q: got ID %q, want %q", + pkgPath, selected.ID, expectedID) + } + } + + // Check that no unexpected packages are present + if len(result) != len(tt.expectedPkgIDs) { + t.Errorf("Expected %d packages in result, got %d", + len(tt.expectedPkgIDs), len(result)) + } + }) + } +} + // TestPackageSelectionRealWorld simulates the real-world go-git scenario func TestPackageSelectionRealWorld(t *testing.T) { // Simulate the actual packages.Load result for go-git repository @@ -221,17 +318,8 @@ func TestPackageSelectionRealWorld(t *testing.T) { }, } - // Simulate the bestPackageIds selection logic - bestPackageIds := make(map[string]*packages.Package) - for _, pkg := range pkgs { - if bestSoFar, present := bestPackageIds[pkg.PkgPath]; present { - if isBetterPackage(pkg, bestSoFar) { - bestPackageIds[pkg.PkgPath] = pkg - } - } else { - bestPackageIds[pkg.PkgPath] = pkg - } - } + // Use the actual selection logic from the extractor + bestPackageIds := selectBestPackages(pkgs) // Verify the correct package was selected selected := bestPackageIds["github.com/go-git/go-git/v6"]