Address PR feedback

Changes based on code review:

1. Remove redundant strings.Contains check in isExactTestPackage
   The equality check on the next line handles both cases, making
   the early return unnecessary.

2. Extract package selection logic into selectBestPackages function
   This reduces code duplication and allows the test to call the
   actual implementation rather than copying the logic.

3. Add TestSelectBestPackages to test the new function
   Comprehensive test covering single packages, test vs production,
   exact vs nested tests, and multiple packages.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

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) {

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"]