diff options
Diffstat (limited to 'src')
14 files changed, 319 insertions, 113 deletions
diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD index 1699cb882b..71a715a85a 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD +++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD @@ -17,6 +17,7 @@ java_library( "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model", "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding", "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/util", + "//src/main/protobuf:worker_protocol_java_proto", "//third_party:junit4", ], ) diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java index 74a6826f88..3f4c966d94 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java +++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java @@ -14,13 +14,20 @@ package com.google.testing.junit.runner; +import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; +import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; import com.google.testing.junit.runner.internal.StackTraces; import com.google.testing.junit.runner.junit4.JUnit4InstanceModules.Config; import com.google.testing.junit.runner.junit4.JUnit4InstanceModules.SuiteClass; import com.google.testing.junit.runner.junit4.JUnit4Runner; import com.google.testing.junit.runner.model.AntXmlResultWriter; import com.google.testing.junit.runner.model.XmlResultWriter; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.PrintStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; @@ -48,6 +55,9 @@ public class BazelTestRunner { */ static final String TEST_SUITE_PROPERTY_NAME = "bazel.test_suite"; + private static URL[] classpaths = null; + private static URLClassLoader targetClassLoader; + private BazelTestRunner() { // utility class; should not be instantiated } @@ -72,9 +82,8 @@ public class BazelTestRunner { String suiteClassName = System.getProperty(TEST_SUITE_PROPERTY_NAME); - if (args.length >= 1 && args[args.length - 1].equals("--persistent_test_runner")) { - System.err.println("Requested test strategy is currently unsupported."); - System.exit(1); + if ("true".equals(System.getenv("PERSISTENT_TEST_RUNNER"))) { + System.exit(runPersistentTestRunner(suiteClassName)); } if (!checkTestSuiteProperty(suiteClassName)) { @@ -126,7 +135,7 @@ public class BazelTestRunner { } private static int runTestsInSuite(String suiteClassName, String[] args) { - Class<?> suite = getTestClass(suiteClassName); + Class<?> suite = getTargetSuiteClass(suiteClassName); if (suite == null) { // No class found corresponding to the system property passed in from Bazel @@ -136,29 +145,123 @@ public class BazelTestRunner { } } - // TODO(kush): Use a new classloader for the following instantiation. JUnit4Runner runner = JUnit4Bazel.builder() .suiteClass(new SuiteClass(suite)) .config(new Config(args)) .build() .runner(); - return runner.run().wasSuccessful() ? 0 : 1; + + // Some frameworks such as Mockito use the Thread's context classloader. + Thread.currentThread().setContextClassLoader(targetClassLoader); + + int result = 1; + try { + result = runner.run().wasSuccessful() ? 0 : 1; + } catch (RuntimeException e) { + System.err.println("Test run failed with exception"); + e.printStackTrace(); + } + return result; } - private static Class<?> getTestClass(String name) { - if (name == null) { + /** + * Run in a loop awaiting instructions for the next test run. + * + * @param suiteClassName name of the class which is passed on to JUnit to determine the test suite + * @return 0 when we encounter an EOF from input, or non-zero values if we encounter an + * unrecoverable error. + */ + private static int runPersistentTestRunner(String suiteClassName) { + PrintStream originalStdOut = System.out; + PrintStream originalStdErr = System.err; + + while (true) { + try { + WorkRequest request = WorkRequest.parseDelimitedFrom(System.in); + + if (request == null) { + break; + } + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + PrintStream printStream = new PrintStream(outputStream, true); + System.setOut(printStream); + System.setErr(printStream); + String[] arguments = request.getArgumentsList().toArray(new String[0]); + int exitCode = -1; + try { + exitCode = runTestsInSuite(suiteClassName, arguments); + } finally { + System.setOut(originalStdOut); + System.setErr(originalStdErr); + } + + WorkResponse response = + WorkResponse + .newBuilder() + .setOutput(outputStream.toString()) + .setExitCode(exitCode) + .build(); + response.writeDelimitedTo(System.out); + System.out.flush(); + + } catch (IOException e) { + e.printStackTrace(); + return 1; + } + } + return 0; + } + + /** + * Get the actual Test Suite class corresponding to the given name. + */ + private static Class<?> getTargetSuiteClass(String suiteClassName) { + if (suiteClassName == null) { return null; } try { - return Class.forName(name); - } catch (ClassNotFoundException e) { + targetClassLoader = new URLClassLoader(getClasspaths()); + Class<?> targetSuiteClass = targetClassLoader.loadClass(suiteClassName); + System.out.printf( + "Running test suites for class: %s, created by classLoader: %s%n", + targetSuiteClass, targetSuiteClass.getClassLoader()); + return targetSuiteClass; + } catch (ClassNotFoundException | MalformedURLException e) { + System.err.println("Exception in loading class:" + e.getMessage()); return null; } } /** + * Used to get the classpaths which should be used to load the classes of the test target. + * + * @throws MalformedURLException when we are unable to create a given classpath. + * @return array of URLs containing the classpaths or null if classpaths could not be located. + */ + private static URL[] getClasspaths() throws MalformedURLException { + // TODO(kush): WARNING THIS DOES NOT RELOAD CLASSPATHS FOR EVERY TEST RUN. b/34712039 + if (classpaths != null) { + return classpaths; + } + String testTargetsClaspaths = System.getenv("TEST_TARGET_CLASSPATH"); + if (testTargetsClaspaths == null || testTargetsClaspaths.isEmpty()) { + throw new IllegalStateException( + "Target's classpath not present in TEST_TARGET_CLASSPATH environment variable"); + } + + String[] targetClassPaths = testTargetsClaspaths.split(":"); + + classpaths = new URL[targetClassPaths.length]; + String workingDir = System.getProperty("user.dir"); + for (int index = 0; index < targetClassPaths.length; index++) { + classpaths[index] = new URL("file://" + workingDir + "/" + targetClassPaths[index]); + } + return classpaths; + } + + /** * Prints out stack traces if the JVM does not exit quickly. This can help detect shutdown hooks * that are preventing the JVM from exiting quickly. * diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD index 95327df8ab..3ba2806cf2 100644 --- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD +++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD @@ -68,8 +68,12 @@ sh_test( args = [ "$(location :TestbedBinary)", "bazel.test_suite", + "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)", + ], + data = [ + ":TestbedBinary", + "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar", ], - data = [":TestbedBinary"], shard_count = 0, deps = [":testenv"], ) @@ -81,8 +85,12 @@ sh_test( args = [ "$(location :TestbedBinary)", "bazel.test_suite", + "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)", + ], + data = [ + ":TestbedBinary", + "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar", ], - data = [":TestbedBinary"], shard_count = 0, deps = [":testenv"], ) @@ -95,10 +103,12 @@ sh_test( "$(location :TestbedBinary)", "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:XmlOutputExercises.ant.xml)", "bazel.test_suite", + "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)", ], data = [ ":TestbedBinary", "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:XmlOutputExercises.ant.xml", + "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar", ], shard_count = 0, deps = [":testenv"], diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh index 609518a6bf..b79ccf14fe 100755 --- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh +++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh @@ -25,6 +25,7 @@ XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml" SUITE_PARAMETER="$3" SUITE="com.google.testing.junit.runner.testbed.XmlOutputExercises" SUITE_FLAG="-D${SUITE_PARAMETER}=${SUITE}" +TESTBED_LIB_LOCATION="${PWD}/$4" shift 3 source ${DIR}/testenv.sh || { echo "testenv.sh not found!" >&2; exit 1; } @@ -32,6 +33,9 @@ source ${DIR}/testenv.sh || { echo "testenv.sh not found!" >&2; exit 1; } function test_XmlOutputExercises() { cd $TEST_TMPDIR + # Pass the test target classpath through the environment variable + declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION" + $TESTBED --jvm_flag=${SUITE_FLAG} || true # Test failures # Remove timestamps and test runtime from the XML files as they will always differ and cause a diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh index b44c34783b..470fece506 100755 --- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh +++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh @@ -25,6 +25,7 @@ DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) TESTBED="${PWD}/$1" SUITE_PARAMETER="$2" +TESTBED_LIB_LOCATION="${PWD}/$3" SUITE_FLAG="-D${SUITE_PARAMETER}=com.google.testing.junit.runner.testbed.JUnit4TestbridgeExercises" XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml" @@ -42,6 +43,9 @@ function set_up() { function test_Junit4() { cd $TEST_TMPDIR + # Pass the test target classpath through the environment variable + declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION" + # Run the test without environment flag; it should fail. declare +x TESTBRIDGE_TEST_ONLY $TESTBED --jvm_flag=${SUITE_FLAG} >& $TEST_log && fail "Expected failure" @@ -66,6 +70,9 @@ function test_Junit4() { function test_Junit4FlagOverridesEnv() { cd $TEST_TMPDIR + # Pass the test target classpath through the environment variable + declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION" + # Run the test with both environment and command line flags. declare -x TESTBRIDGE_TEST_ONLY="doNotRun" $TESTBED --jvm_flag=${SUITE_FLAG} --test_filter doRun >& $TEST_log || \ diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh index 3b2c47c7bb..96dfaee784 100755 --- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh +++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh @@ -23,6 +23,7 @@ TESTBED="${PWD}/$1" SUITE_PARAMETER="$2" SUITE_FLAG="-D${SUITE_PARAMETER}=com.google.testing.junit.runner.testbed.InternationalCharsTest" XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml" +TESTBED_LIB_LOCATION="${PWD}/$3" unset TEST_PREMATURE_EXIT_FILE shift 2 @@ -35,6 +36,9 @@ function expect_log() { } function test_utf8_log() { + # Pass the test target classpath through the environment variable + declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION" + $TESTBED --jvm_flag=${SUITE_FLAG} > $TEST_log && fail "Expected failure" expect_log 'expected:<Test [Japan].> but was:<Test [日本].>' diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index f5fa1ff7ae..d988eb31ae 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; -import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction; @@ -33,18 +32,22 @@ import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Su import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Template; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.bazel.rules.BazelConfiguration; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; import com.google.devtools.build.lib.rules.java.JavaCommon; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; import com.google.devtools.build.lib.rules.java.JavaConfiguration; import com.google.devtools.build.lib.rules.java.JavaHelper; import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; -import com.google.devtools.build.lib.rules.java.JavaRunfilesProvider; import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; import com.google.devtools.build.lib.rules.java.JavaTargetAttributes; @@ -81,6 +84,8 @@ public class BazelJavaSemantics implements JavaSemantics { private static final String JACOCO_COVERAGE_RUNNER_MAIN_CLASS = "com.google.testing.coverage.JacocoCoverageRunner"; + private static final String BAZEL_TEST_RUNNER = "com.google.testing.junit.runner.BazelTestRunner"; + private BazelJavaSemantics() { } @@ -103,10 +108,20 @@ public class BazelJavaSemantics implements JavaSemantics { private static final String JUNIT4_RUNNER = "org.junit.runner.JUnitCore"; + @Nullable private String getMainClassInternal(RuleContext ruleContext, ImmutableList<Artifact> sources) { if (!ruleContext.attributes().get("create_executable", Type.BOOLEAN)) { return null; } + String mainClass = determineMainClassFromRule(ruleContext); + + if (mainClass.isEmpty()) { + mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources); + } + return mainClass; + } + + private String determineMainClassFromRule(RuleContext ruleContext) { String mainClass = ruleContext.attributes().get("main_class", Type.STRING); // Legacy behavior for java_test rules: main_class defaulted to JUnit4 runner. @@ -120,9 +135,8 @@ public class BazelJavaSemantics implements JavaSemantics { if (mainClass.isEmpty()) { if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN) && !useLegacyJavaTest(ruleContext)) { - return "com.google.testing.junit.runner.BazelTestRunner"; + return BAZEL_TEST_RUNNER; } - mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources); } return mainClass; } @@ -164,10 +178,55 @@ public class BazelJavaSemantics implements JavaSemantics { return ruleContext.getPrerequisiteArtifacts("resources", Mode.TARGET).list(); } + /** + * Used to generate the Classpaths contained within the stub. + */ + private static class ComputedClasspathSubstitution extends ComputedSubstitution { + private final NestedSet<Artifact> jars; + private final String workspacePrefix; + private final boolean isRunfilesEnabled; + + ComputedClasspathSubstitution( + String key, NestedSet<Artifact> jars, String workspacePrefix, boolean isRunfilesEnabled) { + super(key); + this.jars = jars; + this.workspacePrefix = workspacePrefix; + this.isRunfilesEnabled = isRunfilesEnabled; + } + + /** + * Builds a class path by concatenating the root relative paths of the artifacts. Each relative + * path entry is prepended with "${RUNPATH}" which will be expanded by the stub script at + * runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string. + */ + @Override + public String getValue() { + StringBuilder buffer = new StringBuilder(); + buffer.append("\""); + for (Artifact artifact : jars) { + if (buffer.length() > 1) { + buffer.append(File.pathSeparatorChar); + } + if (!isRunfilesEnabled) { + buffer.append("$(rlocation "); + PathFragment runfilePath = + new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath()); + buffer.append(runfilePath.normalize().getPathString()); + buffer.append(")"); + } else { + buffer.append("${RUNPATH}"); + buffer.append(artifact.getRunfilesPath().getPathString()); + } + } + buffer.append("\""); + return buffer.toString(); + } + } + @Override public Artifact createStubAction( RuleContext ruleContext, - final JavaCommon javaCommon, + JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -188,18 +247,26 @@ public class BazelJavaSemantics implements JavaSemantics { arguments.add(Substitution.of("%javabin%", javaExecutable)); arguments.add(Substitution.of("%needs_runfiles%", ruleContext.getFragment(Jvm.class).getJavaExecutable().isAbsolute() ? "0" : "1")); + + NestedSet<Artifact> classpath = javaCommon.getRuntimeClasspath(); + NestedSet<Artifact> testTargetClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER); + TransitiveInfoCollection testSupport = getTestSupport(ruleContext); + if (TargetUtils.isTestRule(ruleContext.getRule()) + && determineMainClassFromRule(ruleContext).equals(BAZEL_TEST_RUNNER) + && testSupport != null) { + // Keep only the locations containing the classes to start the test runner itself within, + // classpath variable, and place all the paths required for the test run in + // testTargetClasspath, so that the classes for the test target may be loaded by a separate + // ClassLoader. + testTargetClasspath = classpath; + classpath = getRuntimeJarsForTargets(testSupport); + } arguments.add( - new ComputedSubstitution("%classpath%") { - @Override - public String getValue() { - StringBuilder buffer = new StringBuilder(); - Iterable<Artifact> jars = javaCommon.getRuntimeClasspath(); - char delimiter = File.pathSeparatorChar; - appendRunfilesRelativeEntries( - buffer, jars, workspacePrefix, delimiter, isRunfilesEnabled); - return buffer.toString(); - } - }); + new ComputedClasspathSubstitution( + "%classpath%", classpath, workspacePrefix, isRunfilesEnabled)); + arguments.add( + new ComputedClasspathSubstitution( + "%test_target_classpath%", testTargetClasspath, workspacePrefix, isRunfilesEnabled)); JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts(); String path = @@ -250,40 +317,7 @@ public class BazelJavaSemantics implements JavaSemantics { } } - /** - * Builds a class path by concatenating the root relative paths of the artifacts separated by the - * delimiter. Each relative path entry is prepended with "${RUNPATH}" which will be expanded by - * the stub script at runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string. - * - * @param buffer the buffer to use for concatenating the entries - * @param artifacts the entries to concatenate in the buffer - * @param delimiter the delimiter character to separate the entries - */ - private static void appendRunfilesRelativeEntries( - StringBuilder buffer, - Iterable<Artifact> artifacts, - String workspacePrefix, - char delimiter, - boolean isRunfilesEnabled) { - buffer.append("\""); - for (Artifact artifact : artifacts) { - if (buffer.length() > 1) { - buffer.append(delimiter); - } - if (!isRunfilesEnabled) { - buffer.append("$(rlocation "); - PathFragment runfilePath = - new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath()); - buffer.append(runfilePath.normalize().getPathString()); - buffer.append(")"); - } else { - buffer.append("${RUNPATH}"); - buffer.append(artifact.getRunfilesPath().getPathString()); - } - } - buffer.append("\""); - } - + @Nullable private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) { if (!isJavaBinaryOrJavaTest(ruleContext)) { return null; @@ -300,13 +334,25 @@ public class BazelJavaSemantics implements JavaSemantics { } } + private static NestedSet<Artifact> getRuntimeJarsForTargets(TransitiveInfoCollection... deps) { + // The dep may be a simple JAR and not a java rule, hence we can't simply do + // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(), + // so we reuse the logic within JavaCompilationArgs to handle both scenarios. + JavaCompilationArgs args = + JavaCompilationArgs.builder() + .addTransitiveTargets( + ImmutableList.copyOf(deps), /*recursive=*/ true, ClasspathType.RUNTIME_ONLY) + .build(); + return args.getRuntimeJars(); + } + @Override public void addRunfilesForBinary(RuleContext ruleContext, Artifact launcher, Runfiles.Builder runfilesBuilder) { TransitiveInfoCollection testSupport = getTestSupport(ruleContext); if (testSupport != null) { - runfilesBuilder.addTarget(testSupport, JavaRunfilesProvider.TO_RUNFILES); - runfilesBuilder.addTarget(testSupport, RunfilesProvider.DEFAULT_RUNFILES); + // Not using addTransitiveArtifacts() due to the mismatch in NestedSet ordering. + runfilesBuilder.addArtifacts(getRuntimeJarsForTargets(testSupport)); } } @@ -316,12 +362,16 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public void collectTargetsTreatedAsDeps( - RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder) { + RuleContext ruleContext, + ImmutableList.Builder<TransitiveInfoCollection> builder, + ClasspathType type) { + if (type == ClasspathType.COMPILE_ONLY) { + // We add the test support below, but the test framework's deps are not relevant for + // COMPILE_ONLY, hence we return here. + return; + } TransitiveInfoCollection testSupport = getTestSupport(ruleContext); if (testSupport != null) { - // TODO(bazel-team): The testsupport is used as the test framework - // and really only needs to be on the runtime, not compile-time - // classpath. builder.add(testSupport); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt index cdc7ee4aa7..312976f586 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt @@ -228,6 +228,10 @@ else CLASSPATH=%classpath% fi +# Export the classpaths which would be used to load the classes of the test target as an +# environment variable. +export TEST_TARGET_CLASSPATH=%test_target_classpath% + # If using Jacoco in offline instrumentation mode, the CLASSPATH contains instrumented files. # We need to make the metadata jar with uninstrumented classes available for generating # the lcov-compatible coverage report, and we don't want it on the classpath. diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java index 03c6a594a3..8056e7cb0d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java @@ -95,10 +95,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory { } } - List<TransitiveInfoCollection> deps = - // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference. - Lists.<TransitiveInfoCollection>newArrayList( - common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)); semantics.checkRule(ruleContext, common); semantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, common); String mainClass = semantics.getMainClass(ruleContext, common.getSrcsArtifacts()); @@ -110,6 +106,10 @@ public class JavaBinary implements RuleConfiguredTargetFactory { // Collect the transitive dependencies. JavaCompilationHelper helper = new JavaCompilationHelper( ruleContext, semantics, common.getJavacOpts(), attributesBuilder); + List<TransitiveInfoCollection> deps = + // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference. + Lists.<TransitiveInfoCollection>newArrayList( + common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)); helper.addLibrariesToAttributes(deps); attributesBuilder.addNativeLibraries( collectNativeLibraries(common.targetsTreatedAsDeps(ClasspathType.BOTH))); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 344793423e..b6f0f2b448 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -664,7 +664,7 @@ public class JavaCommon { } builder.addAll(ruleContext.getPrerequisites("deps", Mode.TARGET)); - semantics.collectTargetsTreatedAsDeps(ruleContext, builder); + semantics.collectTargetsTreatedAsDeps(ruleContext, builder, type); // Implicitly add dependency on java launcher cc_binary when --java_launcher= is enabled, // or when launcher attribute is specified in a build rule. diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java index ed638b2147..9f5034c1dc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.PathFragment; @@ -262,7 +263,7 @@ public interface JavaSemantics { */ Artifact createStubAction( RuleContext ruleContext, - final JavaCommon javaCommon, + JavaCommon javaCommon, List<String> jvmFlags, Artifact executable, String javaStartClass, @@ -284,11 +285,11 @@ public interface JavaSemantics { */ Iterable<String> getExtraJavacOpts(RuleContext ruleContext); - /** - * Add additional targets to be treated as direct dependencies. - */ + /** Add additional targets to be treated as direct dependencies. */ void collectTargetsTreatedAsDeps( - RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder); + RuleContext ruleContext, + ImmutableList.Builder<TransitiveInfoCollection> builder, + ClasspathType type); /** * Enables coverage support for the java target - adds instrumented jar to the classpath and diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java index d4a0fa8140..e4193e8420 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java @@ -92,7 +92,7 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { return execInWorker( action, actionExecutionContext, - spawn.getEnvironment(), + addPersistentRunnerVars(spawn.getEnvironment()), startupArgs, actionExecutionContext.getExecutor().getExecRoot(), maxRetries); @@ -107,6 +107,12 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { int retriesLeft) throws ExecException, InterruptedException, IOException { Executor executor = actionExecutionContext.getExecutor(); + // TODO(kush): Remove once we're out of the experimental phase. + executor + .getEventHandler() + .handle( + Event.warn( + "RUNNING TEST IN AN EXPERIMENTAL PERSISTENT WORKER. RESULTS MAY BE INACCURATE")); TestResultData.Builder builder = TestResultData.newBuilder(); Path testLogPath = action.getTestLog().getPath(); @@ -191,6 +197,19 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { } } + private static Map<String, String> addPersistentRunnerVars(Map<String, String> originalEnv) + throws UserExecException { + if (originalEnv.containsKey("PERSISTENT_TEST_RUNNER")) { + throw new UserExecException( + "Found clashing environment variable with persistent_test_runner." + + " Please use another test strategy"); + } + return ImmutableMap.<String, String>builder() + .putAll(originalEnv) + .put("PERSISTENT_TEST_RUNNER", "true") + .build(); + } + private List<String> getStartUpArgs(TestRunnerAction action) throws ExecException { List<String> args = getArgs(/*coverageScript=*/ "", action); ImmutableList.Builder<String> startupArgs = ImmutableList.builder(); @@ -198,8 +217,6 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { startupArgs.add(args.get(0)).add("--no_echo"); // Add remaining of the original args. startupArgs.addAll(args.subList(1, args.size())); - // Make the Test runner run persistently. - startupArgs.add("--persistent_test_runner"); // Add additional flags requested for this invocation. startupArgs.addAll(MoreObjects.firstNonNull( extraFlags.get(action.getMnemonic()), ImmutableList.<String>of())); diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh index 1ce5184edd..6076ea86e0 100755 --- a/src/test/shell/bazel/bazel_java_test.sh +++ b/src/test/shell/bazel/bazel_java_test.sh @@ -169,52 +169,55 @@ EOF function test_java_test_main_class() { mkdir -p java/testrunners || fail "mkdir failed" - cat > java/testrunners/TestRunner.java <<EOF -package testrunners; -import com.google.testing.junit.runner.BazelTestRunner; + cat > java/testrunners/TestRunnerPass.java <<EOF +package testrunners; -public class TestRunner { +public class TestRunnerPass { public static void main(String[] argv) { - System.out.println("Custom test runner was run"); - BazelTestRunner.main(argv); + System.out.println("Custom test runner pass was run"); } } EOF - cat > java/testrunners/Tests.java <<EOF + cat > java/testrunners/TestRunnerFail.java <<EOF package testrunners; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.Test; - -@RunWith(JUnit4.class) -public class Tests { - - @Test - public void testTest() { - System.out.println("testTest was run"); +public class TestRunnerFail { + public static void main(String[] argv) { + System.out.println("Custom failing test runner was run"); + System.exit(1); // signal failure } } EOF cat > java/testrunners/BUILD <<EOF -java_library(name = "test_runner", - srcs = ['TestRunner.java'], - deps = ['@bazel_tools//tools/jdk:TestRunner_deploy.jar'], +java_library(name = "test_runner_pass", + srcs = ['TestRunnerPass.java'], +) + +java_library(name = "test_runner_fail", + srcs = ['TestRunnerFail.java'], +) + +java_test(name = "TestsPass", + main_class = "testrunners.TestRunnerPass", + runtime_deps = [':test_runner_pass'] ) -java_test(name = "Tests", - srcs = ['Tests.java'], - deps = ['@bazel_tools//tools/jdk:TestRunner_deploy.jar'], - main_class = "testrunners.TestRunner", - runtime_deps = [':test_runner'] +java_test(name = "TestsFail", + main_class = "testrunners.TestRunnerFail", + runtime_deps = [':test_runner_fail'] ) EOF - bazel test --test_output=streamed //java/testrunners:Tests &> "$TEST_log" - expect_log "Custom test runner was run" - expect_log "testTest was run" + + bazel test --test_output=streamed //java/testrunners:TestsPass &> "$TEST_log" \ + || fail "Unexpected failure" + expect_log "Custom test runner pass was run" + + bazel test --test_output=streamed //java/testrunners:TestsFail &> "$TEST_log" \ + && fail "Expected failure" || true + expect_log "Custom failing test runner was run" } function test_basic_java_sandwich() { @@ -433,7 +436,10 @@ java_test( name = "MainTest", size = "small", srcs = ["MainTest.java"], - deps = [":custom"] + deps = [ + ":custom", + '@bazel_tools//tools/jdk:TestRunner_deploy.jar', + ] ) java_custom_library( diff --git a/src/test/shell/bazel/persistent_test_runner_test.sh b/src/test/shell/bazel/persistent_test_runner_test.sh index 8af6d04a65..deb2d501d0 100755 --- a/src/test/shell/bazel/persistent_test_runner_test.sh +++ b/src/test/shell/bazel/persistent_test_runner_test.sh @@ -22,7 +22,7 @@ CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" source "${CURRENT_DIR}/../integration_test_setup.sh" \ || { echo "integration_test_setup.sh not found!" >&2; exit 1; } -function DISABLED_test_simple_scenario() { +function test_simple_scenario() { mkdir -p java/testrunners || fail "mkdir failed" cat > java/testrunners/TestsPass.java <<EOF @@ -80,8 +80,7 @@ EOF || true } -# TODO(kush): Enable this test once we're able to reload modified classes in persistent test runner. -function DISABLED_test_reload_modified_classes() { +function test_reload_modified_classes() { mkdir -p java/testrunners || fail "mkdir failed" # Create a passing test. |