From a42d6417b3cdef35b6c2f9e1aa9f6e8ab51c1499 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Tue, 26 Jan 2021 21:57:27 -0500 Subject: [PATCH] Make bp2buildMutators registration local to TestContext. The previous implementation relied on the implicit registration of Bp2Build mutators, resulting in test non-hermeticity. Refactor bp2build tests to explicitly specify the bp2build mutators under test. Test: Soong tests Test: TH Change-Id: I9b9674bad1ea533b3bd31b07077a9e02c99b4c1d --- android/filegroup.go | 4 +- android/testing.go | 4 +- bp2build/build_conversion_test.go | 61 ++++++++++++++++++------------- genrule/genrule.go | 4 +- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/android/filegroup.go b/android/filegroup.go index fd4a2fe49..3d1bbc533 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -23,7 +23,7 @@ import ( func init() { RegisterModuleType("filegroup", FileGroupFactory) - RegisterBp2BuildMutator("filegroup", bp2buildMutator) + RegisterBp2BuildMutator("filegroup", FilegroupBp2Build) } // https://docs.bazel.build/versions/master/be/general.html#filegroup @@ -51,7 +51,7 @@ func (bfg *bazelFilegroup) Name() string { func (bfg *bazelFilegroup) GenerateAndroidBuildActions(ctx ModuleContext) {} // TODO: Create helper functions to avoid this boilerplate. -func bp2buildMutator(ctx TopDownMutatorContext) { +func FilegroupBp2Build(ctx TopDownMutatorContext) { if m, ok := ctx.Module().(*fileGroup); ok { name := "__bp2build__" + m.base().BaseModuleName() ctx.CreateModule(BazelFileGroupFactory, &bazelFilegroupAttributes{ diff --git a/android/testing.go b/android/testing.go index 76172d18e..5640c2981 100644 --- a/android/testing.go +++ b/android/testing.go @@ -89,7 +89,7 @@ func (ctx *TestContext) RegisterBp2BuildMutator(moduleType string, m func(TopDow f := func(ctx RegisterMutatorsContext) { ctx.TopDown(mutatorName, m) } - bp2buildMutators = append(bp2buildMutators, f) + ctx.bp2buildMutators = append(ctx.bp2buildMutators, f) } func (ctx *TestContext) Register() { @@ -100,7 +100,7 @@ func (ctx *TestContext) Register() { // RegisterForBazelConversion prepares a test context for bp2build conversion. func (ctx *TestContext) RegisterForBazelConversion() { - RegisterMutatorsForBazelConversion(ctx.Context.Context, bp2buildMutators) + RegisterMutatorsForBazelConversion(ctx.Context.Context, ctx.bp2buildMutators) } func (ctx *TestContext) ParseFileList(rootDir string, filePaths []string) (deps []string, errs []error) { diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index 367e13f75..73ca92fa1 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -270,16 +270,18 @@ func TestGenerateBazelTargetModules(t *testing.T) { func TestModuleTypeBp2Build(t *testing.T) { testCases := []struct { - moduleTypeUnderTest string - moduleTypeUnderTestFactory android.ModuleFactory - bp string - expectedBazelTarget string - description string + moduleTypeUnderTest string + moduleTypeUnderTestFactory android.ModuleFactory + moduleTypeUnderTestBp2BuildMutator func(android.TopDownMutatorContext) + bp string + expectedBazelTarget string + description string }{ { - description: "filegroup with no srcs", - moduleTypeUnderTest: "filegroup", - moduleTypeUnderTestFactory: android.FileGroupFactory, + description: "filegroup with no srcs", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, bp: `filegroup { name: "foo", srcs: [], @@ -291,9 +293,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "filegroup with srcs", - moduleTypeUnderTest: "filegroup", - moduleTypeUnderTestFactory: android.FileGroupFactory, + description: "filegroup with srcs", + moduleTypeUnderTest: "filegroup", + moduleTypeUnderTestFactory: android.FileGroupFactory, + moduleTypeUnderTestBp2BuildMutator: android.FilegroupBp2Build, bp: `filegroup { name: "foo", srcs: ["a", "b"], @@ -307,9 +310,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "genrule with command line variable replacements", - moduleTypeUnderTest: "genrule", - moduleTypeUnderTestFactory: genrule.GenRuleFactory, + description: "genrule with command line variable replacements", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build, bp: `genrule { name: "foo", out: ["foo.out"], @@ -332,9 +336,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "genrule using $(locations :label)", - moduleTypeUnderTest: "genrule", - moduleTypeUnderTestFactory: genrule.GenRuleFactory, + description: "genrule using $(locations :label)", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build, bp: `genrule { name: "foo", out: ["foo.out"], @@ -357,9 +362,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "genrule using $(location) label should substitute first tool label automatically", - moduleTypeUnderTest: "genrule", - moduleTypeUnderTestFactory: genrule.GenRuleFactory, + description: "genrule using $(location) label should substitute first tool label automatically", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build, bp: `genrule { name: "foo", out: ["foo.out"], @@ -383,9 +389,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "genrule using $(locations) label should substitute first tool label automatically", - moduleTypeUnderTest: "genrule", - moduleTypeUnderTestFactory: genrule.GenRuleFactory, + description: "genrule using $(locations) label should substitute first tool label automatically", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build, bp: `genrule { name: "foo", out: ["foo.out"], @@ -409,9 +416,10 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { - description: "genrule without tools or tool_files can convert successfully", - moduleTypeUnderTest: "genrule", - moduleTypeUnderTestFactory: genrule.GenRuleFactory, + description: "genrule without tools or tool_files can convert successfully", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + moduleTypeUnderTestBp2BuildMutator: genrule.GenruleBp2Build, bp: `genrule { name: "foo", out: ["foo.out"], @@ -436,6 +444,7 @@ func TestModuleTypeBp2Build(t *testing.T) { config := android.TestConfig(buildDir, nil, testCase.bp, nil) ctx := android.NewTestContext(config) ctx.RegisterModuleType(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestFactory) + ctx.RegisterBp2BuildMutator(testCase.moduleTypeUnderTest, testCase.moduleTypeUnderTestBp2BuildMutator) ctx.RegisterForBazelConversion() _, errs := ctx.ParseFileList(dir, []string{"Android.bp"}) diff --git a/genrule/genrule.go b/genrule/genrule.go index 66e5652f7..ddfb4596b 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -47,7 +47,7 @@ func RegisterGenruleBuildComponents(ctx android.RegistrationContext) { ctx.BottomUp("genrule_tool_deps", toolDepsMutator).Parallel() }) - android.RegisterBp2BuildMutator("genrule", bp2buildMutator) + android.RegisterBp2BuildMutator("genrule", GenruleBp2Build) } var ( @@ -794,7 +794,7 @@ func BazelGenruleFactory() android.Module { return module } -func bp2buildMutator(ctx android.TopDownMutatorContext) { +func GenruleBp2Build(ctx android.TopDownMutatorContext) { if m, ok := ctx.Module().(*Module); ok { name := "__bp2build__" + m.Name() // Bazel only has the "tools" attribute.