diff --git a/java/app.go b/java/app.go index 2abc45107..7b2577538 100755 --- a/java/app.go +++ b/java/app.go @@ -1337,6 +1337,8 @@ func (a *AndroidTest) GenerateAndroidBuildActions(ctx android.ModuleContext) { OutputFile: a.OutputFile(), TestConfig: a.testConfig, HostRequiredModuleNames: a.HostRequiredModuleNames(), + TestSuites: a.testProperties.Test_suites, + IsHost: false, }) } diff --git a/java/java.go b/java/java.go index 72536cd6d..c5bdf9dcd 100644 --- a/java/java.go +++ b/java/java.go @@ -1438,6 +1438,14 @@ func (j *TestHost) GenerateAndroidBuildActions(ctx android.ModuleContext) { j.Test.generateAndroidBuildActionsWithConfig(ctx, configs) android.SetProvider(ctx, testing.TestModuleProviderKey, testing.TestModuleProviderData{}) + android.SetProvider(ctx, tradefed.BaseTestProviderKey, tradefed.BaseTestProviderData{ + InstalledFiles: j.data, + OutputFile: j.outputFile, + TestConfig: j.testConfig, + RequiredModuleNames: j.RequiredModuleNames(), + TestSuites: j.testProperties.Test_suites, + IsHost: true, + }) } func (j *Test) GenerateAndroidBuildActions(ctx android.ModuleContext) { diff --git a/tradefed/providers.go b/tradefed/providers.go index f41e09eb6..66cb6253b 100644 --- a/tradefed/providers.go +++ b/tradefed/providers.go @@ -16,6 +16,11 @@ type BaseTestProviderData struct { TestConfig android.Path // Other modules we require to be installed to run tests. We expect base to build them. HostRequiredModuleNames []string + RequiredModuleNames []string + // List of test suites base uses. + TestSuites []string + // Used for bases that are Host + IsHost bool } var BaseTestProviderKey = blueprint.NewProvider[BaseTestProviderData]() diff --git a/tradefed_modules/test_module_config.go b/tradefed_modules/test_module_config.go index ba6ab9400..686753713 100644 --- a/tradefed_modules/test_module_config.go +++ b/tradefed_modules/test_module_config.go @@ -17,6 +17,7 @@ func init() { // Register the license_kind module type. func RegisterTestModuleConfigBuildComponents(ctx android.RegistrationContext) { ctx.RegisterModuleType("test_module_config", TestModuleConfigFactory) + ctx.RegisterModuleType("test_module_config_host", TestModuleConfigHostFactory) } type testModuleConfigModule struct { @@ -32,6 +33,12 @@ type testModuleConfigModule struct { provider tradefed.BaseTestProviderData } +// Host is mostly the same as non-host, just some diffs for AddDependency and +// AndroidMkEntries, but the properties are the same. +type testModuleConfigHostModule struct { + testModuleConfigModule +} + // Properties to list in Android.bp for this module. type tradefedProperties struct { // Module name of the base test that we will run. @@ -68,8 +75,9 @@ type dependencyTag struct { } var ( - testModuleConfigTag = dependencyTag{name: "TestModuleConfigBase"} - pctx = android.NewPackageContext("android/soong/tradefed_modules") + testModuleConfigTag = dependencyTag{name: "TestModuleConfigBase"} + testModuleConfigHostTag = dependencyTag{name: "TestModuleConfigHostBase"} + pctx = android.NewPackageContext("android/soong/tradefed_modules") ) func (m *testModuleConfigModule) InstallInTestcases() bool { @@ -77,6 +85,10 @@ func (m *testModuleConfigModule) InstallInTestcases() bool { } func (m *testModuleConfigModule) DepsMutator(ctx android.BottomUpMutatorContext) { + if m.Base == nil { + ctx.ModuleErrorf("'base' field must be set to a 'android_test' module.") + return + } ctx.AddDependency(ctx.Module(), testModuleConfigTag, *m.Base) } @@ -143,35 +155,41 @@ func (m *testModuleConfigModule) composeOptions() []tradefed.Option { // // If we change to symlinks, this all needs to change. func (m *testModuleConfigModule) GenerateAndroidBuildActions(ctx android.ModuleContext) { + m.validateBase(ctx, &testModuleConfigTag, "android_test", false) + m.generateManifestAndConfig(ctx) - ctx.VisitDirectDepsWithTag(testModuleConfigTag, func(dep android.Module) { - if provider, ok := android.OtherModuleProvider(ctx, dep, tradefed.BaseTestProviderKey); ok { - m.base = dep - m.provider = provider - } else { - ctx.ModuleErrorf("The base module '%s' does not provide test BaseTestProviderData. Only 'android_test' modules are supported.", dep.Name()) - return +} + +// Any test suites in base should not be repeated in the derived class, except "general-tests". +// We may restrict derived tests to only be "general-tests" as it doesn't make sense to add a slice +// of a test to compatibility suite. +// +// Returns ErrorMessage, false on problems +// Returns _, true if okay. +func (m *testModuleConfigModule) validateTestSuites(ctx android.ModuleContext) bool { + if len(m.tradefedProperties.Test_suites) == 0 { + ctx.ModuleErrorf("At least one test-suite must be set or this won't run. Use \"general-tests\"") + return false + } + + derivedSuites := make(map[string]bool) + // See if any suites in base is also in derived (other than general-tests) + for _, s := range m.tradefedProperties.Test_suites { + if s != "general-tests" { + derivedSuites[s] = true } - }) + } + if len(derivedSuites) == 0 { + return true + } + for _, baseSuite := range m.provider.TestSuites { + if derivedSuites[baseSuite] { + ctx.ModuleErrorf("TestSuite %s exists in the base, do not add it here", baseSuite) + return false + } + } - // 1) A manifest file listing the base. - installDir := android.PathForModuleInstall(ctx, ctx.ModuleName()) - out := android.PathForModuleOut(ctx, "test_module_config.manifest") - android.WriteFileRule(ctx, out, fmt.Sprintf("{%q: %q}", "base", *m.tradefedProperties.Base)) - ctx.InstallFile(installDir, out.Base(), out) - - // 2) Module.config / AndroidTest.xml - // Note, there is still a "test-tag" element with base's module name, but - // Tradefed team says its ignored anyway. - m.testConfig = m.fixTestConfig(ctx, m.provider.TestConfig) - - // 3) Write ARCH/Module.apk in testcases. - // Handled by soong_app_prebuilt and OutputFile in entries. - // Nothing to do here. - - // 4) Copy base's data files. - // Handled by soong_app_prebuilt and LOCAL_COMPATIBILITY_SUPPORT_FILES. - // Nothing to do here. + return true } func TestModuleConfigFactory() android.Module { @@ -184,6 +202,16 @@ func TestModuleConfigFactory() android.Module { return module } +func TestModuleConfigHostFactory() android.Module { + module := &testModuleConfigHostModule{} + + module.AddProperties(&module.tradefedProperties) + android.InitAndroidMultiTargetsArchModule(module, android.HostSupported, android.MultilibCommon) + android.InitDefaultableModule(module) + + return module +} + // Implements android.AndroidMkEntriesProvider var _ android.AndroidMkEntriesProvider = (*testModuleConfigModule)(nil) @@ -198,22 +226,96 @@ func (m *testModuleConfigModule) AndroidMkEntries() []android.AndroidMkEntries { // Out update config file with extra options. entries.SetPath("LOCAL_FULL_TEST_CONFIG", m.testConfig) entries.SetString("LOCAL_MODULE_TAGS", "tests") - // Required for atest to run additional tradefed testtypes - entries.AddStrings("LOCAL_HOST_REQUIRED_MODULES", m.provider.HostRequiredModuleNames...) - - // Clear the JNI symbols because they belong to base not us. Either transform the names in the string - // or clear the variable because we don't need it, we are copying bases libraries not generating - // new ones. - entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", "") // Don't append to base's test-suites, only use the ones we define, so clear it before // appending to it. entries.SetString("LOCAL_COMPATIBILITY_SUITE", "") - if len(m.tradefedProperties.Test_suites) > 0 { - entries.AddCompatibilityTestSuites(m.tradefedProperties.Test_suites...) - } else { - entries.AddCompatibilityTestSuites("null-suite") + entries.AddCompatibilityTestSuites(m.tradefedProperties.Test_suites...) + + if len(m.provider.HostRequiredModuleNames) > 0 { + entries.AddStrings("LOCAL_HOST_REQUIRED_MODULES", m.provider.HostRequiredModuleNames...) + } + if len(m.provider.RequiredModuleNames) > 0 { + entries.AddStrings("LOCAL_REQUIRED_MODULES", m.provider.RequiredModuleNames...) + } + + if m.provider.IsHost == false { + // Not needed for jar_host_test + // + // Clear the JNI symbols because they belong to base not us. Either transform the names in the string + // or clear the variable because we don't need it, we are copying bases libraries not generating + // new ones. + entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", "") } }) return entriesList } + +func (m *testModuleConfigHostModule) DepsMutator(ctx android.BottomUpMutatorContext) { + if m.Base == nil { + ctx.ModuleErrorf("'base' field must be set to a 'java_test_host' module") + return + } + ctx.AddVariationDependencies(ctx.Config().BuildOSCommonTarget.Variations(), testModuleConfigHostTag, *m.Base) +} + +// File to write: +// 1) out/host/linux-x86/testcases/derived-module/test_module_config.manifest # contains base's name. +// 2) out/host/linux-x86/testcases/derived-module/derived-module.config # Update AnroidTest.xml +// 3) out/host/linux-x86/testcases/derived-module/base.jar +// - written via soong_java_prebuilt.mk +// +// 4) out/host/linux-x86/testcases/derived-module/* # data dependencies from base. +// - written via soong_java_prebuilt.mk +func (m *testModuleConfigHostModule) GenerateAndroidBuildActions(ctx android.ModuleContext) { + m.validateBase(ctx, &testModuleConfigHostTag, "java_test_host", true) + m.generateManifestAndConfig(ctx) +} + +// Ensure the base listed is the right type by checking that we get the expected provider data. +// Returns false on errors and the context is updated with an error indicating the baseType expected. +func (m *testModuleConfigModule) validateBase(ctx android.ModuleContext, depTag *dependencyTag, baseType string, baseShouldBeHost bool) { + ctx.VisitDirectDepsWithTag(*depTag, func(dep android.Module) { + if provider, ok := android.OtherModuleProvider(ctx, dep, tradefed.BaseTestProviderKey); ok { + if baseShouldBeHost == provider.IsHost { + m.base = dep + m.provider = provider + } else { + if baseShouldBeHost { + ctx.ModuleErrorf("'android_test' module used as base, but 'java_test_host' expected.") + } else { + ctx.ModuleErrorf("'java_test_host' module used as base, but 'android_test' expected.") + } + } + } else { + ctx.ModuleErrorf("'%s' module used as base but it is not a '%s' module.", *m.Base, baseType) + } + }) +} + +// Actions to write: +// 1. manifest file to testcases dir +// 2. New Module.config / AndroidTest.xml file with our options. +func (m *testModuleConfigModule) generateManifestAndConfig(ctx android.ModuleContext) { + if !m.validateTestSuites(ctx) { + return + } + // Ensure the provider is accurate + if m.provider.TestConfig == nil { + return + } + + // 1) A manifest file listing the base, write text to a tiny file. + installDir := android.PathForModuleInstall(ctx, ctx.ModuleName()) + manifest := android.PathForModuleOut(ctx, "test_module_config.manifest") + android.WriteFileRule(ctx, manifest, fmt.Sprintf("{%q: %q}", "base", *m.tradefedProperties.Base)) + // build/soong/android/androidmk.go has this comment: + // Assume the primary install file is last + // so we need to Install our file last. + ctx.InstallFile(installDir, manifest.Base(), manifest) + + // 2) Module.config / AndroidTest.xml + m.testConfig = m.fixTestConfig(ctx, m.provider.TestConfig) +} + +var _ android.AndroidMkEntriesProvider = (*testModuleConfigHostModule)(nil) diff --git a/tradefed_modules/test_module_config_test.go b/tradefed_modules/test_module_config_test.go index ff5304373..41dd3d479 100644 --- a/tradefed_modules/test_module_config_test.go +++ b/tradefed_modules/test_module_config_test.go @@ -16,6 +16,7 @@ package tradefed_modules import ( "android/soong/android" "android/soong/java" + "strconv" "strings" "testing" ) @@ -43,6 +44,7 @@ const bp = ` base: "base", exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], } ` @@ -92,7 +94,7 @@ func TestModuleConfigOptions(t *testing.T) { } // Ensure we error for a base we don't support. -func TestModuleConfigBadBaseShouldFail(t *testing.T) { +func TestModuleConfigWithHostBaseShouldFailWithExplicitMessage(t *testing.T) { badBp := ` java_test_host { name: "base", @@ -104,16 +106,60 @@ func TestModuleConfigBadBaseShouldFail(t *testing.T) { base: "base", exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], }` - ctx := android.GroupFixturePreparers( + android.GroupFixturePreparers( java.PrepareForTestWithJavaDefaultModules, android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), ).ExtendWithErrorHandler( - android.FixtureExpectsAtLeastOneErrorMatchingPattern("does not provide test BaseTestProviderData")). + android.FixtureExpectsAtLeastOneErrorMatchingPattern("'java_test_host' module used as base, but 'android_test' expected")). RunTestWithBp(t, badBp) +} - ctx.ModuleForTests("derived_test", "android_common") +func TestModuleConfigBadBaseShouldFailWithGeneralMessage(t *testing.T) { + badBp := ` + java_library { + name: "base", + srcs: ["a.java"], + } + + test_module_config { + name: "derived_test", + base: "base", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], + }` + + android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).ExtendWithErrorHandler( + android.FixtureExpectsOneErrorPattern("'base' module used as base but it is not a 'android_test' module.")). + RunTestWithBp(t, badBp) +} + +func TestModuleConfigNoBaseShouldFail(t *testing.T) { + badBp := ` + java_library { + name: "base", + srcs: ["a.java"], + } + + test_module_config { + name: "derived_test", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], + }` + + android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).ExtendWithErrorHandler( + android.FixtureExpectsOneErrorPattern("'base' field must be set to a 'android_test' module.")). + RunTestWithBp(t, badBp) } // Ensure we error for a base we don't support. @@ -128,6 +174,7 @@ func TestModuleConfigNoFiltersOrAnnotationsShouldFail(t *testing.T) { test_module_config { name: "derived_test", base: "base", + test_suites: ["general-tests"], }` ctx := android.GroupFixturePreparers( @@ -152,12 +199,14 @@ func TestModuleConfigMultipleDerivedTestsWriteDistinctMakeEntries(t *testing.T) name: "derived_test", base: "base", include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], } test_module_config { name: "another_derived_test", base: "base", include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], }` ctx := android.GroupFixturePreparers( @@ -190,6 +239,114 @@ func TestModuleConfigMultipleDerivedTestsWriteDistinctMakeEntries(t *testing.T) } } +// Test_module_config_host rule is allowed to depend on java_test_host +func TestModuleConfigHostBasics(t *testing.T) { + bp := ` + java_test_host { + name: "base", + srcs: ["a.java"], + test_suites: ["suiteA", "general-tests", "suiteB"], + } + + test_module_config_host { + name: "derived_test", + base: "base", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests"], + }` + + ctx := android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).RunTestWithBp(t, bp) + + variant := ctx.Config.BuildOS.String() + "_common" + derived := ctx.ModuleForTests("derived_test", variant) + mod := derived.Module().(*testModuleConfigHostModule) + allEntries := android.AndroidMkEntriesForTest(t, ctx.TestContext, mod) + entries := allEntries[0] + android.AssertArrayString(t, "", entries.EntryMap["LOCAL_MODULE"], []string{"derived_test"}) + + if !mod.Host() { + t.Errorf("host bit is not set for a java_test_host module.") + } + actualData, _ := strconv.ParseBool(entries.EntryMap["LOCAL_IS_UNIT_TEST"][0]) + android.AssertBoolEquals(t, "LOCAL_IS_UNIT_TEST", true, actualData) + +} + +// When you pass an 'android_test' as base, the warning message is a bit obscure, +// talking about variants, but it is something. Ideally we could do better. +func TestModuleConfigHostBadBaseShouldFailWithVariantWarning(t *testing.T) { + badBp := ` + android_test { + name: "base", + sdk_version: "current", + srcs: ["a.java"], + } + + test_module_config_host { + name: "derived_test", + base: "base", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + }` + + android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).ExtendWithErrorHandler( + android.FixtureExpectsAtLeastOneErrorMatchingPattern("missing variant")). + RunTestWithBp(t, badBp) +} + +func TestModuleConfigHostNeedsATestSuite(t *testing.T) { + badBp := ` + java_test_host { + name: "base", + srcs: ["a.java"], + } + + test_module_config_host { + name: "derived_test", + base: "base", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + }` + + android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).ExtendWithErrorHandler( + android.FixtureExpectsAtLeastOneErrorMatchingPattern("At least one test-suite must be set")). + RunTestWithBp(t, badBp) +} + +func TestModuleConfigHostDuplicateTestSuitesGiveErrors(t *testing.T) { + badBp := ` + java_test_host { + name: "base", + srcs: ["a.java"], + test_suites: ["general-tests", "some-compat"], + } + + test_module_config_host { + name: "derived_test", + base: "base", + exclude_filters: ["android.test.example.devcodelab.DevCodelabTest#testHelloFail"], + include_annotations: ["android.platform.test.annotations.LargeTest"], + test_suites: ["general-tests", "some-compat"], + }` + + android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + android.FixtureRegisterWithContext(RegisterTestModuleConfigBuildComponents), + ).ExtendWithErrorHandler( + android.FixtureExpectsAtLeastOneErrorMatchingPattern("TestSuite some-compat exists in the base")). + RunTestWithBp(t, badBp) +} + // Use for situations where the entries map contains pairs: [srcPath:installedPath1, srcPath2:installedPath2] // and we want to compare the RHS of the pairs, i.e. installedPath1, installedPath2 func assertEntryPairValues(t *testing.T, actual []string, expected []string) {