From 384e1cbc6a62e415648721f56b641001c0a7ae8b Mon Sep 17 00:00:00 2001 From: iirina Date: Wed, 29 May 2019 07:45:22 -0700 Subject: [PATCH] Only add CoverageOutputGenerator in coverage mode. Fixes #8355. Closes #8477. PiperOrigin-RevId: 250489150 --- .../build/lib/analysis/BaseRuleClasses.java | 18 ++++++++ .../lib/analysis/test/TestActionBuilder.java | 7 +-- .../lib/bazel/rules/cpp/BazelCcTestRule.java | 7 +-- .../bazel/rules/java/BazelJavaTestRule.java | 7 +-- .../lib/bazel/rules/sh/BazelShTestRule.java | 9 +--- .../sh/BazelShTestConfiguredTargetTest.java | 46 +++++++++++++++++++ 6 files changed, 72 insertions(+), 22 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestConfiguredTargetTest.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index cfb61b59a34f34..0340500436b2cd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -99,6 +99,9 @@ public static LabelLateBoundDefault coverageSupportAttribute( public static final String DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE = "//tools/test:coverage_report_generator"; + private static final String DEFAULT_COVERAGE_OUTPUT_GENERATOR_VALUE = + "@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main"; + @AutoCodec static final Resolver COVERAGE_REPORT_GENERATOR_CONFIGURATION_RESOLVER = (rule, attributes, configuration) -> configuration.getCoverageReportGenerator(); @@ -109,6 +112,21 @@ public static LabelLateBoundDefault coverageReportGeneratorAt TestConfiguration.class, defaultValue, COVERAGE_REPORT_GENERATOR_CONFIGURATION_RESOLVER); } + public static LabelLateBoundDefault getCoverageOutputGeneratorLabel() { + return LabelLateBoundDefault.fromTargetConfiguration( + BuildConfiguration.class, null, COVERAGE_OUTPUT_GENERATOR_RESOLVER); + } + + @AutoCodec + static final Resolver COVERAGE_OUTPUT_GENERATOR_RESOLVER = + (rule, attributes, configuration) -> { + if (configuration.isCodeCoverageEnabled()) { + return Label.parseAbsoluteUnchecked(DEFAULT_COVERAGE_OUTPUT_GENERATOR_VALUE); + } else { + return null; + } + }; + // TODO(b/65746853): provide a way to do this without passing the entire configuration /** Implementation for the :run_under attribute. */ @AutoCodec 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 ef8c9963c1d905..6e5a58ea3c854a 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 @@ -269,9 +269,9 @@ private TestParams createTestAction(int shards) { extraTestEnv.put(BAZEL_CC_COVERAGE_TOOL, GCOV_TOOL); // We don't add this attribute to non-supported test target - if (ruleContext.isAttrDefined("$lcov_merger", LABEL)) { + if (ruleContext.isAttrDefined(":lcov_merger", LABEL)) { TransitiveInfoCollection lcovMerger = - ruleContext.getPrerequisite("$lcov_merger", Mode.TARGET); + ruleContext.getPrerequisite(":lcov_merger", Mode.TARGET); FilesToRunProvider lcovFilesToRun = lcovMerger.getProvider(FilesToRunProvider.class); if (lcovFilesToRun != null) { extraTestEnv.put(LCOV_MERGER, lcovFilesToRun.getExecutable().getExecPathString()); @@ -285,7 +285,8 @@ private TestParams createTestAction(int shards) { extraTestEnv.put(LCOV_MERGER, lcovMergerArtifact.getExecPathString()); inputsBuilder.add(lcovMergerArtifact); } else { - ruleContext.attributeError("$lcov_merger", + ruleContext.attributeError( + ":lcov_merger", "the LCOV merger should be either an executable or a single artifact"); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java index a200c6d90a2a5a..7995f5b06da8a3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcTestRule.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses.CcBinaryBaseRule; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; @@ -45,11 +44,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) // to decorate data symbols imported from DLL. .override(attr("linkstatic", BOOLEAN).value(OS.getCurrent() == OS.WINDOWS)) .override(attr("stamp", TRISTATE).value(TriState.NO)) - .add( - attr("$lcov_merger", LABEL) - .value( - Label.parseAbsoluteUnchecked( - "@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main"))) + .add(attr(":lcov_merger", LABEL).value(BaseRuleClasses.getCoverageOutputGeneratorLabel())) .add( attr("$collect_cc_coverage", LABEL) .cfg(HostTransition.createFactory()) 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 793b8273dd31a7..a749cffa796361 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 @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses.BaseJavaBinaryRule; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.TriState; @@ -66,11 +65,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .override(attr("use_testrunner", BOOLEAN).value(true)) .override(attr(":java_launcher", LABEL).value(JavaSemantics.JAVA_LAUNCHER)) // Input files for test actions collecting code coverage - .add( - attr("$lcov_merger", LABEL) - .value( - Label.parseAbsoluteUnchecked( - "@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main"))) + .add(attr(":lcov_merger", LABEL).value(BaseRuleClasses.getCoverageOutputGeneratorLabel())) .add( attr("$jacocorunner", LABEL) .value(env.getToolsLabel("//tools/jdk:JacocoCoverageRunner"))) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java index 6548ccd152dfb3..7ef224fa4b5ab0 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestRule.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.bazel.rules.sh.BazelShRuleClasses.ShRule; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -31,13 +30,9 @@ public final class BazelShTestRule implements RuleDefinition { @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) { - // TODO(bazel-team): Add $lcov_merger to every test rule as opposed to particular rules. + // TODO(bazel-team): Add :lcov_merger to every test rule as opposed to particular rules. builder - .add( - attr("$lcov_merger", LABEL) - .value( - Label.parseAbsoluteUnchecked( - "@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main"))) + .add(attr(":lcov_merger", LABEL).value(BaseRuleClasses.getCoverageOutputGeneratorLabel())) .add( attr("$launcher", LABEL) .cfg(HostTransition.createFactory()) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestConfiguredTargetTest.java new file mode 100644 index 00000000000000..bc6a71237bf756 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShTestConfiguredTargetTest.java @@ -0,0 +1,46 @@ +// 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.devtools.build.lib.bazel.rules.sh; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for sh_test configured target. */ +@RunWith(JUnit4.class) +public class BazelShTestConfiguredTargetTest extends BuildViewTestCase { + @Test + public void testCoverageOutputGenerator() throws Exception { + scratch.file("sh/test/BUILD", "sh_test(name = 'foo_test', srcs = ['foo_test.sh'])"); + reporter.removeHandler(failFastHandler); + ConfiguredTarget ct = getConfiguredTarget("//sh/test:foo_test"); + assertThat(getRuleContext(ct).getPrerequisite(":lcov_merger", Mode.HOST)).isNull(); + } + + @Test + public void testCoverageOutputGeneratorCoverageMode() throws Exception { + useConfiguration("--collect_code_coverage"); + scratch.file("sh/test/BUILD", "sh_test(name = 'foo_test', srcs = ['foo_test.sh'])"); + reporter.removeHandler(failFastHandler); + ConfiguredTarget ct = getConfiguredTarget("//sh/test:foo_test"); + assertThat(getRuleContext(ct).getPrerequisite(":lcov_merger", Mode.HOST).getLabel().toString()) + .isEqualTo( + "@bazel_tools//tools/test/CoverageOutputGenerator/java/com/google/devtools/coverageoutputgenerator:Main"); + } +}