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

1 files changed, 7 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionValue.java
index 4f108ba513..e0406ab940 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TestSuiteExpansionValue.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.ResolvedTargets;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -27,6 +28,7 @@ import com.google.devtools.build.skyframe.SkyValue;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
+import java.util.Collection;
 
 /**
  * A value referring to a computed set of resolved targets. This is used for the results of target
@@ -67,8 +69,9 @@ public final class TestSuiteExpansionValue implements SkyValue {
    * @param targets the set of targets to be expanded
    */
   @ThreadSafe
-  public static SkyKey key(ImmutableSet<Label> targets) {
-    return new SkyKey(SkyFunctions.TEST_SUITE_EXPANSION, new TestSuiteExpansion(targets));
+  public static SkyKey key(Collection<Label> targets) {
+    return new SkyKey(SkyFunctions.TEST_SUITE_EXPANSION,
+        new TestSuiteExpansion(ImmutableSortedSet.copyOf(targets)));
   }
 
   /**
@@ -76,9 +79,9 @@ public final class TestSuiteExpansionValue implements SkyValue {
    */
   @ThreadSafe
   static final class TestSuiteExpansion implements Serializable {
-    private final ImmutableSet<Label> targets;
+    private final ImmutableSortedSet<Label> targets;
 
-    public TestSuiteExpansion(ImmutableSet<Label> targets) {
+    public TestSuiteExpansion(ImmutableSortedSet<Label> targets) {
       this.targets = targets;
     }