Add unit test for parsing build files in bp2build

This involves some minor changes to testing infrastructure.

Bug: 285631638
Fixes: 286545783
Test: unit test
Change-Id: If64ba29308d99e63a1cc5526feaf077f2cb14478
diff --git a/android/fixture.go b/android/fixture.go
index dbc3bc5..03abb7f 100644
--- a/android/fixture.go
+++ b/android/fixture.go
@@ -275,6 +275,15 @@
 	})
 }
 
+// Sync the mock filesystem with the current config, then modify the context,
+// This allows context modification that requires filesystem access.
+func FixtureModifyContextWithMockFs(mutator func(ctx *TestContext)) FixturePreparer {
+	return newSimpleFixturePreparer(func(f *fixture) {
+		f.config.mockFileSystem("", f.mockFS)
+		mutator(f.ctx)
+	})
+}
+
 func FixtureRegisterWithContext(registeringFunc func(ctx RegistrationContext)) FixturePreparer {
 	return FixtureModifyContext(func(ctx *TestContext) { registeringFunc(ctx) })
 }
diff --git a/android/register.go b/android/register.go
index 64b0207..5d277e1 100644
--- a/android/register.go
+++ b/android/register.go
@@ -15,9 +15,13 @@
 package android
 
 import (
+	"bufio"
 	"fmt"
+	"path/filepath"
 	"reflect"
+	"regexp"
 
+	"android/soong/shared"
 	"github.com/google/blueprint"
 )
 
@@ -197,6 +201,49 @@
 	RegisterMutatorsForBazelConversion(ctx, bp2buildPreArchMutators)
 }
 
+func (c *Context) ParseBuildFiles(topDir string, existingBazelFiles []string) error {
+	result := map[string][]string{}
+
+	// Search for instances of `name = "$NAME"` (with arbitrary spacing).
+	targetNameRegex := regexp.MustCompile(`(?m)^\s*name\s*=\s*\"([^\"]+)\"`)
+
+	parseBuildFile := func(path string) error {
+		fullPath := shared.JoinPath(topDir, path)
+		sourceDir := filepath.Dir(path)
+
+		fileInfo, err := c.Config().fs.Stat(fullPath)
+		if err != nil {
+			return fmt.Errorf("Error accessing Bazel file '%s': %s", path, err)
+		}
+		if !fileInfo.IsDir() &&
+			(fileInfo.Name() == "BUILD" || fileInfo.Name() == "BUILD.bazel") {
+			f, err := c.Config().fs.Open(fullPath)
+			if err != nil {
+				return fmt.Errorf("Error reading Bazel file '%s': %s", path, err)
+			}
+			defer f.Close()
+			scanner := bufio.NewScanner(f)
+			for scanner.Scan() {
+				line := scanner.Text()
+				matches := targetNameRegex.FindAllStringSubmatch(line, -1)
+				for _, match := range matches {
+					result[sourceDir] = append(result[sourceDir], match[1])
+				}
+			}
+		}
+		return nil
+	}
+
+	for _, path := range existingBazelFiles {
+		err := parseBuildFile(path)
+		if err != nil {
+			return err
+		}
+	}
+	c.Config().SetBazelBuildFileTargets(result)
+	return nil
+}
+
 // RegisterForApiBazelConversion is similar to RegisterForBazelConversion except that
 // it only generates API targets in the generated  workspace
 func (ctx *Context) RegisterForApiBazelConversion() {
diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go
index e127fd5..1a35743 100644
--- a/bp2build/build_conversion_test.go
+++ b/bp2build/build_conversion_test.go
@@ -1946,3 +1946,36 @@
 	actual, _ := prettyPrintAttribute(lla, 0)
 	android.AssertStringEquals(t, "Print the common value if all keys in an axis have the same value", `[":libfoo.impl"]`, actual)
 }
+
+func TestAlreadyPresentBuildTarget(t *testing.T) {
+	bp := `
+	custom {
+		name: "foo",
+	}
+	custom {
+		name: "bar",
+	}
+	`
+	alreadyPresentBuildFile :=
+		MakeBazelTarget(
+			"custom",
+			"foo",
+			AttrNameToString{},
+		)
+	expectedBazelTargets := []string{
+		MakeBazelTarget(
+			"custom",
+			"bar",
+			AttrNameToString{},
+		),
+	}
+	registerCustomModule := func(ctx android.RegistrationContext) {
+		ctx.RegisterModuleType("custom", customModuleFactoryHostAndDevice)
+	}
+	RunBp2BuildTestCase(t, registerCustomModule, Bp2buildTestCase{
+		AlreadyExistingBuildContents: alreadyPresentBuildFile,
+		Blueprint:                    bp,
+		ExpectedBazelTargets:         expectedBazelTargets,
+		Description:                  "Not duplicating work for an already-present BUILD target.",
+	})
+}
diff --git a/bp2build/testing.go b/bp2build/testing.go
index 140b214..89ef07b 100644
--- a/bp2build/testing.go
+++ b/bp2build/testing.go
@@ -21,6 +21,7 @@
 
 import (
 	"fmt"
+	"path/filepath"
 	"sort"
 	"strings"
 	"testing"
@@ -82,7 +83,12 @@
 	// ExpectedBazelTargets compares the BazelTargets generated in `Dir` (if not empty).
 	// Otherwise, it checks the BazelTargets generated by `Blueprint` in the root directory.
 	ExpectedBazelTargets []string
-	Filesystem           map[string]string
+	// AlreadyExistingBuildContents, if non-empty, simulates an already-present source BUILD file
+	// in the directory under test. The BUILD file has the given contents. This BUILD file
+	// will also be treated as "BUILD file to keep" by the simulated bp2build environment.
+	AlreadyExistingBuildContents string
+
+	Filesystem map[string]string
 	// Dir sets the directory which will be compared against the targets in ExpectedBazelTargets.
 	// This should used in conjunction with the Filesystem property to check for targets
 	// generated from a directory that is not the root.
@@ -119,11 +125,22 @@
 
 func runBp2BuildTestCaseWithSetup(t *testing.T, extraPreparer android.FixturePreparer, tc Bp2buildTestCase) {
 	t.Helper()
-	dir := "."
+	checkDir := "."
+	if tc.Dir != "" {
+		checkDir = tc.Dir
+	}
+	keepExistingBuildDirs := tc.KeepBuildFileForDirs
+	buildFilesToParse := []string{}
 	filesystem := make(map[string][]byte)
 	for f, content := range tc.Filesystem {
 		filesystem[f] = []byte(content)
 	}
+	if len(tc.AlreadyExistingBuildContents) > 0 {
+		buildFilePath := filepath.Join(checkDir, "BUILD")
+		filesystem[buildFilePath] = []byte(tc.AlreadyExistingBuildContents)
+		keepExistingBuildDirs = append(keepExistingBuildDirs, checkDir)
+		buildFilesToParse = append(buildFilesToParse, buildFilePath)
+	}
 
 	preparers := []android.FixturePreparer{
 		extraPreparer,
@@ -132,7 +149,7 @@
 		android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) {
 			ctx.RegisterModuleType(tc.ModuleTypeUnderTest, tc.ModuleTypeUnderTestFactory)
 		}),
-		android.FixtureModifyContext(func(ctx *android.TestContext) {
+		android.FixtureModifyContextWithMockFs(func(ctx *android.TestContext) {
 			// A default configuration for tests to not have to specify bp2build_available on top level
 			// targets.
 			bp2buildConfig := android.NewBp2BuildAllowlist().SetDefaultConfig(
@@ -140,7 +157,7 @@
 					android.Bp2BuildTopLevel: allowlists.Bp2BuildDefaultTrueRecursively,
 				},
 			)
-			for _, f := range tc.KeepBuildFileForDirs {
+			for _, f := range keepExistingBuildDirs {
 				bp2buildConfig.SetKeepExistingBuildFile(map[string]bool{
 					f: /*recursive=*/ false,
 				})
@@ -150,6 +167,10 @@
 			// from cloning modules to their original state after mutators run. This
 			// would lose some data intentionally set by these mutators.
 			ctx.SkipCloneModulesAfterMutators = true
+			err := ctx.ParseBuildFiles(".", buildFilesToParse)
+			if err != nil {
+				t.Errorf("error parsing build files in test setup: %s", err)
+			}
 		}),
 		android.FixtureModifyEnv(func(env map[string]string) {
 			if tc.UnconvertedDepsMode == errorModulesUnconvertedDeps {
@@ -168,10 +189,6 @@
 		return
 	}
 
-	checkDir := dir
-	if tc.Dir != "" {
-		checkDir = tc.Dir
-	}
 	expectedTargets := map[string][]string{
 		checkDir: tc.ExpectedBazelTargets,
 	}
diff --git a/cmd/soong_build/main.go b/cmd/soong_build/main.go
index 989dd7f..6975677 100644
--- a/cmd/soong_build/main.go
+++ b/cmd/soong_build/main.go
@@ -21,7 +21,6 @@
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"time"
 
@@ -737,43 +736,6 @@
 	return excluded
 }
 
-// buildTargetsByPackage parses Bazel BUILD.bazel and BUILD files under
-// the workspace, and returns a map containing names of Bazel targets defined in
-// these BUILD files.
-// For example, maps "//foo/bar" to ["baz", "qux"] if `//foo/bar:{baz,qux}` exist.
-func buildTargetsByPackage(ctx *android.Context) map[string][]string {
-	existingBazelFiles, err := getExistingBazelRelatedFiles(topDir)
-	maybeQuit(err, "Error determining existing Bazel-related files")
-
-	result := map[string][]string{}
-
-	// Search for instances of `name = "$NAME"` (with arbitrary spacing).
-	targetNameRegex := regexp.MustCompile(`(?m)^\s*name\s*=\s*\"([^\"]+)\"`)
-
-	for _, path := range existingBazelFiles {
-		if !ctx.Config().Bp2buildPackageConfig.ShouldKeepExistingBuildFileForDir(filepath.Dir(path)) {
-			continue
-		}
-		fullPath := shared.JoinPath(topDir, path)
-		sourceDir := filepath.Dir(path)
-		fileInfo, err := os.Stat(fullPath)
-		maybeQuit(err, "Error accessing Bazel file '%s'", fullPath)
-
-		if !fileInfo.IsDir() &&
-			(fileInfo.Name() == "BUILD" || fileInfo.Name() == "BUILD.bazel") {
-			// Process this BUILD file.
-			buildFileContent, err := os.ReadFile(fullPath)
-			maybeQuit(err, "Error reading Bazel file '%s'", fullPath)
-
-			matches := targetNameRegex.FindAllStringSubmatch(string(buildFileContent), -1)
-			for _, match := range matches {
-				result[sourceDir] = append(result[sourceDir], match[1])
-			}
-		}
-	}
-	return result
-}
-
 // Run Soong in the bp2build mode. This creates a standalone context that registers
 // an alternate pipeline of mutators and singletons specifically for generating
 // Bazel BUILD files instead of Ninja files.
@@ -782,7 +744,11 @@
 	ctx.EventHandler.Do("bp2build", func() {
 
 		ctx.EventHandler.Do("read_build", func() {
-			ctx.Config().SetBazelBuildFileTargets(buildTargetsByPackage(ctx))
+			existingBazelFiles, err := getExistingBazelRelatedFiles(topDir)
+			maybeQuit(err, "Error determining existing Bazel-related files")
+
+			err = ctx.ParseBuildFiles(topDir, existingBazelFiles)
+			maybeQuit(err, "Error parsing existing Bazel-related files")
 		})
 
 		// Propagate "allow misssing dependencies" bit. This is normally set in