Implemented fix for case when 'use_testrunner' attribute works interconnected with 'main_class' in java_test rule.

for manual testing I used BUILD file:

java_test(
  name = "mytest",
  srcs = glob(["*.java"]),
  main_class = "com.test.Test",
  use_testrunner = 1,
)

RELNOTES: java_tests no complain when use_testrunner is explicitly set to 1 and main_class is set.
PiperOrigin-RevId: 177291746

5 files changed, 119 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java
index e309112ca0..179bbc12d7 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaTestRule.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
+import com.google.devtools.build.lib.rules.java.JavaRuleClasses;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.Jvm;
 
@@ -58,7 +59,9 @@ public final class BazelJavaTestRule implements RuleDefinition {
         .add(attr(":proguard", LABEL).cfg(HOST).value(JavaSemantics.PROGUARD).exec())
         .add(attr(":extra_proguard_specs", LABEL_LIST).value(JavaSemantics.EXTRA_PROGUARD_SPECS))
         .override(attr("stamp", TRISTATE).value(TriState.NO))
-        .override(attr("use_testrunner", BOOLEAN).value(true))
+        .override(
+            attr("use_testrunner", BOOLEAN)
+                .value(JavaRuleClasses.createUseTestrunnerComputedDefault()))
         .override(attr(":java_launcher", LABEL).value(JavaSemantics.JAVA_LAUNCHER))
         // Input files for test actions collecting code coverage
         .add(
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 ed229fee77..80249e9f24 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
@@ -97,6 +97,8 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
       }
     }
 
+    validateTestMainClass(ruleContext);
+
     semantics.checkRule(ruleContext, common);
     semantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, common);
     String mainClass = semantics.getMainClass(ruleContext, common.getSrcsArtifacts());
@@ -454,6 +456,14 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
         .build();
   }
 
+  private void validateTestMainClass(RuleContext ruleContext) {
+    boolean useTestrunner = ruleContext.attributes().get("use_testrunner", Type.BOOLEAN);
+    if (useTestrunner
+        && ruleContext.attributes().isAttributeValueExplicitlySpecified("main_class")) {
+      ruleContext.ruleError("cannot use use_testrunner with main_class specified.");
+    }
+  }
+
   // Create the deploy jar and make it dependent on the runfiles middleman if an executable is
   // created. Do not add the deploy jar to files to build, so we will only build it when it gets
   // requested.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleClasses.java
index 3afa1f5c37..780f71ec12 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuleClasses.java
@@ -21,15 +21,18 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.analysis.platform.ToolchainInfo;
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
+import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 
-/**
- * Common rule class definitions for Java rules.
- */
-public class JavaRuleClasses {
+/** Common rule class definitions for Java rules. */
+public final class JavaRuleClasses {
+  private JavaRuleClasses() {}
+
   /**
    * Common attributes for rules that depend on ijar.
    */
@@ -55,6 +58,15 @@ public class JavaRuleClasses {
     }
   }
 
+  public static Attribute.ComputedDefault createUseTestrunnerComputedDefault() {
+    return new ComputedDefault() {
+      @Override
+      public Object getDefault(AttributeMap rule) {
+        return !rule.isAttributeValueExplicitlySpecified("main_class");
+      }
+    };
+  }
+
   /**
    * Meant to be an element of {@code mandatoryProvidersLists} in order to accept rules providing a
    * {@link JavaInfo} through an attribute. Other providers can be included in {@code
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaTestTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaTestTest.java
new file mode 100644
index 0000000000..ee43992257
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaTestTest.java
@@ -0,0 +1,85 @@
+// Copyright 2017 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.devtools.build.lib.rules.java;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.syntax.Type;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for java_test rile.
+ */
+@RunWith(JUnit4.class)
+public class JavaTestTest extends BuildViewTestCase {
+
+  @Test
+  public void testMainClassWithoutUseTestRunner() throws Exception {
+
+    ConfiguredTarget ct = scratchConfiguredTarget(
+            "a",
+            "UXMessageTest",
+            "java_test("
+                + "    name = 'UXMessageTest',"
+                + "    srcs = ['UXMessageTest.java'],"
+                + "    main_class = 'com.google.devtools.build.lib.view.uxmessage.UXMessage'"
+                + ")");
+
+    assertThatUseTestrunnerIsFalse(ct);
+  }
+
+  @Test
+  public void testWithUseTestrunnerDefinedAsFalse() throws Exception {
+
+    ConfiguredTarget ct = scratchConfiguredTarget(
+            "a",
+            "UXMessageTest",
+            "java_test("
+                + "    name = 'UXMessageTest',"
+                + "    srcs = ['UXMessageTest.java'],"
+                + "    main_class = 'com.google.devtools.build.lib.view.uxmessage.UXMessage',"
+                + "    use_testrunner = 0"
+                + ")");
+
+    assertThatUseTestrunnerIsFalse(ct);
+  }
+
+  private void assertThatUseTestrunnerIsFalse(ConfiguredTarget configuredTarget) throws Exception {
+
+    Boolean useTestrunner =
+        getRuleContext(configuredTarget).attributes().get("use_testrunner", Type.BOOLEAN);
+
+    assertWithMessage("attribute 'use_testrunner' value should be false")
+        .that(useTestrunner)
+        .isFalse();
+  }
+
+  @Test
+  public void testWithUseTestrunnerDefinedAsTrue() throws Exception {
+
+    checkError("java",
+        "UXMessageTest",
+        "cannot use use_testrunner with main_class specified.",
+        "java_test("
+            + "    name = 'UXMessageTest',"
+            + "    srcs = ['UXMessageTest.java'],"
+            + "    main_class = 'com.google.devtools.build.lib.view.uxmessage.UXMessage',"
+            + "    use_testrunner = 1,"
+            + ")");
+  }
+}
diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
index 852463140f..abb16d7c03 100755
--- a/src/test/shell/bazel/bazel_java_test.sh
+++ b/src/test/shell/bazel/bazel_java_test.sh
@@ -510,6 +510,9 @@ java_library(name = "test_runner",
 
 java_test(name = "Tests",
           srcs = ['Tests.java'],
+          jvm_flags = [
+              "-Dbazel.test_suite=testrunners.Tests",
+          ],
           deps = ['//third_party:junit4'],
           main_class = "testrunners.TestRunner",
           runtime_deps = [':test_runner']
@@ -518,6 +521,7 @@ EOF
   bazel test --test_output=streamed //java/testrunners:Tests &> "$TEST_log"
   expect_log "Custom test runner was run"
   expect_log "testTest was run"
+  rm -fr java/testrunners
 }
 
 function test_basic_java_sandwich() {