Reapply "Change test_module_config from copying files to symlink ..."

This reverts commit c6a321e4de.

Original commit broke whe building sdk/ndk in postsubmit because of
missing target arch.

See: https://android-build.corp.google.com/artifact/submitted/11914755/mainline_modules_sdks-trunk_staging-userdebug/latest/view/logs%2Fbuild_error.log

Reapplying originaly commit and adding fix in patchest.

Test: DIST_DIR=/usr/local/google/dist/bug TARGET_RELEASE=trunk_staging TARGET_BUILD_VARIANT=userdebug UNBUNDLED_BUILD_SDKS_FROM_SOURCE=true packages/modules/common/build/mainline_modules_sdks.sh
Test: m general-tests
Change-Id: Id844feb7ff9750bcd5af890a9fd26f7342344965

java/app.go

@@ -1380,6 +1380,8 @@ func (a *AndroidTest) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 		HostRequiredModuleNames: a.HostRequiredModuleNames(),
 		TestSuites:              a.testProperties.Test_suites,
 		IsHost:                  false,
+		LocalCertificate:        a.certificate.AndroidMkString(),
+		IsUnitTest:              Bool(a.testProperties.Test_options.Unit_test),
 	})
 	android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{
 		TestOnly:       true,

java/java.go

@@ -1504,6 +1504,8 @@ func (j *TestHost) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 		RequiredModuleNames: j.RequiredModuleNames(),
 		TestSuites:          j.testProperties.Test_suites,
 		IsHost:              true,
+		LocalSdkVersion:     j.sdkVersion.String(),
+		IsUnitTest:          Bool(j.testProperties.Test_options.Unit_test),
 	})
 }
 

tradefed/providers.go

@@ -6,7 +6,8 @@ import (
 	"github.com/google/blueprint"
 )
 
-// Output files we need from a base test that we derive from.
+// Data that test_module_config[_host] modules types will need from
+// their dependencies to write out build rules and AndroidMkEntries.
 type BaseTestProviderData struct {
 	// data files and apps for android_test
 	InstalledFiles android.Paths
@@ -19,8 +20,14 @@ type BaseTestProviderData struct {
 	RequiredModuleNames     []string
 	// List of test suites base uses.
 	TestSuites []string
-	// Used for bases that are Host
+	// True indicates the base modules is built for Host.
 	IsHost bool
+	// Base's sdk version for AndroidMkEntries, generally only used for Host modules.
+	LocalSdkVersion string
+	// Base's certificate for AndroidMkEntries, generally only used for device modules.
+	LocalCertificate string
+	// Indicates if the base module was a unit test.
+	IsUnitTest bool
 }
 
 var BaseTestProviderKey = blueprint.NewProvider[BaseTestProviderData]()

tradefed_modules/test_module_config.go

@@ -5,6 +5,8 @@ import (
 	"android/soong/tradefed"
 	"encoding/json"
 	"fmt"
+	"io"
+	"strings"
 
 	"github.com/google/blueprint"
 	"github.com/google/blueprint/proptools"
@@ -23,14 +25,17 @@ func RegisterTestModuleConfigBuildComponents(ctx android.RegistrationContext) {
 type testModuleConfigModule struct {
 	android.ModuleBase
 	android.DefaultableModuleBase
-	base android.Module
 
 	tradefedProperties
 
 	// Our updated testConfig.
 	testConfig android.OutputPath
-	manifest   android.InstallPath
+	manifest   android.OutputPath
 	provider   tradefed.BaseTestProviderData
+
+	supportFiles android.InstallPaths
+
+	isHost bool
 }
 
 // Host is mostly the same as non-host, just some diffs for AddDependency and
@@ -94,7 +99,6 @@ func (m *testModuleConfigModule) DepsMutator(ctx android.BottomUpMutatorContext)
 
 // Takes base's Tradefed Config xml file and generates a new one with the test properties
 // appeneded from this module.
-// Rewrite the name of the apk in "test-file-name" to be our module's name, rather than the original one.
 func (m *testModuleConfigModule) fixTestConfig(ctx android.ModuleContext, baseTestConfig android.Path) android.OutputPath {
 	// Test safe to do when no test_runner_options, but check for that earlier?
 	fixedConfig := android.PathForModuleOut(ctx, "test_config_fixer", ctx.ModuleName()+".config")
@@ -106,9 +110,8 @@ func (m *testModuleConfigModule) fixTestConfig(ctx android.ModuleContext, baseTe
 	}
 	xmlTestModuleConfigSnippet, _ := json.Marshal(options)
 	escaped := proptools.NinjaAndShellEscape(string(xmlTestModuleConfigSnippet))
-	command.FlagWithArg("--test-file-name=", ctx.ModuleName()+".apk").
+	command.FlagWithArg("--test-runner-options=", escaped)
-		FlagWithArg("--orig-test-file-name=", *m.tradefedProperties.Base+".apk").
+
-		FlagWithArg("--test-runner-options=", escaped)
 	rule.Build("fix_test_config", "fix test config")
 	return fixedConfig.OutputPath
 }
@@ -190,6 +193,7 @@ func TestModuleConfigHostFactory() android.Module {
 	module.AddProperties(&module.tradefedProperties)
 	android.InitAndroidMultiTargetsArchModule(module, android.HostSupported, android.MultilibCommon)
 	android.InitDefaultableModule(module)
+	module.isHost = true
 
 	return module
 }
@@ -198,39 +202,66 @@ func TestModuleConfigHostFactory() android.Module {
 var _ android.AndroidMkEntriesProvider = (*testModuleConfigModule)(nil)
 
 func (m *testModuleConfigModule) AndroidMkEntries() []android.AndroidMkEntries {
-	// We rely on base writing LOCAL_COMPATIBILITY_SUPPORT_FILES for its data files
+	appClass := "APPS"
-	entriesList := m.base.(android.AndroidMkEntriesProvider).AndroidMkEntries()
+	include := "$(BUILD_SYSTEM)/soong_app_prebuilt.mk"
-	entries := &entriesList[0]
+	if m.isHost {
-	entries.OutputFile = android.OptionalPathForPath(m.provider.OutputFile)
+		appClass = "JAVA_LIBRARIES"
-	entries.ExtraEntries = append(entries.ExtraEntries, func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) {
+		include = "$(BUILD_SYSTEM)/soong_java_prebuilt.mk"
-		entries.SetString("LOCAL_MODULE", m.Name()) //  out module name, not base's
+	}
-
+	return []android.AndroidMkEntries{{
-		// Out update config file with extra options.
+		Class:      appClass,
+		OutputFile: android.OptionalPathForPath(m.manifest),
+		Include:    include,
+		Required:   []string{*m.Base},
+		ExtraEntries: []android.AndroidMkExtraEntriesFunc{
+			func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) {
 				entries.SetPath("LOCAL_FULL_TEST_CONFIG", m.testConfig)
 				entries.SetString("LOCAL_MODULE_TAGS", "tests")
+				entries.SetString("LOCAL_TEST_MODULE_CONFIG_BASE", *m.Base)
+				if m.provider.LocalSdkVersion != "" {
+					entries.SetString("LOCAL_SDK_VERSION", m.provider.LocalSdkVersion)
+				}
+				if m.provider.LocalCertificate != "" {
+					entries.SetString("LOCAL_CERTIFICATE", m.provider.LocalCertificate)
+				}
 
-		// Don't append to base's test-suites, only use the ones we define, so clear it before
+				entries.SetBoolIfTrue("LOCAL_IS_UNIT_TEST", m.provider.IsUnitTest)
-		// appending to it.
-		entries.SetString("LOCAL_COMPATIBILITY_SUITE", "")
 				entries.AddCompatibilityTestSuites(m.tradefedProperties.Test_suites...)
 
-		if len(m.provider.HostRequiredModuleNames) > 0 {
+				// The app_prebuilt_internal.mk files try create a copy of the OutputFile as an .apk.
-			entries.AddStrings("LOCAL_HOST_REQUIRED_MODULES", m.provider.HostRequiredModuleNames...)
+				// Normally, this copies the "package.apk" from the intermediate directory here.
-		}
+				// To prevent the copy of the large apk and to prevent confusion with the real .apk we
-		if len(m.provider.RequiredModuleNames) > 0 {
+				// link to, we set the STEM here to a bogus name and we set OutputFile to a small file (our manifest).
-			entries.AddStrings("LOCAL_REQUIRED_MODULES", m.provider.RequiredModuleNames...)
+				// We do this so we don't have to add more conditionals to base_rules.mk
-		}
+				// soong_java_prebult has the same issue for .jars so use this in both module types.
+				entries.SetString("LOCAL_MODULE_STEM", fmt.Sprintf("UNUSED-%s", *m.Base))
 
-		if m.provider.IsHost == false {
+				// In normal java/app modules, the module writes LOCAL_COMPATIBILITY_SUPPORT_FILES
-			// Not needed for jar_host_test
+				// and then base_rules.mk ends up copying each of those dependencies from .intermediates to the install directory.
+				// tasks/general-tests.mk, tasks/devices-tests.mk also use these to figure out
+				// which testcase files to put in a zip for running tests on another machine.
 				//
-			// Clear the JNI symbols because they belong to base not us. Either transform the names in the string
+				// We need our files to end up in the zip, but we don't want \.mk files to
-			// or clear the variable because we don't need it, we are copying bases libraries not generating
+				// `install` files for us.
-			// new ones.
+				// So we create a new make variable to indicate these should be in the zip
-			entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", "")
+				// but not installed.
+				entries.AddStrings("LOCAL_SOONG_INSTALLED_COMPATIBILITY_SUPPORT_FILES", m.supportFiles.Strings()...)
+			},
+		},
+		// Ensure each of our supportFiles depends on the installed file in base so that our symlinks will always
+		// resolve.  The provider gives us the .intermediate path for the support file in base, we change it to
+		// the installed path with a string substitution.
+		ExtraFooters: []android.AndroidMkExtraFootersFunc{
+			func(w io.Writer, name, prefix, moduleDir string) {
+				for _, f := range m.supportFiles.Strings() {
+					// convert out/.../testcases/FrameworksServicesTests_contentprotection/file1.apk
+					// to      out/.../testcases/FrameworksServicesTests/file1.apk
+					basePath := strings.Replace(f, "/"+m.Name()+"/", "/"+*m.Base+"/", 1)
+					fmt.Fprintf(w, "%s: %s\n", f, basePath)
 				}
-	})
+			},
-	return entriesList
+		},
+	}}
 }
 
 func (m *testModuleConfigHostModule) DepsMutator(ctx android.BottomUpMutatorContext) {
@@ -248,7 +279,7 @@ func (m *testModuleConfigHostModule) DepsMutator(ctx android.BottomUpMutatorCont
 //   - written via soong_java_prebuilt.mk
 //
 // 4) out/host/linux-x86/testcases/derived-module/* # data dependencies from base.
-//   - written via soong_java_prebuilt.mk
+//   - written via our InstallSymlink
 func (m *testModuleConfigHostModule) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	m.validateBase(ctx, &testModuleConfigHostTag, "java_test_host", true)
 	m.generateManifestAndConfig(ctx)
@@ -260,7 +291,6 @@ func (m *testModuleConfigModule) validateBase(ctx android.ModuleContext, depTag
 	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 {
@@ -277,7 +307,9 @@ func (m *testModuleConfigModule) validateBase(ctx android.ModuleContext, depTag
 
 // Actions to write:
 //  1. manifest file to testcases dir
-//  2. New Module.config / AndroidTest.xml file with our options.
+//  2. Symlink to base.apk under base's arch dir
+//  3. Symlink to all data dependencies
+//  4. New Module.config / AndroidTest.xml file with our options.
 func (m *testModuleConfigModule) generateManifestAndConfig(ctx android.ModuleContext) {
 	// Keep before early returns.
 	android.SetProvider(ctx, android.TestOnlyProviderKey, android.TestModuleInformation{
@@ -292,7 +324,6 @@ func (m *testModuleConfigModule) generateManifestAndConfig(ctx android.ModuleCon
 	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")
@@ -301,9 +332,53 @@ func (m *testModuleConfigModule) generateManifestAndConfig(ctx android.ModuleCon
 	//    Assume the primary install file is last
 	// so we need to Install our file last.
 	ctx.InstallFile(installDir, manifest.Base(), manifest)
+	m.manifest = manifest.OutputPath
 
-	// 2) Module.config / AndroidTest.xml
+	// 2) Symlink to base.apk
+	baseApk := m.provider.OutputFile
+
+	// Typically looks like this for baseApk
+	// FrameworksServicesTests
+	// └── x86_64
+	//    └── FrameworksServicesTests.apk
+	symlinkName := fmt.Sprintf("%s/%s", ctx.DeviceConfig().DeviceArch(), baseApk.Base())
+	// Only android_test, not java_host_test puts the output in the DeviceArch dir.
+	if m.provider.IsHost || ctx.DeviceConfig().DeviceArch() == "" {
+		// testcases/CtsDevicePolicyManagerTestCases
+		// ├── CtsDevicePolicyManagerTestCases.jar
+		symlinkName = baseApk.Base()
+	}
+	target := installedBaseRelativeToHere(symlinkName, *m.tradefedProperties.Base)
+	installedApk := ctx.InstallAbsoluteSymlink(installDir, symlinkName, target)
+	m.supportFiles = append(m.supportFiles, installedApk)
+
+	// 3) Symlink for all data deps
+	// And like this for data files and required modules
+	// FrameworksServicesTests
+	// ├── data
+	// │   └── broken_shortcut.xml
+	// ├── JobTestApp.apk
+	for _, f := range m.provider.InstalledFiles {
+		symlinkName := f.Rel()
+		target := installedBaseRelativeToHere(symlinkName, *m.tradefedProperties.Base)
+		installedPath := ctx.InstallAbsoluteSymlink(installDir, symlinkName, target)
+		m.supportFiles = append(m.supportFiles, installedPath)
+	}
+
+	// 4) Module.config / AndroidTest.xml
 	m.testConfig = m.fixTestConfig(ctx, m.provider.TestConfig)
 }
 
 var _ android.AndroidMkEntriesProvider = (*testModuleConfigHostModule)(nil)
+
+// Given a relative path to a file in the current directory or a subdirectory,
+// return a relative path under our sibling directory named `base`.
+// There should be one "../" for each subdir we descend plus one to backup to "base".
+//
+//	 ThisDir/file1
+//	 ThisDir/subdir/file2
+//	would return "../base/file1" or "../../subdir/file2"
+func installedBaseRelativeToHere(targetFileName string, base string) string {
+	backup := strings.Repeat("../", strings.Count(targetFileName, "/")+1)
+	return fmt.Sprintf("%s%s/%s", backup, base, targetFileName)
+}

tradefed_modules/test_module_config_test.go

@@ -16,6 +16,7 @@ package tradefed_modules
 import (
 	"android/soong/android"
 	"android/soong/java"
+	"fmt"
 	"strconv"
 	"strings"
 	"testing"
@@ -69,15 +70,36 @@ func TestModuleConfigAndroidTest(t *testing.T) {
 	entries := android.AndroidMkEntriesForTest(t, ctx.TestContext, derived.Module())[0]
 
 	// Ensure some entries from base are there, specifically support files for data and helper apps.
-	assertEntryPairValues(t, entries.EntryMap["LOCAL_COMPATIBILITY_SUPPORT_FILES"], []string{"HelperApp.apk", "data/testfile"})
+	// Do not use LOCAL_COMPATIBILITY_SUPPORT_FILES, but instead use LOCAL_SOONG_INSTALLED_COMPATIBILITY_SUPPORT_FILES
+	android.AssertStringPathsRelativeToTopEquals(t, "support-files", ctx.Config,
+		[]string{"out/soong/target/product/test_device/testcases/derived_test/arm64/base.apk",
+			"out/soong/target/product/test_device/testcases/derived_test/HelperApp.apk",
+			"out/soong/target/product/test_device/testcases/derived_test/data/testfile"},
+		entries.EntryMap["LOCAL_SOONG_INSTALLED_COMPATIBILITY_SUPPORT_FILES"])
+	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_COMPATIBILITY_SUPPORT_FILES"], []string{})
+
+	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_REQUIRED_MODULES"], []string{"base"})
+	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_CERTIFICATE"], []string{"build/make/target/product/security/testkey.x509.pem"})
+	android.AssertStringEquals(t, "", entries.Class, "APPS")
 
 	// And some new derived entries are there.
 	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_MODULE_TAGS"], []string{"tests"})
 
-	// And ones we override
-	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_SOONG_JNI_LIBS_SYMBOLS"], []string{""})
-
 	android.AssertStringMatches(t, "", entries.EntryMap["LOCAL_FULL_TEST_CONFIG"][0], "derived_test/android_common/test_config_fixer/derived_test.config")
+
+	// Check the footer lines.  Our support files should depend on base's support files.
+	convertedActual := make([]string, 5)
+	for i, e := range entries.FooterLinesForTests() {
+		// AssertStringPathsRelativeToTop doesn't replace both instances
+		convertedActual[i] = strings.Replace(e, ctx.Config.SoongOutDir(), "", 2)
+	}
+	android.AssertArrayString(t, fmt.Sprintf("%s", ctx.Config.SoongOutDir()), convertedActual, []string{
+		"include $(BUILD_SYSTEM)/soong_app_prebuilt.mk",
+		"/target/product/test_device/testcases/derived_test/arm64/base.apk: /target/product/test_device/testcases/base/arm64/base.apk",
+		"/target/product/test_device/testcases/derived_test/HelperApp.apk: /target/product/test_device/testcases/base/HelperApp.apk",
+		"/target/product/test_device/testcases/derived_test/data/testfile: /target/product/test_device/testcases/base/data/testfile",
+		"",
+	})
 }
 
 // Make sure we call test-config-fixer with the right args.
@@ -92,7 +114,7 @@ func TestModuleConfigOptions(t *testing.T) {
 	derived := ctx.ModuleForTests("derived_test", "android_common")
 	rule_cmd := derived.Rule("fix_test_config").RuleParams.Command
 	android.AssertStringDoesContain(t, "Bad FixConfig rule inputs", rule_cmd,
-		`--test-file-name=derived_test.apk --orig-test-file-name=base.apk --test-runner-options='[{"Name":"exclude-filter","Key":"","Value":"android.test.example.devcodelab.DevCodelabTest#testHelloFail"},{"Name":"include-annotation","Key":"","Value":"android.platform.test.annotations.LargeTest"}]'`)
+		`--test-runner-options='[{"Name":"exclude-filter","Key":"","Value":"android.test.example.devcodelab.DevCodelabTest#testHelloFail"},{"Name":"include-annotation","Key":"","Value":"android.platform.test.annotations.LargeTest"}]'`)
 }
 
 // Ensure we error for a base we don't support.
@@ -195,6 +217,12 @@ func TestModuleConfigMultipleDerivedTestsWriteDistinctMakeEntries(t *testing.T)
 			name: "base",
 			sdk_version: "current",
                         srcs: ["a.java"],
+                        data: [":HelperApp", "data/testfile"],
+		}
+
+                android_test_helper_app {
+                        name: "HelperApp",
+                        srcs: ["helper.java"],
                 }
 
                 test_module_config {
@@ -220,8 +248,12 @@ func TestModuleConfigMultipleDerivedTestsWriteDistinctMakeEntries(t *testing.T)
 		derived := ctx.ModuleForTests("derived_test", "android_common")
 		entries := android.AndroidMkEntriesForTest(t, ctx.TestContext, derived.Module())[0]
 		// All these should be the same in both derived tests
-		assertEntryPairValues(t, entries.EntryMap["LOCAL_COMPATIBILITY_SUPPORT_FILES"], []string{"HelperApp.apk", "data/testfile"})
+		android.AssertStringPathsRelativeToTopEquals(t, "support-files", ctx.Config,
-		android.AssertArrayString(t, "", entries.EntryMap["LOCAL_SOONG_JNI_LIBS_SYMBOLS"], []string{""})
+			[]string{"out/soong/target/product/test_device/testcases/derived_test/arm64/base.apk",
+				"out/soong/target/product/test_device/testcases/derived_test/HelperApp.apk",
+				"out/soong/target/product/test_device/testcases/derived_test/data/testfile"},
+			entries.EntryMap["LOCAL_SOONG_INSTALLED_COMPATIBILITY_SUPPORT_FILES"])
+
 		// Except this one, which points to the updated tradefed xml file.
 		android.AssertStringMatches(t, "", entries.EntryMap["LOCAL_FULL_TEST_CONFIG"][0], "derived_test/android_common/test_config_fixer/derived_test.config")
 		// And this one, the module name.
@@ -232,8 +264,11 @@ func TestModuleConfigMultipleDerivedTestsWriteDistinctMakeEntries(t *testing.T)
 		derived := ctx.ModuleForTests("another_derived_test", "android_common")
 		entries := android.AndroidMkEntriesForTest(t, ctx.TestContext, derived.Module())[0]
 		// All these should be the same in both derived tests
-		assertEntryPairValues(t, entries.EntryMap["LOCAL_COMPATIBILITY_SUPPORT_FILES"], []string{"HelperApp.apk", "data/testfile"})
+		android.AssertStringPathsRelativeToTopEquals(t, "support-files", ctx.Config,
-		android.AssertArrayString(t, "", entries.EntryMap["LOCAL_SOONG_JNI_LIBS_SYMBOLS"], []string{""})
+			[]string{"out/soong/target/product/test_device/testcases/another_derived_test/arm64/base.apk",
+				"out/soong/target/product/test_device/testcases/another_derived_test/HelperApp.apk",
+				"out/soong/target/product/test_device/testcases/another_derived_test/data/testfile"},
+			entries.EntryMap["LOCAL_SOONG_INSTALLED_COMPATIBILITY_SUPPORT_FILES"])
 		// Except this one, which points to the updated tradefed xml file.
 		android.AssertStringMatches(t, "", entries.EntryMap["LOCAL_FULL_TEST_CONFIG"][0], "another_derived_test/android_common/test_config_fixer/another_derived_test.config")
 		// And this one, the module name.
@@ -269,6 +304,8 @@ func TestModuleConfigHostBasics(t *testing.T) {
 	allEntries := android.AndroidMkEntriesForTest(t, ctx.TestContext, mod)
 	entries := allEntries[0]
 	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_MODULE"], []string{"derived_test"})
+	android.AssertArrayString(t, "", entries.EntryMap["LOCAL_SDK_VERSION"], []string{"private_current"})
+	android.AssertStringEquals(t, "", entries.Class, "JAVA_LIBRARIES")
 
 	if !mod.Host() {
 		t.Errorf("host bit is not set for a java_test_host module.")
@@ -385,16 +422,3 @@ func TestTestOnlyProvider(t *testing.T) {
 		t.Errorf("test-only: Expected but not found: %v, Found but not expected: %v", left, right)
 	}
 }
-
-// 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) {
-	for i, e := range actual {
-		parts := strings.Split(e, ":")
-		if len(parts) != 2 {
-			t.Errorf("Expected entry to have a value delimited by :, received: %s", e)
-			return
-		}
-		android.AssertStringEquals(t, "", parts[1], expected[i])
-	}
-}