Automated rollback of commit 9700ee6778706e6be65c377cb13f758e18f4a3ae. RELNOTES: None. PiperOrigin-RevId: 284187275
diff --git a/.bazelci/postsubmit.yml b/.bazelci/postsubmit.yml index 56b7506..2f220ad 100644 --- a/.bazelci/postsubmit.yml +++ b/.bazelci/postsubmit.yml
@@ -96,7 +96,6 @@ - "-//src/java_tools/import_deps_checker/..." # TODO(katre): Re-enable after 0.29.0: https://github.com/bazelbuild/bazel/issues/9148 - "-//src/tools/singlejar:combiners_test" - - "-//src/test/shell/bazel:bazel_java_persistent_test_runner_test" include_json_profile: - build - test
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index d3fd6e3..676ebf7 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml
@@ -102,7 +102,6 @@ - "-//src/java_tools/import_deps_checker/..." # TODO(katre): Re-enable after 0.29.0: https://github.com/bazelbuild/bazel/issues/9148 - "-//src/tools/singlejar:combiners_test" - - "-//src/test/shell/bazel:bazel_java_persistent_test_runner_test" ubuntu1804_clang: platform: ubuntu1804 environment:
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 4a98882..aa067d4 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
@@ -22,31 +22,16 @@ ":common_runner_java_files", ], data = ["//tools:test_sharding_compliant"], - runtime_deps = [":persistent_test_runner"], deps = [ "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal", "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/junit4", "//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:guava", "//third_party:junit4", ], ) -java_library( - name = "persistent_test_runner", - srcs = [ - "PersistentTestRunner.java", - "SuiteTestRunner.java", - ], - deps = [ - "//src/main/protobuf:worker_protocol_java_proto", - "//third_party:guava", - ], -) - java_binary( name = "Runner", main_class = "com.google.testing.junit.runner.BazelTestRunner",
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 5adb989..f735bc2 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
@@ -27,14 +27,17 @@ /** * A class to run JUnit tests in a controlled environment. * - * <p>Currently sets up a security manager to catch undesirable behaviour; System.exit. Also has - * nice command line options - run with "-help" for details. + * <p>Currently sets up a security manager to catch undesirable behaviour; + * System.exit. Also has nice command line options - run with "-help" for + * details. * - * <p>This class traps writes to <code>System.err.println()</code> and <code>System.out.println() - * </code> including the output of failed tests in the error report. + * <p>This class traps writes to <code>System.err.println()</code> and + * <code>System.out.println()</code> including the output of failed tests in + * the error report. * - * <p>It also traps SIGTERM signals to make sure that the test report is written when the signal is - * closed by the unit test framework for running over time. + * <p>It also traps SIGTERM signals to make sure that the test report is + * written when the signal is closed by the unit test framework for running + * over time. */ public class BazelTestRunner { /** @@ -66,22 +69,19 @@ PrintStream stderr = System.err; 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 (!checkTestSuiteProperty(suiteClassName)) { System.exit(2); } - if (PersistentTestRunner.isPersistentTestRunner()) { - System.exit( - PersistentTestRunner.runPersistentTestRunner( - suiteClassName, - System.getenv("WORKSPACE_PREFIX"), - (suitClass, testArgs, classLoader) -> - runTestsInSuite(suitClass, testArgs, classLoader))); - } - int exitCode; try { - exitCode = runTestsInSuite(suiteClassName, args, /* classLoader= */ null); + exitCode = runTestsInSuite(suiteClassName, args); } catch (Throwable e) { // An exception was thrown by the runner. Print the error to the output stream so it will be // logged @@ -132,13 +132,8 @@ return true; } - /** - * Runs the tests in the specified suite. Looks for the suite class in the given classLoader, or - * in the system classloader if none is specified. - */ - private static int runTestsInSuite( - String suiteClassName, String[] args, ClassLoader classLoader) { - Class<?> suite = PersistentTestRunner.getTestClass(suiteClassName, classLoader); + private static int runTestsInSuite(String suiteClassName, String[] args) { + Class<?> suite = getTestClass(suiteClassName); if (suite == null) { // No class found corresponding to the system property passed in from Bazel @@ -158,6 +153,18 @@ return runner.run().wasSuccessful() ? 0 : 1; } + private static Class<?> getTestClass(String name) { + if (name == null) { + return null; + } + + try { + return Class.forName(name); + } catch (ClassNotFoundException e) { + return null; + } + } + /** * 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/java/com/google/testing/junit/runner/PersistentTestRunner.java b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/PersistentTestRunner.java deleted file mode 100644 index b2a268c..0000000 --- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/PersistentTestRunner.java +++ /dev/null
@@ -1,157 +0,0 @@ -// Copyright 2019 The Bazel Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.testing.junit.runner; - -import static java.nio.charset.StandardCharsets.UTF_8; - -import com.google.common.io.Files; -import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; -import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.PrintStream; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.ArrayList; -import java.util.List; - -/** A utility class for running Java tests using persistent workers. */ -final class PersistentTestRunner { - - private PersistentTestRunner() { - // utility class; should not be instantiated - } - - /** - * Runs new tests in the same process. Communicates with bazel using the worker's protocol. Reads - * a {@link WorkRequest} sent by bazel and sends back a {@link WorkResponse}. - * - * <p>Before running a test it creates a new classloader loading the test and its dependencies' - * classes. A class already loaded in a classloader can not be unloaded. To overcome this issue a - * new classloader has to be created at every test run. - */ - static int runPersistentTestRunner( - String suitClassName, String workspacePrefix, SuiteTestRunner suiteTestRunner) { - PrintStream originalStdOut = System.out; - PrintStream originalStdErr = System.err; - - String testRuntimeClasspathFile = System.getenv("TEST_RUNTIME_CLASSPATH_FILE"); - String javaRunfilesPath = System.getenv("JAVA_RUNFILES"); - - // Reading the work requests and solving them in sequence is not a problem because Bazel creates - // up to --worker_max_instances (defaults to 4) instances per worker key. - while (true) { - try { - WorkRequest request = WorkRequest.parseDelimitedFrom(System.in); - - if (request == null) { - // null is only returned when the stream reaches EOF - break; - } - - URLClassLoader testRunnerClassLoader = - recreateClassLoader(testRuntimeClasspathFile, javaRunfilesPath, workspacePrefix); - - 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 = - suiteTestRunner.runTestsInSuite(suitClassName, arguments, testRunnerClassLoader); - } 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; - } - - /** - * Returns a {@link Class} with the given name. Loads the class for the given classloader, or from - * the system classloader if none is specified. - */ - static Class<?> getTestClass(String name, ClassLoader classLoader) { - if (name == null) { - return null; - } - - try { - if (classLoader != null) { - return Class.forName(name, true, classLoader); - } - return Class.forName(name); - } catch (ClassNotFoundException e) { - return null; - } - } - - /** - * Returns a classloader containing the jars read from the given runtime classpath file. - * - * <p>Sets the classloader of the current thread to the newly created classloader. - * - * <p>Sets the classloader used to load the test classes and their dependencies. - * - * <p>Needs to be called before every test run to avoid having stale classes. A class already - * loaded in a classloader can not be unloaded. To overcome this a new classloader has to be - * created at every test run. - */ - private static URLClassLoader recreateClassLoader( - String runtimeClasspathFilename, String javaRunfilesPath, String workspacePrefix) - throws IOException { - URLClassLoader classLoader = - new URLClassLoader( - createURLsFromRelativePathsInFile( - runtimeClasspathFilename, javaRunfilesPath, workspacePrefix)); - Thread.currentThread().setContextClassLoader(classLoader); - return classLoader; - } - - private static URL[] createURLsFromRelativePathsInFile( - String runtimeClasspathFilename, String javaRunfilesPath, String workspacePrefix) - throws IOException { - List<String> testRuntimeClasspath = Files.readLines(new File(runtimeClasspathFilename), UTF_8); - ArrayList<URL> urlList = new ArrayList<>(); - for (String classPathEntry : testRuntimeClasspath) { - urlList.add( - new File(javaRunfilesPath + File.separator + workspacePrefix + classPathEntry) - .toURI() - .toURL()); - } - URL[] urls = new URL[urlList.size()]; - return urlList.toArray(urls); - } - - static boolean isPersistentTestRunner() { - return Boolean.parseBoolean(System.getenv("PERSISTENT_TEST_RUNNER")); - } -}
diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/SuiteTestRunner.java b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/SuiteTestRunner.java deleted file mode 100644 index 640dba9..0000000 --- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/SuiteTestRunner.java +++ /dev/null
@@ -1,21 +0,0 @@ -// Copyright 2019 The Bazel Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.testing.junit.runner; - -/** An interface for running the tests in a specified suite. */ -@FunctionalInterface -interface SuiteTestRunner { - int runTestsInSuite(String suiteClassName, String[] testArgs, ClassLoader testRunnerClassLoader); -}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index f3375c3..bce9635 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -84,7 +84,6 @@ private NestedSetBuilder<Artifact> filesToRunBuilder = NestedSetBuilder.stableOrder(); private RunfilesSupport runfilesSupport; - private Runfiles persistentTestRunnerRunfiles; private Artifact executable; private ImmutableSet<ActionAnalysisMetadata> actionsWithoutExtraAction = ImmutableSet.of(); @@ -420,9 +419,9 @@ TestParams testParams = testActionBuilder .setFilesToRunProvider(filesToRunProvider) - .setPersistentTestRunnerRunfiles(persistentTestRunnerRunfiles) .setExecutionRequirements( - (ExecutionInfo) providersBuilder.getProvider(ExecutionInfo.PROVIDER.getKey())) + (ExecutionInfo) providersBuilder + .getProvider(ExecutionInfo.PROVIDER.getKey())) .setShardCount(explicitShardCount) .build(); ImmutableList<String> testTags = ImmutableList.copyOf(ruleContext.getRule().getRuleTags()); @@ -579,11 +578,6 @@ return this; } - public RuleConfiguredTargetBuilder setPersistentTestRunnerRunfiles(Runfiles testSupportRunfiles) { - this.persistentTestRunnerRunfiles = testSupportRunfiles; - return this; - } - /** * Set the files to build. */
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java index 56305a1..c59c4b7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -23,13 +23,11 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; -import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; import com.google.devtools.build.lib.analysis.RunfilesSupport; import com.google.devtools.build.lib.analysis.ShToolchain; @@ -71,7 +69,6 @@ private final RuleContext ruleContext; private RunfilesSupport runfilesSupport; - private Runfiles persistentTestRunnerRunfiles; private Artifact executable; private ExecutionInfo executionRequirements; private InstrumentedFilesInfo instrumentedFiles; @@ -124,11 +121,6 @@ return this; } - public TestActionBuilder setPersistentTestRunnerRunfiles(Runfiles runfiles) { - this.persistentTestRunnerRunfiles = runfiles; - return this; - } - public TestActionBuilder setInstrumentedFiles(@Nullable InstrumentedFilesInfo instrumentedFiles) { this.instrumentedFiles = instrumentedFiles; return this; @@ -315,7 +307,6 @@ runfilesSupport, executable, instrumentedFileManifest, - /* persistentTestRunnerFlagFile= */ null, shards, runsPerTest); inputsBuilder.add(instrumentedFileManifest); @@ -324,16 +315,9 @@ extraTestEnv.put(coverageEnvEntry.getFirst(), coverageEnvEntry.getSecond()); } } else { - Artifact flagFile = null; - // The worker spawn runner expects a flag file containg the worker's flags. - if (testConfiguration.isPersistentTestRunner()) { - flagFile = ruleContext.getBinArtifact(ruleContext.getLabel().getName() + "_flag_file.txt"); - inputsBuilder.add(flagFile); - } - executionSettings = new TestTargetExecutionSettings( - ruleContext, runfilesSupport, executable, null, flagFile, shards, runsPerTest); + ruleContext, runfilesSupport, executable, null, shards, runsPerTest); } extraTestEnv.putAll(extraEnv); @@ -385,30 +369,11 @@ boolean cancelConcurrentTests = testConfiguration.runsPerTestDetectsFlakes() && testConfiguration.cancelConcurrentTests(); - RunfilesSupplier testRunfilesSupplier; - if (testConfiguration.isPersistentTestRunner()) { - // Create a RunfilesSupplier from the persistent test runner's runfiles. Pass only the - // test runner's runfiles to avoid using a different worker for every test run. - testRunfilesSupplier = - new RunfilesSupplierImpl( - /* runfilesDir= */ persistentTestRunnerRunfiles.getSuffix(), - /* runfiles= */ persistentTestRunnerRunfiles, - /* buildRunfileLinks= */ false, - /* runfileLinksEnabled= */ false); - } else { - testRunfilesSupplier = RunfilesSupplierImpl.create(runfilesSupport); - } - - ImmutableList.Builder<Artifact> tools = new ImmutableList.Builder<>(); - if (testConfiguration.isPersistentTestRunner()) { - tools.add(testActionExecutable); - tools.add(executionSettings.getExecutable()); - } TestRunnerAction testRunnerAction = new TestRunnerAction( ruleContext.getActionOwner(), inputs, - testRunfilesSupplier, + RunfilesSupplierImpl.create(runfilesSupport), testActionExecutable, isUsingTestWrapperInsteadOfTestSetupScript, testXmlGeneratorExecutable, @@ -427,8 +392,7 @@ || executionSettings.needsShell(isExecutedOnWindows)) ? ShToolchain.getPathOrError(ruleContext) : null, - cancelConcurrentTests, - tools.build()); + cancelConcurrentTests); testOutputs.addAll(testRunnerAction.getSpawnOutputs()); testOutputs.addAll(testRunnerAction.getOutputs());
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index cdb6de2..3f5e72d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
@@ -156,18 +156,6 @@ public TestActionBuilder.TestShardingStrategy testShardingStrategy; @Option( - name = "experimental_persistent_test_runner", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, - effectTags = {OptionEffectTag.UNKNOWN}, - help = - "Allows running java_test targets locally within a persistent worker. " - + "To enable the persistent test runner one must run bazel test with the flags:" - + "--test_strategy=local --strategy=TestRunner=worker " - + " --experimental_persistent_test_runner") - public boolean persistentTestRunner; - - @Option( name = "runs_per_test", allowMultiple = true, defaultValue = "1", @@ -325,10 +313,6 @@ return options.testShardingStrategy; } - public boolean isPersistentTestRunner() { - return options.persistentTestRunner; - } - public Label getCoverageSupport(){ return options.coverageSupport; }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 5413d94..4db8d46 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -171,11 +171,10 @@ BuildConfiguration configuration, String workspaceName, @Nullable PathFragment shExecutable, - boolean cancelConcurrentTestsOnSuccess, - Iterable<Artifact> tools) { + boolean cancelConcurrentTestsOnSuccess) { super( owner, - tools, + /*tools=*/ ImmutableList.of(), inputs, runfilesSupplier, list(testLog, cacheStatus, coverageArtifact), @@ -561,11 +560,6 @@ env.put("RUNFILES_MANIFEST_ONLY", "1"); } - if (testConfiguration.isPersistentTestRunner()) { - // Let the test runner know it runs persistently within a worker. - env.put("PERSISTENT_TEST_RUNNER", "true"); - } - if (isCoverageMode()) { // Instruct remote-runtest.sh/local-runtest.sh not to cd into the runfiles directory. // TODO(ulfjack): Find a way to avoid setting this variable.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java index f0ca758..3e7ca16 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java
@@ -14,18 +14,15 @@ package com.google.devtools.build.lib.analysis.test; -import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLine; -import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesSupport; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; -import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; @@ -57,7 +54,6 @@ RunfilesSupport runfilesSupport, Artifact executable, Artifact instrumentedFileManifest, - Artifact persistentTestRunnerFlagFile, int shards, int runs) { Preconditions.checkArgument(TargetUtils.isTestRule(ruleContext.getRule())); @@ -66,35 +62,8 @@ TestConfiguration testConfig = config.getFragment(TestConfiguration.class); CommandLine targetArgs = runfilesSupport.getArgs(); - if (persistentTestRunnerFlagFile != null) { - // If an flag artifact exists for the persistent test runner, register an action that writes - // the test arguments to the said artifact. When the test runner runs in a persistent worker, - // the worker expects to find the test arguments in a special flag file. - ImmutableList.Builder<String> testTargetArgs = new ImmutableList.Builder<>(); - try { - testTargetArgs.addAll(targetArgs.arguments()); - } catch (CommandLineExpansionException e) { - // Don't fail the build and ignore the runfiles arguments. - ruleContext.ruleError("Could not expand test target arguments: " + e.getMessage()); - } - testTargetArgs.addAll(testConfig.getTestArguments()); - ruleContext.registerAction( - FileWriteAction.create( - ruleContext.getActionOwner(), - persistentTestRunnerFlagFile, - /* fileContents= */ Joiner.on(System.lineSeparator()).join(testTargetArgs.build()), - /* makeExecutable= */ false, - /* allowCompression= */ FileWriteAction.Compression.DISALLOW)); - - // When using the persistent test runner the test arguments are passed through --flagfile. - testArguments = - CommandLine.of( - ImmutableList.of( - "--flagfile=" + persistentTestRunnerFlagFile.getRootRelativePathString())); - } else { - testArguments = - CommandLine.concat(targetArgs, ImmutableList.copyOf(testConfig.getTestArguments())); - } + testArguments = + CommandLine.concat(targetArgs, ImmutableList.copyOf(testConfig.getTestArguments())); totalShards = shards; totalRuns = runs;
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 b814d40..06c5741 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
@@ -306,42 +306,7 @@ } NestedSet<Artifact> classpath = classpathBuilder.build(); - if (JavaSemantics.isPersistentTestRunner(ruleContext)) { - // Create an artifact that stores the test's runtime classpath (excluding the test support - // classpath). The file is read by the test runner. The jars inside the file are loaded - // dynamically for every test run into a custom classloader. - arguments.add( - new ComputedClasspathSubstitution( - JavaSemantics.getTestSupportRuntimeClasspath(ruleContext), - workspacePrefix, - isRunfilesEnabled)); - - // Create an artifact that stores the test's runtime classpath. - Artifact testRuntimeClasspathArtifact = - ruleContext.getBinArtifact( - ruleContext.getLabel().getName() + "_test_runtime_classpath.txt"); - ruleContext.registerAction( - new LazyWritePathsFileAction( - ruleContext.getActionOwner(), - testRuntimeClasspathArtifact, - javaCommon.getRuntimeClasspath(), - true)); - filesBuilder.add(testRuntimeClasspathArtifact); - - arguments.add( - Substitution.of( - TEST_RUNTIME_CLASSPATH_FILE_PLACEHOLDER, - "export WORKSPACE_PREFIX=" - + workspacePrefix - + "\nexport TEST_RUNTIME_CLASSPATH_FILE=${JAVA_RUNFILES}" - + File.separator - + workspacePrefix - + testRuntimeClasspathArtifact.getRootRelativePathString())); - } else { - arguments.add( - new ComputedClasspathSubstitution(classpath, workspacePrefix, isRunfilesEnabled)); - arguments.add(Substitution.of(TEST_RUNTIME_CLASSPATH_FILE_PLACEHOLDER, "")); - } + arguments.add(new ComputedClasspathSubstitution(classpath, workspacePrefix, isRunfilesEnabled)); if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { if (createCoverageMetadataJar) { @@ -494,14 +459,9 @@ // targets may break, we are keeping it behind this flag. return; } - if (!JavaSemantics.isPersistentTestRunner(ruleContext)) { - // Only add the test support to the dependencies when running in regular mode. - // In persistent test runner mode don't pollute the classpath of the test with - // the test support classes. - TransitiveInfoCollection testSupport = JavaSemantics.getTestSupport(ruleContext); - if (testSupport != null) { - builder.add(testSupport); - } + TransitiveInfoCollection testSupport = JavaSemantics.getTestSupport(ruleContext); + if (testSupport != null) { + builder.add(testSupport); } } @@ -768,4 +728,3 @@ @Override public void checkDependencyRuleKinds(RuleContext ruleContext) {} } -
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 19d460d..eeeccb9 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
@@ -106,11 +106,6 @@ fi done -# A file storing the test's runtime classpath, excluding the test support. -# The file is read by the persistent test runner. The jars inside the file are -# loaded dynamically for every test run into a custom classloader. -%test_runtime_classpath_file% - # Find our runfiles tree. We need this to construct the classpath # (unless --singlejar was passed). #
diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 83451be..66c527c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.test.TestActionContext; -import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; import com.google.devtools.build.lib.analysis.test.TestRunnerAction.ResolvedPaths; @@ -118,9 +117,6 @@ executionInfo.put(ExecutionRequirements.NO_CACHE, ""); } executionInfo.put(ExecutionRequirements.TIMEOUT, "" + getTimeout(action).getSeconds()); - if (action.getConfiguration().getFragment(TestConfiguration.class).isPersistentTestRunner()) { - executionInfo.put(ExecutionRequirements.SUPPORTS_WORKERS, "1"); - } ResourceSet localResourceUsage = action @@ -137,9 +133,7 @@ action.getRunfilesSupplier(), ImmutableMap.of(), /*inputs=*/ ImmutableList.copyOf(action.getInputs()), - action.getConfiguration().getFragment(TestConfiguration.class).isPersistentTestRunner() - ? ImmutableList.copyOf(action.getTools()) - : ImmutableList.of(), + /*tools=*/ ImmutableList.<Artifact>of(), ImmutableList.copyOf(action.getSpawnOutputs()), localResourceUsage); return new StandaloneTestRunnerSpawn(
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 6372646..dacae1b 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
@@ -334,7 +334,6 @@ Runfiles defaultRunfiles = runfilesBuilder.build(); RunfilesSupport runfilesSupport = null; - Runfiles persistentTestRunnerRunfiles = null; NestedSetBuilder<Artifact> extraFilesToRunBuilder = NestedSetBuilder.stableOrder(); if (createExecutable) { List<String> extraArgs = @@ -348,9 +347,6 @@ RunfilesSupport.withExecutable( ruleContext, defaultRunfiles, executableForRunfiles, extraArgs); extraFilesToRunBuilder.add(runfilesSupport.getRunfilesMiddleman()); - if (JavaSemantics.isPersistentTestRunner(ruleContext)) { - persistentTestRunnerRunfiles = JavaSemantics.getTestSupportRunfiles(ruleContext); - } } RunfilesProvider runfilesProvider = @@ -474,7 +470,6 @@ // shell script), on Windows they are different (the executable to run is a batch file, the // executable for runfiles is the shell script). .setRunfilesSupport(runfilesSupport, executableToRun) - .setPersistentTestRunnerRunfiles(persistentTestRunnerRunfiles) .addFilesToRun(extraFilesToRunBuilder.build()) .add( JavaRuntimeClasspathProvider.class,
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 4f55aef..795bce1 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
@@ -29,16 +29,13 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.Runfiles.Builder; -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.Substitution.ComputedSubstitution; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; -import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.Label; 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.Attribute.LabelLateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.LabelListLateBoundDefault; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; @@ -87,7 +84,6 @@ fromTemplates("%{name}_deploy-src.jar"); SafeImplicitOutputsFunction JAVA_TEST_CLASSPATHS_FILE = fromTemplates("%{name}_classpaths_file"); - String TEST_RUNTIME_CLASSPATH_FILE_PLACEHOLDER = "%test_runtime_classpath_file%"; FileType JAVA_SOURCE = FileType.of(".java"); FileType JAR = FileType.of(".jar"); @@ -323,29 +319,6 @@ */ boolean isJavaExecutableSubstitution(); - static boolean isPersistentTestRunner(RuleContext ruleContext) { - return ruleContext.isTestTarget() - && ruleContext.getFragment(TestConfiguration.class).isPersistentTestRunner(); - } - - static Runfiles getTestSupportRunfiles(RuleContext ruleContext) { - TransitiveInfoCollection testSupport = getTestSupport(ruleContext); - if (testSupport == null) { - return Runfiles.EMPTY; - } - - RunfilesProvider testSupportRunfilesProvider = testSupport.getProvider(RunfilesProvider.class); - return testSupportRunfilesProvider.getDefaultRunfiles(); - } - - static NestedSet<Artifact> getTestSupportRuntimeClasspath(RuleContext ruleContext) { - TransitiveInfoCollection testSupport = JavaSemantics.getTestSupport(ruleContext); - if (testSupport == null) { - return NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } - return JavaInfo.getProvider(JavaCompilationArgsProvider.class, testSupport).getRuntimeJars(); - } - static TransitiveInfoCollection getTestSupport(RuleContext ruleContext) { if (!isJavaBinaryOrJavaTest(ruleContext)) { return null; @@ -363,16 +336,16 @@ } } - static boolean useLegacyJavaTest(RuleContext ruleContext) { - return !ruleContext.attributes().isAttributeValueExplicitlySpecified("test_class") - && ruleContext.getFragment(JavaConfiguration.class).useLegacyBazelJavaTest(); - } - static boolean isJavaBinaryOrJavaTest(RuleContext ruleContext) { return ruleContext.getRule().getRuleClass().equals("java_binary") || ruleContext.getRule().getRuleClass().equals("java_test"); } + static boolean useLegacyJavaTest(RuleContext ruleContext) { + return !ruleContext.attributes().isAttributeValueExplicitlySpecified("test_class") + && ruleContext.getFragment(JavaConfiguration.class).useLegacyBazelJavaTest(); + } + /** Adds extra runfiles for a {@code java_binary} rule. */ void addRunfilesForBinary( RuleContext ruleContext, Artifact launcher, Runfiles.Builder runfilesBuilder);
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 12c29a3..1a2efa5 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD
@@ -171,19 +171,6 @@ ) sh_test( - name = "bazel_java_persistent_test_runner_test", - srcs = ["bazel_java_persistent_test_runner_test.sh"], - data = [ - ":test-deps", - "//src:java_tools_java11_zip", - "@bazel_tools//tools/bash/runfiles", - ], - tags = [ - "no_windows", - ], -) - -sh_test( name = "bazel_java_test_defaults", srcs = ["bazel_java_test_defaults.sh"], data = [
diff --git a/src/test/shell/bazel/bazel_java_persistent_test_runner_test.sh b/src/test/shell/bazel/bazel_java_persistent_test_runner_test.sh deleted file mode 100755 index 8fa81da..0000000 --- a/src/test/shell/bazel/bazel_java_persistent_test_runner_test.sh +++ /dev/null
@@ -1,266 +0,0 @@ -#!/bin/bash -# Copyright 2019 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# --- begin runfiles.bash initialization --- -if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - if [[ -f "$0.runfiles_manifest" ]]; then - export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" - elif [[ -f "$0.runfiles/MANIFEST" ]]; then - export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" - elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then - export RUNFILES_DIR="$0.runfiles" - fi -fi -if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then - source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" -elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ - "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" -else - echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" - exit 1 -fi -# --- end runfiles.bash initialization --- - -source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ - || { echo "integration_test_setup.sh not found!" >&2; exit 1; } - -case "$(uname -s | tr [:upper:] [:lower:])" in -msys*|mingw*|cygwin*) - declare -r is_windows=true - ;; -*) - declare -r is_windows=false - ;; -esac - -if "$is_windows"; then - export MSYS_NO_PATHCONV=1 - export MSYS2_ARG_CONV_EXCL="*" -fi - -declare -a PTR_BAZEL_ARGS=("--test_strategy=standalone" \ - "--strategy=TestRunner=worker" \ - "--experimental_persistent_test_runner" \ - "--test_output=all" \ - "--worker_verbose") - -function set_up() { - setup_javatest_support - - if "$is_windows"; then - java_tools_url="file:///$(rlocation io_bazel/src/java_tools_java11.zip)" - else - java_tools_url="file://$(rlocation io_bazel/src/java_tools_java11.zip)" - fi - - cat >>WORKSPACE <<EOF -load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -http_archive( - name = "remote_java_tools_linux", - urls = ["${java_tools_url}"] -) -http_archive( - name = "remote_java_tools_darwin", - urls = ["${java_tools_url}"] -) -http_archive( - name = "remote_java_tools_windows", - urls = ["${java_tools_url}"] -) -EOF -} - -function test_java_test_persistent_test_runner() { - mkdir -p javatests/com/google/ptr - - cat > javatests/com/google/ptr/DummyTest.java <<EOF -package com.google.ptr; - -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.Test; - -@RunWith(JUnit4.class) -public class DummyTest { - - @Test - public void dummyTest() { - System.out.println("dummyTest was run"); - } -} -EOF - - cat > javatests/com/google/ptr/BUILD <<EOF -java_test( - name = "DummyTest", - srcs = ["DummyTest.java"], - deps = [ - "//third_party:junit4", - ], -) -EOF - - # Make sure we start clean with no workers already in use. - bazel clean - # The first test run creates the TestRunner worker. - bazel test javatests/com/google/ptr:DummyTest "${PTR_BAZEL_ARGS[@]}" \ - &> "${TEST_log}" || fail "Expected success" - expect_log "dummyTest was run" - expect_log "Created new non-sandboxed TestRunner worker (id [0-9]\+)" - - # Change the test to fail in order to make sure the persistent test runner - # picks up the latest test changes. - cat > javatests/com/google/ptr/DummyTest.java <<EOF -package com.google.ptr; - -import static org.junit.Assert.fail; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.Test; - -@RunWith(JUnit4.class) -public class DummyTest { - - @Test - public void dummyTest() { - System.out.println("dummyTest will fail"); - fail(); - } -} -EOF - - # The second run uses the previously created worker. Does not create - # any new workers. - bazel test javatests/com/google/ptr:DummyTest "${PTR_BAZEL_ARGS[@]}" \ - &> "${TEST_log}" && fail "Expected failure" || true - - expect_log "dummyTest will fail" - expect_log "There was 1 failure" - expect_log "at com.google.ptr.DummyTest.dummyTest(DummyTest.java:14)" - expect_not_log "Created new non-sandboxed TestRunner worker" - expect_not_log "Destroying TestRunner worker (id [0-9]\+)" - - # Change the test to pass again but with a different content to avoid - # cached test results. - cat > javatests/com/google/ptr/DummyTest.java <<EOF -package com.google.ptr; - -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.Test; - -@RunWith(JUnit4.class) -public class DummyTest { - - @Test - public void dummyTest() { - System.out.println("Re-running dummyTest without failure."); - } -} -EOF - - # The third run uses the previously created worker. Does not create - # any new workers. - bazel test javatests/com/google/ptr:DummyTest "${PTR_BAZEL_ARGS[@]}" \ - &> "${TEST_log}" || fail "Expected success" - expect_log "Re-running dummyTest without failure." - expect_not_log "Created new non-sandboxed TestRunner worker" - expect_not_log "Destroying TestRunner worker (id [0-9]\+)" -} - -function test_java_test_persistent_test_runner_with_dep() { - mkdir -p javatests/com/google/ptr - mkdir -p java/com/google/ptr - - cat > java/com/google/ptr/Dummy.java <<EOF -package com.google.ptr; - -public class Dummy { - public Dummy() { } - - public void printValue() { - System.out.println("dummyTest was run"); - } -} -EOF - - cat > java/com/google/ptr/BUILD <<EOF -package(default_visibility = ["//visibility:public"]) -java_library( - name = "dummy", - srcs = ["Dummy.java"], -) -EOF - - cat > javatests/com/google/ptr/DummyTest.java <<EOF -package com.google.ptr; - -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.junit.Test; - -@RunWith(JUnit4.class) -public class DummyTest { - - @Test - public void dummyTest() { - new Dummy().printValue(); - } -} -EOF - - cat > javatests/com/google/ptr/BUILD <<EOF -java_test( - name = "DummyTest", - srcs = ["DummyTest.java"], - deps = [ - "//java/com/google/ptr:dummy", - "//third_party:junit4", - ], -) -EOF - - # Make sure we start clean with no workers already in use. - bazel clean - # The first test run creates the TestRunner worker. - bazel test javatests/com/google/ptr:DummyTest "${PTR_BAZEL_ARGS[@]}" \ - &> "${TEST_log}" || fail "Expected success" - expect_log "dummyTest was run" - expect_log "Created new non-sandboxed TestRunner worker (id [0-9]\+)" - - # Change the library content to test if the same worker is re-used - # and if the library jar was reloaded in the PTR's classloader. - cat > java/com/google/ptr/Dummy.java <<EOF -package com.google.ptr; - -public class Dummy { - public Dummy() { } - - public void printValue() { - System.out.println("Re-printing dummy message."); - } -} -EOF - # The second run uses the previously created worker. Does not create - # any new workers. - bazel test javatests/com/google/ptr:DummyTest "${PTR_BAZEL_ARGS[@]}" \ - &> "${TEST_log}" || fail "Expected success" - expect_log "Re-printing dummy message." - expect_not_log "Created new non-sandboxed TestRunner worker" - expect_not_log "Destroying TestRunner worker (id [0-9]\+)" -} - -run_suite "Persistent Java Test Runner integration tests"
diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh index 2be7728..098a898 100755 --- a/tools/test/test-setup.sh +++ b/tools/test/test-setup.sh
@@ -14,30 +14,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -function echo_err() { - echo "$@" 1>&2; -} +# shift stderr to stdout. +exec 2>&1 -function printf_err() { - printf "$@" >&2 -} - -echo=echo -printf=printf -if [[ -z "$PERSISTENT_TEST_RUNNER" ]]; then - # Redirect all subsequent output to stdout. - # Only redirect in non-persistent test mode. - exec 2>&1 -else - # Don't print anything to stdout in this special case, only to stderr. +no_echo= +if [[ "$1" = "--no_echo" ]]; then + # Don't print anything to stdout in this special case. # Currently needed for persistent test runner. - echo=echo_err - printf=printf_err + no_echo="true" + shift +else + echo 'exec ${PAGER:-/usr/bin/less} "$0" || exit 1' + echo "Executing tests from ${TEST_TARGET}" fi -$echo 'exec ${PAGER:-/usr/bin/less} "$0" || exit 1' -$echo "Executing tests from ${TEST_TARGET}" - function is_absolute { [[ "$1" = /* ]] || [[ "$1" =~ ^[a-zA-Z]:[/\\].* ]] } @@ -158,7 +148,9 @@ fi # This header marks where --test_output=streamed will start being printed. -$echo "-----------------------------------------------------------------------------" +if [[ -z "$no_echo" ]]; then + echo "-----------------------------------------------------------------------------" +fi # Unused if EXPERIMENTAL_SPLIT_XML_GENERATION is set. function encode_stream { @@ -297,7 +289,7 @@ else "$1" "$TEST_PATH" "${@:3}" 2>&1 || exitCode=$? fi -elif [ "$has_tail" == true ]; then +elif [ "$has_tail" == true ] && [ -z "$no_echo" ]; then touch "${XML_OUTPUT_FILE}.log" if [ -z "$COVERAGE_DIR" ]; then ("${TEST_PATH}" "$@" &>"${XML_OUTPUT_FILE}.log") <&0 &