From 41805bedbfb53ae03ac6d017402bb082ae8ea050 Mon Sep 17 00:00:00 2001 From: Chih-Hung Hsieh Date: Thu, 31 Oct 2019 20:56:47 -0700 Subject: [PATCH] Add TestProperties, gen test config, fix names * Rename testBinaryDecorator to testDecorator * Add TestProperties * Add install function for testDecorator to install config files * Add tradefed.AutoGenRustHostTestConfig * Depend on new build/make/core/rust_host_test_config_template.xml and new tradefed.testtype.rust.RustBinaryHostTest class * Add autogenTemplateWithName in tradefed/autogen.go to generate config files with customized(mutated) executable name. * Make rust_test module names more robust and easy to use. * Use crate name instead of source file name as the Stem for single source file modules, to match original user specified output file name in Cargo.toml. * Do not set up test module SubName when Stem is empty or when the module name already contains Stem suffix. That happens when TestPerSrcMutator is disabled or when there is only one source file with renamed output file name. * In TEST_MAPPING, references to mutated rust_test modules should be (1) for single source file modules without mutation, or (2) _ for single source file modules, or (3) _ for multi-file modules. Bug: 140938178 Test: mm in rust projects, check output test file names Change-Id: Ifdbfa14d5eed4f10b4fb983f82c93bbb9be3f899 --- rust/androidmk.go | 18 +++++++++-- rust/rust.go | 1 + rust/test.go | 77 +++++++++++++++++++++++++++++++++++++------- rust/test_test.go | 20 ++++++++++++ tradefed/autogen.go | 21 +++++++++++- tradefed/config.go | 1 + tradefed/makevars.go | 1 + 7 files changed, 124 insertions(+), 15 deletions(-) diff --git a/rust/androidmk.go b/rust/androidmk.go index 49115f219..2a9a6c0e2 100644 --- a/rust/androidmk.go +++ b/rust/androidmk.go @@ -87,9 +87,23 @@ func (binary *binaryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.Andr }) } -func (test *testBinaryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkData) { +func (test *testDecorator) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkData) { test.binaryDecorator.AndroidMk(ctx, ret) - ret.SubName = "_" + String(test.baseCompiler.Properties.Stem) + stem := String(test.baseCompiler.Properties.Stem) + if stem != "" && !strings.HasSuffix(ctx.Name(), "_"+stem) { + // Avoid repeated suffix in the module name. + ret.SubName = "_" + stem + } + ret.Extra = append(ret.Extra, func(w io.Writer, outputFile android.Path) { + if len(test.Properties.Test_suites) > 0 { + fmt.Fprintln(w, "LOCAL_COMPATIBILITY_SUITE :=", + strings.Join(test.Properties.Test_suites, " ")) + } + if test.testConfig != nil { + fmt.Fprintln(w, "LOCAL_FULL_TEST_CONFIG :=", test.testConfig.String()) + } + }) + // TODO(chh): add test data with androidMkWriteTestData(test.data, ctx, ret) } func (library *libraryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkData) { diff --git a/rust/rust.go b/rust/rust.go index 612e25727..aa11d80a9 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -218,6 +218,7 @@ func DefaultsFactory(props ...interface{}) android.Module { &LibraryCompilerProperties{}, &ProcMacroCompilerProperties{}, &PrebuiltProperties{}, + &TestProperties{}, ) android.InitDefaultsModule(module) diff --git a/rust/test.go b/rust/test.go index 816e3c731..cb64e8f83 100644 --- a/rust/test.go +++ b/rust/test.go @@ -19,19 +19,41 @@ import ( "strings" "android/soong/android" + "android/soong/tradefed" ) +type TestProperties struct { + // the name of the test configuration (for example "AndroidTest.xml") that should be + // installed with the module. + Test_config *string `android:"arch_variant"` + + // the name of the test configuration template (for example "AndroidTestTemplate.xml") that + // should be installed with the module. + Test_config_template *string `android:"arch_variant"` + + // list of compatibility suites (for example "cts", "vts") that the module should be + // installed into. + Test_suites []string `android:"arch_variant"` + + // Flag to indicate whether or not to create test config automatically. If AndroidTest.xml + // doesn't exist next to the Android.bp, this attribute doesn't need to be set to true + // explicitly. + Auto_gen_config *bool +} + // A test module is a binary module with extra --test compiler flag // and different default installation directory. // In golang, inheriance is written as a component. -type testBinaryDecorator struct { +type testDecorator struct { *binaryDecorator + Properties TestProperties + testConfig android.Path } -func NewRustTest(hod android.HostOrDeviceSupported) (*Module, *testBinaryDecorator) { +func NewRustTest(hod android.HostOrDeviceSupported) (*Module, *testDecorator) { module := newModule(hod, android.MultilibFirst) - test := &testBinaryDecorator{ + test := &testDecorator{ binaryDecorator: &binaryDecorator{ // TODO(chh): set up dir64? baseCompiler: NewBaseCompiler("testcases", ""), @@ -43,7 +65,27 @@ func NewRustTest(hod android.HostOrDeviceSupported) (*Module, *testBinaryDecorat return module, test } -func (test *testBinaryDecorator) compilerFlags(ctx ModuleContext, flags Flags) Flags { +func (test *testDecorator) compilerProps() []interface{} { + return append(test.binaryDecorator.compilerProps(), &test.Properties) +} + +func (test *testDecorator) install(ctx ModuleContext, file android.Path) { + name := ctx.ModuleName() // default executable name + if stem := String(test.baseCompiler.Properties.Stem); stem != "" { + name = stem + } + if path := test.baseCompiler.relativeInstallPath(); path != "" { + name = path + "/" + name + } + test.testConfig = tradefed.AutoGenRustHostTestConfig(ctx, name, + test.Properties.Test_config, + test.Properties.Test_config_template, + test.Properties.Test_suites, + test.Properties.Auto_gen_config) + test.binaryDecorator.install(ctx, file) +} + +func (test *testDecorator) compilerFlags(ctx ModuleContext, flags Flags) Flags { flags = test.binaryDecorator.compilerFlags(ctx, flags) flags.RustFlags = append(flags.RustFlags, "--test") return flags @@ -65,21 +107,21 @@ func RustTestHostFactory() android.Module { return module.Init() } -func (test *testBinaryDecorator) testPerSrc() bool { +func (test *testDecorator) testPerSrc() bool { return true } -func (test *testBinaryDecorator) srcs() []string { - return test.Properties.Srcs +func (test *testDecorator) srcs() []string { + return test.binaryDecorator.Properties.Srcs } -func (test *testBinaryDecorator) setSrc(name, src string) { - test.Properties.Srcs = []string{src} +func (test *testDecorator) setSrc(name, src string) { + test.binaryDecorator.Properties.Srcs = []string{src} test.baseCompiler.Properties.Stem = StringPtr(name) } -func (test *testBinaryDecorator) unsetSrc() { - test.Properties.Srcs = nil +func (test *testDecorator) unsetSrc() { + test.binaryDecorator.Properties.Srcs = nil test.baseCompiler.Properties.Stem = StringPtr("") } @@ -90,7 +132,7 @@ type testPerSrc interface { unsetSrc() } -var _ testPerSrc = (*testBinaryDecorator)(nil) +var _ testPerSrc = (*testDecorator)(nil) func TestPerSrcMutator(mctx android.BottomUpMutatorContext) { if m, ok := mctx.Module().(*Module); ok { @@ -101,10 +143,21 @@ func TestPerSrcMutator(mctx android.BottomUpMutatorContext) { mctx.PropertyErrorf("srcs", "found a duplicate entry %q", duplicate) return } + // Rust compiler always compiles one source file at a time and + // uses the crate name as output file name. + // Cargo uses the test source file name as default crate name, + // but that can be redefined. + // So when there are multiple source files, the source file names will + // be the output file names, but when there is only one test file, + // use the crate name. testNames := make([]string, numTests) for i, src := range test.srcs() { testNames[i] = strings.TrimSuffix(filepath.Base(src), filepath.Ext(src)) } + crateName := m.compiler.crateName() + if numTests == 1 && crateName != "" { + testNames[0] = crateName + } // TODO(chh): Add an "all tests" variation like cc/test.go? tests := mctx.CreateLocalVariations(testNames...) for i, src := range test.srcs() { diff --git a/rust/test_test.go b/rust/test_test.go index aa4c3c8cb..f131c6ebe 100644 --- a/rust/test_test.go +++ b/rust/test_test.go @@ -25,6 +25,7 @@ func TestRustTest(t *testing.T) { rust_test_host { name: "my_test", srcs: ["foo.rs", "src/bar.rs"], + crate_name: "new_test", // not used for multiple source files relative_install_path: "rust/my-test", }`) @@ -41,3 +42,22 @@ func TestRustTest(t *testing.T) { } } } + +// crate_name is output file name, when there is only one source file. +func TestRustTestSingleFile(t *testing.T) { + ctx := testRust(t, ` + rust_test_host { + name: "my-test", + srcs: ["foo.rs"], + crate_name: "new_test", + relative_install_path: "my-pkg", + }`) + + name := "new_test" + testingModule := ctx.ModuleForTests("my-test", "linux_glibc_x86_64_"+name) + outPath := "/my-test/linux_glibc_x86_64_" + name + "/" + name + testingBuildParams := testingModule.Output(name) + if !strings.Contains(testingBuildParams.Output.String(), outPath) { + t.Errorf("wrong output: %v expect: %v", testingBuildParams.Output, outPath) + } +} diff --git a/tradefed/autogen.go b/tradefed/autogen.go index 3d30cfa13..905acfacb 100644 --- a/tradefed/autogen.go +++ b/tradefed/autogen.go @@ -105,6 +105,10 @@ func (ob Object) Config() string { } func autogenTemplate(ctx android.ModuleContext, output android.WritablePath, template string, configs []Config) { + autogenTemplateWithName(ctx, ctx.ModuleName(), output, template, configs) +} + +func autogenTemplateWithName(ctx android.ModuleContext, name string, output android.WritablePath, template string, configs []Config) { var configStrings []string for _, config := range configs { configStrings = append(configStrings, config.Config()) @@ -117,7 +121,7 @@ func autogenTemplate(ctx android.ModuleContext, output android.WritablePath, tem Description: "test config", Output: output, Args: map[string]string{ - "name": ctx.ModuleName(), + "name": name, "template": template, "extraConfigs": extraConfigs, }, @@ -193,6 +197,21 @@ func AutoGenPythonBinaryHostTestConfig(ctx android.ModuleContext, testConfigProp return path } +func AutoGenRustHostTestConfig(ctx android.ModuleContext, name string, testConfigProp *string, + testConfigTemplateProp *string, testSuites []string, autoGenConfig *bool) android.Path { + path, autogenPath := testConfigPath(ctx, testConfigProp, testSuites, autoGenConfig) + if autogenPath != nil { + templatePathString := "${RustHostTestConfigTemplate}" + templatePath := getTestConfigTemplate(ctx, testConfigTemplateProp) + if templatePath.Valid() { + templatePathString = templatePath.String() + } + autogenTemplateWithName(ctx, name, autogenPath, templatePathString, nil) + return autogenPath + } + return path +} + var autogenInstrumentationTest = pctx.StaticRule("autogenInstrumentationTest", blueprint.RuleParams{ Command: "${AutoGenTestConfigScript} $out $in ${EmptyTestConfig} $template", CommandDeps: []string{ diff --git a/tradefed/config.go b/tradefed/config.go index 141e0c570..8249ffe81 100644 --- a/tradefed/config.go +++ b/tradefed/config.go @@ -31,6 +31,7 @@ func init() { pctx.SourcePathVariable("NativeHostTestConfigTemplate", "build/make/core/native_host_test_config_template.xml") pctx.SourcePathVariable("NativeTestConfigTemplate", "build/make/core/native_test_config_template.xml") pctx.SourcePathVariable("PythonBinaryHostTestConfigTemplate", "build/make/core/python_binary_host_test_config_template.xml") + pctx.SourcePathVariable("RustHostTestConfigTemplate", "build/make/core/rust_host_test_config_template.xml") pctx.SourcePathVariable("EmptyTestConfig", "build/make/core/empty_test_config.xml") } diff --git a/tradefed/makevars.go b/tradefed/makevars.go index aad7273ec..e6b88ea1d 100644 --- a/tradefed/makevars.go +++ b/tradefed/makevars.go @@ -31,6 +31,7 @@ func makeVarsProvider(ctx android.MakeVarsContext) { ctx.Strict("NATIVE_HOST_TEST_CONFIG_TEMPLATE", "${NativeHostTestConfigTemplate}") ctx.Strict("NATIVE_TEST_CONFIG_TEMPLATE", "${NativeTestConfigTemplate}") ctx.Strict("PYTHON_BINARY_HOST_TEST_CONFIG_TEMPLATE", "${PythonBinaryHostTestConfigTemplate}") + ctx.Strict("RUST_HOST_TEST_CONFIG_TEMPLATE", "${RustHostTestConfigTemplate}") ctx.Strict("EMPTY_TEST_CONFIG", "${EmptyTestConfig}") }