From cc51843e5253f270e2249edf97c9fb5534af3efc Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Wed, 30 Mar 2022 15:50:34 +0000 Subject: [PATCH] add unit test for bp2build allowlist Test: go test ./android Change-Id: I0ea1d97444cbedfd285f1fc4bd7ff246ce699dc9 --- android/bazel_test.go | 251 +++++++++++++++++++++++++++++++++++++++ bazel/properties_test.go | 16 +-- bazel/testing.go | 62 +++++----- 3 files changed, 290 insertions(+), 39 deletions(-) diff --git a/android/bazel_test.go b/android/bazel_test.go index 925cc3ea2..482df2abd 100644 --- a/android/bazel_test.go +++ b/android/bazel_test.go @@ -15,7 +15,12 @@ package android import ( "android/soong/android/allowlists" + "android/soong/bazel" + "fmt" "testing" + + "github.com/google/blueprint" + "github.com/google/blueprint/proptools" ) func TestConvertAllModulesInPackage(t *testing.T) { @@ -135,3 +140,249 @@ func TestModuleOptIn(t *testing.T) { } } } + +type TestBazelModule struct { + bazel.TestModuleInfo + BazelModuleBase +} + +var _ blueprint.Module = TestBazelModule{} + +func (m TestBazelModule) Name() string { + return m.TestModuleInfo.ModuleName +} + +func (m TestBazelModule) GenerateBuildActions(blueprint.ModuleContext) { +} + +type TestBazelConversionContext struct { + omc bazel.OtherModuleTestContext + allowlist bp2BuildConversionAllowlist + errors []string +} + +var _ bazelOtherModuleContext = &TestBazelConversionContext{} + +func (bcc *TestBazelConversionContext) OtherModuleType(m blueprint.Module) string { + return bcc.omc.OtherModuleType(m) +} + +func (bcc *TestBazelConversionContext) OtherModuleName(m blueprint.Module) string { + return bcc.omc.OtherModuleName(m) +} + +func (bcc *TestBazelConversionContext) OtherModuleDir(m blueprint.Module) string { + return bcc.omc.OtherModuleDir(m) +} + +func (bcc *TestBazelConversionContext) ModuleErrorf(format string, args ...interface{}) { + bcc.errors = append(bcc.errors, fmt.Sprintf(format, args...)) +} + +func (bcc *TestBazelConversionContext) Config() Config { + return Config{ + &config{ + bp2buildPackageConfig: bcc.allowlist, + }, + } +} + +var bazelableBazelModuleBase = BazelModuleBase{ + bazelProperties: properties{ + Bazel_module: bazelModuleProperties{ + CanConvertToBazel: true, + }, + }, +} + +func TestBp2BuildAllowlist(t *testing.T) { + testCases := []struct { + description string + shouldConvert bool + expectedErrors []string + module TestBazelModule + allowlist bp2BuildConversionAllowlist + }{ + { + description: "allowlist enables module", + shouldConvert: true, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "dir1", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + }, + }, + { + description: "module in name allowlist and type allowlist fails", + shouldConvert: false, + expectedErrors: []string{"A module cannot be in moduleAlwaysConvert and also be in moduleTypeAlwaysConvert"}, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "dir1", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + moduleTypeAlwaysConvert: map[string]bool{ + "rule1": true, + }, + }, + }, + { + description: "module in allowlist and denylist fails", + shouldConvert: false, + expectedErrors: []string{"a module cannot be in moduleDoNotConvert and also be in moduleAlwaysConvert"}, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "dir1", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + moduleDoNotConvert: map[string]bool{ + "foo": true, + }, + }, + }, + { + description: "module in allowlist and existing BUILD file", + shouldConvert: false, + expectedErrors: []string{"A module cannot be in a directory listed in keepExistingBuildFile and also be in moduleAlwaysConvert. Directory: 'existing/build/dir'"}, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "existing/build/dir", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + keepExistingBuildFile: map[string]bool{ + "existing/build/dir": true, + }, + }, + }, + { + description: "module allowlist and enabled directory", + shouldConvert: false, + expectedErrors: []string{"A module cannot be in a directory marked Bp2BuildDefaultTrue or Bp2BuildDefaultTrueRecursively and also be in moduleAlwaysConvert. Directory: 'existing/build/dir'"}, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "existing/build/dir", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + defaultConfig: allowlists.Bp2BuildConfig{ + "existing/build/dir": allowlists.Bp2BuildDefaultTrue, + }, + }, + }, + { + description: "module allowlist and enabled subdirectory", + shouldConvert: false, + expectedErrors: []string{"A module cannot be in a directory marked Bp2BuildDefaultTrue or Bp2BuildDefaultTrueRecursively and also be in moduleAlwaysConvert. Directory: 'existing/build/dir'"}, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: "existing/build/dir/subdir", + }, + BazelModuleBase: bazelableBazelModuleBase, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + defaultConfig: allowlists.Bp2BuildConfig{ + "existing/build/dir": allowlists.Bp2BuildDefaultTrueRecursively, + }, + }, + }, + { + description: "module enabled in unit test short-circuits other allowlists", + shouldConvert: true, + module: TestBazelModule{ + TestModuleInfo: bazel.TestModuleInfo{ + ModuleName: "foo", + Typ: "rule1", + Dir: ".", + }, + BazelModuleBase: BazelModuleBase{ + bazelProperties: properties{ + Bazel_module: bazelModuleProperties{ + CanConvertToBazel: true, + Bp2build_available: proptools.BoolPtr(true), + }, + }, + }, + }, + allowlist: bp2BuildConversionAllowlist{ + moduleAlwaysConvert: map[string]bool{ + "foo": true, + }, + moduleDoNotConvert: map[string]bool{ + "foo": true, + }, + }, + }, + } + + for _, test := range testCases { + t.Run(test.description, func(t *testing.T) { + bcc := &TestBazelConversionContext{ + omc: bazel.OtherModuleTestContext{ + Modules: []bazel.TestModuleInfo{ + test.module.TestModuleInfo, + }, + }, + allowlist: test.allowlist, + } + + shouldConvert := test.module.shouldConvertWithBp2build(bcc, test.module.TestModuleInfo) + if test.shouldConvert != shouldConvert { + t.Errorf("Module shouldConvert expected to be: %v, but was: %v", test.shouldConvert, shouldConvert) + } + + errorsMatch := true + if len(test.expectedErrors) != len(bcc.errors) { + errorsMatch = false + } else { + for i, err := range test.expectedErrors { + if err != bcc.errors[i] { + errorsMatch = false + } + } + } + if !errorsMatch { + t.Errorf("Expected errors to be: %v, but were: %v", test.expectedErrors, bcc.errors) + } + }) + } +} diff --git a/bazel/properties_test.go b/bazel/properties_test.go index c7f977678..7b76b747b 100644 --- a/bazel/properties_test.go +++ b/bazel/properties_test.go @@ -329,7 +329,7 @@ func labelAddSuffixForTypeMapper(suffix, typ string) LabelMapper { func TestPartitionLabelListAttribute(t *testing.T) { testCases := []struct { name string - ctx *otherModuleTestContext + ctx *OtherModuleTestContext labelList LabelListAttribute filters LabelPartitions expected PartitionToLabelListAttribute @@ -337,7 +337,7 @@ func TestPartitionLabelListAttribute(t *testing.T) { }{ { name: "no configurable values", - ctx: &otherModuleTestContext{}, + ctx: &OtherModuleTestContext{}, labelList: LabelListAttribute{ Value: makeLabelList([]string{"a.a", "b.b", "c.c", "d.d", "e.e"}, []string{}), }, @@ -354,7 +354,7 @@ func TestPartitionLabelListAttribute(t *testing.T) { }, { name: "no configurable values, remainder partition", - ctx: &otherModuleTestContext{}, + ctx: &OtherModuleTestContext{}, labelList: LabelListAttribute{ Value: makeLabelList([]string{"a.a", "b.b", "c.c", "d.d", "e.e"}, []string{}), }, @@ -371,7 +371,7 @@ func TestPartitionLabelListAttribute(t *testing.T) { }, { name: "no configurable values, empty partition", - ctx: &otherModuleTestContext{}, + ctx: &OtherModuleTestContext{}, labelList: LabelListAttribute{ Value: makeLabelList([]string{"a.a", "c.c"}, []string{}), }, @@ -387,8 +387,8 @@ func TestPartitionLabelListAttribute(t *testing.T) { }, { name: "no configurable values, has map", - ctx: &otherModuleTestContext{ - modules: []testModuleInfo{testModuleInfo{name: "srcs", typ: "fg", dir: "dir"}}, + ctx: &OtherModuleTestContext{ + Modules: []TestModuleInfo{{ModuleName: "srcs", Typ: "fg", Dir: "dir"}}, }, labelList: LabelListAttribute{ Value: makeLabelList([]string{"a.a", "srcs", "b.b", "c.c"}, []string{}), @@ -406,7 +406,7 @@ func TestPartitionLabelListAttribute(t *testing.T) { }, { name: "configurable values, keeps empty if excludes", - ctx: &otherModuleTestContext{}, + ctx: &OtherModuleTestContext{}, labelList: LabelListAttribute{ ConfigurableValues: configurableLabelLists{ ArchConfigurationAxis: labelListSelectValues{ @@ -450,7 +450,7 @@ func TestPartitionLabelListAttribute(t *testing.T) { }, { name: "error for multiple partitions same value", - ctx: &otherModuleTestContext{}, + ctx: &OtherModuleTestContext{}, labelList: LabelListAttribute{ Value: makeLabelList([]string{"a.a", "b.b", "c.c", "d.d", "e.e"}, []string{}), }, diff --git a/bazel/testing.go b/bazel/testing.go index 23c835084..9a43b61d7 100644 --- a/bazel/testing.go +++ b/bazel/testing.go @@ -20,86 +20,86 @@ import ( "github.com/google/blueprint" ) -// testModuleInfo implements blueprint.Module interface with sufficient information to mock a subset of +// TestModuleInfo implements blueprint.Module interface with sufficient information to mock a subset of // a blueprint ModuleContext -type testModuleInfo struct { - name string - typ string - dir string +type TestModuleInfo struct { + ModuleName string + Typ string + Dir string } // Name returns name for testModuleInfo -- required to implement blueprint.Module -func (mi testModuleInfo) Name() string { - return mi.name +func (mi TestModuleInfo) Name() string { + return mi.ModuleName } // GenerateBuildActions unused, but required to implmeent blueprint.Module -func (mi testModuleInfo) GenerateBuildActions(blueprint.ModuleContext) {} +func (mi TestModuleInfo) GenerateBuildActions(blueprint.ModuleContext) {} -func (mi testModuleInfo) equals(other testModuleInfo) bool { - return mi.name == other.name && mi.typ == other.typ && mi.dir == other.dir +func (mi TestModuleInfo) equals(other TestModuleInfo) bool { + return mi.ModuleName == other.ModuleName && mi.Typ == other.Typ && mi.Dir == other.Dir } // ensure testModuleInfo implements blueprint.Module -var _ blueprint.Module = testModuleInfo{} +var _ blueprint.Module = TestModuleInfo{} -// otherModuleTestContext is a mock context that implements OtherModuleContext -type otherModuleTestContext struct { - modules []testModuleInfo +// OtherModuleTestContext is a mock context that implements OtherModuleContext +type OtherModuleTestContext struct { + Modules []TestModuleInfo errors []string } // ModuleFromName retrieves the testModuleInfo corresponding to name, if it exists -func (omc *otherModuleTestContext) ModuleFromName(name string) (blueprint.Module, bool) { - for _, m := range omc.modules { - if m.name == name { +func (omc *OtherModuleTestContext) ModuleFromName(name string) (blueprint.Module, bool) { + for _, m := range omc.Modules { + if m.ModuleName == name { return m, true } } - return testModuleInfo{}, false + return TestModuleInfo{}, false } // testModuleInfo returns the testModuleInfo corresponding to a blueprint.Module if it exists in omc -func (omc *otherModuleTestContext) testModuleInfo(m blueprint.Module) (testModuleInfo, bool) { - mi, ok := m.(testModuleInfo) +func (omc *OtherModuleTestContext) testModuleInfo(m blueprint.Module) (TestModuleInfo, bool) { + mi, ok := m.(TestModuleInfo) if !ok { - return testModuleInfo{}, false + return TestModuleInfo{}, false } - for _, other := range omc.modules { + for _, other := range omc.Modules { if other.equals(mi) { return mi, true } } - return testModuleInfo{}, false + return TestModuleInfo{}, false } // OtherModuleType returns type of m if it exists in omc -func (omc *otherModuleTestContext) OtherModuleType(m blueprint.Module) string { +func (omc *OtherModuleTestContext) OtherModuleType(m blueprint.Module) string { if mi, ok := omc.testModuleInfo(m); ok { - return mi.typ + return mi.Typ } return "" } // OtherModuleName returns name of m if it exists in omc -func (omc *otherModuleTestContext) OtherModuleName(m blueprint.Module) string { +func (omc *OtherModuleTestContext) OtherModuleName(m blueprint.Module) string { if mi, ok := omc.testModuleInfo(m); ok { - return mi.name + return mi.ModuleName } return "" } // OtherModuleDir returns dir of m if it exists in omc -func (omc *otherModuleTestContext) OtherModuleDir(m blueprint.Module) string { +func (omc *OtherModuleTestContext) OtherModuleDir(m blueprint.Module) string { if mi, ok := omc.testModuleInfo(m); ok { - return mi.dir + return mi.Dir } return "" } -func (omc *otherModuleTestContext) ModuleErrorf(format string, args ...interface{}) { +func (omc *OtherModuleTestContext) ModuleErrorf(format string, args ...interface{}) { omc.errors = append(omc.errors, fmt.Sprintf(format, args...)) } // Ensure otherModuleTestContext implements OtherModuleContext -var _ OtherModuleContext = &otherModuleTestContext{} +var _ OtherModuleContext = &OtherModuleTestContext{}