From 0e396b827bca5ee507aadc875169e47d959a7136 Mon Sep 17 00:00:00 2001 From: Damien Martin-Guillerez Date: Wed, 13 Jan 2016 09:39:16 +0000 Subject: Use our java test runner in Bazel RELNOTES[NEW]: A new java test runner that support XML output and test filtering is supported. It can be used by specifying --nolegacy_bazel_java_test or by speicifying the test_class attribute on a java_test. -- MOS_MIGRATED_REVID=112028955 --- .../src/test/java/com/example/myproject/BUILD | 10 ++ .../lib/bazel/rules/java/BazelJavaRuleClasses.java | 34 +++++ .../lib/bazel/rules/java/BazelJavaSemantics.java | 162 ++++++++++++++++++--- .../lib/bazel/rules/java/BazelJavaTestRule.java | 46 +++++- .../build/lib/rules/java/JavaConfiguration.java | 17 ++- .../devtools/build/lib/rules/java/JavaOptions.java | 6 + .../build/lib/analysis/mock/BazelAnalysisMock.java | 2 +- src/test/shell/bazel/BUILD | 1 + src/test/shell/bazel/bazel_example_test.sh | 7 + src/test/shell/bazel/test-setup.sh | 1 + src/test/shell/bazel/testenv.sh | 1 + 11 files changed, 260 insertions(+), 27 deletions(-) diff --git a/examples/java-native/src/test/java/com/example/myproject/BUILD b/examples/java-native/src/test/java/com/example/myproject/BUILD index 9a91a8288b..b9d81a1ff8 100644 --- a/examples/java-native/src/test/java/com/example/myproject/BUILD +++ b/examples/java-native/src/test/java/com/example/myproject/BUILD @@ -16,6 +16,16 @@ java_test( ], ) +java_test( + name = "custom_with_test_class", + srcs = glob(["Test*.java"]), + test_class = "com.example.myproject.TestCustomGreeting", + deps = [ + "//examples/java-native/src/main/java/com/example/myproject:custom-greeting", + "//third_party:junit4", + ], +) + java_test( name = "fail", srcs = ["Fail.java"], diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java index 0dea311192..b065913bcb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaRuleClasses.java @@ -25,10 +25,13 @@ import static com.google.devtools.build.lib.syntax.Type.STRING; import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.Constants; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses; +import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.PredicateWithMessage; import com.google.devtools.build.lib.packages.Rule; @@ -38,6 +41,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.RuleClass.PackageNameConstraint; import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.java.JavaSemantics; +import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; import java.util.Set; @@ -50,6 +54,9 @@ public class BazelJavaRuleClasses { public static final PredicateWithMessage JAVA_PACKAGE_NAMES = new PackageNameConstraint( PackageNameConstraint.ANY_SEGMENT, "java", "javatests"); + protected static final String JUNIT_TESTRUNNER = + Constants.TOOLS_REPOSITORY + "//tools/jdk:TestRunner_deploy.jar"; + public static final ImplicitOutputsFunction JAVA_BINARY_IMPLICIT_OUTPUTS = fromFunctions( JavaSemantics.JAVA_BINARY_CLASS_JAR, @@ -268,6 +275,7 @@ public class BazelJavaRuleClasses { * Base class for rule definitions producing Java binaries. */ public static final class BaseJavaBinaryRule implements RuleDefinition { + @Override public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) { return builder @@ -298,6 +306,23 @@ public class BazelJavaRuleClasses {

*/ .add(attr("jvm_flags", STRING_LIST)) + /* + Use the + com.google.testing.junit.runner.GoogleTestRunner class as the + main entry point for a Java program. + ${SYNOPSIS} + + You can use this to override the default + behavior, which is to use BazelTestRunner for + java_test rules, + and not use it for java_binary rules. It is unlikely + you will want to do this. One use is for AllTest + rules that are invoked by another rule (to set up a database + before running the tests, for example). The AllTest + rule must be declared as a java_binary, but should + still use the test runner as its main entry point. + */ + .add(attr("use_testrunner", BOOLEAN).value(false)) /* Name of class with main() method to use as entry point. ${SYNOPSIS} @@ -322,6 +347,15 @@ public class BazelJavaRuleClasses { .add(attr("create_executable", BOOLEAN) .nonconfigurable("internal") .value(true)) + .add(attr("$testsupport", LABEL).value( + new Attribute.ComputedDefault("use_testrunner") { + @Override + public Object getDefault(AttributeMap rule) { + return rule.get("use_testrunner", Type.BOOLEAN) + ? env.getLabel(JUNIT_TESTRUNNER) + : null; + } + })) /* ${SYNOPSIS} A list of lines to add to the META-INF/manifest.mf file generated for the 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 182c2130f5..f55c0d939f 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 @@ -16,11 +16,14 @@ package com.google.devtools.build.lib.bazel.rules.java; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; 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; @@ -38,6 +41,7 @@ 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.JavaPrimaryClassProvider; +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.JavaTargetAttributes; import com.google.devtools.build.lib.rules.java.JavaUtil; @@ -51,6 +55,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Set; /** * Semantics for Bazel Java rules @@ -80,14 +85,29 @@ public class BazelJavaSemantics implements JavaSemantics { } } - private String getMainClassInternal(RuleContext ruleContext) { - return ruleContext.getRule().isAttrDefined("main_class", Type.STRING) + private String getMainClassInternal(RuleContext ruleContext, JavaCommon javaCommon) { + String mainClass = ruleContext.getRule().isAttrDefined("main_class", Type.STRING) ? ruleContext.attributes().get("main_class", Type.STRING) : ""; + boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN); + boolean useTestrunner = ruleContext.attributes().get("use_testrunner", Type.BOOLEAN) + && !useLegacyJavaTest(ruleContext); + + if (createExecutable) { + if (useTestrunner) { + mainClass = "com.google.testing.junit.runner.BazelTestRunner"; + } else { /* java_binary or non-Junit java_test */ + if (mainClass.isEmpty()) { + mainClass = javaCommon.determinePrimaryClass(javaCommon.getSrcsArtifacts()); + } + } + } + + return mainClass; } private void checkMainClass(RuleContext ruleContext, JavaCommon javaCommon) { boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN); - String mainClass = getMainClassInternal(ruleContext); + String mainClass = getMainClassInternal(ruleContext, javaCommon); if (!createExecutable && !mainClass.isEmpty()) { ruleContext.ruleError("main class must not be specified when executable is not created"); @@ -95,8 +115,7 @@ public class BazelJavaSemantics implements JavaSemantics { if (createExecutable && mainClass.isEmpty()) { if (javaCommon.getSrcsArtifacts().isEmpty()) { - ruleContext.ruleError( - "need at least one of 'main_class', 'use_testrunner' or Java source files"); + ruleContext.ruleError("need at least one of 'main_class' or Java source files"); } mainClass = javaCommon.determinePrimaryClass(javaCommon.getSrcsArtifacts()); if (mainClass == null) { @@ -111,7 +130,7 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public String getMainClass(RuleContext ruleContext, JavaCommon javaCommon) { checkMainClass(ruleContext, javaCommon); - return getMainClassInternal(ruleContext); + return getMainClassInternal(ruleContext, javaCommon); } @Override @@ -198,9 +217,30 @@ public class BazelJavaSemantics implements JavaSemantics { } } + private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) { + 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)) { + return Iterables.getOnlyElement(ruleContext.getPrerequisites("$testsupport", Mode.TARGET)); + } else { + return null; + } + } + @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); + } } @Override @@ -210,6 +250,13 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public void collectTargetsTreatedAsDeps( RuleContext ruleContext, ImmutableList.Builder builder) { + 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); + } } @Override @@ -230,17 +277,87 @@ public class BazelJavaSemantics implements JavaSemantics { NestedSetBuilder filesBuilder, RuleConfiguredTargetBuilder ruleBuilder) { if (isJavaBinaryOrJavaTest(ruleContext)) { - boolean createExec = ruleContext.attributes().get("create_executable", Type.BOOLEAN); - ruleBuilder.add(JavaPrimaryClassProvider.class, - new JavaPrimaryClassProvider(createExec ? getMainClassInternal(ruleContext) : null)); + ruleBuilder.add( + JavaPrimaryClassProvider.class, + new JavaPrimaryClassProvider(getPrimaryClass(ruleContext, javaCommon))); + } + } + + // TODO(dmarting): simplify that logic when we remove the legacy Bazel java_test behavior. + private String getPrimaryClassLegacy(RuleContext ruleContext, JavaCommon javaCommon) { + boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN); + if (!createExecutable) { + return null; } + return getMainClassInternal(ruleContext, javaCommon); } + private String getPrimaryClassNew(RuleContext ruleContext, JavaCommon javaCommon) { + boolean createExecutable = ruleContext.attributes().get("create_executable", Type.BOOLEAN); + Set sourceFiles = ImmutableSet.copyOf(javaCommon.getSrcsArtifacts()); + + if (!createExecutable) { + return null; + } + + boolean useTestrunner = ruleContext.attributes().get("use_testrunner", Type.BOOLEAN); + + String testClass = ruleContext.getRule().isAttrDefined("test_class", Type.STRING) + ? ruleContext.attributes().get("test_class", Type.STRING) : ""; + + if (useTestrunner) { + if (testClass.isEmpty()) { + testClass = javaCommon.determinePrimaryClass(sourceFiles); + if (testClass == null) { + ruleContext.ruleError("cannot determine junit.framework.Test class " + + "(Found no source file '" + ruleContext.getTarget().getName() + + ".java' and package name doesn't include 'java' or 'javatests'. " + + "You might want to rename the rule or add a 'test_class' " + + "attribute.)"); + } + } + return testClass; + } else { + if (!testClass.isEmpty()) { + ruleContext.attributeError("test_class", "this attribute is only meaningful to " + + "BazelTestRunner, but you are not using it (use_testrunner = 0)"); + } + + return getMainClassInternal(ruleContext, javaCommon); + } + } + + private String getPrimaryClass(RuleContext ruleContext, JavaCommon javaCommon) { + return useLegacyJavaTest(ruleContext) ? getPrimaryClassLegacy(ruleContext, javaCommon) + : getPrimaryClassNew(ruleContext, javaCommon); + } @Override public Iterable getJvmFlags( RuleContext ruleContext, JavaCommon javaCommon, List userJvmFlags) { - return userJvmFlags; + ImmutableList.Builder jvmFlags = ImmutableList.builder(); + jvmFlags.addAll(userJvmFlags); + + 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(javaCommon.getSrcsArtifacts()); + } + + 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(); } @Override @@ -309,22 +426,29 @@ public class BazelJavaSemantics implements JavaSemantics { @Override public List getExtraArguments(RuleContext ruleContext, JavaCommon javaCommon) { if (ruleContext.getRule().getRuleClass().equals("java_test")) { - if (ruleContext.getConfiguration().getTestArguments().isEmpty() - && !ruleContext.attributes().isAttributeValueExplicitlySpecified("args")) { - ImmutableList.Builder builder = ImmutableList.builder(); - for (Artifact artifact : javaCommon.getSrcsArtifacts()) { - PathFragment path = artifact.getRootRelativePath(); - String className = JavaUtil.getJavaFullClassname(FileSystemUtils.removeExtension(path)); - if (className != null) { - builder.add(className); + if (useLegacyJavaTest(ruleContext)) { + if (ruleContext.getConfiguration().getTestArguments().isEmpty() + && !ruleContext.attributes().isAttributeValueExplicitlySpecified("args")) { + ImmutableList.Builder builder = ImmutableList.builder(); + for (Artifact artifact : javaCommon.getSrcsArtifacts()) { + PathFragment path = artifact.getRootRelativePath(); + String className = JavaUtil.getJavaFullClassname(FileSystemUtils.removeExtension(path)); + if (className != null) { + builder.add(className); + } } + return builder.build(); } - return builder.build(); } } return ImmutableList.of(); } + private boolean useLegacyJavaTest(RuleContext ruleContext) { + return !ruleContext.attributes().isAttributeValueExplicitlySpecified("test_class") + && ruleContext.getFragment(JavaConfiguration.class).useLegacyBazelJavaTest(); + } + @Override public String getJavaBuilderMainClass() { return JAVABUILDER_CLASS_NAME; 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 e9d44bebe8..c3c5b7558c 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 @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.bazel.rules.java; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; +import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; import static com.google.devtools.build.lib.syntax.Type.STRING; import com.google.devtools.build.lib.analysis.BaseRuleClasses; @@ -50,9 +51,45 @@ public final class BazelJavaTestRule implements RuleDefinition { return builder .requiresConfigurationFragments(JavaConfiguration.class, Jvm.class) .setImplicitOutputsFunction(BazelJavaRuleClasses.JAVA_BINARY_IMPLICIT_OUTPUTS) - .override(attr("main_class", STRING).value(JUNIT4_RUNNER)) .override(attr("stamp", TRISTATE).value(TriState.NO)) + .override(attr("use_testrunner", BOOLEAN).value(true)) .override(attr(":java_launcher", LABEL).value(JavaSemantics.JAVA_LAUNCHER)) + // TODO(dmarting): remove once we drop the legacy bazel java_test behavior. + .override(attr("main_class", STRING).value(JUNIT4_RUNNER)) + /* + The Java class to be loaded by the test runner.
+ ${SYNOPSIS} +

+ By default, if this argument is not defined then the legacy mode is used and the + test arguments are used instead. Set the --nolegacy_bazel_java_test flag + to not fallback on the first argument. +

+

+ This attribute specifies the name of a Java class to be run by + this test. It is rare to need to set this. If this argument is omitted, the Java class + whose name corresponds to the name of this + java_test rule will be used. +

+

+ For JUnit3, the test class needs to either be a subclass of + junit.framework.TestCase or it needs to have a public + static suite() method that returns a + junit.framework.Test (or a subclass of Test). + For JUnit4, the class needs to be annotated with + org.junit.runner.RunWith. +

+

+ This attribute allows several java_test rules to + share the same Test + (TestCase, TestSuite, ...). Typically + additional information is passed to it + (e.g. via jvm_flags=['-Dkey=value']) so that its + behavior differs in each case, such as running a different + subset of the tests. This attribute also enables the use of + Java tests outside the javatests tree. +

+ */ + .add(attr("test_class", STRING)) .build(); } @@ -81,10 +118,9 @@ ${IMPLICIT_OUTPUTS} ${ATTRIBUTE_DEFINITION}

-See the section on java_binary() arguments, with the caveat -that there is no main_class argument. This rule also supports all -attributes common to all test rules -(*_test). +See the section on java_binary() arguments. This rule also +supports all attributes common +to all test rules (*_test).

Examples

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 246253a406..315cf67797 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 @@ -142,12 +142,16 @@ public final class JavaConfiguration extends Fragment { private final ImmutableList