From fdaa672ad1faacf0fb279adf876841dc2ffb404e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 23 Aug 2024 11:58:08 -0700 Subject: [PATCH] Remove obsolete robolectric test runner integration The `m Run*RoboTests` test runner is no longer supported, remove the code to generate the Make rules for it. Also reduce the number of combining steps by passing the extra jars that need to be combined into the compile step. This relands Icf05079bf570bed7a10962cbf03459bd53c51f99 with a fix to reorder the instrumented app classes after dependency classes. Test: atest --host SystemUiRoboTests Flag: EXEMPT refactor Change-Id: I174ceb95af5557e1859eeb12a2f5fac2332975a8 --- java/aar.go | 2 +- java/app.go | 2 +- java/base.go | 4 +- java/java.go | 4 +- java/robolectric.go | 193 +++++++------------------------------------- 5 files changed, 37 insertions(+), 168 deletions(-) diff --git a/java/aar.go b/java/aar.go index 42866f8b6..8ceeace20 100644 --- a/java/aar.go +++ b/java/aar.go @@ -884,7 +884,7 @@ func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) extraSrcJars = android.Paths{a.aapt.aaptSrcJar} } - a.Module.compile(ctx, extraSrcJars, extraClasspathJars, extraCombinedJars) + a.Module.compile(ctx, extraSrcJars, extraClasspathJars, extraCombinedJars, nil) a.aarFile = android.PathForModuleOut(ctx, ctx.ModuleName()+".aar") var res android.Paths diff --git a/java/app.go b/java/app.go index d5c220d9d..4ac42a750 100644 --- a/java/app.go +++ b/java/app.go @@ -690,7 +690,7 @@ func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) (android.Path, a extraSrcJars = android.Paths{a.aapt.aaptSrcJar} } - a.Module.compile(ctx, extraSrcJars, extraClasspathJars, extraCombinedJars) + a.Module.compile(ctx, extraSrcJars, extraClasspathJars, extraCombinedJars, nil) if a.dexProperties.resourceShrinkingEnabled(ctx) { binaryResources := android.PathForModuleOut(ctx, packageResources.Base()+".binary.out.apk") aapt2Convert(ctx, binaryResources, a.dexer.resourcesOutput.Path(), "binary") diff --git a/java/base.go b/java/base.go index 75b552fd6..a31c8489b 100644 --- a/java/base.go +++ b/java/base.go @@ -1156,7 +1156,7 @@ func (j *Module) addGeneratedSrcJars(path android.Path) { j.properties.Generated_srcjars = append(j.properties.Generated_srcjars, path) } -func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspathJars, extraCombinedJars android.Paths) { +func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspathJars, extraCombinedJars, extraDepCombinedJars android.Paths) { // Auto-propagating jarjar rules jarjarProviderData := j.collectJarJarRules(ctx) if jarjarProviderData != nil { @@ -1521,6 +1521,8 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath jars = append(jars, deps.staticJars...) } + jars = append(jars, extraDepCombinedJars...) + manifest := j.overrideManifest if !manifest.Valid() && j.properties.Manifest != nil { manifest = android.OptionalPathForPath(android.PathForModuleSrc(ctx, *j.properties.Manifest)) diff --git a/java/java.go b/java/java.go index 8cc108541..f30d892b7 100644 --- a/java/java.go +++ b/java/java.go @@ -553,7 +553,7 @@ type deps struct { // are provided by systemModules. java9Classpath classpath - processorPath classpath + processorPath classpath `` errorProneProcessorPath classpath processorClasses []string staticJars android.Paths @@ -980,7 +980,7 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { j.dexpreopter.disableDexpreopt() } } - j.compile(ctx, nil, nil, nil) + j.compile(ctx, nil, nil, nil, nil) // If this module is an impl library created from java_sdk_library, // install the files under the java_sdk_library module outdir instead of this module outdir. diff --git a/java/robolectric.go b/java/robolectric.go index 26f4b7176..374fc5f78 100644 --- a/java/robolectric.go +++ b/java/robolectric.go @@ -16,9 +16,6 @@ package java import ( "fmt" - "io" - "strconv" - "strings" "android/soong/android" "android/soong/java/config" @@ -88,16 +85,6 @@ type robolectricTest struct { robolectricProperties robolectricProperties testProperties testProperties - libs []string - tests []string - - manifest android.Path - resourceApk android.Path - - combinedJar android.WritablePath - - roboSrcJar android.Path - testConfig android.Path data android.Paths @@ -121,12 +108,12 @@ func (r *robolectricTest) DepsMutator(ctx android.BottomUpMutatorContext) { } if v := String(r.robolectricProperties.Robolectric_prebuilt_version); v != "" { - ctx.AddVariationDependencies(nil, libTag, fmt.Sprintf(robolectricPrebuiltLibPattern, v)) + ctx.AddVariationDependencies(nil, staticLibTag, fmt.Sprintf(robolectricPrebuiltLibPattern, v)) } else if !proptools.BoolDefault(r.robolectricProperties.Strict_mode, true) { if proptools.Bool(r.robolectricProperties.Upstream) { - ctx.AddVariationDependencies(nil, libTag, robolectricCurrentLib+"_upstream") + ctx.AddVariationDependencies(nil, staticLibTag, robolectricCurrentLib+"_upstream") } else { - ctx.AddVariationDependencies(nil, libTag, robolectricCurrentLib) + ctx.AddVariationDependencies(nil, staticLibTag, robolectricCurrentLib) } } @@ -134,10 +121,10 @@ func (r *robolectricTest) DepsMutator(ctx android.BottomUpMutatorContext) { ctx.AddVariationDependencies(nil, roboRuntimeOnlyTag, robolectricCurrentLib+"_upstream") } else { // opting out from strict mode, robolectric_non_strict_mode_permission lib should be added - ctx.AddVariationDependencies(nil, libTag, "robolectric_non_strict_mode_permission") + ctx.AddVariationDependencies(nil, staticLibTag, "robolectric_non_strict_mode_permission") } - ctx.AddVariationDependencies(nil, libTag, robolectricDefaultLibs...) + ctx.AddVariationDependencies(nil, staticLibTag, robolectricDefaultLibs...) ctx.AddVariationDependencies(nil, roboCoverageLibsTag, r.robolectricProperties.Coverage_libs...) @@ -163,9 +150,6 @@ func (r *robolectricTest) GenerateAndroidBuildActions(ctx android.ModuleContext) }) r.data = android.PathsForModuleSrc(ctx, r.testProperties.Data) - roboTestConfig := android.PathForModuleGen(ctx, "robolectric"). - Join(ctx, "com/android/tools/test_config.properties") - var ok bool var instrumentedApp *AndroidApp @@ -181,90 +165,58 @@ func (r *robolectricTest) GenerateAndroidBuildActions(ctx android.ModuleContext) panic(fmt.Errorf("expected exactly 1 instrumented dependency, got %d", len(instrumented))) } + var resourceApk android.Path + var manifest android.Path if instrumentedApp != nil { - r.manifest = instrumentedApp.mergedManifestFile - r.resourceApk = instrumentedApp.outputFile - - generateRoboTestConfig(ctx, roboTestConfig, instrumentedApp) - r.extraResources = android.Paths{roboTestConfig} - } - - r.Library.GenerateAndroidBuildActions(ctx) - - roboSrcJar := android.PathForModuleGen(ctx, "robolectric", ctx.ModuleName()+".srcjar") - - if instrumentedApp != nil { - r.generateRoboSrcJar(ctx, roboSrcJar, instrumentedApp) - r.roboSrcJar = roboSrcJar + manifest = instrumentedApp.mergedManifestFile + resourceApk = instrumentedApp.outputFile } roboTestConfigJar := android.PathForModuleOut(ctx, "robolectric_samedir", "samedir_config.jar") generateSameDirRoboTestConfigJar(ctx, roboTestConfigJar) - combinedJarJars := android.Paths{ - // roboTestConfigJar comes first so that its com/android/tools/test_config.properties - // overrides the one from r.extraResources. The r.extraResources one can be removed - // once the Make test runner is removed. - roboTestConfigJar, - r.outputFile, - } + extraCombinedJars := android.Paths{roboTestConfigJar} - if instrumentedApp != nil { - combinedJarJars = append(combinedJarJars, instrumentedApp.implementationAndResourcesJar) - } - - handleLibDeps := func(dep android.Module, runtimeOnly bool) { - if !runtimeOnly { - r.libs = append(r.libs, ctx.OtherModuleName(dep)) - } + handleLibDeps := func(dep android.Module) { if !android.InList(ctx.OtherModuleName(dep), config.FrameworkLibraries) { if m, ok := android.OtherModuleProvider(ctx, dep, JavaInfoProvider); ok { - combinedJarJars = append(combinedJarJars, m.ImplementationAndResourcesJars...) + extraCombinedJars = append(extraCombinedJars, m.ImplementationAndResourcesJars...) } } } for _, dep := range ctx.GetDirectDepsWithTag(libTag) { - handleLibDeps(dep, false) + handleLibDeps(dep) } for _, dep := range ctx.GetDirectDepsWithTag(sdkLibTag) { - handleLibDeps(dep, false) + handleLibDeps(dep) } // handle the runtimeOnly tag for strict_mode for _, dep := range ctx.GetDirectDepsWithTag(roboRuntimeOnlyTag) { - handleLibDeps(dep, true) + handleLibDeps(dep) } - r.combinedJar = android.PathForModuleOut(ctx, "robolectric_combined", r.outputFile.Base()) - TransformJarsToJar(ctx, r.combinedJar, "combine jars", combinedJarJars, android.OptionalPath{}, - false, nil, nil) - - // TODO: this could all be removed if tradefed was used as the test runner, it will find everything - // annotated as a test and run it. - for _, src := range r.uniqueSrcFiles { - s := src.Rel() - if !strings.HasSuffix(s, "Test.java") && !strings.HasSuffix(s, "Test.kt") { - continue - } else if strings.HasSuffix(s, "/BaseRobolectricTest.java") { - continue - } else { - s = strings.TrimPrefix(s, "src/") - } - r.tests = append(r.tests, s) + if instrumentedApp != nil { + extraCombinedJars = append(extraCombinedJars, instrumentedApp.implementationAndResourcesJar) } + r.stem = proptools.StringDefault(r.overridableProperties.Stem, ctx.ModuleName()) + r.classLoaderContexts = r.usesLibrary.classLoaderContextForUsesLibDeps(ctx) + r.dexpreopter.disableDexpreopt() + r.compile(ctx, nil, nil, nil, extraCombinedJars) + installPath := android.PathForModuleInstall(ctx, r.BaseModuleName()) var installDeps android.InstallPaths - if r.manifest != nil { - r.data = append(r.data, r.manifest) - installedManifest := ctx.InstallFile(installPath, ctx.ModuleName()+"-AndroidManifest.xml", r.manifest) + if manifest != nil { + r.data = append(r.data, manifest) + installedManifest := ctx.InstallFile(installPath, ctx.ModuleName()+"-AndroidManifest.xml", manifest) installDeps = append(installDeps, installedManifest) } - if r.resourceApk != nil { - r.data = append(r.data, r.resourceApk) - installedResourceApk := ctx.InstallFile(installPath, ctx.ModuleName()+".apk", r.resourceApk) + if resourceApk != nil { + r.data = append(r.data, resourceApk) + installedResourceApk := ctx.InstallFile(installPath, ctx.ModuleName()+".apk", resourceApk) installDeps = append(installDeps, installedResourceApk) } @@ -287,29 +239,10 @@ func (r *robolectricTest) GenerateAndroidBuildActions(ctx android.ModuleContext) installDeps = append(installDeps, installJni) } - r.installFile = ctx.InstallFile(installPath, ctx.ModuleName()+".jar", r.combinedJar, installDeps...) + r.installFile = ctx.InstallFile(installPath, ctx.ModuleName()+".jar", r.outputFile, installDeps...) android.SetProvider(ctx, testing.TestModuleProviderKey, testing.TestModuleProviderData{}) } -func generateRoboTestConfig(ctx android.ModuleContext, outputFile android.WritablePath, - instrumentedApp *AndroidApp) { - rule := android.NewRuleBuilder(pctx, ctx) - - manifest := instrumentedApp.mergedManifestFile - resourceApk := instrumentedApp.outputFile - - rule.Command().Text("rm -f").Output(outputFile) - rule.Command(). - Textf(`echo "android_merged_manifest=%s" >>`, manifest.String()).Output(outputFile).Text("&&"). - Textf(`echo "android_resource_apk=%s" >>`, resourceApk.String()).Output(outputFile). - // Make it depend on the files to which it points so the test file's timestamp is updated whenever the - // contents change - Implicit(manifest). - Implicit(resourceApk) - - rule.Build("generate_test_config", "generate test_config.properties") -} - func generateSameDirRoboTestConfigJar(ctx android.ModuleContext, outputFile android.ModuleOutPath) { rule := android.NewRuleBuilder(pctx, ctx) @@ -332,22 +265,6 @@ func generateSameDirRoboTestConfigJar(ctx android.ModuleContext, outputFile andr rule.Build("generate_test_config_samedir", "generate test_config.properties") } -func (r *robolectricTest) generateRoboSrcJar(ctx android.ModuleContext, outputFile android.WritablePath, - instrumentedApp *AndroidApp) { - - srcJarArgs := android.CopyOf(instrumentedApp.srcJarArgs) - srcJarDeps := append(android.Paths(nil), instrumentedApp.srcJarDeps...) - - for _, m := range ctx.GetDirectDepsWithTag(roboCoverageLibsTag) { - if dep, ok := android.OtherModuleProvider(ctx, m, JavaInfoProvider); ok { - srcJarArgs = append(srcJarArgs, dep.SrcJarArgs...) - srcJarDeps = append(srcJarDeps, dep.SrcJarDeps...) - } - } - - TransformResourcesToJar(ctx, outputFile, srcJarArgs, srcJarDeps) -} - func (r *robolectricTest) AndroidMkEntries() []android.AndroidMkEntries { entriesList := r.Library.AndroidMkEntries() entries := &entriesList[0] @@ -359,61 +276,11 @@ func (r *robolectricTest) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_FULL_TEST_CONFIG", r.testConfig) } }) - - entries.ExtraFooters = []android.AndroidMkExtraFootersFunc{ - func(w io.Writer, name, prefix, moduleDir string) { - if s := r.robolectricProperties.Test_options.Shards; s != nil && *s > 1 { - numShards := int(*s) - shardSize := (len(r.tests) + numShards - 1) / numShards - shards := android.ShardStrings(r.tests, shardSize) - for i, shard := range shards { - r.writeTestRunner(w, name, "Run"+name+strconv.Itoa(i), shard) - } - - // TODO: add rules to dist the outputs of the individual tests, or combine them together? - fmt.Fprintln(w, "") - fmt.Fprintln(w, ".PHONY:", "Run"+name) - fmt.Fprintln(w, "Run"+name, ": \\") - for i := range shards { - fmt.Fprintln(w, " ", "Run"+name+strconv.Itoa(i), "\\") - } - fmt.Fprintln(w, "") - } else { - r.writeTestRunner(w, name, "Run"+name, r.tests) - } - }, - } - return entriesList } -func (r *robolectricTest) writeTestRunner(w io.Writer, module, name string, tests []string) { - fmt.Fprintln(w, "") - fmt.Fprintln(w, "include $(CLEAR_VARS)", " # java.robolectricTest") - fmt.Fprintln(w, "LOCAL_MODULE :=", name) - android.AndroidMkEmitAssignList(w, "LOCAL_JAVA_LIBRARIES", []string{module}, r.libs) - fmt.Fprintln(w, "LOCAL_TEST_PACKAGE :=", String(r.robolectricProperties.Instrumentation_for)) - if r.roboSrcJar != nil { - fmt.Fprintln(w, "LOCAL_INSTRUMENT_SRCJARS :=", r.roboSrcJar.String()) - } - android.AndroidMkEmitAssignList(w, "LOCAL_ROBOTEST_FILES", tests) - if t := r.robolectricProperties.Test_options.Timeout; t != nil { - fmt.Fprintln(w, "LOCAL_ROBOTEST_TIMEOUT :=", *t) - } - if v := String(r.robolectricProperties.Robolectric_prebuilt_version); v != "" { - fmt.Fprintf(w, "-include prebuilts/misc/common/robolectric/%s/run_robotests.mk\n", v) - } else { - fmt.Fprintln(w, "-include external/robolectric-shadows/run_robotests.mk") - } -} - // An android_robolectric_test module compiles tests against the Robolectric framework that can run on the local host -// instead of on a device. It also generates a rule with the name of the module prefixed with "Run" that can be -// used to run the tests. Running the tests with build rule will eventually be deprecated and replaced with atest. -// -// The test runner considers any file listed in srcs whose name ends with Test.java to be a test class, unless -// it is named BaseRobolectricTest.java. The path to the each source file must exactly match the package -// name, or match the package name when the prefix "src/" is removed. +// instead of on a device. func RobolectricTestFactory() android.Module { module := &robolectricTest{}