diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java index 55e084acfae340..bdec79ed8cb90d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java +++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java @@ -381,6 +381,29 @@ public static Predicate tagFilter(List tagFilterList) { }; } + /** + * Returns a predicate to be used for test rule name filtering, i.e., that only accepts tests that match + * a required rule name and not an excluded rule name. + */ + public static Predicate ruleFilter(List ruleFilterList) { + Pair, Collection> ruleLists = + TestTargetUtils.sortTagsBySense(ruleFilterList); + final Collection requiredRules = ruleLists.first; + final Collection excludedRules = ruleLists.second; + return input -> { + if (requiredRules.isEmpty() && excludedRules.isEmpty()) { + return true; + } + + if (!(input instanceof Rule)) { + return requiredRules.isEmpty(); + } + + return TestTargetUtils.testMatchesRuleFilters( + ((Rule) input).getRuleClass(), requiredRules, excludedRules); + }; + } + /** Return {@link Location} for {@link Target} target, if it should not be null. */ @Nullable public static Location getLocationMaybe(Target target) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java index 3f5006e2930e76..ef28c72bd5c169 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java +++ b/src/main/java/com/google/devtools/build/lib/packages/TestTargetUtils.java @@ -91,6 +91,18 @@ private static boolean testMatchesFilters( return testMatchesFilters(testTags, requiredTags, excludedTags); } + /** + * Returns whether a test with a rule name matches a filter (as specified by the set + * of its positive and its negative filters). + */ + public static boolean testMatchesRuleFilters( + String ruleName, + Collection requiredRules, + Collection excludedRules) { + return (requiredRules.isEmpty() || requiredRules.contains(ruleName)) && + !excludedRules.contains(ruleName); + } + /** * Filters 'tests' (by mutation) according to the 'tags' attribute, specifically those that * match ALL of the tags in tagsAttribute. diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java index 436ae9c932a5d5..42392309b00952 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java @@ -128,6 +128,35 @@ public class LoadingOptions extends OptionsBase { ) public List testLangFilterList; + @Option( + name = "build_rule_filters", + converter = CommaSeparatedOptionListConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Specifies a comma-separated list of rule names. Each rule name can be optionally " + + "preceded with '-' to specify excluded rule names. Only those targets will be built that " + + "equal the positive rule name or do not equal the negative rule name. This option " + + "does not affect the set of tests executed with the 'test' command; those are be " + + "governed by the test filtering options, for example '--test_rule_filters'" + ) + public List buildRuleFilterList; + + @Option( + name = "test_rule_filters", + converter = CommaSeparatedOptionListConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "Specifies a comma-separated list of rule names. Each rule name can be optionally " + + "preceded with '-' to specify excluded rule names. Only those test targets will be " + + "found that equal the positive rule name or do not equal the negative rule name." + + "This option affects --build_tests_only behavior and the test command." + ) + public List testRuleFilterList; + @Option( name = "build_manual_tests", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TestFilter.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TestFilter.java index fb59fd9d64ac28..009e6ab29c18b4 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/TestFilter.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TestFilter.java @@ -46,13 +46,15 @@ public static TestFilter forOptions(LoadingOptions options) { ImmutableSet.copyOf(options.testSizeFilterSet), ImmutableSet.copyOf(options.testTimeoutFilterSet), ImmutableList.copyOf(options.testTagFilterList), - ImmutableList.copyOf(options.testLangFilterList)); + ImmutableList.copyOf(options.testLangFilterList), + ImmutableList.copyOf(options.testRuleFilterList)); } private final ImmutableSet testSizeFilterSet; private final ImmutableSet testTimeoutFilterSet; private final ImmutableList testTagFilterList; private final ImmutableList testLangFilterList; + private final ImmutableList testRuleFilterList; private final Predicate impl; @VisibleForSerialization @@ -60,11 +62,13 @@ public static TestFilter forOptions(LoadingOptions options) { ImmutableSet testSizeFilterSet, ImmutableSet testTimeoutFilterSet, ImmutableList testTagFilterList, - ImmutableList testLangFilterList) { + ImmutableList testLangFilterList, + ImmutableList testRuleFilterList) { this.testSizeFilterSet = testSizeFilterSet; this.testTimeoutFilterSet = testTimeoutFilterSet; this.testTagFilterList = testTagFilterList; this.testLangFilterList = testLangFilterList; + this.testRuleFilterList = testRuleFilterList; Predicate testFilter = ALWAYS_TRUE; if (!testSizeFilterSet.isEmpty()) { testFilter = testFilter.and(testSizeFilter(testSizeFilterSet)); @@ -78,6 +82,9 @@ public static TestFilter forOptions(LoadingOptions options) { if (!testLangFilterList.isEmpty()) { testFilter = testFilter.and(testLangFilter(testLangFilterList)); } + if (!testRuleFilterList.isEmpty()) { + testFilter = testFilter.and(testRuleFilter(testRuleFilterList)); + } impl = testFilter; } @@ -89,7 +96,7 @@ public boolean apply(@Nullable Target input) { @Override public int hashCode() { return Objects.hash(testSizeFilterSet, testTimeoutFilterSet, testTagFilterList, - testLangFilterList); + testLangFilterList, testRuleFilterList); } @Override @@ -103,7 +110,8 @@ public boolean equals(Object o) { return f.testSizeFilterSet.equals(testSizeFilterSet) && f.testTimeoutFilterSet.equals(testTimeoutFilterSet) && f.testTagFilterList.equals(testTagFilterList) - && f.testLangFilterList.equals(testLangFilterList); + && f.testLangFilterList.equals(testLangFilterList) + && f.testRuleFilterList.equals(testRuleFilterList); } @Override @@ -113,6 +121,7 @@ public String toString() { .add("testTimeoutFilterSet", testTimeoutFilterSet) .add("testTagFilterList", testTagFilterList) .add("testLangFilterList", testLangFilterList) + .add("testRuleFilterList", testRuleFilterList) .toString(); } @@ -159,4 +168,28 @@ private static Predicate testLangFilter(List langFilterList) { && !excludedLangs.contains(ruleLang); }; } + + /** + * Returns a predicate to be used for test rule filtering, i.e., that only accepts tests of + * the specified rule names. + */ + private static Predicate testRuleFilter(List ruleFilterList) { + final Set requiredRules = new HashSet<>(); + final Set excludedRules = new HashSet<>(); + + for (String ruleFilter : ruleFilterList) { + if (ruleFilter.startsWith("-")) { + ruleFilter = ruleFilter.substring(1); + excludedRules.add(ruleFilter); + } else { + requiredRules.add(ruleFilter); + } + } + + return rule -> { + String ruleName = rule.getRuleClass(); + return (requiredRules.isEmpty() || requiredRules.contains(ruleName)) + && !excludedRules.contains(ruleName); + }; + } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index e705a678aa755b..889790d1b1c895 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -3002,6 +3002,7 @@ public TargetPatternPhaseValue loadTargetPatternsWithFilters( options.buildTestsOnly, determineTests, ImmutableList.copyOf(options.buildTagFilterList), + ImmutableList.copyOf(options.buildRuleFilterList), options.buildManualTests, options.expandTestSuites, TestFilter.forOptions(options)); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index 381a7868455bb2..9d8aa7f8b36b75 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -367,8 +367,10 @@ private static ResolvedTargets mergeAll( } } - ResolvedTargets result = - builder.filter(TargetUtils.tagFilter(options.getBuildTargetFilter())).build(); + builder.filter(TargetUtils.tagFilter(options.getBuildTargetFilter())); + builder.filter(TargetUtils.ruleFilter(options.getBuildRuleFilter())); + + ResolvedTargets result = builder.build(); if (options.getCompileOneDependency()) { EnvironmentBackedRecursivePackageProvider environmentBackedRecursivePackageProvider = new EnvironmentBackedRecursivePackageProvider(env); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java index cf6e31b59bc5da..9a17708c4f5722 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java @@ -155,6 +155,7 @@ public static TargetPatternPhaseKey key( boolean buildTestsOnly, boolean determineTests, ImmutableList buildTargetFilter, + ImmutableList buildRuleFilter, boolean buildManualTests, boolean expandTestSuites, @Nullable TestFilter testFilter) { @@ -165,6 +166,7 @@ public static TargetPatternPhaseKey key( buildTestsOnly, determineTests, buildTargetFilter, + buildRuleFilter, buildManualTests, expandTestSuites, testFilter); @@ -181,7 +183,7 @@ public static TargetPatternPhaseKey key( public static SkyKey keyWithoutFilters( ImmutableList targetPatterns, PathFragment offset) { return new TargetPatternPhaseKey( - targetPatterns, offset, false, false, false, ImmutableList.of(), false, false, null); + targetPatterns, offset, false, false, false, ImmutableList.of(), ImmutableList.of(), false, false, null); } /** The configuration needed to run the target pattern evaluation phase. */ @@ -194,6 +196,7 @@ static final class TargetPatternPhaseKey implements SkyKey { private final boolean buildTestsOnly; private final boolean determineTests; private final ImmutableList buildTargetFilter; + private final ImmutableList buildRuleFilter; private final boolean buildManualTests; private final boolean expandTestSuites; @Nullable private final TestFilter testFilter; @@ -205,6 +208,7 @@ private TargetPatternPhaseKey( boolean buildTestsOnly, boolean determineTests, ImmutableList buildTargetFilter, + ImmutableList buildRuleFilter, boolean buildManualTests, boolean expandTestSuites, @Nullable TestFilter testFilter) { @@ -214,6 +218,7 @@ private TargetPatternPhaseKey( this.buildTestsOnly = buildTestsOnly; this.determineTests = determineTests; this.buildTargetFilter = Preconditions.checkNotNull(buildTargetFilter); + this.buildRuleFilter = Preconditions.checkNotNull(buildRuleFilter); this.buildManualTests = buildManualTests; this.expandTestSuites = expandTestSuites; this.testFilter = testFilter; @@ -251,6 +256,10 @@ public ImmutableList getBuildTargetFilter() { return buildTargetFilter; } + public ImmutableList getBuildRuleFilter() { + return buildRuleFilter; + } + public boolean getBuildManualTests() { return buildManualTests; } @@ -305,6 +314,7 @@ public boolean equals(Object obj) { && other.buildTestsOnly == buildTestsOnly && other.determineTests == determineTests && other.buildTargetFilter.equals(buildTargetFilter) + && other.buildRuleFilter.equals(buildRuleFilter) && other.buildManualTests == buildManualTests && other.expandTestSuites == expandTestSuites && Objects.equals(other.testFilter, testFilter); diff --git a/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java b/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java index 5352474f57b985..14ddd99697c4f0 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/TargetUtilsTest.java @@ -96,6 +96,48 @@ public void testFilterByTag() throws Exception { assertThat(tagFilter.apply(tag1b)).isFalse(); } + @Test + public void testFilterByRuleName() throws Exception { + scratch.file( + "tests/BUILD", + """ + load("//test_defs:foo_binary.bzl", "foo_binary") + load("//test_defs:foo_test.bzl", "foo_test") + foo_binary( + name = "foo_binary", + ) + + foo_test( + name = "foo_test", + ) + """); + + Target fooBinary = getTarget("//tests:foo_binary"); + Target fooTest = getTarget("//tests:foo_test"); + + Predicate ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList()); + assertThat(ruleFilter.apply(fooBinary)).isTrue(); + assertThat(ruleFilter.apply(fooTest)).isTrue(); + ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList("foo_binary", "foo_test")); + assertThat(ruleFilter.apply(fooBinary)).isTrue(); + assertThat(ruleFilter.apply(fooTest)).isTrue(); + ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList("foo_binary")); + assertThat(ruleFilter.apply(fooBinary)).isTrue(); + assertThat(ruleFilter.apply(fooTest)).isFalse(); + ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList("-foo_test")); + assertThat(ruleFilter.apply(fooBinary)).isTrue(); + assertThat(ruleFilter.apply(fooTest)).isFalse(); + // Applying same tag as positive and negative filter produces an empty + // result because the negative filter is applied first and positive filter will + // not match anything. + ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList("foo_test", "-foo_test")); + assertThat(ruleFilter.apply(fooBinary)).isFalse(); + assertThat(ruleFilter.apply(fooTest)).isFalse(); + ruleFilter = TargetUtils.ruleFilter(Lists.newArrayList("foo_test", "-foo_binary")); + assertThat(ruleFilter.apply(fooBinary)).isFalse(); + assertThat(ruleFilter.apply(fooTest)).isTrue(); + } + @Test public void testExecutionInfo() throws Exception { scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/packages/TestTargetUtilsTest.java b/src/test/java/com/google/devtools/build/lib/packages/TestTargetUtilsTest.java index cc87e63177b3f5..1534eb84c6f333 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/TestTargetUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/TestTargetUtilsTest.java @@ -118,6 +118,34 @@ public void testFilterBySize() { public void testFilterByLang() { LoadingOptions options = new LoadingOptions(); options.testLangFilterList = ImmutableList.of("positive", "-negative"); + options.testRuleFilterList = ImmutableList.of(); + options.testSizeFilterSet = ImmutableSet.of(); + options.testTimeoutFilterSet = ImmutableSet.of(); + options.testTagFilterList = ImmutableList.of(); + TestFilter filter = TestFilter.forOptions(options); + Package pkg = mock(Package.class); + RuleClass ruleClass = mock(RuleClass.class); + when(ruleClass.getDefaultImplicitOutputsFunction()) + .thenReturn(SafeImplicitOutputsFunction.NONE); + when(ruleClass.getAttributeProvider()).thenReturn(mock(AttributeProvider.class)); + Rule mockRule = + new Rule( + pkg, + Label.parseCanonicalUnchecked("//pkg:a"), + ruleClass, + Location.fromFile(""), + /* interiorCallStack= */ null); + when(ruleClass.getName()).thenReturn("positive_test"); + assertThat(filter.apply(mockRule)).isTrue(); + when(ruleClass.getName()).thenReturn("negative_test"); + assertThat(filter.apply(mockRule)).isFalse(); + } + + @Test + public void testFilterByRule() { + LoadingOptions options = new LoadingOptions(); + options.testLangFilterList = ImmutableList.of(); + options.testRuleFilterList = ImmutableList.of("positive_test", "-negative_test"); options.testSizeFilterSet = ImmutableSet.of(); options.testTimeoutFilterSet = ImmutableSet.of(); options.testTagFilterList = ImmutableList.of(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java index 8b47da5e2d8c3e..bf70629fc216f1 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java @@ -53,6 +53,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), false, true, null, @@ -62,6 +63,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), false, false, null, @@ -71,6 +73,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), true, true, null, @@ -80,6 +83,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), true, false, null, @@ -89,6 +93,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), false, true, emptyTestFilter(), @@ -98,6 +103,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), true, true, emptyTestFilter(), @@ -107,6 +113,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), false, true, emptyTestFilter(), @@ -116,6 +123,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of(), + ImmutableList.of(), true, true, emptyTestFilter(), @@ -125,6 +133,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of("a"), + ImmutableList.of("a"), false, true, null)) @@ -133,6 +142,7 @@ public void testEquality() throws Exception { ImmutableList.of(), PathFragment.EMPTY_FRAGMENT, ImmutableList.of("a"), + ImmutableList.of("a"), true, true, null)) @@ -143,6 +153,7 @@ private static TargetPatternPhaseKey of( ImmutableList targetPatterns, PathFragment offset, ImmutableList buildTagFilter, + ImmutableList buildRuleFilter, boolean includeManualTests, boolean expandTestSuites, @Nullable TestFilter testFilter, @@ -158,6 +169,7 @@ private static TargetPatternPhaseKey of( buildTestsOnly, determineTests, buildTagFilter, + buildRuleFilter, includeManualTests, expandTestSuites, testFilter); @@ -165,7 +177,7 @@ private static TargetPatternPhaseKey of( private static TargetPatternPhaseKey of( ImmutableList targetPatterns, PathFragment offset) { - return of(targetPatterns, offset, ImmutableList.of(), false, true, null); + return of(targetPatterns, offset, ImmutableList.of(), ImmutableList.of(), false, true, null); } private static TestFilter emptyTestFilter() {