Skip to content

Commit d0c75ff

Browse files
committed
Provide --experimental_instrumentation_filter_fragment cli option
This option is similar to `--instrumentation_filter` option. The difference is this option can be used multiple times to accumulate its values. If this option is provided `--instrumentation_filter` has no effect. Multiple uses of `--instrumentation_filter` are not accumulated. This makes it difficult to have short hand configuration flags for a combination of coverage flags in a bazelrc file. e.g., ``` build:covA --instrumentation_filter=afoo/abar,-afoo/abaz build:covB --instrumentation_filter=bfoo/bbar,-bfoo/bbaz build:covC --instrumentation_filter=cfoo/cbar,-cfoo/cbaz \# To combine the flags concatenate the respective filter strings build:covAB --instrumentation_filter=afoo/abar,-afoo/abaz,bfoo/bbar,-bfoo/bbaz build:covAC --instrumentation_filter=afoo/abar,-afoo/abaz,cfoo/cbar,-cfoo/cbaz build:covABC --instrumentation_filter=afoo/abar,-afoo/abaz,bfoo/bbar,-bfoo/bbaz,cfoo/cbar,-cfoo/cbaz ``` With a large set of flags and their respective large filter strings it is uneasy to combine the short hand configuration flags because the respective filter strings need to be concatenated into a single string. This is uneasy to maintain because filter strings are repeated in the combinations. `--experimental_instrumentation_filter_fragment` simplifies combining multiple shorthand configuration flags because its multiple uses are accumulated. e.g., ``` build:covA --experimental_instrumentation_filter_fragment=afoo/abar,-afoo/abaz build:covB --experimental_instrumentation_filter_fragment=bfoo/bbar,-bfoo/bbaz build:covC --experimental_instrumentation_filter_fragment=cfoo/cbar,-cfoo/cbaz build:covAB --config covA --config covB build:covAC --config covA --config covC build:covABC --config covA --config covB --config covC ``` Add tests for --experimental_instrumentation_filter_fragment cli to verify that its multiple uses are accumulated and --instrumentation_filter has no effect when --experimental_instrumentation_filter_fragment is used. bazelbuild#22959 is the upstream issue. Testing Done: ``` $ bazelisk test src/test/java/com/google/devtools/build/lib/starlark:all INFO: Analyzed 4 targets (0 packages loaded, 0 targets configured). INFO: Found 1 target and 3 test targets... INFO: Elapsed time: 242.197s, Critical Path: 216.72s INFO: 27 processes: 1 internal, 25 darwin-sandbox, 1 worker. INFO: Build completed successfully, 27 total actions //src/test/java/com/google/devtools/build/lib/starlark:BindTest (cached) PASSED in 7.5s //src/test/java/com/google/devtools/build/lib/starlark:StarlarkRuleClassFunctionsTest (cached) PASSED in 78.0s Stats over 5 runs: max = 78.0s, min = 59.8s, avg = 73.9s, dev = 7.1s //src/test/java/com/google/devtools/build/lib/starlark:StarlarkTests PASSED in 121.1s Stats over 25 runs: max = 121.1s, min = 24.8s, avg = 88.6s, dev = 32.1s Executed 1 out of 3 tests: 3 tests pass. There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are. ```
1 parent 29f7c86 commit d0c75ff

File tree

3 files changed

+109
-1
lines changed

3 files changed

+109
-1
lines changed

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.devtools.build.lib.analysis.config;
1616

1717
import com.google.common.annotations.VisibleForTesting;
18+
import com.google.common.base.Joiner;
1819
import com.google.common.collect.ImmutableCollection;
1920
import com.google.common.collect.ImmutableMap;
2021
import com.google.common.collect.ImmutableSet;
@@ -44,6 +45,7 @@
4445
import com.google.devtools.build.lib.vfs.PathFragment;
4546
import com.google.devtools.build.skyframe.SkyValue;
4647
import com.google.devtools.common.options.TriState;
48+
import com.google.devtools.common.options.OptionsParsingException;
4749
import java.io.PrintStream;
4850
import java.util.HashMap;
4951
import java.util.List;
@@ -143,6 +145,8 @@ public void reportInvalidOptions(EventHandler reporter) {
143145
}
144146
}
145147

148+
private final RegexFilter instrumentationFilter;
149+
146150
/**
147151
* Compute the test environment, which, at configuration level, is a pair consisting of the
148152
* statically set environment variables with their values and the set of environment variables to
@@ -298,6 +302,19 @@ private static ImmutableSortedMap<Class<? extends Fragment>, Fragment> getConfig
298302
this.reservedActionMnemonics = reservedActionMnemonics;
299303
this.commandLineLimits = new CommandLineLimits(options.minParamFileSize);
300304
this.defaultFeatures = FeatureSet.parse(options.defaultFeatures);
305+
306+
if (!options.instrumentationFilterFragment.isEmpty()) {
307+
// A regex-based instrumentation filter instance is formed by concatenating
308+
// string values of --experimental_instrumentation_filter_fragment.
309+
try {
310+
this.instrumentationFilter = new RegexFilter.RegexFilterConverter().convert(
311+
Joiner.on(",").join(options.instrumentationFilterFragment));
312+
} catch (OptionsParsingException e) {
313+
throw new RuntimeException(e);
314+
}
315+
} else {
316+
this.instrumentationFilter = options.instrumentationFilter;
317+
}
301318
}
302319

303320
@Override
@@ -566,7 +583,7 @@ public Iterable<String> getVariableShellEnvironment() {
566583
* identify targets to be instrumented in the coverage mode.
567584
*/
568585
public RegexFilter getInstrumentationFilter() {
569-
return options.instrumentationFilter;
586+
return instrumentationFilter;
570587
}
571588

572589
/**

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.collect.ImmutableList.toImmutableList;
1818
import static com.google.common.collect.ImmutableMap.toImmutableMap;
1919

20+
import com.google.common.base.Joiner;
2021
import com.google.common.collect.ImmutableList;
2122
import com.google.common.collect.ImmutableMap;
2223
import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.LabelListConverter;
@@ -274,6 +275,22 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
274275
+ "instrumented unless --instrument_test_targets is enabled.")
275276
public RegexFilter instrumentationFilter;
276277

278+
@Option(
279+
name = "experimental_instrumentation_filter_fragment",
280+
allowMultiple = true,
281+
defaultValue = "null",
282+
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
283+
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
284+
help =
285+
"When coverage is enabled, only rules with names included by the "
286+
+ "specified regex-based filter will be instrumented. Rules prefixed "
287+
+ "with '-' are excluded instead. Note that only non-test rules are "
288+
+ "instrumented unless --instrument_test_targets is enabled. "
289+
+ "Excluded filters always override included ones. This option can be used "
290+
+ "multiple times. If this option is provided --instrumentation_filter "
291+
+ "has no effect.")
292+
public List<String> instrumentationFilterFragment;
293+
277294
@Option(
278295
name = "instrument_test_targets",
279296
defaultValue = "false",

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3020,6 +3020,80 @@ private void setUpCoverageInstrumentedTest() throws Exception {
30203020
""");
30213021
}
30223022

3023+
@Test
3024+
public void testCoverageExperimentalInstrumentedFragmentCoverageDisabled() throws Exception {
3025+
setUpCoverageInstrumentedTest();
3026+
useConfiguration(
3027+
"--nocollect_code_coverage",
3028+
"--experimental_instrumentation_filter_fragment=.");
3029+
StarlarkRuleContext ruleContext = createRuleContext("//test:foo");
3030+
setRuleContext(ruleContext);
3031+
Object result = ev.eval("ruleContext.coverage_instrumented()");
3032+
assertThat((Boolean) result).isFalse();
3033+
}
3034+
3035+
@Test
3036+
public void testCoverageExperimentalInstrumentedFragmentFalseForSourceFileLabel() throws Exception {
3037+
setUpCoverageInstrumentedTest();
3038+
useConfiguration(
3039+
"--collect_code_coverage",
3040+
"--experimental_instrumentation_filter_fragment=:foo",
3041+
"--experimental_instrumentation_filter_fragment=:bar");
3042+
setRuleContext(createRuleContext("//test:foo"));
3043+
Object result = ev.eval("ruleContext.coverage_instrumented(ruleContext.attr.srcs[0])");
3044+
assertThat((Boolean) result).isFalse();
3045+
}
3046+
3047+
@Test
3048+
public void testCoverageExperimentalInstrumentedFragmentDoesNotMatchFilter() throws Exception {
3049+
setUpCoverageInstrumentedTest();
3050+
useConfiguration(
3051+
"--collect_code_coverage",
3052+
"--experimental_instrumentation_filter_fragment=:foo",
3053+
"--experimental_instrumentation_filter_fragment=-:bar");
3054+
setRuleContext(createRuleContext("//test:bar"));
3055+
Object result = ev.eval("ruleContext.coverage_instrumented()");
3056+
assertThat((Boolean) result).isFalse();
3057+
}
3058+
3059+
@Test
3060+
public void testCoverageExperimentalInstrumentedFragmentMatchesFilter() throws Exception {
3061+
setUpCoverageInstrumentedTest();
3062+
useConfiguration(
3063+
"--collect_code_coverage",
3064+
"--experimental_instrumentation_filter_fragment=:foo",
3065+
"--experimental_instrumentation_filter_fragment=:bar");
3066+
setRuleContext(createRuleContext("//test:foo"));
3067+
Object result = ev.eval("ruleContext.coverage_instrumented()");
3068+
assertThat((Boolean) result).isTrue();
3069+
}
3070+
3071+
@Test
3072+
public void testCoverageExperimentalInstrumentedFragmentDoesNotMatchFilterNonDefaultLabel() throws Exception {
3073+
setUpCoverageInstrumentedTest();
3074+
useConfiguration(
3075+
"--collect_code_coverage",
3076+
"--experimental_instrumentation_filter_fragment=:foo",
3077+
// --instrumentation_filter has no effect because --experimental_instrumentation_filter_fragment is used.
3078+
"--instrumentation_filter=:bar");
3079+
setRuleContext(createRuleContext("//test:foo"));
3080+
// //test:bar does not match :foo, though //test:foo would.
3081+
Object result = ev.eval("ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
3082+
assertThat((Boolean) result).isFalse();
3083+
}
3084+
3085+
@Test
3086+
public void testCoverageExperimentalInstrumentedFragmentMatchesFilterNonDefaultLabel() throws Exception {
3087+
setUpCoverageInstrumentedTest();
3088+
useConfiguration(
3089+
"--collect_code_coverage",
3090+
"--experimental_instrumentation_filter_fragment=:bar");
3091+
setRuleContext(createRuleContext("//test:foo"));
3092+
// //test:bar does match :bar, though //test:foo would not.
3093+
Object result = ev.eval("ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
3094+
assertThat((Boolean) result).isTrue();
3095+
}
3096+
30233097
@Test
30243098
public void testCoverageInstrumentedCoverageDisabled() throws Exception {
30253099
setUpCoverageInstrumentedTest();

0 commit comments

Comments
 (0)