Add test framework for OptionsBase classes and their Converters.

Because OptionsBase implements equals() as a final method, subclasses can
only add fields in certain ways for OptionsBase to properly obey equals()
semantics. Specifically, all fields must be public and @Option annotated.
The OptionsTester checks for these two things.

Additionally, Converters must make sure to always return equals() values
on equals() (or equivalent) input. The OptionsTester includes a check that
all Converters named by the OptionsBase subclass being tested have matching
ConverterTesters, and if valid default values are specified (i.e., on
Options which are not multi-valued or default null), that these defaults
are among the values tested by the ConverterTesters.

The ConverterTesters themselves are wrapped EqualsTesters, testing that
the output of a Converter obeys equals() as expected for the same input
(or equivalent ones), and is consistent across calls to the same Converter
instance or different Converter instances.

Between these two, OptionsBase subclasses can have reasonable certainty
that two instances of themselves which were parsed equally - or underwent
equivalent transformations - will be equal.

This does not actually test any OptionsBase subclasses or Converter
implementations; it merely adds a framework. Future changes will cover
automatically testing all of the OptionsBase subclasses in a
RuleClassProvider, but naturally, this requires writing test data for
each Converter in the Bazel codebase first.

RELNOTES: None.
PiperOrigin-RevId: 162522445

4 files changed, 451 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/common/options/testing/BUILD b/src/main/java/com/google/devtools/common/options/testing/BUILD
new file mode 100644
index 0000000000..fc57c99d03
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/testing/BUILD
@@ -0,0 +1,29 @@
+# Description:
+#   Testing tools for the devtools-common options parser.
+package(
+    default_testonly = 1,
+    default_visibility = ["//src:__subpackages__"],
+)
+
+licenses(["notice"])  # Apache 2.0
+
+filegroup(
+    name = "srcs",
+    testonly = 0,
+    srcs = glob(
+        ["**"],
+    ),
+    visibility = ["//src/main/java/com/google/devtools/common/options:__pkg__"],
+)
+
+java_library(
+    name = "testing",
+    srcs = glob(["*.java"]),
+    deps = [
+        "//src/main/java/com/google/devtools/common/options",
+        "//third_party:guava",
+        "//third_party:guava-testlib",
+        "//third_party:junit4",
+        "//third_party:truth",
+    ],
+)
diff --git a/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java b/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java
new file mode 100644
index 0000000000..7d0b8d3974
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/testing/ConverterTester.java
@@ -0,0 +1,198 @@
+// 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.common.options.testing;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.testing.EqualsTester;
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.OptionsParsingException;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+
+/**
+ * A tester to confirm that {@link Converter} instances produce equal results on multiple calls with
+ * the same input.
+ */
+public final class ConverterTester {
+
+  private final Converter<?> converter;
+  private final Class<? extends Converter<?>> converterClass;
+  private final EqualsTester tester = new EqualsTester();
+  private final LinkedHashSet<String> testedInputs = new LinkedHashSet<>();
+  private final ArrayList<ImmutableList<String>> inputLists = new ArrayList<>();
+
+  /** Creates a new ConverterTester which will test the given Converter class. */
+  public ConverterTester(Class<? extends Converter<?>> converterClass) {
+    this.converterClass = converterClass;
+    this.converter = createConverter();
+  }
+
+  private Converter<?> createConverter() {
+    try {
+      return converterClass.getDeclaredConstructor().newInstance();
+    } catch (ReflectiveOperationException ex) {
+      throw new AssertionError("Failed to create converter", ex);
+    }
+  }
+
+  /** Returns the class this ConverterTester is testing. */
+  public Class<? extends Converter<?>> getConverterClass() {
+    return converterClass;
+  }
+
+  /**
+   * Returns whether this ConverterTester has a test for the given input, i.e., addEqualityGroup
+   * was called with the given string.
+   */
+  public boolean hasTestForInput(String input) {
+    return testedInputs.contains(input);
+  }
+
+  /**
+   * Adds a set of valid inputs which are expected to convert to equal values.
+   *
+   * <p>The inputs added here will be converted to values using the Converter class passed to the
+   * constructor of this instance; the resulting values must be equal (and have equal hashCodes):
+   *
+   * <ul>
+   * <li>to themselves
+   * <li>to another copy of themselves generated from the same Converter instance
+   * <li>to another copy of themselves generated from a different Converter instance
+   * <li>to the other values converted from inputs in the same addEqualityGroup call
+   * </ul>
+   *
+   * <p>They must NOT be equal:
+   *
+   * <ul>
+   * <li>to null
+   * <li>to an instance of an arbitrary class
+   * <li>to any values converted from inputs in a different addEqualityGroup call
+   * </ul>
+   *
+   * @throws AssertionError if an {@link OptionsParsingException} is thrown from the
+   *     {@link Converter#convert} method when converting any of the inputs.
+   * @see EqualsTester#addEqualityGroup
+   */
+  public ConverterTester addEqualityGroup(String... inputs) {
+    ImmutableList.Builder<WrappedItem> wrapped = new ImmutableList.Builder<>();
+    ImmutableList<String> inputList = ImmutableList.copyOf(inputs);
+    inputLists.add(inputList);
+    for (String input : inputList) {
+      testedInputs.add(input);
+      try {
+        wrapped.add(new WrappedItem(input, converter.convert(input)));
+      } catch (OptionsParsingException ex) {
+        throw new AssertionError("Failed to parse input: \"" + input + "\"", ex);
+      }
+    }
+    tester.addEqualityGroup(wrapped.build().toArray());
+    return this;
+  }
+
+  /**
+   * Tests the convert method of the wrapped Converter class, verifying the properties listed in the
+   * Javadoc listed for {@link #addEqualityGroup}.
+   *
+   * @throws AssertionError if one of the expected properties did not hold up
+   * @see EqualsTester#testEquals
+   */
+  public ConverterTester testConvert() {
+    tester.testEquals();
+    testItems();
+    return this;
+  }
+
+  private void testItems() {
+    for (ImmutableList<String> inputList : inputLists) {
+      for (String input : inputList) {
+        Converter<?> converter = createConverter();
+        Converter<?> converter2 = createConverter();
+
+        Object converted;
+        Object convertedAgain;
+        Object convertedDifferentConverterInstance;
+        try {
+          converted = converter.convert(input);
+          convertedAgain = converter.convert(input);
+          convertedDifferentConverterInstance = converter2.convert(input);
+        } catch (OptionsParsingException ex) {
+          throw new AssertionError("Failed to parse input: \"" + input + "\"", ex);
+        }
+
+        assertWithMessage(
+                "Input \""
+                    + input
+                    + "\" was not equal to itself when converted twice by the same Converter")
+            .that(convertedAgain)
+            .isEqualTo(converted);
+        assertWithMessage(
+                "Input \""
+                    + input
+                    + "\" did not have a consistent hashCode when converted twice "
+                    + "by the same Converter")
+            .that(convertedAgain.hashCode())
+            .isEqualTo(converted.hashCode());
+        assertWithMessage(
+            "Input \""
+                + input
+                + "\" was not equal to itself when converted twice by a different Converter")
+            .that(convertedDifferentConverterInstance)
+            .isEqualTo(converted);
+        assertWithMessage(
+            "Input \""
+                + input
+                + "\" did not have a consistent hashCode when converted twice "
+                + "by a different Converter")
+            .that(convertedDifferentConverterInstance.hashCode())
+            .isEqualTo(converted.hashCode());
+      }
+    }
+  }
+
+  /**
+   * A wrapper around the objects passed to EqualsTester to give them a more useful toString() so
+   * that the mapping between the input text which actually appears in the source file and the
+   * object produced from parsing it is more obvious.
+   */
+  private static final class WrappedItem {
+    private final String argument;
+    private final Object wrapped;
+
+    private WrappedItem(String argument, Object wrapped) {
+      this.argument = argument;
+      this.wrapped = wrapped;
+    }
+
+    @Override
+    public String toString() {
+      return String.format("Converted input \"%s\" => [%s]", argument, wrapped);
+    }
+
+    @Override
+    public int hashCode() {
+      return wrapped.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      if (other instanceof WrappedItem) {
+        return this.wrapped.equals(((WrappedItem) other).wrapped);
+      }
+      return this.wrapped.equals(other);
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java b/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java
new file mode 100644
index 0000000000..afa0231ea7
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/testing/ConverterTesterMap.java
@@ -0,0 +1,84 @@
+// 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.common.options.testing;
+
+import com.google.common.collect.ForwardingMap;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.common.options.Converter;
+import java.util.Map;
+
+/**
+ * An immutable mapping from {@link Converter} classes to {@link ConverterTester}s which test them.
+ *
+ * <p>Note that the ConverterTesters within are NOT immutable.
+ */
+public final class ConverterTesterMap
+    extends ForwardingMap<Class<? extends Converter<?>>, ConverterTester> {
+
+  private final ImmutableMap<Class<? extends Converter<?>>, ConverterTester> delegate;
+
+  private ConverterTesterMap(
+      ImmutableMap<Class<? extends Converter<?>>, ConverterTester> delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  protected Map<Class<? extends Converter<?>>, ConverterTester> delegate() {
+    return delegate;
+  }
+
+  /** A builder to construct new {@link ConverterTesterMap}s. */
+  public static final class Builder {
+    private final ImmutableMap.Builder<Class<? extends Converter<?>>, ConverterTester> delegate;
+
+    public Builder() {
+      this.delegate = new ImmutableMap.Builder<>();
+    }
+
+    /**
+     * Adds a new ConverterTester, mapping it to the class of converter it tests. Only one tester
+     * for each class is permitted; duplicates will cause {@link #build} to fail.
+     */
+    public Builder add(ConverterTester item) {
+      delegate.put(item.getConverterClass(), item);
+      return this;
+    }
+
+    /**
+     * Adds the entries from the given {@link ConverterTesterMap}. Only one tester for each class is
+     * permitted; duplicates will cause {@link #build} to fail.
+     */
+    public Builder addAll(ConverterTesterMap map) {
+      // this is safe because we know the other map was constructed the same way this one was
+      delegate.putAll(map);
+      return this;
+    }
+
+    /**
+     * Adds all of the ConverterTesters from the given iterable. Only one tester for each class is
+     * permitted; duplicates will cause {@link #build} to fail.
+     */
+    public Builder addAll(Iterable<ConverterTester> items) {
+      for (ConverterTester item : items) {
+        add(item);
+      }
+      return this;
+    }
+
+    public ConverterTesterMap build() {
+      return new ConverterTesterMap(delegate.build());
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java b/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java
new file mode 100644
index 0000000000..fa58462494
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/testing/OptionsTester.java
@@ -0,0 +1,140 @@
+// 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.common.options.testing;
+
+import static com.google.common.truth.Truth.assertWithMessage;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.devtools.common.options.Converter;
+import com.google.devtools.common.options.Option;
+import com.google.devtools.common.options.OptionsBase;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+
+/**
+ * A tester to validate certain useful properties of OptionsBase subclasses. These are not required
+ * for parsing options in these classes, but can be helpful for e.g. ensuring that equality is not
+ * violated.
+ */
+public final class OptionsTester {
+
+  private final Class<? extends OptionsBase> optionsClass;
+
+  public OptionsTester(Class<? extends OptionsBase> optionsClass) {
+    this.optionsClass = optionsClass;
+  }
+
+  private static ImmutableList<Field> getAllFields(Class<? extends OptionsBase> optionsClass) {
+    ImmutableList.Builder<Field> builder = new ImmutableList.Builder<>();
+    Class<? extends OptionsBase> current = optionsClass;
+    while (!OptionsBase.class.equals(current)) {
+      builder.add(current.getDeclaredFields());
+      // the input extends OptionsBase and we haven't seen OptionsBase yet, so this must also extend
+      // (or be) OptionsBase
+      @SuppressWarnings("unchecked")
+      Class<? extends OptionsBase> superclass =
+          (Class<? extends OptionsBase>) current.getSuperclass();
+      current = superclass;
+    }
+    return builder.build();
+  }
+
+  /**
+   * Tests that there are no non-Option instance fields. Fields not annotated with @Option will not
+   * be considered for equality.
+   */
+  public OptionsTester testAllInstanceFieldsAnnotatedWithOption() {
+    for (Field field : getAllFields(optionsClass)) {
+      if (!Modifier.isStatic(field.getModifiers())) {
+        assertWithMessage(
+                field
+                    + " is missing an @Option annotation; it will not be considered for equality.")
+            .that(field.getAnnotation(Option.class))
+            .isNotNull();
+      }
+    }
+    return this;
+  }
+
+  /** Tests that there are no non-public fields which would interfere with option parsing. */
+  public OptionsTester testAllOptionFieldsPublic() {
+    for (Field field : getAllFields(optionsClass)) {
+      if (field.isAnnotationPresent(Option.class)) {
+        assertWithMessage(
+                field
+                    + " is Option-annotated, but is not public; it will not be considered as part"
+                    + " of the options. Change the visibility to public.")
+            .that(Modifier.isPublic(field.getModifiers()))
+            .isTrue();
+      }
+      if (Modifier.isStatic(field.getModifiers()) || Modifier.isFinal(field.getModifiers())) {
+        assertWithMessage(
+                field
+                    + " is Option-annotated, but is either static or final; it cannot be properly"
+                    + " set by the option parser. Remove either the annotation or the modifier(s).")
+            .that(field.getAnnotation(Option.class))
+            .isNull();
+      }
+    }
+    return this;
+  }
+
+  /**
+   * Tests that the default values of this class were part of the test data for the appropriate
+   * ConverterTester, ensuring that the defaults at least obey proper equality semantics.
+   *
+   * <p>The default converters are not tested in this way.
+   *
+   * <p>Note that testConvert is not actually run on the ConverterTesters; it is expected that they
+   * are run elsewhere.
+   */
+  public OptionsTester testAllDefaultValuesTestedBy(ConverterTesterMap testers) {
+    ImmutableListMultimap.Builder<Class<? extends Converter<?>>, Field> converterClassesBuilder =
+        new ImmutableListMultimap.Builder<>();
+    for (Field field : getAllFields(optionsClass)) {
+      Option option = field.getAnnotation(Option.class);
+      if (option != null && !Converter.class.equals(option.converter())) {
+        @SuppressWarnings("unchecked") // converter is rawtyped; see comment on Option.converter()
+        Class<? extends Converter<?>> converter =
+            (Class<? extends Converter<?>>) option.converter();
+        converterClassesBuilder.put(converter, field);
+      }
+    }
+    ImmutableListMultimap<Class<? extends Converter<?>>, Field> converterClasses =
+        converterClassesBuilder.build();
+    for (Class<? extends Converter<?>> converter : converterClasses.keySet()) {
+      assertWithMessage(
+              "Converter " + converter.getCanonicalName() + " has no corresponding ConverterTester")
+          .that(testers)
+          .containsKey(converter);
+      for (Field field : converterClasses.get(converter)) {
+        Option option = field.getAnnotation(Option.class);
+        if (!option.allowMultiple() && !"null".equals(option.defaultValue())) {
+          assertWithMessage(
+                  "Default value \""
+                      + option.defaultValue()
+                      + "\" on "
+                      + field
+                      + " is not tested in the corresponding ConverterTester for "
+                      + converter.getCanonicalName())
+              .that(testers.get(converter).hasTestForInput(option.defaultValue()))
+              .isTrue();
+        }
+      }
+    }
+    return this;
+  }
+}