Rollback of commit bd3be73b84001dbdb2f05297f215e8eb5bacf0e1.

*** Reason for rollback ***

To deprecate that flag, we need to fix #2539 first.

*** Original change description ***

Deprecate the legacy_java_test flag, now that nolegacy_java_test has been the default for quite a while now.

RELNOTES[INC]: legacy_java_test flag will not have any effect now.

--
PiperOrigin-RevId: 148107667
MOS_MIGRATED_REVID=148107667

3 files changed, 84 insertions, 20 deletions

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 7a91443a50..f5fa1ff7ae 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
@@ -55,6 +55,7 @@ import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.ShellEscaper;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.File;
 import java.util.ArrayList;
@@ -100,14 +101,25 @@ public class BazelJavaSemantics implements JavaSemantics {
   public void checkProtoDeps(
       RuleContext ruleContext, Collection<? extends TransitiveInfoCollection> deps) {}
 
+  private static final String JUNIT4_RUNNER = "org.junit.runner.JUnitCore";
+
   private String getMainClassInternal(RuleContext ruleContext, ImmutableList<Artifact> sources) {
     if (!ruleContext.attributes().get("create_executable", Type.BOOLEAN)) {
       return null;
     }
     String mainClass = ruleContext.attributes().get("main_class", Type.STRING);
 
+    // Legacy behavior for java_test rules: main_class defaulted to JUnit4 runner.
+    // TODO(dmarting): remove once we drop the legacy bazel java_test behavior.
+    if (mainClass.isEmpty()
+        && useLegacyJavaTest(ruleContext)
+        && "java_test".equals(ruleContext.getRule().getRuleClass())) {
+      mainClass = JUNIT4_RUNNER;
+    }
+
     if (mainClass.isEmpty()) {
-      if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)) {
+      if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)
+          && !useLegacyJavaTest(ruleContext)) {
         return "com.google.testing.junit.runner.BazelTestRunner";
       }
       mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources);
@@ -276,6 +288,9 @@ public class BazelJavaSemantics implements JavaSemantics {
     if (!isJavaBinaryOrJavaTest(ruleContext)) {
       return null;
     }
+    if (useLegacyJavaTest(ruleContext)) {
+      return null;
+    }
 
     boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN);
     if (createExecutable && ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)) {
@@ -329,8 +344,16 @@ public class BazelJavaSemantics implements JavaSemantics {
       RuleConfiguredTargetBuilder ruleBuilder) {
   }
 
-  @Override
-  public String getPrimaryClass(RuleContext ruleContext, ImmutableList<Artifact> sources) {
+  // TODO(dmarting): simplify that logic when we remove the legacy Bazel java_test behavior.
+  private String getPrimaryClassLegacy(RuleContext ruleContext, ImmutableList<Artifact> sources) {
+    boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN);
+    if (!createExecutable) {
+      return null;
+    }
+    return getMainClassInternal(ruleContext, sources);
+  }
+
+  private String getPrimaryClassNew(RuleContext ruleContext, ImmutableList<Artifact> sources) {
     boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN);
 
     if (!createExecutable) {
@@ -365,28 +388,37 @@ public class BazelJavaSemantics implements JavaSemantics {
   }
 
   @Override
+  public String getPrimaryClass(RuleContext ruleContext, ImmutableList<Artifact> sources) {
+    return useLegacyJavaTest(ruleContext)
+        ? getPrimaryClassLegacy(ruleContext, sources)
+        : getPrimaryClassNew(ruleContext, sources);
+  }
+
+  @Override
   public Iterable<String> getJvmFlags(
       RuleContext ruleContext, ImmutableList<Artifact> sources, List<String> userJvmFlags) {
     ImmutableList.Builder<String> jvmFlags = ImmutableList.builder();
     jvmFlags.addAll(userJvmFlags);
-    if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)) {
-      String testClass =
-          ruleContext.getRule().isAttrDefined("test_class", Type.STRING)
-              ? ruleContext.attributes().get("test_class", Type.STRING)
-              : "";
-      if (testClass.isEmpty()) {
-        testClass = JavaCommon.determinePrimaryClass(ruleContext, sources);
-      }
 
-      if (testClass == null) {
-        ruleContext.ruleError("cannot determine test class");
-      } else {
-        // Always run junit tests with -ea (enable assertion)
-        jvmFlags.add("-ea");
-        // "suite" is a misnomer.
-        jvmFlags.add("-Dbazel.test_suite=" + ShellEscaper.escapeString(testClass));
+    if (!useLegacyJavaTest(ruleContext)) {
+      if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)) {
+        String testClass = ruleContext.getRule().isAttrDefined("test_class", Type.STRING)
+            ? ruleContext.attributes().get("test_class", Type.STRING) : "";
+        if (testClass.isEmpty()) {
+          testClass = JavaCommon.determinePrimaryClass(ruleContext, sources);
+        }
+
+        if (testClass == null) {
+          ruleContext.ruleError("cannot determine test class");
+        } else {
+          // Always run junit tests with -ea (enable assertion)
+          jvmFlags.add("-ea");
+          // "suite" is a misnomer.
+          jvmFlags.add("-Dbazel.test_suite=" +  ShellEscaper.escapeString(testClass));
+        }
       }
     }
+
     return jvmFlags.build();
   }
 
@@ -504,7 +536,28 @@ public class BazelJavaSemantics implements JavaSemantics {
 
   @Override
   public List<String> getExtraArguments(RuleContext ruleContext, ImmutableList<Artifact> sources) {
-     return ImmutableList.of();
+    if (ruleContext.getRule().getRuleClass().equals("java_test")) {
+      if (useLegacyJavaTest(ruleContext)) {
+        if (ruleContext.getConfiguration().getTestArguments().isEmpty()
+            && !ruleContext.attributes().isAttributeValueExplicitlySpecified("args")) {
+          ImmutableList.Builder<String> builder = ImmutableList.builder();
+          for (Artifact artifact : sources) {
+            PathFragment path = artifact.getRootRelativePath();
+            String className = JavaUtil.getJavaFullClassname(FileSystemUtils.removeExtension(path));
+            if (className != null) {
+              builder.add(className);
+            }
+          }
+          return builder.build();
+        }
+      }
+    }
+    return ImmutableList.<String>of();
+  }
+
+  private boolean useLegacyJavaTest(RuleContext ruleContext) {
+    return !ruleContext.attributes().isAttributeValueExplicitlySpecified("test_class")
+        && ruleContext.getFragment(JavaConfiguration.class).useLegacyBazelJavaTest();
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
index ede6e57af7..9dd26c46c4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
@@ -144,6 +144,9 @@ public final class JavaConfiguration extends Fragment {
   private final JavaOptimizationMode javaOptimizationMode;
   private final Label javaToolchain;
 
+  // TODO(dmarting): remove once we have a proper solution for #2539
+  private final boolean legacyBazelJavaTest;
+
   JavaConfiguration(
       boolean generateJavaDeps,
       List<String> defaultJvmFlags,
@@ -169,6 +172,7 @@ public final class JavaConfiguration extends Fragment {
     this.bundleTranslations = javaOptions.bundleTranslations;
     this.javaToolchain = javaToolchain;
     this.javaOptimizationMode = javaOptions.javaOptimizationMode;
+    this.legacyBazelJavaTest = javaOptions.legacyBazelJavaTest;
     this.strictDepsJavaProtos = javaOptions.strictDepsJavaProtos;
     this.enforceOneVersion = javaOptions.enforceOneVersion;
 
@@ -329,6 +333,14 @@ public final class JavaConfiguration extends Fragment {
   }
 
   /**
+   * Returns true if java_test in Bazel should behave in legacy mode that existed before we
+   * open-sourced our test runner.
+   */
+  public boolean useLegacyBazelJavaTest() {
+    return legacyBazelJavaTest;
+  }
+
+  /**
    * Returns true if Bazel should attempt to enforce one-version correctness on java_binary rules
    * using the 'oneversion' tool in the java_toolchain. One-version correctness will inspect for
    * multiple non-identical versions of java classes in the transitive dependencies for a
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index 7335ad00ed..a7b7086ee8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -304,7 +304,6 @@ public class JavaOptions extends FragmentOptions {
       help = "Applies desired link-time optimizations to Java binaries and tests.")
   public JavaOptimizationMode javaOptimizationMode;
 
-  @Deprecated
   @Option(name = "legacy_bazel_java_test",
       defaultValue = "false",
       category = "undocumented",