Add --[no]instrument_test_targets option to Blaze
When true (the default), test rules included by --instrumentation_filter are instrumented. When false, test rules are excluded from instrumentation (whether or not their names would be matched by --instrumentation_filter).
This option provides a superior way to exclude test rules from instrumentation, compared to setting --instrumentation_filter exclusions based on test rule naming conventions. Naming conventions vary from language to language and are not always followed. Blaze already has enforced semantics for test targets (test rule build function names _must_ end in "_test", see TargetUtils.isTestRule). In general, when either option is available, it's better to rely on enforced semantics than unenforced conventions.
Note that this default preserves present behavior. In the future, I think it would make sense to:
* Change the default value of --instrument_test_targets to false.
* Change the default value of --instrumentation_filter to an empty string (match everything).
RELNOTES: Add --instrument_test_targets option.
--
MOS_MIGRATED_REVID=124732226
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index f7671a2..3977caf 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1284,6 +1284,13 @@
}
/**
+ * Returns true if the target for this context is a test target.
+ */
+ public boolean isTestTarget() {
+ return TargetUtils.isTestRule(getTarget());
+ }
+
+ /**
* Returns true if runfiles support should create the runfiles tree, or
* false if it should just create the manifest.
*/
@@ -1298,7 +1305,7 @@
// b. host tools could potentially use data files, but currently don't
// (they're run from the execution root, not a runfiles tree).
// Currently hostConfiguration.buildRunfiles() returns true.
- if (TargetUtils.isTestRule(getTarget())) {
+ if (isTestTarget()) {
// Tests are only executed during testing (duh),
// and their runfiles are generated lazily on local
// execution (see LocalTestStrategy). Therefore, it
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 3f8c6be..f3d7b7f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -559,7 +559,7 @@
public boolean stampBinaries;
// TODO(bazel-team): delete from OSS tree
- // This value is always overwritten in the case of "blaze coverage" by :
+ // This default value is always overwritten in the case of "blaze coverage" by
// CoverageCommand.setDefaultInstrumentationFilter()
@Option(name = "instrumentation_filter",
converter = RegexFilter.RegexFilterConverter.class,
@@ -571,6 +571,15 @@
+ "'javatests' or ending with '_test' will not be instrumented.")
public RegexFilter instrumentationFilter;
+ @Option(name = "instrument_test_targets",
+ defaultValue = "true",
+ category = "semantics",
+ help = "When coverage is enabled, specifies whether to consider instrumenting test rules. "
+ + "When true (the default), test rules included by --instrumentation_filter are "
+ + "instrumented. When false, test rules are always excluded from coverage "
+ + "instrumentation.")
+ public boolean instrumentTestTargets;
+
@Option(name = "show_cached_analysis_results",
defaultValue = "true",
category = "undocumented",
@@ -2045,6 +2054,15 @@
}
/**
+ * Returns a boolean of whether to include targets created by *_test rules in the set of targets
+ * matched by --instrumentation_filter. If this is false, all test targets are excluded from
+ * instrumentation.
+ */
+ public boolean shouldInstrumentTestTargets() {
+ return options.instrumentTestTargets;
+ }
+
+ /**
* Returns the set of labels for coverage.
*/
public Set<Label> getCoverageLabels() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
index 9951f63..bd20a52 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFilesCollector.java
@@ -21,6 +21,7 @@
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
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;
@@ -250,8 +251,9 @@
public static final LocalMetadataCollector NO_METADATA_COLLECTOR = null;
private static boolean shouldIncludeLocalSources(RuleContext ruleContext) {
- return ruleContext.getConfiguration().getInstrumentationFilter().isIncluded(
- ruleContext.getLabel().toString());
+ BuildConfiguration config = ruleContext.getConfiguration();
+ return ((config.shouldInstrumentTestTargets() || !ruleContext.isTestTarget())
+ && config.getInstrumentationFilter().isIncluded(ruleContext.getLabel().toString()));
}
private static Iterable<TransitiveInfoCollection> getAllPrerequisites(
