From bdf5d7170ac859e28f458c1d46c2b2494061de3f Mon Sep 17 00:00:00 2001 From: Pete Gillin Date: Mon, 21 Oct 2019 14:29:58 +0100 Subject: [PATCH] Refactor and strengthen sdk_test.go. This change: - Removes usage of the EXPERIMENTAL_JAVA_LANGUAGE_LEVEL_9=false option from tests, since it is going to be removed. The java_version property is used instead. - Commons up some of the assertions between the tests for language levels 8 and 9 in sdk_test.go. - In commoning the code up, some additional assertions are made in the language level 9 cases, strengthening the tests. Test: m nothing Bug: 115604102 Change-Id: I693c5d299b5592b851c44dde4434d92a931f15cd --- java/java_test.go | 68 ++++++++++++++++++++++------------ java/sdk_test.go | 94 +++++++++++++++++++++++++++++------------------ 2 files changed, 103 insertions(+), 59 deletions(-) diff --git a/java/java_test.go b/java/java_test.go index a3499ccd8..3767d1b24 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1073,32 +1073,32 @@ func checkPatchModuleFlag(t *testing.T, ctx *android.TestContext, moduleName str } func TestPatchModule(t *testing.T) { - bp := ` - java_library { - name: "foo", - srcs: ["a.java"], - } - - java_library { - name: "bar", - srcs: ["b.java"], - sdk_version: "none", - system_modules: "none", - patch_module: "java.base", - } - - java_library { - name: "baz", - srcs: ["c.java"], - patch_module: "java.base", - } - ` - t.Run("Java language level 8", func(t *testing.T) { // Test with legacy javac -source 1.8 -target 1.8 - config := testConfig(map[string]string{"EXPERIMENTAL_JAVA_LANGUAGE_LEVEL_9": "false"}) - ctx := testContext(bp, nil) - run(t, ctx, config) + bp := ` + java_library { + name: "foo", + srcs: ["a.java"], + java_version: "1.8", + } + + java_library { + name: "bar", + srcs: ["b.java"], + sdk_version: "none", + system_modules: "none", + patch_module: "java.base", + java_version: "1.8", + } + + java_library { + name: "baz", + srcs: ["c.java"], + patch_module: "java.base", + java_version: "1.8", + } + ` + ctx, _ := testJava(t, bp) checkPatchModuleFlag(t, ctx, "foo", "") checkPatchModuleFlag(t, ctx, "bar", "") @@ -1107,6 +1107,26 @@ func TestPatchModule(t *testing.T) { t.Run("Java language level 9", func(t *testing.T) { // Test with default javac -source 9 -target 9 + bp := ` + java_library { + name: "foo", + srcs: ["a.java"], + } + + java_library { + name: "bar", + srcs: ["b.java"], + sdk_version: "none", + system_modules: "none", + patch_module: "java.base", + } + + java_library { + name: "baz", + srcs: ["c.java"], + patch_module: "java.base", + } + ` ctx, _ := testJava(t, bp) checkPatchModuleFlag(t, ctx, "foo", "") diff --git a/java/sdk_test.go b/java/sdk_test.go index 5e0e592f6..fd47d81c9 100644 --- a/java/sdk_test.go +++ b/java/sdk_test.go @@ -206,7 +206,7 @@ func TestClasspath(t *testing.T) { moduleType = testcase.moduleType } - bp := moduleType + ` { + props := ` name: "foo", srcs: ["a.java"], target: { @@ -214,6 +214,10 @@ func TestClasspath(t *testing.T) { srcs: ["bar-doc/IFoo.aidl"], }, }, + ` + bp := moduleType + " {" + props + testcase.properties + ` + }` + bpJava8 := moduleType + " {" + props + `java_version: "1.8", ` + testcase.properties + ` }` @@ -233,45 +237,64 @@ func TestClasspath(t *testing.T) { bootclasspath := convertModulesToPaths(testcase.bootclasspath) classpath := convertModulesToPaths(testcase.classpath) - bc := strings.Join(bootclasspath, ":") - if bc != "" { - bc = "-bootclasspath " + bc + bc := "" + var bcDeps []string + if len(bootclasspath) > 0 { + bc = "-bootclasspath " + strings.Join(bootclasspath, ":") + if bootclasspath[0] != `""` { + bcDeps = bootclasspath + } } - c := strings.Join(classpath, ":") - if c != "" { - c = "-classpath " + c + c := "" + if len(classpath) > 0 { + c = "-classpath " + strings.Join(classpath, ":") } + system := "" + var systemDeps []string if testcase.system == "none" { system = "--system=none" + } else if testcase.system == "bootclasspath" { + system = bc + systemDeps = bcDeps } else if testcase.system != "" { system = "--system=" + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system") + // The module-relative parts of these paths are hardcoded in system_modules.go: + systemDeps = []string{ + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system", "lib", "modules"), + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system", "lib", "jrt-fs.jar"), + filepath.Join(buildDir, ".intermediates", testcase.system, "android_common", "system", "release"), + } } - checkClasspath := func(t *testing.T, ctx *android.TestContext) { + checkClasspath := func(t *testing.T, ctx *android.TestContext, isJava8 bool) { foo := ctx.ModuleForTests("foo", variant) javac := foo.Rule("javac") + var deps []string aidl := foo.MaybeRule("aidl") + if aidl.Rule != nil { + deps = append(deps, aidl.Output.String()) + } got := javac.Args["bootClasspath"] - if got != bc { - t.Errorf("bootclasspath expected %q != got %q", bc, got) + expected := "" + if isJava8 { + expected = bc + deps = append(deps, bcDeps...) + } else { + expected = system + deps = append(deps, systemDeps...) + } + if got != expected { + t.Errorf("bootclasspath expected %q != got %q", expected, got) } got = javac.Args["classpath"] if got != c { t.Errorf("classpath expected %q != got %q", c, got) } - - var deps []string - if aidl.Rule != nil { - deps = append(deps, aidl.Output.String()) - } - if len(bootclasspath) > 0 && bootclasspath[0] != `""` { - deps = append(deps, bootclasspath...) - } deps = append(deps, classpath...) if !reflect.DeepEqual(javac.Implicits.Strings(), deps) { @@ -281,17 +304,17 @@ func TestClasspath(t *testing.T) { // Test with legacy javac -source 1.8 -target 1.8 t.Run("Java language level 8", func(t *testing.T) { - config := testConfig(map[string]string{"EXPERIMENTAL_JAVA_LANGUAGE_LEVEL_9": "false"}) + config := testConfig(nil) if testcase.unbundled { config.TestProductVariables.Unbundled_build = proptools.BoolPtr(true) } if testcase.pdk { config.TestProductVariables.Pdk = proptools.BoolPtr(true) } - ctx := testContext(bp, nil) + ctx := testContext(bpJava8, nil) run(t, ctx, config) - checkClasspath(t, ctx) + checkClasspath(t, ctx, true /* isJava8 */) if testcase.host != android.Host { aidl := ctx.ModuleForTests("foo", variant).Rule("aidl") @@ -314,21 +337,20 @@ func TestClasspath(t *testing.T) { ctx := testContext(bp, nil) run(t, ctx, config) - javac := ctx.ModuleForTests("foo", variant).Rule("javac") - got := javac.Args["bootClasspath"] - expected := system - if testcase.system == "bootclasspath" { - expected = bc - } - if got != expected { - t.Errorf("bootclasspath expected %q != got %q", expected, got) + checkClasspath(t, ctx, false /* isJava8 */) + + if testcase.host != android.Host { + aidl := ctx.ModuleForTests("foo", variant).Rule("aidl") + + if g, w := aidl.RuleParams.Command, testcase.aidl+" -I."; !strings.Contains(g, w) { + t.Errorf("want aidl command to contain %q, got %q", w, g) + } } }) - // Test again with PLATFORM_VERSION_CODENAME=REL - t.Run("REL", func(t *testing.T) { - // TODO(b/115604102): This test should be rewritten with language level 9 - config := testConfig(map[string]string{"EXPERIMENTAL_JAVA_LANGUAGE_LEVEL_9": "false"}) + // Test again with PLATFORM_VERSION_CODENAME=REL, javac -source 8 -target 8 + t.Run("REL + Java language level 8", func(t *testing.T) { + config := testConfig(nil) config.TestProductVariables.Platform_sdk_codename = proptools.StringPtr("REL") config.TestProductVariables.Platform_sdk_final = proptools.BoolPtr(true) @@ -338,11 +360,13 @@ func TestClasspath(t *testing.T) { if testcase.pdk { config.TestProductVariables.Pdk = proptools.BoolPtr(true) } - ctx := testContext(bp, nil) + ctx := testContext(bpJava8, nil) run(t, ctx, config) - checkClasspath(t, ctx) + checkClasspath(t, ctx, true /* isJava8 */) }) + + // TODO(b/142896162): Add a with PLATFORM_VERSION_CODENAME=REL, javac -source 9 -target 9, when that all works. }) }