From 5e6a79798280bec894ffe8ab5b7e9f30e2eca0eb Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 7 Jun 2020 16:56:32 -0700 Subject: [PATCH 1/2] Allow tests to bypass PathForSource existence checks Forcing every test to specify every file it wants to pass to PathForSource or PathForModuleSrc is painful to maintain and doesn't add any value. Allow tests to reference paths through PathForSource and PathForModuleSrc without specifying them in the mock FS. Test: all soong tests Change-Id: Ia8a8fd965a338d0645b3721314bf91f50146ad21 --- android/config.go | 8 ++++++++ android/paths.go | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/android/config.go b/android/config.go index 59118ce86..a37afe897 100644 --- a/android/config.go +++ b/android/config.go @@ -111,6 +111,10 @@ type config struct { fs pathtools.FileSystem mockBpList string + // If testAllowNonExistentPaths is true then PathForSource and PathForModuleSrc won't error + // in tests when a path doesn't exist. + testAllowNonExistentPaths bool + OncePer } @@ -230,6 +234,10 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string buildDir: buildDir, captureBuild: true, env: envCopy, + + // Set testAllowNonExistentPaths so that test contexts don't need to specify every path + // passed to PathForSource or PathForModuleSrc. + testAllowNonExistentPaths: true, } config.deviceConfig = &deviceConfig{ config: config, diff --git a/android/paths.go b/android/paths.go index 3ad27acbe..bed6f3f58 100644 --- a/android/paths.go +++ b/android/paths.go @@ -405,7 +405,7 @@ func expandOneSrcPath(ctx ModuleContext, s string, expandedExcludes []string) (P p := pathForModuleSrc(ctx, s) if exists, _, err := ctx.Config().fs.Exists(p.String()); err != nil { reportPathErrorf(ctx, "%s: %s", p, err.Error()) - } else if !exists { + } else if !exists && !ctx.Config().testAllowNonExistentPaths { reportPathErrorf(ctx, "module source path %q does not exist", p) } @@ -798,7 +798,7 @@ func PathForSource(ctx PathContext, pathComponents ...string) SourcePath { } } else if exists, _, err := ctx.Config().fs.Exists(path.String()); err != nil { reportPathErrorf(ctx, "%s: %s", path, err.Error()) - } else if !exists { + } else if !exists && !ctx.Config().testAllowNonExistentPaths { reportPathErrorf(ctx, "source path %q does not exist", path) } return path From 238c1f3903eef027c7f1f9448bb6bcc6d4c669cd Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 7 Jun 2020 16:58:18 -0700 Subject: [PATCH 2/2] Remove most paths from java.TestConfig Now that tests don't need to specify every path passed to PathForSource or PathForModuleSrc, remove most of them from java.TestConfig. Leave a few that are globbed by lots of tests, and move a few that are globbed by a single test into the tests. Test: all soong tests Change-Id: Ica91d7203a6a7dbca0fd4fed84c78f149b8699e1 --- java/app_test.go | 9 +++++-- java/droiddoc.go | 2 +- java/java_test.go | 46 ++++++++++++++++++++++++++--------- java/testing.go | 62 ----------------------------------------------- 4 files changed, 42 insertions(+), 77 deletions(-) diff --git a/java/app_test.go b/java/app_test.go index eeba161b3..e45ba70d5 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2657,7 +2657,7 @@ func TestCodelessApp(t *testing.T) { } func TestEmbedNotice(t *testing.T) { - ctx, _ := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + ctx, _ := testJavaWithFS(t, cc.GatherRequiredDepsForTest(android.Android)+` android_app { name: "foo", srcs: ["a.java"], @@ -2713,7 +2713,12 @@ func TestEmbedNotice(t *testing.T) { srcs: ["b.java"], notice: "TOOL_NOTICE", } - `) + `, map[string][]byte{ + "APP_NOTICE": nil, + "GENRULE_NOTICE": nil, + "LIB_NOTICE": nil, + "TOOL_NOTICE": nil, + }) // foo has NOTICE files to process, and embed_notices is true. foo := ctx.ModuleForTests("foo", "android_common") diff --git a/java/droiddoc.go b/java/droiddoc.go index 4c3e11236..68cfe9fde 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -1470,7 +1470,7 @@ func metalavaCmd(ctx android.ModuleContext, rule *android.RuleBuilder, javaVersi FlagWithInput("@", srcJarList). FlagWithOutput("--strict-input-files:warn ", android.PathForModuleOut(ctx, ctx.ModuleName()+"-"+"violations.txt")) - if implicitsRsp.String() != "" { + if implicitsRsp != nil { cmd.FlagWithArg("--strict-input-files-exempt ", "@"+implicitsRsp.String()) } diff --git a/java/java_test.go b/java/java_test.go index 8ea34d975..215070e6f 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -142,9 +142,14 @@ func testJavaErrorWithConfig(t *testing.T, pattern string, config android.Config return ctx, config } +func testJavaWithFS(t *testing.T, bp string, fs map[string][]byte) (*android.TestContext, android.Config) { + t.Helper() + return testJavaWithConfig(t, testConfig(nil, bp, fs)) +} + func testJava(t *testing.T, bp string) (*android.TestContext, android.Config) { t.Helper() - return testJavaWithConfig(t, testConfig(nil, bp, nil)) + return testJavaWithFS(t, bp, nil) } func testJavaWithConfig(t *testing.T, config android.Config) (*android.TestContext, android.Config) { @@ -740,7 +745,7 @@ func TestResources(t *testing.T) { for _, test := range table { t.Run(test.name, func(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -750,7 +755,13 @@ func TestResources(t *testing.T) { ], `+test.prop+`, } - `+test.extra) + `+test.extra, + map[string][]byte{ + "java-res/a/a": nil, + "java-res/b/b": nil, + "java-res2/a": nil, + }, + ) foo := ctx.ModuleForTests("foo", "android_common").Output("withres/foo.jar") fooRes := ctx.ModuleForTests("foo", "android_common").Output("res/foo.jar") @@ -769,7 +780,7 @@ func TestResources(t *testing.T) { } func TestIncludeSrcs(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -790,7 +801,11 @@ func TestIncludeSrcs(t *testing.T) { java_resource_dirs: ["java-res"], include_srcs: true, } - `) + `, map[string][]byte{ + "java-res/a/a": nil, + "java-res/b/b": nil, + "java-res2/a": nil, + }) // Test a library with include_srcs: true foo := ctx.ModuleForTests("foo", "android_common").Output("withres/foo.jar") @@ -832,7 +847,7 @@ func TestIncludeSrcs(t *testing.T) { } func TestGeneratedSources(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` java_library { name: "foo", srcs: [ @@ -847,7 +862,10 @@ func TestGeneratedSources(t *testing.T) { tool_files: ["java-res/a"], out: ["gen.java"], } - `) + `, map[string][]byte{ + "a.java": nil, + "b.java": nil, + }) javac := ctx.ModuleForTests("foo", "android_common").Rule("javac") genrule := ctx.ModuleForTests("gen", "").Rule("generator") @@ -932,7 +950,7 @@ func TestSharding(t *testing.T) { } func TestDroiddoc(t *testing.T) { - ctx, _ := testJava(t, ` + ctx, _ := testJavaWithFS(t, ` droiddoc_exported_dir { name: "droiddoc-templates-sdk", path: ".", @@ -945,7 +963,7 @@ func TestDroiddoc(t *testing.T) { droiddoc { name: "bar-doc", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", "bar-doc/IFoo.aidl", ":bar-doc-aidl-srcs", ], @@ -963,7 +981,11 @@ func TestDroiddoc(t *testing.T) { todo_file: "libcore-docs-todo.html", args: "-offlinemode -title \"libcore\"", } - `) + `, + map[string][]byte{ + "bar-doc/a.java": nil, + "bar-doc/b.java": nil, + }) barDoc := ctx.ModuleForTests("bar-doc", "android_common").Rule("javadoc") var javaSrcs []string @@ -989,7 +1011,7 @@ func TestDroidstubsWithSystemModules(t *testing.T) { droidstubs { name: "stubs-source-system-modules", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", ], sdk_version: "none", system_modules: "source-system-modules", @@ -1010,7 +1032,7 @@ func TestDroidstubsWithSystemModules(t *testing.T) { droidstubs { name: "stubs-prebuilt-system-modules", srcs: [ - "bar-doc/*.java", + "bar-doc/a.java", ], sdk_version: "none", system_modules: "prebuilt-system-modules", diff --git a/java/testing.go b/java/testing.go index f993f56b3..f34c64af6 100644 --- a/java/testing.go +++ b/java/testing.go @@ -25,35 +25,12 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string bp += GatherRequiredDepsForTest() mockFS := map[string][]byte{ - "a.java": nil, - "b.java": nil, - "c.java": nil, - "b.kt": nil, - "a.jar": nil, - "b.jar": nil, - "c.jar": nil, - "APP_NOTICE": nil, - "GENRULE_NOTICE": nil, - "LIB_NOTICE": nil, - "TOOL_NOTICE": nil, - "AndroidTest.xml": nil, - "java-res/a/a": nil, - "java-res/b/b": nil, - "java-res2/a": nil, - "java-fg/a.java": nil, - "java-fg/b.java": nil, - "java-fg/c.java": nil, "api/current.txt": nil, "api/removed.txt": nil, "api/system-current.txt": nil, "api/system-removed.txt": nil, "api/test-current.txt": nil, "api/test-removed.txt": nil, - "framework/aidl/a.aidl": nil, - "aidl/foo/IFoo.aidl": nil, - "aidl/bar/IBar.aidl": nil, - "assets_a/a": nil, - "assets_b/b": nil, "prebuilts/sdk/14/public/android.jar": nil, "prebuilts/sdk/14/public/framework.aidl": nil, @@ -103,45 +80,6 @@ func TestConfig(buildDir string, env map[string]string, bp string, fs map[string "prebuilts/sdk/30/test/api/bar-removed.txt": nil, "prebuilts/sdk/tools/core-lambda-stubs.jar": nil, "prebuilts/sdk/Android.bp": []byte(`prebuilt_apis { name: "sdk", api_dirs: ["14", "28", "30", "current"],}`), - - "prebuilts/apk/app.apk": nil, - "prebuilts/apk/app_arm.apk": nil, - "prebuilts/apk/app_arm64.apk": nil, - "prebuilts/apk/app_xhdpi.apk": nil, - "prebuilts/apk/app_xxhdpi.apk": nil, - - "prebuilts/apks/app.apks": nil, - - // For framework-res, which is an implicit dependency for framework - "AndroidManifest.xml": nil, - "build/make/target/product/security/testkey": nil, - - "build/soong/scripts/jar-wrapper.sh": nil, - - "build/make/core/verify_uses_libraries.sh": nil, - - "build/make/core/proguard.flags": nil, - "build/make/core/proguard_basic_keeps.flags": nil, - - "jdk8/jre/lib/jce.jar": nil, - "jdk8/jre/lib/rt.jar": nil, - "jdk8/lib/tools.jar": nil, - - "bar-doc/a.java": nil, - "bar-doc/b.java": nil, - "bar-doc/IFoo.aidl": nil, - "bar-doc/IBar.aidl": nil, - "bar-doc/known_oj_tags.txt": nil, - "external/doclava/templates-sdk": nil, - - "cert/new_cert.x509.pem": nil, - "cert/new_cert.pk8": nil, - "lineage.bin": nil, - - "testdata/data": nil, - - "stubs-sources/foo/Foo.java": nil, - "stubs/sources/foo/Foo.java": nil, } cc.GatherRequiredFilesForTest(mockFS)