Add ObjectCodecTester, migrating a few tests to use it

This provides a composition-based alternative to the existing inheritance-based
testing style. The inheritance style has been showing its age/has become quite
cumbersome when multiple codecs are in the same class or codecs are colocated
with their encoded class. This will hopefully get rid of some friction when
adding new codecs.

RELNOTES: None
PiperOrigin-RevId: 172778555

2 files changed, 186 insertions, 31 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/AbstractObjectCodecTest.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/AbstractObjectCodecTest.java
index 117f4d8bac..08f675c5dd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/AbstractObjectCodecTest.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/AbstractObjectCodecTest.java
@@ -15,30 +15,25 @@
 package com.google.devtools.build.lib.skyframe.serialization.testutils;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
 import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
-import com.google.protobuf.CodedInputStream;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import javax.annotation.Nullable;
 import org.junit.Before;
 import org.junit.Test;
 
-/** Common ObjectCodec tests. */
+/**
+ * Base class for {@link ObjectCodec} tests. This is a slim wrapper around {@link ObjectCodecTester}
+ * and exists mostly to support existing tests.
+ */
 public abstract class AbstractObjectCodecTest<T> {
 
   @Nullable protected ObjectCodec<T> underTest;
   @Nullable protected ImmutableList<T> subjects;
-
-  /**
-   * Override to false to skip testDeserializeBadDataThrowsSerializationException(). Codecs that
-   * cannot distinguish good and bad data should do this.
-   */
-  protected boolean shouldTestDeserializeBadData = true;
+  private ObjectCodecTester<T> objectCodecTester;
 
   /** Construct with the given codec and subjects. */
   protected AbstractObjectCodecTest(
@@ -57,41 +52,29 @@ public abstract class AbstractObjectCodecTest<T> {
   protected AbstractObjectCodecTest() {}
 
   @Before
-  public void checkInitialized() {
+  public void initialize() {
     Preconditions.checkNotNull(underTest);
     Preconditions.checkNotNull(subjects);
+    objectCodecTester = ObjectCodecTester.newBuilder(underTest)
+        .verificationFunction(
+            (original, deserialized) -> this.verifyDeserialization(deserialized, original))
+        .addSubjects(subjects)
+        .build();
   }
 
   @Test
   public void testSuccessfulSerializationDeserialization() throws Exception {
-    for (T subject : subjects) {
-      byte[] serialized = toBytes(subject);
-      Object deserialized = fromBytes(serialized);
-      verifyDeserialization(deserialized, subject);
-    }
+    objectCodecTester.testSerializeDeserialize();
   }
 
   @Test
   public void testSerializationRoundTripBytes() throws Exception {
-    for (T subject : subjects) {
-      byte[] serialized = toBytes(subject);
-      T deserialized = fromBytes(serialized);
-      byte[] reserialized = toBytes(deserialized);
-      assertThat(reserialized).isEqualTo(serialized);
-    }
+    objectCodecTester.testStableSerialization();
   }
 
   @Test
   public void testDeserializeBadDataThrowsSerializationException() {
-    if (!shouldTestDeserializeBadData) {
-      return;
-    }
-    try {
-      underTest.deserialize(CodedInputStream.newInstance("junk".getBytes(StandardCharsets.UTF_8)));
-      fail("Expected exception");
-    } catch (SerializationException | IOException e) {
-      // Expected.
-    }
+    objectCodecTester.testDeserializeJunkData();
   }
 
   protected T fromBytes(byte[] bytes) throws SerializationException, IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/ObjectCodecTester.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/ObjectCodecTester.java
new file mode 100644
index 0000000000..1f370f68de
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/ObjectCodecTester.java
@@ -0,0 +1,172 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.skyframe.serialization.testutils;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationException;
+import com.google.protobuf.CodedInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+/** Utility for testing {@link ObjectCodec} instances. */
+public class ObjectCodecTester<T> {
+
+  /** Interface for testing successful deserialization of an object. */
+  @FunctionalInterface
+  public interface VerificationFunction<T> {
+    /**
+     * Verify whether or not the original object was sufficiently serialized/deserialized. Typically
+     * this will be some sort of assertion.
+     *
+     * @throws Exception on verification failure
+     */
+    void verifyDeserialized(T original, Object deserialized) throws Exception;
+  }
+
+  /**
+   * Create an {@link ObjectCodecTester.Builder} for the supplied instance. See
+   * {@link ObjectCodecTester.Builder} for details.
+   */
+  public static <T> ObjectCodecTester.Builder<T> newBuilder(ObjectCodec<T> toTest) {
+    return new ObjectCodecTester.Builder<>(toTest);
+  }
+
+  private final ObjectCodec<T> underTest;
+  private final ImmutableList<T> subjects;
+  private final boolean skipBadDataTest;
+  private final VerificationFunction<T> verificationFunction;
+
+  private ObjectCodecTester(
+      ObjectCodec<T> underTest,
+      ImmutableList<T> subjects,
+      boolean skipBadDataTest,
+      VerificationFunction<T> verificationFunction) {
+    this.underTest = underTest;
+    Preconditions.checkState(!subjects.isEmpty(), "No subjects provided");
+    this.subjects = subjects;
+    this.skipBadDataTest = skipBadDataTest;
+    this.verificationFunction = verificationFunction;
+  }
+
+  private void runTests() throws Exception {
+    testSerializeDeserialize();
+    testStableSerialization();
+    if (!skipBadDataTest) {
+      testDeserializeJunkData();
+    }
+  }
+
+  /** Runs serialization/deserialization tests. */
+  void testSerializeDeserialize() throws Exception {
+    for (T subject : subjects) {
+      byte[] serialized = toBytes(subject);
+      Object deserialized = fromBytes(serialized);
+      verificationFunction.verifyDeserialized(subject, deserialized);
+    }
+  }
+
+  /** Runs serialized bytes stability tests. */
+  void testStableSerialization() throws Exception {
+    for (T subject : subjects) {
+      byte[] serialized = toBytes(subject);
+      T deserialized = fromBytes(serialized);
+      byte[] reserialized = toBytes(deserialized);
+      assertThat(reserialized).isEqualTo(serialized);
+    }
+  }
+
+  /** Runs junk-data recognition tests. */
+  void testDeserializeJunkData() {
+    try {
+      underTest.deserialize(CodedInputStream.newInstance("junk".getBytes(StandardCharsets.UTF_8)));
+      fail("Expected exception");
+    } catch (SerializationException | IOException e) {
+      // Expected.
+    }
+  }
+
+  private T fromBytes(byte[] bytes) throws SerializationException, IOException {
+    return TestUtils.fromBytes(underTest, bytes);
+  }
+
+  private byte[] toBytes(T subject) throws IOException, SerializationException {
+    return TestUtils.toBytes(underTest, subject);
+  }
+
+  /** Builder for {@link ObjectCodecTester}. */
+  public static class Builder<T> {
+    private final ObjectCodec<T> underTest;
+    private final ImmutableList.Builder<T> subjectsBuilder = ImmutableList.builder();
+    private boolean skipBadDataTest = false;
+    private VerificationFunction<T> verificationFunction =
+        (original, deserialized) -> assertThat(deserialized).isEqualTo(original);
+
+    private Builder(ObjectCodec<T> underTest) {
+      this.underTest = underTest;
+    }
+
+    /** Add subjects to be tested for serialization/deserialization. */
+    public Builder<T> addSubjects(@SuppressWarnings("unchecked") T... subjects) {
+      return addSubjects(ImmutableList.copyOf(subjects));
+    }
+
+    /** Add subjects to be tested for serialization/deserialization. */
+    Builder<T> addSubjects(ImmutableList<T> subjects) {
+      subjectsBuilder.addAll(subjects);
+      return this;
+    }
+
+    /**
+     * Skip tests that check for the ability to detect bad data. This may be useful for simpler
+     * codecs which don't do any error verification.
+     */
+    public Builder<T> skipBadDataTest() {
+      this.skipBadDataTest = true;
+      return this;
+    }
+
+    /**
+     * Sets {@link ObjectCodecTester.VerificationFunction} for verifying deserialization. Default
+     * is simple equality assertion, a custom version may be provided for more, or less, detailed
+     * checks.
+     */
+    public Builder<T> verificationFunction(VerificationFunction<T> verificationFunction) {
+      this.verificationFunction = Preconditions.checkNotNull(verificationFunction);
+      return this;
+    }
+
+    /** Captures the state of this builder and run all associated tests. */
+    public void buildAndRunTests() throws Exception {
+      build().runTests();
+    }
+
+    /**
+     * Creates a new {@link ObjectCodecTester} from this builder. Exposed to allow running tests
+     * individually.
+     */
+    ObjectCodecTester<T> build() {
+      return new ObjectCodecTester<>(
+          underTest,
+          subjectsBuilder.build(),
+          skipBadDataTest,
+          verificationFunction);
+    }
+  }
+}