From 880827312919e5829ab056b3f27284e24c2411c2 Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Mon, 19 Oct 2015 16:53:21 +0000 Subject: Use a sorted set for the test suite expansion key to prevent non-determinism. While a normal set is theoretically sufficient, it can cause hard-to-reproduce problems. In particular, the iteration order of the expansion result depends on the iteration order of the requested targets. If there are multiple requests for the same set of targets, but with different orders, the results would depend on which request was made first. If a downstream function also inadvertantly depends on the iteration order, it can be hard to debug why it ended up with a different order than it requested. Alternatively, we could sort the result before returning it. We generally expect the key set to be smaller than the result set, so this should be ever so slightly more efficient. -- MOS_MIGRATED_REVID=105765514 --- .../build/lib/skyframe/TestSuiteExpansionFunction.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionFunction.java') diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionFunction.java index bba5946413..ad2cff7a29 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionFunction.java @@ -55,8 +55,8 @@ final class TestSuiteExpansionFunction implements SkyFunction { return null; } - ResolvedTargets.Builder result = ResolvedTargets.builder(); - result.mergeError(targets.hasError()); + Set result = new LinkedHashSet<>(); + boolean hasError = targets.hasError(); for (Target target : targets.getTargets()) { if (TargetUtils.isTestRule(target)) { result.add(target); @@ -64,7 +64,8 @@ final class TestSuiteExpansionFunction implements SkyFunction { TestsInSuiteValue value = (TestsInSuiteValue) testsInSuites.get( TestsInSuiteValue.key(target, true)); if (value != null) { - result.merge(value.getTargets()); + result.addAll(value.getTargets().getTargets()); + hasError |= value.getTargets().hasError(); } } else { result.add(target); @@ -73,7 +74,9 @@ final class TestSuiteExpansionFunction implements SkyFunction { if (env.valuesMissing()) { return null; } - return new TestSuiteExpansionValue(result.build()); + // We use ResolvedTargets in order to associate an error flag; the result should never contain + // any filtered targets. + return new TestSuiteExpansionValue(new ResolvedTargets(result, hasError)); } static ResolvedTargets labelsToTargets( -- cgit v1.2.3