From f8e3d8341aec34c42a600ad5c9b13e0f41215f53 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 23 Mar 2021 14:47:18 +0000 Subject: [PATCH 1/3] Use more inclusive language in dexpreopt/testing.go Bug: 177892522 Test: m nothing Change-Id: Idbb37485a573ddd25c4da48ab88f9c559fca5434 --- dexpreopt/testing.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dexpreopt/testing.go b/dexpreopt/testing.go index 8e90295b1..8d0fb622d 100644 --- a/dexpreopt/testing.go +++ b/dexpreopt/testing.go @@ -18,29 +18,29 @@ import ( "android/soong/android" ) -type dummyToolBinary struct { +type fakeToolBinary struct { android.ModuleBase } -func (m *dummyToolBinary) GenerateAndroidBuildActions(ctx android.ModuleContext) {} +func (m *fakeToolBinary) GenerateAndroidBuildActions(ctx android.ModuleContext) {} -func (m *dummyToolBinary) HostToolPath() android.OptionalPath { +func (m *fakeToolBinary) HostToolPath() android.OptionalPath { return android.OptionalPathForPath(android.PathForTesting("dex2oat")) } -func dummyToolBinaryFactory() android.Module { - module := &dummyToolBinary{} +func fakeToolBinaryFactory() android.Module { + module := &fakeToolBinary{} android.InitAndroidArchModule(module, android.HostSupported, android.MultilibFirst) return module } func RegisterToolModulesForTest(ctx android.RegistrationContext) { - ctx.RegisterModuleType("dummy_tool_binary", dummyToolBinaryFactory) + ctx.RegisterModuleType("fake_tool_binary", fakeToolBinaryFactory) } func BpToolModulesForTest() string { return ` - dummy_tool_binary { + fake_tool_binary { name: "dex2oatd", } ` From 3c84eaaa984805936e71d678ffd67032beefb2c8 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 23 Mar 2021 15:23:33 +0000 Subject: [PATCH 2/3] Separate methods used for fixture based and legacy tests The fixture mechanism makes it easy to refactor by splitting up an existing preparer into separate ones and then combining them back together. Unfortunately, that becomes slightly more tricky when preparers and legacy tests use the same functions to register build components and define default modules. This change splits the RegisterRequiredBuildComponentsForTest and GatherRequiredDepsForTest methods into two methods each, with the existing method used for legacy tests and calling the new method that is used for the preparer. At the moment all the functionality is in the new methods but over time, as functionality is extracted into separate preparers, the functionality can also be moved from the method that is common to both legacy and fixture based tests into the legacy only method. Bug: 177892522 Test: m nothing Change-Id: I233a4fe1fb072a00292acc2bb20821ec82a9bd67 --- java/testing.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/java/testing.go b/java/testing.go index 82a2103fd..d5be5b8db 100644 --- a/java/testing.go +++ b/java/testing.go @@ -43,7 +43,7 @@ var PrepareForTestWithJavaBuildComponents = android.GroupFixturePreparers( // Make sure that mutators and module types, e.g. prebuilt mutators available. android.PrepareForTestWithAndroidBuildComponents, // Make java build components available to the test. - android.FixtureRegisterWithContext(RegisterRequiredBuildComponentsForTest), + android.FixtureRegisterWithContext(registerRequiredBuildComponentsForTest), android.FixtureRegisterWithContext(registerJavaPluginBuildComponents), ) @@ -52,7 +52,7 @@ var PrepareForTestWithJavaDefaultModules = android.GroupFixturePreparers( // Make sure that all the module types used in the defaults are registered. PrepareForTestWithJavaBuildComponents, // The java default module definitions. - android.FixtureAddTextFile(defaultJavaDir+"/Android.bp", GatherRequiredDepsForTest()), + android.FixtureAddTextFile(defaultJavaDir+"/Android.bp", gatherRequiredDepsForTest()), ) var PrepareForTestWithOverlayBuildComponents = android.FixtureRegisterWithContext(registerOverlayBuildComponents) @@ -178,7 +178,19 @@ func prebuiltApisFilesForLibs(apiLevels []string, sdkLibs []string) map[string][ // // In particular this must register all the components that are used in the `Android.bp` snippet // returned by GatherRequiredDepsForTest() +// +// deprecated: Use test fixtures instead, e.g. PrepareForTestWithJavaBuildComponents func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { + registerRequiredBuildComponentsForTest(ctx) +} + +// registerRequiredBuildComponentsForTest registers the build components used by +// PrepareForTestWithJavaDefaultModules. +// +// As functionality is moved out of here into separate FixturePreparer instances they should also +// be moved into GatherRequiredDepsForTest for use by tests that have not yet switched to use test +// fixtures. +func registerRequiredBuildComponentsForTest(ctx android.RegistrationContext) { RegisterAARBuildComponents(ctx) RegisterAppBuildComponents(ctx) RegisterAppImportBuildComponents(ctx) @@ -201,7 +213,19 @@ func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { // Gather the module definitions needed by tests that depend upon code from this package. // // Returns an `Android.bp` snippet that defines the modules that are needed by this package. +// +// deprecated: Use test fixtures instead, e.g. PrepareForTestWithJavaDefaultModules func GatherRequiredDepsForTest() string { + return gatherRequiredDepsForTest() +} + +// gatherRequiredDepsForTest gathers the module definitions used by +// PrepareForTestWithJavaDefaultModules. +// +// As functionality is moved out of here into separate FixturePreparer instances they should also +// be moved into GatherRequiredDepsForTest for use by tests that have not yet switched to use test +// fixtures. +func gatherRequiredDepsForTest() string { var bp string extraModules := []string{ From 9fc9f53423d59adc9c7f249fcbb6d0f3a44952bd Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 23 Mar 2021 15:41:11 +0000 Subject: [PATCH 3/3] Group all the preparations needed for testing dexpreopt Make it easier to test dexpreopt functionality by grouping all the fixture preparations together. Bug: 177892522 Test: m nothing Change-Id: I94f66e3ec82efc4fd791f4fdab678d298565e452 --- dexpreopt/testing.go | 43 +++++++++++++++++++++++++++++++++++++++-- java/java_test.go | 4 ++-- java/testing.go | 46 ++++++++++++++++++++------------------------ 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/dexpreopt/testing.go b/dexpreopt/testing.go index 8d0fb622d..48014824b 100644 --- a/dexpreopt/testing.go +++ b/dexpreopt/testing.go @@ -15,6 +15,8 @@ package dexpreopt import ( + "fmt" + "android/soong/android" ) @@ -46,8 +48,45 @@ func BpToolModulesForTest() string { ` } -// Prepares a test fixture by enabling dexpreopt. -var PrepareForTestWithDexpreopt = FixtureModifyGlobalConfig(func(*GlobalConfig) {}) +func CompatLibDefinitionsForTest() string { + bp := "" + + // For class loader context and tests. + dexpreoptModules := []string{"android.test.runner"} + dexpreoptModules = append(dexpreoptModules, CompatUsesLibs...) + dexpreoptModules = append(dexpreoptModules, OptionalCompatUsesLibs...) + + for _, extra := range dexpreoptModules { + bp += fmt.Sprintf(` + java_library { + name: "%s", + srcs: ["a.java"], + sdk_version: "none", + system_modules: "stable-core-platform-api-stubs-system-modules", + compile_dex: true, + installable: true, + } + `, extra) + } + + return bp +} + +var PrepareForTestWithDexpreoptCompatLibs = android.GroupFixturePreparers( + android.FixtureAddFile("defaults/dexpreopt/compat/a.java", nil), + android.FixtureAddTextFile("defaults/dexpreopt/compat/Android.bp", CompatLibDefinitionsForTest()), +) + +var PrepareForTestWithFakeDex2oatd = android.GroupFixturePreparers( + android.FixtureRegisterWithContext(RegisterToolModulesForTest), + android.FixtureAddTextFile("defaults/dexpreopt/Android.bp", BpToolModulesForTest()), +) + +// Prepares a test fixture by enabling dexpreopt, registering the fake_tool_binary module type and +// using that to define the `dex2oatd` module. +var PrepareForTestByEnablingDexpreopt = android.GroupFixturePreparers( + FixtureModifyGlobalConfig(func(*GlobalConfig) {}), +) // FixtureModifyGlobalConfig enables dexpreopt (unless modified by the mutator) and modifies the // configuration. diff --git a/java/java_test.go b/java/java_test.go index b6f639f98..31eeb6b05 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -53,7 +53,7 @@ var prepareForJavaTest = android.GroupFixturePreparers( android.FixtureRegisterWithContext(func(ctx android.RegistrationContext) { ctx.RegisterPreSingletonType("sdk_versions", sdkPreSingletonFactory) }), - dexpreopt.PrepareForTestWithDexpreopt, + PrepareForTestWithDexpreopt, ) func TestMain(m *testing.M) { @@ -68,7 +68,7 @@ func TestMain(m *testing.M) { func testJavaError(t *testing.T, pattern string, bp string) (*android.TestContext, android.Config) { t.Helper() result := android.GroupFixturePreparers( - prepareForJavaTest, dexpreopt.PrepareForTestWithDexpreopt). + prepareForJavaTest, dexpreopt.PrepareForTestByEnablingDexpreopt). ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(pattern)). RunTestWithBp(t, bp) return result.TestContext, result.Config diff --git a/java/testing.go b/java/testing.go index d5be5b8db..295b8d0a7 100644 --- a/java/testing.go +++ b/java/testing.go @@ -53,6 +53,15 @@ var PrepareForTestWithJavaDefaultModules = android.GroupFixturePreparers( PrepareForTestWithJavaBuildComponents, // The java default module definitions. android.FixtureAddTextFile(defaultJavaDir+"/Android.bp", gatherRequiredDepsForTest()), + // Add dexpreopt compat libs (android.test.base, etc.) and a fake dex2oatd module. + dexpreopt.PrepareForTestWithDexpreoptCompatLibs, + dexpreopt.PrepareForTestWithFakeDex2oatd, +) + +// Provides everything needed by dexpreopt. +var PrepareForTestWithDexpreopt = android.GroupFixturePreparers( + PrepareForTestWithJavaDefaultModules, + dexpreopt.PrepareForTestByEnablingDexpreopt, ) var PrepareForTestWithOverlayBuildComponents = android.FixtureRegisterWithContext(registerOverlayBuildComponents) @@ -182,6 +191,9 @@ func prebuiltApisFilesForLibs(apiLevels []string, sdkLibs []string) map[string][ // deprecated: Use test fixtures instead, e.g. PrepareForTestWithJavaBuildComponents func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { registerRequiredBuildComponentsForTest(ctx) + + // Make sure that any tool related module types needed by dexpreopt have been registered. + dexpreopt.RegisterToolModulesForTest(ctx) } // registerRequiredBuildComponentsForTest registers the build components used by @@ -205,9 +217,6 @@ func registerRequiredBuildComponentsForTest(ctx android.RegistrationContext) { RegisterSdkLibraryBuildComponents(ctx) RegisterStubsBuildComponents(ctx) RegisterSystemModulesBuildComponents(ctx) - - // Make sure that any tool related module types needed by dexpreopt have been registered. - dexpreopt.RegisterToolModulesForTest(ctx) } // Gather the module definitions needed by tests that depend upon code from this package. @@ -216,7 +225,15 @@ func registerRequiredBuildComponentsForTest(ctx android.RegistrationContext) { // // deprecated: Use test fixtures instead, e.g. PrepareForTestWithJavaDefaultModules func GatherRequiredDepsForTest() string { - return gatherRequiredDepsForTest() + bp := gatherRequiredDepsForTest() + + // For class loader context and tests. + bp += dexpreopt.CompatLibDefinitionsForTest() + + // Make sure that any tools needed for dexpreopting are defined. + bp += dexpreopt.BpToolModulesForTest() + + return bp } // gatherRequiredDepsForTest gathers the module definitions used by @@ -257,24 +274,6 @@ func gatherRequiredDepsForTest() string { `, extra) } - // For class loader context and tests. - dexpreoptModules := []string{"android.test.runner"} - dexpreoptModules = append(dexpreoptModules, dexpreopt.CompatUsesLibs...) - dexpreoptModules = append(dexpreoptModules, dexpreopt.OptionalCompatUsesLibs...) - - for _, extra := range dexpreoptModules { - bp += fmt.Sprintf(` - java_library { - name: "%s", - srcs: ["a.java"], - sdk_version: "none", - system_modules: "stable-core-platform-api-stubs-system-modules", - compile_dex: true, - installable: true, - } - `, extra) - } - bp += ` java_library { name: "framework", @@ -311,9 +310,6 @@ func gatherRequiredDepsForTest() string { `, extra) } - // Make sure that any tools needed for dexpreopting are defined. - bp += dexpreopt.BpToolModulesForTest() - // Make sure that the dex_bootjars singleton module is instantiated for the tests. bp += ` dex_bootjars {