Make BuildOptions$OptionsDiffForReconstruction deterministic in its construction. Also restrict visibility of some test-only OptionsDiff methods, and remove a server-specific part of the ConfiguredTargetKey #toString representation.

PiperOrigin-RevId: 197830577

2 files changed, 51 insertions, 2 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 887620485a..e8d568d834 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -699,6 +699,7 @@ java_test(
         ":guava_junit_truth",
         ":test_runner",
         ":testutil",
+        "//src/main/java/com/google/devtools/build/lib:android-rules",
         "//src/main/java/com/google/devtools/build/lib:bazel-main",
         "//src/main/java/com/google/devtools/build/lib:bazel-rules",
         "//src/main/java/com/google/devtools/build/lib:build-base",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
index a8a120a1e0..3f0f93a513 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildOptionsTest.java
@@ -17,10 +17,17 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff;
 import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiffForReconstruction;
+import com.google.devtools.build.lib.rules.android.AndroidConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppOptions;
+import com.google.devtools.build.lib.rules.java.JavaOptions;
+import com.google.devtools.build.lib.rules.proto.ProtoConfiguration;
+import com.google.devtools.build.lib.rules.python.PythonOptions;
+import com.google.devtools.build.lib.skyframe.serialization.testutils.TestUtils;
 import com.google.devtools.common.options.OptionsParser;
+import java.util.stream.Collectors;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -100,8 +107,13 @@ public class BuildOptionsTest {
     OptionsDiff diff = BuildOptions.diff(one, two);
 
     assertThat(diff.areSame()).isFalse();
-    assertThat(diff.getExtraFirstFragmentClasses()).containsExactly(CppOptions.class);
-    assertThat(diff.getExtraSecondFragmentClasses()).containsExactlyElementsIn(TEST_OPTIONS);
+    assertThat(diff.getExtraFirstFragmentClassesForTesting()).containsExactly(CppOptions.class);
+    assertThat(
+            diff.getExtraSecondFragmentsForTesting()
+                .stream()
+                .map(Object::getClass)
+                .collect(Collectors.toSet()))
+        .containsExactlyElementsIn(TEST_OPTIONS);
   }
 
   @Test
@@ -165,4 +177,40 @@ public class BuildOptionsTest {
     assertThat(otherFragment.applyDiff(BuildOptions.diffForReconstruction(otherFragment, one)))
         .isEqualTo(one);
   }
+
+  private static ImmutableList.Builder<Class<? extends FragmentOptions>> makeOptionsClassBuilder() {
+    return ImmutableList.<Class<? extends FragmentOptions>>builder()
+        .addAll(TEST_OPTIONS)
+        .add(CppOptions.class);
+  }
+
+  /**
+   * Tests that an {@link OptionsDiffForReconstruction} serializes stably. Unfortunately, still
+   * passes without fixes! (Perhaps more classes and diffs are needed?)
+   */
+  @Test
+  public void codecStability() throws Exception {
+    BuildOptions one =
+        BuildOptions.of(
+            makeOptionsClassBuilder()
+                .add(AndroidConfiguration.Options.class)
+                .add(ProtoConfiguration.Options.class)
+                .build());
+    BuildOptions two =
+        BuildOptions.of(
+            makeOptionsClassBuilder().add(JavaOptions.class).add(PythonOptions.class).build(),
+            "--cpu=k8",
+            "--compilation_mode=opt",
+            "--compiler=gcc",
+            "--copt=-Dfoo",
+            "--javacopt=--javacoption",
+            "--experimental_fix_deps_tool=fake",
+            "--build_python_zip=no",
+            "--force_python=py2");
+    OptionsDiffForReconstruction diff1 = BuildOptions.diffForReconstruction(one, two);
+    OptionsDiffForReconstruction diff2 = BuildOptions.diffForReconstruction(one, two);
+    assertThat(diff2).isEqualTo(diff1);
+    assertThat(TestUtils.toBytes(diff2, ImmutableMap.of()))
+        .isEqualTo(TestUtils.toBytes(diff1, ImmutableMap.of()));
+  }
 }