From cdc66f4268d5f185fc39208352de51965ed7aa53 Mon Sep 17 00:00:00 2001 From: Ronald Braunstein Date: Fri, 12 Apr 2024 11:23:19 -0700 Subject: [PATCH] Add "test-only" flag for java modules As part of aosp/3022586 where we added the idea of "test-only" modules and top_level_test_targets, this CL implements that for java modules. We let users set "test-only" on java_library, but not on other modules where the module kind is implicitly test-only, like java_test. The implementation, not the user decides it is test-only. We also exclude it from java_defaults. % gqui from "flatten(~/aosp-main-with-phones/out/soong/ownership/all_teams.pb, teams)" proto team.proto:AllTeams 'select teams.kind, count(*) where teams.test_only = true and teams.kind not like "%cc_%" group by teams.kind' +--------------------------+----------+ | teams.kind | count(*) | +--------------------------+----------+ | android_test | 1382 | | android_test_helper_app | 1680 | | java_fuzz | 5 | | java_test | 774 | | java_test_helper_library | 29 | +--------------------------+----------+ % gqui from "flatten(~/aosp-main-with-phones/out/soong/ownership/all_teams.pb, teams)" proto team.proto:AllTeams 'select teams.kind, count(*) where teams.top_level_target = true and teams.kind not like "%cc_%" group by teams.kind' +--------------+----------+ | teams.kind | count(*) | +--------------+----------+ | android_test | 1382 | | java_fuzz | 5 | | java_test | 774 | +--------------+----------+ Test: m nothing --no-skip-soong-tests Test: go test ./java Test: m all_teams Bug: b/327280661 Change-Id: I9c3ad947dc3d68d6427abada27449526d69daa6b --- java/aar.go | 3 +- java/app.go | 18 ++++++- java/app_test.go | 38 ++++++++++++++ java/base.go | 1 + java/fuzz.go | 2 + java/java.go | 13 ++++- java/java_test.go | 124 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 195 insertions(+), 4 deletions(-) diff --git a/java/aar.go b/java/aar.go index fef0d8c58..edf04f6d5 100644 --- a/java/aar.go +++ b/java/aar.go @@ -922,7 +922,8 @@ func AndroidLibraryFactory() android.Module { module.Module.addHostAndDeviceProperties() module.AddProperties( &module.aaptProperties, - &module.androidLibraryProperties) + &module.androidLibraryProperties, + &module.sourceProperties) module.androidLibraryProperties.BuildAAR = true module.Module.linter.library = true diff --git a/java/app.go b/java/app.go index 7b2577538..1aa3afe8e 100644 --- a/java/app.go +++ b/java/app.go @@ -329,6 +329,10 @@ func (a *AndroidTestHelperApp) GenerateAndroidBuildActions(ctx android.ModuleCon a.aapt.manifestValues.applicationId = *applicationId } a.generateAndroidBuildActions(ctx) + android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ + TestOnly: true, + }) + } func (a *AndroidApp) GenerateAndroidBuildActions(ctx android.ModuleContext) { @@ -1191,7 +1195,8 @@ func AndroidAppFactory() android.Module { module.AddProperties( &module.aaptProperties, &module.appProperties, - &module.overridableAppProperties) + &module.overridableAppProperties, + &module.Library.sourceProperties) module.usesLibrary.enforce = true @@ -1340,6 +1345,11 @@ func (a *AndroidTest) GenerateAndroidBuildActions(ctx android.ModuleContext) { TestSuites: a.testProperties.Test_suites, IsHost: false, }) + android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ + TestOnly: true, + TopLevelTarget: true, + }) + } func (a *AndroidTest) FixTestConfig(ctx android.ModuleContext, testConfig android.Path) android.Path { @@ -1532,9 +1542,13 @@ type OverrideAndroidTest struct { android.OverrideModuleBase } -func (i *OverrideAndroidTest) GenerateAndroidBuildActions(_ android.ModuleContext) { +func (i *OverrideAndroidTest) GenerateAndroidBuildActions(ctx android.ModuleContext) { // All the overrides happen in the base module. // TODO(jungjw): Check the base module type. + android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ + TestOnly: true, + TopLevelTarget: true, + }) } // override_android_test is used to create an android_app module based on another android_test by overriding diff --git a/java/app_test.go b/java/app_test.go index 8262777b2..0c2800041 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -4432,6 +4432,44 @@ func TestNoDexpreoptOptionalUsesLibDoesNotHaveImpl(t *testing.T) { android.AssertBoolEquals(t, "dexpreopt should be disabled if optional_uses_libs does not have an implementation", true, dexpreopt == nil) } +func TestTestOnlyApp(t *testing.T) { + t.Parallel() + ctx := android.GroupFixturePreparers( + prepareForJavaTest, + ).RunTestWithBp(t, ` + // These should be test-only + android_test { + name: "android-test", + } + android_test_helper_app { + name: "helper-app", + } + override_android_test { + name: "override-test", + base: "android-app", + } + // And these should not be + android_app { + name: "android-app", + srcs: ["b.java"], + sdk_version: "current", + } + `) + + expectedTestOnly := []string{ + "android-test", + "helper-app", + "override-test", + } + + expectedTopLevel := []string{ + "android-test", + "override-test", + } + + assertTestOnlyAndTopLevel(t, ctx, expectedTestOnly, expectedTopLevel) +} + func TestAppStem(t *testing.T) { ctx := testApp(t, ` android_app { diff --git a/java/base.go b/java/base.go index 391ba8355..ef61f1cc2 100644 --- a/java/base.go +++ b/java/base.go @@ -435,6 +435,7 @@ type Module struct { deviceProperties DeviceProperties overridableProperties OverridableProperties + sourceProperties android.SourceProperties // jar file containing header classes including static library dependencies, suitable for // inserting into the bootclasspath/classpath of another compile diff --git a/java/fuzz.go b/java/fuzz.go index dc4c6bec5..fb31ce794 100644 --- a/java/fuzz.go +++ b/java/fuzz.go @@ -64,6 +64,8 @@ func JavaFuzzFactory() android.Module { module.Module.properties.Installable = proptools.BoolPtr(true) module.Module.dexpreopter.isTest = true module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true) + module.Module.sourceProperties.Test_only = proptools.BoolPtr(true) + module.Module.sourceProperties.Top_level_test_target = true android.AddLoadHook(module, func(ctx android.LoadHookContext) { disableLinuxBionic := struct { diff --git a/java/java.go b/java/java.go index 778c9a2d9..ca99e6991 100644 --- a/java/java.go +++ b/java/java.go @@ -958,6 +958,11 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { } j.installFile = ctx.InstallFile(installDir, j.Stem()+".jar", j.outputFile, extraInstallDeps...) } + + android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{ + TestOnly: Bool(j.sourceProperties.Test_only), + TopLevelTarget: j.sourceProperties.Top_level_test_target, + }) } func (j *Library) DepsMutator(ctx android.BottomUpMutatorContext) { @@ -1123,6 +1128,7 @@ func LibraryFactory() android.Module { module := &Library{} module.addHostAndDeviceProperties() + module.AddProperties(&module.sourceProperties) module.initModuleAndImport(module) @@ -1604,6 +1610,8 @@ func TestFactory() android.Module { module.Module.properties.Installable = proptools.BoolPtr(true) module.Module.dexpreopter.isTest = true module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true) + module.Module.sourceProperties.Test_only = proptools.BoolPtr(true) + module.Module.sourceProperties.Top_level_test_target = true InitJavaModule(module, android.HostAndDeviceSupported) return module @@ -1619,6 +1627,7 @@ func TestHelperLibraryFactory() android.Module { module.Module.properties.Installable = proptools.BoolPtr(true) module.Module.dexpreopter.isTest = true module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true) + module.Module.sourceProperties.Test_only = proptools.BoolPtr(true) InitJavaModule(module, android.HostAndDeviceSupported) return module @@ -1674,6 +1683,8 @@ func InitTestHost(th *TestHost, installable *bool, testSuites []string, autoGenC th.properties.Installable = installable th.testProperties.Auto_gen_config = autoGenConfig th.testProperties.Test_suites = testSuites + th.sourceProperties.Test_only = proptools.BoolPtr(true) + th.sourceProperties.Top_level_test_target = true } // @@ -1799,7 +1810,7 @@ func BinaryFactory() android.Module { module := &Binary{} module.addHostAndDeviceProperties() - module.AddProperties(&module.binaryProperties) + module.AddProperties(&module.binaryProperties, &module.sourceProperties) module.Module.properties.Installable = proptools.BoolPtr(true) diff --git a/java/java_test.go b/java/java_test.go index 7969d9798..a1192bb5f 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -2838,6 +2838,99 @@ func TestApiLibraryAconfigDeclarations(t *testing.T) { android.AssertStringDoesContain(t, "flagged api hide command not included", cmdline, "revert-annotations-exportable.txt") } +func TestTestOnly(t *testing.T) { + t.Parallel() + ctx := android.GroupFixturePreparers( + prepareForJavaTest, + ).RunTestWithBp(t, ` + // These should be test-only + java_library { + name: "lib1-test-only", + srcs: ["a.java"], + test_only: true, + } + java_test { + name: "java-test", + } + java_test_host { + name: "java-test-host", + } + java_test_helper_library { + name: "helper-library", + } + java_binary { + name: "java-data-binary", + srcs: ["foo.java"], + main_class: "foo.bar.jb", + test_only: true, + } + + // These are NOT + java_library { + name: "lib2-app", + srcs: ["b.java"], + } + java_import { + name: "bar", + jars: ["bar.jar"], + } + java_binary { + name: "java-binary", + srcs: ["foo.java"], + main_class: "foo.bar.jb", + } + `) + + expectedTestOnlyModules := []string{ + "lib1-test-only", + "java-test", + "java-test-host", + "helper-library", + "java-data-binary", + } + expectedTopLevelTests := []string{ + "java-test", + "java-test-host", + } + assertTestOnlyAndTopLevel(t, ctx, expectedTestOnlyModules, expectedTopLevelTests) +} + +// Don't allow setting test-only on things that are always tests or never tests. +func TestInvalidTestOnlyTargets(t *testing.T) { + testCases := []string{ + ` java_test { name: "java-test", test_only: true, srcs: ["foo.java"], } `, + ` java_test_host { name: "java-test-host", test_only: true, srcs: ["foo.java"], } `, + ` java_test_import { name: "java-test-import", test_only: true, } `, + ` java_api_library { name: "java-api-library", test_only: true, } `, + ` java_test_helper_library { name: "test-help-lib", test_only: true, } `, + ` java_defaults { name: "java-defaults", test_only: true, } `, + } + + for i, bp := range testCases { + android.GroupFixturePreparers(prepareForJavaTest). + ExtendWithErrorHandler( + expectOneError("unrecognized property \"test_only\"", + fmt.Sprintf("testcase: %d", i))). + RunTestWithBp(t, bp) + } +} + +// Expect exactly one that matches 'expected'. +// Append 'msg' to the Errorf that printed. +func expectOneError(expected string, msg string) android.FixtureErrorHandler { + return android.FixtureCustomErrorHandler(func(t *testing.T, result *android.TestResult) { + t.Helper() + if len(result.Errs) != 1 { + t.Errorf("Expected exactly one error, but found: %d when setting test_only on: %s", len(result.Errs), msg) + return + } + actualErrMsg := result.Errs[0].Error() + if !strings.Contains(actualErrMsg, expected) { + t.Errorf("Different error than expected. Received: [%v] on %s expected: %s", actualErrMsg, msg, expected) + } + }) +} + func TestJavaLibHostWithStem(t *testing.T) { ctx, _ := testJava(t, ` java_library_host { @@ -2917,3 +3010,34 @@ func TestJavaLibraryOutputFilesRel(t *testing.T) { android.AssertStringEquals(t, "baz relative output path", "baz.jar", bazOutputPath.Rel()) } + +func assertTestOnlyAndTopLevel(t *testing.T, ctx *android.TestResult, expectedTestOnly []string, expectedTopLevel []string) { + t.Helper() + actualTrueModules := []string{} + actualTopLevelTests := []string{} + addActuals := func(m blueprint.Module, key blueprint.ProviderKey[android.TestModuleInformation]) { + if provider, ok := android.OtherModuleProvider(ctx.TestContext.OtherModuleProviderAdaptor(), m, key); ok { + if provider.TestOnly { + actualTrueModules = append(actualTrueModules, m.Name()) + } + if provider.TopLevelTarget { + actualTopLevelTests = append(actualTopLevelTests, m.Name()) + } + } + } + + ctx.VisitAllModules(func(m blueprint.Module) { + addActuals(m, android.TestOnlyProviderKey) + + }) + + notEqual, left, right := android.ListSetDifference(expectedTestOnly, actualTrueModules) + if notEqual { + t.Errorf("test-only: Expected but not found: %v, Found but not expected: %v", left, right) + } + + notEqual, left, right = android.ListSetDifference(expectedTopLevel, actualTopLevelTests) + if notEqual { + t.Errorf("top-level: Expected but not found: %v, Found but not expected: %v", left, right) + } +}