Add an IdentityHashMap to the BuildOptions.OptionsDiffForReconstruction codec.

PiperOrigin-RevId: 196310244

4 files changed, 38 insertions, 20 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index fb3d52c6ff..1502f25437 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -704,6 +704,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:java-compilation",
         "//src/main/java/com/google/devtools/build/lib:java-rules",
         "//src/main/java/com/google/devtools/build/lib:packages",
+        "//src/main/java/com/google/devtools/build/lib:proto-rules",
         "//src/main/java/com/google/devtools/build/lib:python-rules",
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
index aa36ebf127..13d4e7a3a8 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.rules.objc.J2ObjcConfiguration;
 import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.common.options.Options;
+import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.regex.Pattern;
 import org.junit.Test;
@@ -452,6 +453,9 @@ public class BuildConfigurationTest extends ConfigurationTestCase {
                 "--define",
                 "#a=pounda"))
         .addDependency(FileSystem.class, getScratch().getFileSystem())
+        .addDependency(
+            IdentityHashMap.class,
+            new IdentityHashMap<BuildOptions.OptionsDiffForReconstruction, byte[]>())
         .setVerificationFunction(BuildConfigurationTest::verifyDeserialized)
         .runTests();
   }
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 421db8e93c..778378201b 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
@@ -20,7 +20,10 @@ import com.google.common.collect.ImmutableList;
 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.cpp.CppOptions;
+import com.google.devtools.build.lib.rules.proto.ProtoConfiguration;
+import com.google.devtools.build.lib.skyframe.serialization.testutils.ObjectCodecTester;
 import com.google.devtools.common.options.OptionsParser;
+import java.util.IdentityHashMap;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -31,7 +34,8 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class BuildOptionsTest {
   private static final ImmutableList<Class<? extends FragmentOptions>> TEST_OPTIONS =
-      ImmutableList.<Class<? extends FragmentOptions>>of(BuildConfiguration.Options.class);
+      ImmutableList.<Class<? extends FragmentOptions>>of(
+          BuildConfiguration.Options.class, ProtoConfiguration.Options.class);
 
   @Test
   public void optionSetCaching() {
@@ -165,4 +169,32 @@ public class BuildOptionsTest {
     assertThat(otherFragment.applyDiff(BuildOptions.diffForReconstruction(otherFragment, one)))
         .isEqualTo(one);
   }
+
+  @Test
+  public void testCodec() throws Exception {
+    BuildOptions one =
+        BuildOptions.of(
+            TEST_OPTIONS,
+            "--compilation_mode=opt",
+            "cpu=k8",
+            "--proto_compiler=//net/proto2/compiler/public:protocol_compiler",
+            "--proto_toolchain_for_java=//tools/proto/toolchains:java");
+    BuildOptions two =
+        BuildOptions.of(
+            TEST_OPTIONS,
+            "--compilation_mode=dbg",
+            "cpu=k8",
+            "--proto_compiler=@com_google_protobuf//:protoc");
+    ObjectCodecTester.newBuilder(new BuildOptions.OptionsDiffForReconstructionCodec())
+        .addSubjects(BuildOptions.diffForReconstruction(one, two))
+        .addDependency(
+            IdentityHashMap.class,
+            new IdentityHashMap<BuildOptions.OptionsDiffForReconstruction, byte[]>())
+        .skipBadDataTest() // Bad data doesn't make sense with our caching.
+        .verificationFunction(
+            ((original, deserialized) -> {
+              assertThat(original).isEqualTo(deserialized);
+            }))
+        .buildAndRunTests();
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java b/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java
index c49d5e5f31..dade507c4c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/MiddlemanActionTest.java
@@ -23,14 +23,11 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MiddlemanAction;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
-import com.google.devtools.build.lib.actions.OutputBaseSupplier;
 import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
-import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
 import com.google.devtools.build.lib.testutil.Suite;
 import com.google.devtools.build.lib.testutil.TestSpec;
-import com.google.devtools.build.lib.vfs.FileSystem;
 import java.util.ArrayList;
 import java.util.Arrays;
 import org.junit.Before;
@@ -138,20 +135,4 @@ public class MiddlemanActionTest extends BuildViewTestCase {
     assertThat(Sets.newHashSet(middlemanActionForD.getOutputs()))
         .isNotEqualTo(Sets.newHashSet(middlemanActionForC.getOutputs()));
   }
-
-  @Test
-  public void testCodec() throws Exception {
-    new SerializationTester(getGeneratingAction(middle))
-        .addDependency(FileSystem.class, scratch.getFileSystem())
-        .addDependency(OutputBaseSupplier.class, () -> outputBase)
-        .setVerificationFunction(MiddlemanActionTest::verifyEquivalent)
-        .runTests();
-  }
-
-  private static void verifyEquivalent(MiddlemanAction first, MiddlemanAction second) {
-    assertThat(first.getActionType()).isEqualTo(second.getActionType());
-    assertThat(first.getInputs()).isEqualTo(second.getInputs());
-    assertThat(first.getOutputs()).isEqualTo(second.getOutputs());
-    assertThat(first.getOwner()).isEqualTo(second.getOwner());
-  }
 }