Make "blaze test <alias target>" run the test.
In order to stay consistent, also fix the tests() query operator. Not binaries(), though: query logic is kinda complicated so I don't want to shave that particular yak. Implementing ConfiguredTargetValueAccessor#getPrerequisites() was enough yak shaving for the day.
A drive-by cleanup that I *could* *have* done, but didn't, is to allow aliases in test_suite.tests and maybe make it configurable: now the logic is all there, so the only change that would be required is to actually allow aliases where we now allow only tests and test_suite rules and remove the non-configurable bit from the attribute. If we want that, it should be done in a separate change because it's separate functionality.
Taking all the select() branches for alias.actual is a questionable approach, but we don't have enough data where we (currently) compute the resolution of target patterns to do better and it's consistent with "blaze query", so I guess that'll do for now. I'll file that under the "blaze query should really be aware of configurations" label.
RELNOTES: None.
PiperOrigin-RevId: 281730608
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
index 4458b77..c51f3cf 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
@@ -91,7 +91,7 @@
private static class FilterTests extends AbstractFilteringPolicy {
@Override
public boolean shouldRetain(Target target, boolean explicit) {
- return TargetUtils.isTestOrTestSuiteRule(target)
+ return (TargetUtils.isTestOrTestSuiteRule(target) || TargetUtils.isAlias(target))
&& FILTER_MANUAL.shouldRetain(target, explicit);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
index c44b78a0..fdd77c0 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
@@ -14,16 +14,27 @@
package com.google.devtools.build.lib.query2;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.AttributeMap.DepEdge;
+import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.query2.aquery.ActionGraphQueryEnvironment;
import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetAccessor;
import com.google.devtools.build.lib.query2.engine.KeyExtractor;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.query2.engine.QueryExpression;
import com.google.devtools.build.lib.query2.engine.QueryVisibility;
+import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
import com.google.devtools.build.lib.skyframe.AspectValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
@@ -39,19 +50,22 @@
/**
* A {@link TargetAccessor} for {@link ConfiguredTargetValue} objects.
*
- * <p>Incomplete; we'll implement getPrerequisites and getVisibility when needed.
+ * <p>Incomplete; we'll implement getVisibility when needed.
*/
public class ConfiguredTargetValueAccessor implements TargetAccessor<ConfiguredTargetValue> {
private final WalkableGraph walkableGraph;
private final KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey>
configuredTargetKeyExtractor;
+ private final ActionGraphQueryEnvironment queryEnvironment;
public ConfiguredTargetValueAccessor(
WalkableGraph walkableGraph,
- KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey> configuredTargetKeyExtractor) {
+ KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey> configuredTargetKeyExtractor,
+ ActionGraphQueryEnvironment queryEnvironment) {
this.walkableGraph = walkableGraph;
this.configuredTargetKeyExtractor = configuredTargetKeyExtractor;
+ this.queryEnvironment = queryEnvironment;
}
@Override
@@ -94,14 +108,54 @@
}
@Override
+ public boolean isAlias(ConfiguredTargetValue configuredTargetValue) {
+ Target actualTarget = getTargetFromConfiguredTargetValue(configuredTargetValue);
+ return TargetUtils.isAlias(actualTarget);
+ }
+
+ private static Label getOriginalLabel(ConfiguredTargetValue configuredTargetValue) {
+ ConfiguredTarget configuredTarget = configuredTargetValue.getConfiguredTarget();
+
+ return configuredTarget instanceof AliasConfiguredTarget
+ ? ((AliasConfiguredTarget) configuredTarget).getOriginalLabel()
+ : configuredTarget.getLabel();
+ }
+
+ @Override
public List<ConfiguredTargetValue> getPrerequisites(
QueryExpression caller,
ConfiguredTargetValue configuredTargetValue,
String attrName,
String errorMsgPrefix)
- throws QueryException, InterruptedException {
- // TODO(bazel-team): implement this if needed.
- throw new UnsupportedOperationException();
+ throws InterruptedException {
+
+ ConfiguredTarget configuredTarget = configuredTargetValue.getConfiguredTarget();
+
+ Multimap<Label, ConfiguredTargetValue> depsByLabel =
+ Multimaps.index(
+ queryEnvironment.getFwdDeps(ImmutableList.of(configuredTargetValue)),
+ ConfiguredTargetValueAccessor::getOriginalLabel);
+
+ ImmutableMap<Label, ConfigMatchingProvider> configConditions;
+ if (configuredTarget instanceof RuleConfiguredTarget) {
+ configConditions = ((RuleConfiguredTarget) configuredTarget).getConfigConditions();
+ } else if (configuredTarget instanceof AliasConfiguredTarget) {
+ configConditions = ((AliasConfiguredTarget) configuredTarget).getConfigConditions();
+ } else {
+ throw new IllegalStateException();
+ }
+
+ ConfiguredAttributeMapper mapper =
+ ConfiguredAttributeMapper.of(
+ (Rule) getTargetFromConfiguredTargetValue(configuredTargetValue), configConditions);
+
+ // Doing this with streams results in a type List<? super ConfiguredTargetValue>
+ ImmutableList.Builder<ConfiguredTargetValue> result = ImmutableList.builder();
+ for (DepEdge depEdge : mapper.visitLabels(mapper.getAttributeDefinition(attrName))) {
+ result.addAll(depsByLabel.get(depEdge.getLabel()));
+ }
+
+ return result.build();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
index f004f83..279b367 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
@@ -110,7 +110,7 @@
};
this.accessor =
new ConfiguredTargetValueAccessor(
- walkableGraphSupplier.get(), this.configuredTargetKeyExtractor);
+ walkableGraphSupplier.get(), this.configuredTargetKeyExtractor, this);
}
public ActionGraphQueryEnvironment(
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
index b6973fb..04a56c9 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
@@ -99,6 +99,12 @@
}
@Override
+ public boolean isAlias(ConfiguredTarget target) {
+ Target actualTarget = getTargetFromConfiguredTarget(target);
+ return TargetUtils.isAlias(actualTarget);
+ }
+
+ @Override
public List<ConfiguredTarget> getPrerequisites(
QueryExpression caller,
ConfiguredTarget configuredTarget,
@@ -114,11 +120,18 @@
Multimap<Label, ConfiguredTarget> depsByLabel =
Multimaps.index(
queryEnvironment.getFwdDeps(ImmutableList.of(configuredTarget)),
- ConfiguredTarget::getLabel);
+ ConfiguredTargetAccessor::getOriginalLabel);
Rule rule = (Rule) getTargetFromConfiguredTarget(configuredTarget);
- ImmutableMap<Label, ConfigMatchingProvider> configConditions =
- ((RuleConfiguredTarget) configuredTarget).getConfigConditions();
+ ImmutableMap<Label, ConfigMatchingProvider> configConditions;
+ if (configuredTarget instanceof RuleConfiguredTarget) {
+ configConditions = ((RuleConfiguredTarget) configuredTarget).getConfigConditions();
+ } else if (configuredTarget instanceof AliasConfiguredTarget) {
+ configConditions = ((AliasConfiguredTarget) configuredTarget).getConfigConditions();
+ } else {
+ throw new IllegalStateException();
+ }
+
ConfiguredAttributeMapper attributeMapper =
ConfiguredAttributeMapper.of(rule, configConditions);
if (!attributeMapper.has(attrName)) {
@@ -163,14 +176,17 @@
return getTargetFromConfiguredTarget(configuredTarget, walkableGraph);
}
+ private static Label getOriginalLabel(ConfiguredTarget configuredTarget) {
+ return configuredTarget instanceof AliasConfiguredTarget
+ ? ((AliasConfiguredTarget) configuredTarget).getOriginalLabel()
+ : configuredTarget.getLabel();
+ }
+
public static Target getTargetFromConfiguredTarget(
ConfiguredTarget configuredTarget, WalkableGraph walkableGraph) {
Target target = null;
try {
- Label label =
- configuredTarget instanceof AliasConfiguredTarget
- ? ((AliasConfiguredTarget) configuredTarget).getOriginalLabel()
- : configuredTarget.getLabel();
+ Label label = getOriginalLabel(configuredTarget);
target =
((PackageValue) walkableGraph.getValue(PackageValue.key(label.getPackageIdentifier())))
.getPackage()
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
index b3e0dbe..a61ad74 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
@@ -559,6 +559,9 @@
*/
boolean isTestSuite(T target);
+ /** Returns whether the given target is an alias rule. */
+ boolean isAlias(T target);
+
/**
* If the attribute of the given name on the given target is a label or label list, then this
* method returns the list of corresponding target instances. Otherwise returns an empty list.
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/TestsFunction.java b/src/main/java/com/google/devtools/build/lib/query2/engine/TestsFunction.java
index 1bed494..f1b77fe 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/TestsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/TestsFunction.java
@@ -15,6 +15,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Argument;
@@ -77,6 +78,8 @@
closure
.getUniqueTestSuites(partitionResult.testSuiteTargets)
.forEach(closure::visitUniqueTestsInUniqueSuite);
+ closure.getUniqueAliases(partitionResult.aliasTargets).forEach(closure::visitUniqueAlias);
+
callback.process(closure.getUniqueTests(partitionResult.testTargets));
};
@@ -96,14 +99,17 @@
private static class PartitionResult<T> {
final ImmutableList<T> testTargets;
final ImmutableList<T> testSuiteTargets;
+ final ImmutableList<T> aliasTargets;
final ImmutableList<T> otherTargets;
private PartitionResult(
ImmutableList<T> testTargets,
ImmutableList<T> testSuiteTargets,
+ ImmutableList<T> aliasTargets,
ImmutableList<T> otherTargets) {
this.testTargets = testTargets;
this.testSuiteTargets = testSuiteTargets;
+ this.aliasTargets = aliasTargets;
this.otherTargets = otherTargets;
}
}
@@ -120,6 +126,7 @@
private final boolean strict;
private final Uniquifier<T> testUniquifier;
private final Uniquifier<T> testSuiteUniquifier;
+ private final Uniquifier<T> aliasUniquifier;
private final List<QueryTaskFuture<Void>> topLevelRecursiveVisitationFutures =
Collections.synchronizedList(new ArrayList<>());
@@ -131,6 +138,7 @@
this.strict = env.isSettingEnabled(Setting.TESTS_EXPRESSION_STRICT);
this.testUniquifier = env.createUniquifier();
this.testSuiteUniquifier = env.createUniquifier();
+ this.aliasUniquifier = env.createUniquifier();
}
private Iterable<T> getUniqueTests(Iterable<T> tests) throws QueryException {
@@ -141,6 +149,10 @@
return testSuiteUniquifier.unique(testSuites);
}
+ private Iterable<T> getUniqueAliases(Iterable<T> aliases) throws QueryException {
+ return aliasUniquifier.unique(aliases);
+ }
+
private void visitUniqueTestsInUniqueSuite(T testSuite) {
topLevelRecursiveVisitationFutures.add(
env.executeAsync(() -> recursivelyVisitUniqueTestsInUniqueSuite(testSuite)));
@@ -154,18 +166,28 @@
return ImmutableList.copyOf(topLevelRecursiveVisitationFutures);
}
- private QueryTaskFuture<Void> recursivelyVisitUniqueTestsInUniqueSuite(T testSuite) {
- List<String> tagsAttribute = accessor.getStringListAttr(testSuite, "tags");
- // Split the tags list into positive and negative tags
- Set<String> requiredTags = new HashSet<>();
- Set<String> excludedTags = new HashSet<>();
- sortTagsBySense(tagsAttribute, requiredTags, excludedTags);
+ private QueryTaskFuture<Void> visitUniqueAlias(T alias) {
+ Iterable<T> actual;
+ try {
+ actual = getPrerequisites(alias, "actual");
+ } catch (InterruptedException e) {
+ return env.immediateCancelledFuture();
+ } catch (QueryException e) {
+ return env.immediateFailedFuture(e);
+ }
+
+ return resolveTests(accessor.getLabel(alias), actual, ImmutableSet.of(), ImmutableSet.of());
+ }
+
+ private QueryTaskFuture<Void> resolveTests(
+ String current, Iterable<T> deps, Set<String> requiredTags, Set<String> excludedTags) {
List<T> testsToProcess = new ArrayList<>();
+ List<T> aliasesToProcess = new ArrayList<>();
List<T> testSuites;
try {
- PartitionResult<T> partitionResult = partition(getPrerequisites(testSuite, "tests"));
+ PartitionResult<T> partitionResult = partition(deps);
for (T testTarget : partitionResult.testTargets) {
if (includeTest(requiredTags, excludedTags, testTarget)
@@ -174,6 +196,12 @@
}
}
+ for (T aliasTarget : partitionResult.aliasTargets) {
+ if (aliasUniquifier.unique(aliasTarget)) {
+ aliasesToProcess.add(aliasTarget);
+ }
+ }
+
testSuites = testSuiteUniquifier.unique(partitionResult.testSuiteTargets);
// If strict mode is enabled, then give an error for any non-test, non-test-suite target.
@@ -184,24 +212,11 @@
"The label '"
+ accessor.getLabel(otherTarget)
+ "' in the test_suite '"
- + accessor.getLabel(testSuite)
- + "' does not refer to a test or test_suite "
+ + current
+ + "' does not refer to a test or test_suite or alias "
+ "rule!");
}
}
-
- // Add implicit dependencies on tests in same package, if any.
- for (T target : getPrerequisites(testSuite, "$implicit_tests")) {
- // The Package construction of $implicit_tests ensures that this check never fails, but we
- // add it here anyway for compatibility with future code.
- if (accessor.isTestRule(target)
- && includeTest(requiredTags, excludedTags, target)
- && testUniquifier.unique(target)) {
- testsToProcess.add(target);
- }
- }
- } catch (InterruptedException e) {
- return env.immediateCancelledFuture();
} catch (QueryException e) {
return env.immediateFailedFuture(e);
}
@@ -214,18 +229,46 @@
return null;
});
+ // Resolve aliases
+ QueryTaskFuture<Void> allAliasesVisitedFuture =
+ env.whenAllSucceed(Iterables.transform(aliasesToProcess, this::visitUniqueAlias));
+
// Visit all suites recursively, asynchronously.
QueryTaskFuture<Void> allTestSuitsVisitedFuture =
env.whenAllSucceed(
Iterables.transform(testSuites, this::recursivelyVisitUniqueTestsInUniqueSuite));
return env.whenAllSucceed(
- ImmutableList.of(allTestsProcessedFuture, allTestSuitsVisitedFuture));
+ ImmutableList.of(
+ allTestsProcessedFuture, allTestSuitsVisitedFuture, allAliasesVisitedFuture));
+ }
+
+ private QueryTaskFuture<Void> recursivelyVisitUniqueTestsInUniqueSuite(T testSuite) {
+ List<String> tagsAttribute = accessor.getStringListAttr(testSuite, "tags");
+ // Split the tags list into positive and negative tags
+ Set<String> requiredTags = new HashSet<>();
+ Set<String> excludedTags = new HashSet<>();
+ sortTagsBySense(tagsAttribute, requiredTags, excludedTags);
+
+ Iterable<T> deps;
+ try {
+ deps =
+ Iterables.concat(
+ getPrerequisites(testSuite, "tests"),
+ getPrerequisites(testSuite, "$implicit_tests"));
+ } catch (InterruptedException e) {
+ return env.immediateCancelledFuture();
+ } catch (QueryException e) {
+ return env.immediateFailedFuture(e);
+ }
+
+ return resolveTests(accessor.getLabel(testSuite), deps, requiredTags, excludedTags);
}
private PartitionResult<T> partition(Iterable<T> targets) {
ImmutableList.Builder<T> testTargetsBuilder = ImmutableList.builder();
ImmutableList.Builder<T> testSuiteTargetsBuilder = ImmutableList.builder();
+ ImmutableList.Builder<T> aliasesBuilder = ImmutableList.builder();
ImmutableList.Builder<T> otherTargetsBuilder = ImmutableList.builder();
for (T target : targets) {
@@ -233,13 +276,18 @@
testTargetsBuilder.add(target);
} else if (accessor.isTestSuite(target)) {
testSuiteTargetsBuilder.add(target);
+ } else if (accessor.isAlias(target)) {
+ aliasesBuilder.add(target);
} else {
otherTargetsBuilder.add(target);
}
}
return new PartitionResult<>(
- testTargetsBuilder.build(), testSuiteTargetsBuilder.build(), otherTargetsBuilder.build());
+ testTargetsBuilder.build(),
+ testSuiteTargetsBuilder.build(),
+ aliasesBuilder.build(),
+ otherTargetsBuilder.build());
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeTargetAccessor.java
index 332076c..af02a04 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeTargetAccessor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeTargetAccessor.java
@@ -131,6 +131,11 @@
}
@Override
+ public boolean isAlias(Target target) {
+ return TargetUtils.isAlias(target);
+ }
+
+ @Override
public Set<QueryVisibility<Target>> getVisibility(Target target)
throws QueryException, InterruptedException {
ImmutableSet.Builder<QueryVisibility<Target>> result = ImmutableSet.builder();
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 1d4e59a..2517378 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
@@ -123,7 +123,8 @@
Map<Label, SkyKey> testExpansionKeys = new LinkedHashMap<>();
if (targets != null) {
for (Target target : targets.getTargets()) {
- if (TargetUtils.isTestSuiteRule(target) && options.isExpandTestSuites()) {
+ if (options.isExpandTestSuites()
+ && (TargetUtils.isTestSuiteRule(target) || TargetUtils.isAlias(target))) {
Label label = target.getLabel();
SkyKey testExpansionKey = TestsForTargetPatternValue.key(ImmutableSet.of(label));
testExpansionKeys.put(label, testExpansionKey);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionFunction.java
index ac6777b..178787f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionFunction.java
@@ -43,8 +43,8 @@
import javax.annotation.Nullable;
/**
- * TestExpansionFunction takes a single test_suite target and expands all of the tests it contains,
- * possibly recursively.
+ * TestExpansionFunction takes a single expandable test target ({@code test_suite} or {@code alias})
+ * and expands it recursively.
*/
// TODO(ulfjack): What about test_suite rules that include each other.
final class TestExpansionFunction implements SkyFunction {
@@ -80,15 +80,19 @@
boolean hasError = false;
List<Target> prerequisites = new ArrayList<>();
- // Note that prerequisites can contain input file targets; the test_suite rule does not
- // restrict the set of targets that can appear in tests or suites.
- hasError |= getPrerequisites(env, rule, "tests", prerequisites);
+ if (TargetUtils.isTestSuiteRule(rule)) {
+ // Note that prerequisites can contain input file targets; the test_suite rule does not
+ // restrict the set of targets that can appear in tests or suites.
+ hasError |= getPrerequisites(env, rule, "tests", prerequisites);
+ } else if (TargetUtils.isAlias(rule)) {
+ hasError |= getPrerequisites(env, rule, "actual", prerequisites);
+ }
- // 1. Add all tests
+ // Add all tests
for (Target test : prerequisites) {
if (TargetUtils.isTestRule(test)) {
result.add(test);
- } else if (strict && !TargetUtils.isTestSuiteRule(test)) {
+ } else if (strict && !TargetUtils.isTestSuiteRule(test) && !TargetUtils.isAlias(rule)) {
// If strict mode is enabled, then give an error for any non-test, non-test-suite targets.
// TODO(ulfjack): We need to throw to end the process if we happen to be in --nokeep_going,
// but we can't know whether or not we are at this point.
@@ -105,27 +109,29 @@
}
}
- // 2. Add implicit dependencies on tests in same package, if any.
- List<Target> implicitTests = new ArrayList<>();
- hasError |= getPrerequisites(env, rule, "$implicit_tests", implicitTests);
- for (Target target : implicitTests) {
- // The Package construction of $implicit_tests ensures that this check never fails, but we
- // add it here anyway for compatibility with future code.
- if (TargetUtils.isTestRule(target)) {
- result.add(target);
+ if (TargetUtils.isTestSuiteRule(rule)) {
+ // Add implicit dependencies on tests in same package, if any.
+ List<Target> implicitTests = new ArrayList<>();
+ hasError |= getPrerequisites(env, rule, "$implicit_tests", implicitTests);
+ for (Target target : implicitTests) {
+ // The Package construction of $implicit_tests ensures that this check never fails, but we
+ // add it here anyway for compatibility with future code.
+ if (TargetUtils.isTestRule(target)) {
+ result.add(target);
+ }
}
+
+ // Filter based on tags, size, env.
+ TestTargetUtils.filterTests(rule, result);
}
- // 3. Filter based on tags, size, env.
- TestTargetUtils.filterTests(rule, result);
-
- // 4. Expand all rules recursively, collecting labels.
+ // Expand all rules recursively, collecting labels.
ResolvedTargets.Builder<Label> labelsBuilder = ResolvedTargets.builder();
- // Don't set filtered targets; they would be removed from the containing test suite.
+ // Don't set filtered targets; they would be removed from the containing rule.
labelsBuilder.merge(new ResolvedTargets<>(toLabels(result), ImmutableSet.of(), hasError));
for (Target suite : prerequisites) {
- if (TargetUtils.isTestSuiteRule(suite)) {
+ if (TargetUtils.isTestSuiteRule(suite) || TargetUtils.isAlias(suite)) {
TestExpansionValue value =
(TestExpansionValue) env.getValue(TestExpansionValue.key(suite, strict));
if (value == null) {
@@ -140,9 +146,8 @@
/**
* Adds the set of targets found in the attribute named {@code attrName}, which must be of label
- * or label list type, of the {@code test_suite} rule named {@code testSuite}. Returns true if the
- * method found a problem during the lookup process; the actual error message is reported to the
- * environment.
+ * or label list list type, Returns true if the method found a problem during the lookup process;
+ * the actual error message is reported to the environment.
*/
private static boolean getPrerequisites(
Environment env, Rule rule, String attrName, List<Target> targets)
@@ -154,6 +159,7 @@
.collect(Collectors.toList());
Set<PackageIdentifier> pkgIdentifiers = new LinkedHashSet<>();
+
for (Label label : labels) {
pkgIdentifiers.add(label.getPackageIdentifier());
}
@@ -163,8 +169,10 @@
if (env.valuesMissing()) {
return false;
}
+
boolean hasError = false;
Map<PackageIdentifier, Package> packageMap = new HashMap<>();
+
for (Map.Entry<SkyKey, ValueOrException<NoSuchPackageException>> entry : packages.entrySet()) {
try {
packageMap.put(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionValue.java
index 1b1d88f..dbd3e46 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestExpansionValue.java
@@ -30,8 +30,9 @@
import java.util.Objects;
/**
- * A value referring to a computed set of resolved targets. This is used for the results of target
- * pattern parsing.
+ * A value containing all the tests expanded from a single rule.
+ *
+ * <p>The rule in question should be a {@code test_suite} or {@code alias}.
*/
@Immutable
@ThreadSafe
@@ -64,11 +65,11 @@
/**
* Create a target pattern value key.
*
- * @param target the target to be expanded
+ * @param target the test suite target to be expanded
*/
@ThreadSafe
public static SkyKey key(Target target, boolean strict) {
- Preconditions.checkState(TargetUtils.isTestSuiteRule(target));
+ Preconditions.checkState(TargetUtils.isTestSuiteRule(target) || TargetUtils.isAlias(target));
return new TestExpansionKey(target.getLabel(), strict);
}
@@ -98,7 +99,7 @@
@Override
public String toString() {
- return "TestsInSuite(" + label + ", strict=" + strict + ")";
+ return "TestExpansionKey(" + label + ", strict=" + strict + ")";
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestsForTargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestsForTargetPatternFunction.java
index 45f767e..29df965 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestsForTargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestsForTargetPatternFunction.java
@@ -36,7 +36,7 @@
/**
* Returns all tests that need to be run when testing is requested for a given set of targets.
*
- * <p>This requires resolving {@code test_suite} rules.
+ * <p>This requires resolving {@code test_suite} rules and aliases.
*/
final class TestsForTargetPatternFunction implements SkyFunction {
@Override
@@ -45,10 +45,12 @@
ResolvedTargets<Target> targets = labelsToTargets(env, expansion.getTargets(), false);
List<SkyKey> testsInSuitesKeys = new ArrayList<>();
for (Target target : targets.getTargets()) {
- if (TargetUtils.isTestSuiteRule(target)) {
+ if (TargetUtils.isTestSuiteRule(target)
+ || TargetUtils.isAlias(target)) { // TestExpansionFunction also handles aliases
testsInSuitesKeys.add(TestExpansionValue.key(target, true));
}
}
+
Map<SkyKey, SkyValue> testsInSuites = env.getValues(testsInSuitesKeys);
if (env.valuesMissing()) {
return null;
@@ -59,7 +61,7 @@
for (Target target : targets.getTargets()) {
if (TargetUtils.isTestRule(target)) {
result.add(target.getLabel());
- } else if (TargetUtils.isTestSuiteRule(target)) {
+ } else if (TargetUtils.isTestSuiteRule(target) || TargetUtils.isAlias(target)) {
TestExpansionValue value =
(TestExpansionValue) testsInSuites.get(TestExpansionValue.key(target, true));
if (value != null) {
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 a71401c..af33408 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
@@ -15,7 +15,6 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.base.Function;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -166,19 +165,12 @@
Sets.newHashSet(test1, test2, test1b), ImmutableSet.<Target>of(test1b, suite));
}
- private static final Function<Target, Label> TO_LABEL =
- new Function<Target, Label>() {
- @Override
- public Label apply(Target input) {
- return input.getLabel();
- }
- };
-
private void assertExpandedSuitesSkyframe(Iterable<Target> expected, Collection<Target> suites)
throws Exception {
ImmutableSet<Label> expectedLabels =
- ImmutableSet.copyOf(Iterables.transform(expected, TO_LABEL));
- ImmutableSet<Label> suiteLabels = ImmutableSet.copyOf(Iterables.transform(suites, TO_LABEL));
+ ImmutableSet.copyOf(Iterables.transform(expected, Target::getLabel));
+ ImmutableSet<Label> suiteLabels =
+ ImmutableSet.copyOf(Iterables.transform(suites, Target::getLabel));
SkyKey key = TestsForTargetPatternValue.key(suiteLabels);
EvaluationContext evaluationContext =
EvaluationContext.newBuilder()
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 1ab99d4..fd11d96 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -484,6 +484,48 @@
}
@Test
+ public void testAliasedBuildTestsOnly() throws Exception {
+ tester.addFile(
+ "t/BUILD",
+ "sh_test(name='t', srcs=['t.sh'])",
+ "sh_binary(name='b', srcs=['b.sh'])",
+ "alias(name='ta', actual=':t')",
+ "alias(name='ba', actual=':b')");
+ tester.useLoadingOptions("--build_tests_only");
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//t:ta", "//t:ba"));
+ assertThat(loadingResult.getTargetLabels()).containsExactlyElementsIn(getLabels("//t:t"));
+ }
+
+ @Test
+ public void testAliasedTestExpansion() throws Exception {
+ tester.addFile("sh/BUILD", "sh_test(name='t', srcs=['t.sh'])", "alias(name='a', actual=':t')");
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//sh:a"));
+ assertThat(loadingResult.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//sh:t"));
+ }
+
+ @Test
+ public void testAliasedTestSuite() throws Exception {
+ tester.addFile(
+ "sh/BUILD",
+ "sh_test(name='t', srcs=['t.sh'])",
+ "test_suite(name='ts', tests=[':t'])",
+ "alias(name='a', actual=':ts')");
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//sh:a"));
+ assertThat(loadingResult.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//sh:t"));
+ }
+
+ @Test
+ public void testDoubleAliasedTestExpansion() throws Exception {
+ tester.addFile(
+ "sh/BUILD",
+ "sh_test(name='t', srcs=['t.sh'])",
+ "alias(name='a1', actual=':t')",
+ "alias(name='a2', actual=':a1')");
+ TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//sh:a2"));
+ assertThat(loadingResult.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//sh:t"));
+ }
+
+ @Test
public void testTestSuiteExpansionFails() throws Exception {
tester.addFile("ts/BUILD",
"test_suite(name = 'tests', tests = ['//nonexistent:my_test'])");
diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
index 38f9e40..e62003f 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/AbstractQueryTest.java
@@ -944,6 +944,36 @@
}
@Test
+ public void testTestsOperatorFollowsAliasesToTests() throws Exception {
+ writeFile(
+ "a/BUILD", "sh_test(name='test', srcs=['test.sh'])", "alias(name='alias', actual=':test')");
+
+ assertThat(eval("tests(//a:alias)")).isEqualTo(eval("//a:test"));
+ }
+
+ @Test
+ public void testTestsOperatorFollowsDoubleAliasesToTests() throws Exception {
+ writeFile(
+ "a/BUILD",
+ "sh_test(name='test', srcs=['test.sh'])",
+ "alias(name='alias1', actual=':test')",
+ "alias(name='alias2', actual=':alias1')");
+
+ assertThat(eval("tests(//a:alias2)")).isEqualTo(eval("//a:test"));
+ }
+
+ @Test
+ public void testTestsOperatorFollowsAliasesToTestSuites() throws Exception {
+ writeFile(
+ "a/BUILD",
+ "sh_test(name='test', srcs=['test.sh'])",
+ "test_suite(name='suite', tests=[':test'])",
+ "alias(name='alias', actual=':suite')");
+
+ assertThat(eval("tests(//a:alias)")).isEqualTo(eval("//a:test"));
+ }
+
+ @Test
public void testDotDotDotWithUnrelatedCycle() throws Exception {
writeFile("a/BUILD", "sh_library(name = 'a')");
writeFile(
@@ -1219,7 +1249,7 @@
assertThat(evalThrows("tests(//x:a)", false))
.isEqualTo(
"The label '//x:a.txt' in the test_suite '//x:a' does not refer to a test or "
- + "test_suite rule!");
+ + "test_suite or alias rule!");
}
@Test
