From 3a02c7ba1a927e296fbdcc0a680701f908f8de74 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 21 May 2024 13:46:22 -0700 Subject: [PATCH] Remove test_per_src Remove test_per_src, it never worked well and is incompatible with all of the test infrastructure. Uses in the platform have been removed, generally by replacing them with individual cc_test modules. Test: builds Flag: TEST_ONLY Change-Id: I257c035da35ca8358ae9423b46453878f54efeb1 --- apex/apex.go | 31 ++---------- apex/apex_test.go | 28 ----------- cc/androidmk.go | 3 -- cc/cc.go | 26 +--------- cc/cc_test_only_property_test.go | 32 ------------ cc/test.go | 86 +------------------------------- rust/benchmark.go | 2 +- rust/rust.go | 1 - rust/test.go | 2 +- 9 files changed, 9 insertions(+), 202 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index e6815bc29..09b086ddd 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -2118,20 +2118,10 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext, } case testTag: if ccTest, ok := child.(*cc.Module); ok { - if ccTest.IsTestPerSrcAllTestsVariation() { - // Multiple-output test module (where `test_per_src: true`). - // - // `ccTest` is the "" ("all tests") variation of a `test_per_src` module. - // We do not add this variation to `filesInfo`, as it has no output; - // however, we do add the other variations of this module as indirect - // dependencies (see below). - } else { - // Single-output test module (where `test_per_src: false`). - af := apexFileForExecutable(ctx, ccTest) - af.class = nativeTest - vctx.filesInfo = append(vctx.filesInfo, af) - addAconfigFiles(vctx, ctx, child) - } + af := apexFileForExecutable(ctx, ccTest) + af.class = nativeTest + vctx.filesInfo = append(vctx.filesInfo, af) + addAconfigFiles(vctx, ctx, child) return true // track transitive dependencies } else { ctx.PropertyErrorf("tests", "%q is not a cc module", depName) @@ -2220,19 +2210,6 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext, addAconfigFiles(vctx, ctx, child) return true // track transitive dependencies } - } else if cc.IsTestPerSrcDepTag(depTag) { - if ch, ok := child.(*cc.Module); ok { - af := apexFileForExecutable(ctx, ch) - // Handle modules created as `test_per_src` variations of a single test module: - // use the name of the generated test binary (`fileToCopy`) instead of the name - // of the original test module (`depName`, shared by all `test_per_src` - // variations of that module). - af.androidMkModuleName = filepath.Base(af.builtFile.String()) - // these are not considered transitive dep - af.transitiveDep = false - vctx.filesInfo = append(vctx.filesInfo, af) - return true // track transitive dependencies - } } else if cc.IsHeaderDepTag(depTag) { // nothing } else if java.IsJniDepTag(depTag) { diff --git a/apex/apex_test.go b/apex/apex_test.go index 3bb396692..dd77ab018 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -5728,7 +5728,6 @@ func TestApexWithTests(t *testing.T) { updatable: false, tests: [ "mytest", - "mytests", ], } @@ -5771,25 +5770,6 @@ func TestApexWithTests(t *testing.T) { "testdata/baz" ], } - - cc_test { - name: "mytests", - gtest: false, - srcs: [ - "mytest1.cpp", - "mytest2.cpp", - "mytest3.cpp", - ], - test_per_src: true, - relative_install_path: "test", - system_shared_libs: [], - static_executable: true, - stl: "none", - data: [ - ":fg", - ":fg2", - ], - } `) apexRule := ctx.ModuleForTests("myapex", "android_common_myapex").Rule("apexRule") @@ -5803,11 +5783,6 @@ func TestApexWithTests(t *testing.T) { ensureContains(t, copyCmds, "image.apex/bin/test/baz") ensureContains(t, copyCmds, "image.apex/bin/test/bar/baz") - // Ensure that test deps built with `test_per_src` are copied into apex. - ensureContains(t, copyCmds, "image.apex/bin/test/mytest1") - ensureContains(t, copyCmds, "image.apex/bin/test/mytest2") - ensureContains(t, copyCmds, "image.apex/bin/test/mytest3") - // Ensure the module is correctly translated. bundle := ctx.ModuleForTests("myapex", "android_common_myapex").Module().(*apexBundle) data := android.AndroidMkDataForTest(t, ctx, bundle) @@ -5817,9 +5792,6 @@ func TestApexWithTests(t *testing.T) { data.Custom(&builder, name, prefix, "", data) androidMk := builder.String() ensureContains(t, androidMk, "LOCAL_MODULE := mytest.myapex\n") - ensureContains(t, androidMk, "LOCAL_MODULE := mytest1.myapex\n") - ensureContains(t, androidMk, "LOCAL_MODULE := mytest2.myapex\n") - ensureContains(t, androidMk, "LOCAL_MODULE := mytest3.myapex\n") ensureContains(t, androidMk, "LOCAL_MODULE := myapex\n") } diff --git a/cc/androidmk.go b/cc/androidmk.go index 143e86f09..41346538f 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -351,9 +351,6 @@ func (test *testBinary) AndroidMkEntries(ctx AndroidMkContext, entries *android. ctx.subAndroidMk(entries, test.testDecorator) entries.Class = "NATIVE_TESTS" - if Bool(test.Properties.Test_per_src) { - entries.SubName = "_" + String(test.binaryDecorator.Properties.Stem) - } entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { if test.testConfig != nil { entries.SetString("LOCAL_FULL_TEST_CONFIG", test.testConfig.String()) diff --git a/cc/cc.go b/cc/cc.go index 24f6b839c..134d89c07 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -51,7 +51,6 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) { ctx.BottomUp("sdk", sdkMutator).Parallel() ctx.BottomUp("llndk", llndkMutator).Parallel() ctx.BottomUp("link", LinkageMutator).Parallel() - ctx.BottomUp("test_per_src", TestPerSrcMutator).Parallel() ctx.BottomUp("version", versionMutator).Parallel() ctx.BottomUp("begin", BeginMutator).Parallel() }) @@ -799,7 +798,6 @@ var ( dataLibDepTag = dependencyTag{name: "data lib"} dataBinDepTag = dependencyTag{name: "data bin"} runtimeDepTag = installDependencyTag{name: "runtime lib"} - testPerSrcDepTag = dependencyTag{name: "test_per_src"} stubImplDepTag = dependencyTag{name: "stub_impl"} JniFuzzLibTag = dependencyTag{name: "jni_fuzz_lib_tag"} FdoProfileTag = dependencyTag{name: "fdo_profile"} @@ -826,11 +824,6 @@ func IsRuntimeDepTag(depTag blueprint.DependencyTag) bool { return depTag == runtimeDepTag } -func IsTestPerSrcDepTag(depTag blueprint.DependencyTag) bool { - ccDepTag, ok := depTag.(dependencyTag) - return ok && ccDepTag == testPerSrcDepTag -} - // Module contains the properties and members used by all C/C++ module types, and implements // the blueprint.Module interface. It delegates to compiler, linker, and installer interfaces // to construct the output file. Behavior can be customized with a Customizer, or "decorator", @@ -1786,11 +1779,6 @@ func (c *Module) Symlinks() []string { return nil } -func (c *Module) IsTestPerSrcAllTestsVariation() bool { - test, ok := c.linker.(testPerSrc) - return ok && test.isAllTestsVariation() -} - func (c *Module) DataPaths() []android.DataPath { if p, ok := c.installer.(interface { dataPaths() []android.DataPath @@ -1943,16 +1931,6 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { TopLevelTarget: c.testModule, }) - // Handle the case of a test module split by `test_per_src` mutator. - // - // The `test_per_src` mutator adds an extra variation named "", depending on all the other - // `test_per_src` variations of the test module. Set `outputFile` to an empty path for this - // module and return early, as this module does not produce an output file per se. - if c.IsTestPerSrcAllTestsVariation() { - c.outputFile = android.OptionalPath{} - return - } - c.Properties.SubName = GetSubnameProperty(actx, c) apexInfo, _ := android.ModuleProvider(actx, android.ApexInfoProvider) if !apexInfo.IsForPlatform() { @@ -3758,14 +3736,12 @@ func GetMakeLinkType(actx android.ModuleContext, c LinkableInterface) string { } // Overrides ApexModule.IsInstallabeToApex() -// Only shared/runtime libraries and "test_per_src" tests are installable to APEX. +// Only shared/runtime libraries . func (c *Module) IsInstallableToApex() bool { if lib := c.library; lib != nil { // Stub libs and prebuilt libs in a versioned SDK are not // installable to APEX even though they are shared libs. return lib.shared() && !lib.buildStubs() - } else if _, ok := c.linker.(testPerSrc); ok { - return true } return false } diff --git a/cc/cc_test_only_property_test.go b/cc/cc_test_only_property_test.go index c14f34ecb..972e86bc5 100644 --- a/cc/cc_test_only_property_test.go +++ b/cc/cc_test_only_property_test.go @@ -78,38 +78,6 @@ func TestTestOnlyProvider(t *testing.T) { } } -func TestTestOnlyValueWithTestPerSrcProp(t *testing.T) { - t.Parallel() - ctx := android.GroupFixturePreparers( - prepareForCcTest, - ).RunTestWithBp(t, ` - // These should be test-only - cc_test { name: "cc-test", - gtest: false, - test_per_src: true, - srcs: ["foo_test.cpp"], - test_options: { unit_test: false, }, - } - `) - - // Ensure all variation of test-per-src tests are marked test-only. - ctx.VisitAllModules(func(m blueprint.Module) { - testOnly := false - if provider, ok := android.OtherModuleProvider(ctx.TestContext.OtherModuleProviderAdaptor(), m, android.TestOnlyProviderKey); ok { - if provider.TestOnly { - testOnly = true - } - } - if module, ok := m.(*Module); ok { - if testModule, ok := module.installer.(*testBinary); ok { - if !testOnly && *testModule.Properties.Test_per_src { - t.Errorf("%v is not test-only but should be", m) - } - } - } - }) -} - func TestTestOnlyInTeamsProto(t *testing.T) { t.Parallel() ctx := android.GroupFixturePreparers( diff --git a/cc/test.go b/cc/test.go index a96af31bb..f5bb7610c 100644 --- a/cc/test.go +++ b/cc/test.go @@ -15,11 +15,9 @@ package cc import ( + "github.com/google/blueprint/proptools" "path/filepath" "strconv" - "strings" - - "github.com/google/blueprint/proptools" "android/soong/android" "android/soong/tradefed" @@ -75,13 +73,9 @@ type TestOptions struct { } type TestBinaryProperties struct { - // Create a separate binary for each source file. Useful when there is - // global state that can not be torn down and reset between each test suite. - Test_per_src *bool - // Disables the creation of a test-specific directory when used with // relative_install_path. Useful if several tests need to be in the same - // directory, but test_per_src doesn't work. + // directory. No_named_install_directory *bool // list of files or filegroup modules that provide data that should be installed alongside @@ -174,86 +168,14 @@ func BenchmarkHostFactory() android.Module { return module.Init() } -type testPerSrc interface { - testPerSrc() bool - srcs() []string - isAllTestsVariation() bool - setSrc(string, string) - unsetSrc() -} - -func (test *testBinary) testPerSrc() bool { - return Bool(test.Properties.Test_per_src) -} - -func (test *testBinary) srcs() []string { - return test.baseCompiler.Properties.Srcs -} - func (test *testBinary) dataPaths() []android.DataPath { return test.data } -func (test *testBinary) isAllTestsVariation() bool { - stem := test.binaryDecorator.Properties.Stem - return stem != nil && *stem == "" -} - -func (test *testBinary) setSrc(name, src string) { - test.baseCompiler.Properties.Srcs = []string{src} - test.binaryDecorator.Properties.Stem = StringPtr(name) -} - -func (test *testBinary) unsetSrc() { - test.baseCompiler.Properties.Srcs = nil - test.binaryDecorator.Properties.Stem = StringPtr("") -} - func (test *testBinary) testBinary() bool { return true } -var _ testPerSrc = (*testBinary)(nil) - -func TestPerSrcMutator(mctx android.BottomUpMutatorContext) { - if m, ok := mctx.Module().(*Module); ok { - if test, ok := m.linker.(testPerSrc); ok { - numTests := len(test.srcs()) - if test.testPerSrc() && numTests > 0 { - if duplicate, found := android.CheckDuplicate(test.srcs()); found { - mctx.PropertyErrorf("srcs", "found a duplicate entry %q", duplicate) - return - } - testNames := make([]string, numTests) - for i, src := range test.srcs() { - testNames[i] = strings.TrimSuffix(filepath.Base(src), filepath.Ext(src)) - } - // In addition to creating one variation per test source file, - // create an additional "all tests" variation named "", and have it - // depends on all other test_per_src variations. This is useful to - // create subsequent dependencies of a given module on all - // test_per_src variations created above: by depending on - // variation "", that module will transitively depend on all the - // other test_per_src variations without the need to know their - // name or even their number. - testNames = append(testNames, "") - tests := mctx.CreateLocalVariations(testNames...) - allTests := tests[numTests] - allTests.(*Module).linker.(testPerSrc).unsetSrc() - // Prevent the "all tests" variation from being installable nor - // exporting to Make, as it won't create any output file. - allTests.(*Module).Properties.PreventInstall = true - allTests.(*Module).Properties.HideFromMake = true - for i, src := range test.srcs() { - tests[i].(*Module).linker.(testPerSrc).setSrc(testNames[i], src) - mctx.AddInterVariantDependency(testPerSrcDepTag, allTests, tests[i]) - } - mctx.AliasVariation("") - } - } - } -} - type testDecorator struct { LinkerProperties TestLinkerProperties InstallerProperties TestInstallerProperties @@ -382,10 +304,6 @@ func (test *testBinary) moduleInfoJSON(ctx ModuleContext, moduleInfoJSON *androi } moduleInfoJSON.TestConfig = append(moduleInfoJSON.TestConfig, test.extraTestConfigs.Strings()...) - if Bool(test.Properties.Test_per_src) { - moduleInfoJSON.SubName = "_" + String(test.binaryDecorator.Properties.Stem) - } - moduleInfoJSON.DataDependencies = append(moduleInfoJSON.DataDependencies, test.Properties.Data_bins...) if len(test.InstallerProperties.Test_suites) > 0 { diff --git a/rust/benchmark.go b/rust/benchmark.go index c0f1e24d1..8c3e5151e 100644 --- a/rust/benchmark.go +++ b/rust/benchmark.go @@ -22,7 +22,7 @@ import ( type BenchmarkProperties struct { // Disables the creation of a test-specific directory when used with // relative_install_path. Useful if several tests need to be in the same - // directory, but test_per_src doesn't work. + // directory. No_named_install_directory *bool // the name of the test configuration (for example "AndroidBenchmark.xml") that should be diff --git a/rust/rust.go b/rust/rust.go index 6b7b2fbae..32998616e 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -1107,7 +1107,6 @@ var ( rlibDepTag = dependencyTag{name: "rlibTag", library: true} dylibDepTag = dependencyTag{name: "dylib", library: true, dynamic: true} procMacroDepTag = dependencyTag{name: "procMacro", procMacro: true} - testPerSrcDepTag = dependencyTag{name: "rust_unit_tests"} sourceDepTag = dependencyTag{name: "source"} dataLibDepTag = dependencyTag{name: "data lib"} dataBinDepTag = dependencyTag{name: "data bin"} diff --git a/rust/test.go b/rust/test.go index 258389343..3087d8d94 100644 --- a/rust/test.go +++ b/rust/test.go @@ -27,7 +27,7 @@ import ( type TestProperties struct { // Disables the creation of a test-specific directory when used with // relative_install_path. Useful if several tests need to be in the same - // directory, but test_per_src doesn't work. + // directory. No_named_install_directory *bool // the name of the test configuration (for example "AndroidTest.xml") that should be