Add ignoringAllOverriddenEquals

Refactor tests

Author: joel-costigliola

src/main/java/org/assertj/core/api/recursive/comparison/DualKey.java

@@ -60,4 +60,8 @@ public List<String> getPath() {
   public String getConcatenatedPath() {
     return concatenatedPath;
   }
+
+  public boolean isBasicType() {
+    return key1.getClass().getName().startsWith("java.lang");
+  }
 }

src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert.java

@@ -16,6 +16,7 @@
 import static org.assertj.core.error.ShouldBeEqualByComparingFieldByFieldRecursively.shouldBeEqualByComparingFieldByFieldRecursively;
 
 import java.util.Comparator;
+import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
@@ -316,6 +317,66 @@ public RecursiveComparisonAssert ignoringFieldsByRegexes(String... regexes) {
     return this;
   }
 
+  /**
+   * By default the recursive comparison uses overridden {@code equals} methods to compare fields,
+   * this method allows to force a recursive comparison for all fields at the exception of basic types (i.e. java.lang types)
+   * - at some point we need to compare something!
+   * <p>
+   * For the recursive comparison to use the overridden {@code equals} of a given type anyway (like {@link Date}) you can register
+   * a type comparator using {@link #withComparatorForType(Class, Comparator)}.
+   * <p>
+   * TODO introduce {@code ignoringAllOverriddenEqualsExceptFor(Class<?>... classes)} ?
+   * <p>
+   * Example:
+   * <pre><code class='java'> public class Person {
+   *   String name;
+   *   double height;
+   *   Home home = new Home();
+   * }
+   *
+   * public class Home {
+   *   Address address = new Address();
+   * }
+   *
+   * public static class Address {
+   *   int number;
+   *   String street;
+   *
+   *   // only compares number, ouch!
+   *   {@literal @}Override
+   *   public boolean equals(final Object other) {
+   *     if (!(other instanceof Address)) return false;
+   *     Address castOther = (Address) other;
+   *     return Objects.equals(number, castOther.number);
+   *   }
+   * }
+   *
+   * Person sherlock = new Person("Sherlock", 1.80);
+   * sherlock.home.address.street = "Baker Street";
+   * sherlock.home.address.number = 221;
+   *
+   * Person sherlock2 = new Person("Sherlock", 1.80);
+   * sherlock2.home.address.street = "Butcher Street";
+   * sherlock2.home.address.number = 221;
+   *
+   * // assertion succeeds but that's not what we expected since the home.address.street fields differ
+   * // but the equals implemenation in Address does not compare them.
+   * assertThat(sherlock).usingRecursiveComparison()
+   *                     .isEqualTo(sherlock2);
+   *
+   * // to avoid the previous issue, we force a recursive comparison on the home.address field
+   * // now this assertion fails as we expect since the home.address.street fields differ
+   * assertThat(sherlock).usingRecursiveComparison()
+   *                     .ignoringAllOverriddenEquals()
+   *                     .isEqualTo(sherlock2);</code></pre>
+   *
+   * @return this {@link RecursiveComparisonAssert} to chain other methods.
+   */
+  public RecursiveComparisonAssert ignoringAllOverriddenEquals() {
+    recursiveComparisonConfiguration.ignoreAllOverriddenEquals();
+    return this;
+  }
+
   /**
    * By default the recursive comparison uses overridden {@code equals} methods to compare fields,
    * this method allows to force a recursive comparison for the given fields (it adds them to the already registered ones).

src/main/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonConfiguration.java

@@ -50,6 +50,7 @@ public class RecursiveComparisonConfiguration {
   private List<Class<?>> ignoredOverriddenEqualsForTypes = new ArrayList<>();
   private List<FieldLocation> ignoredOverriddenEqualsForFields = new ArrayList<>();
   private List<Pattern> ignoredOverriddenEqualsRegexes = new ArrayList<>();
+  private boolean ignoreAllOverriddenEquals = false;
 
   // registered comparators section
   private TypeComparators typeComparators = defaultTypeComparators();
@@ -254,7 +255,10 @@ private static String concatenatedPath(String parentPath, String name) {
   }
 
   boolean shouldIgnoreOverriddenEqualsOf(DualKey dualKey) {
-    return matchesAnIgnoredOverriddenEqualsField(dualKey) || shouldIgnoreOverriddenEqualsOf(dualKey.key1.getClass());
+    if (dualKey.isBasicType()) return false; // we must compare basic types otherwise the recursive comparison loops infinitely!
+    return ignoreAllOverriddenEquals
+           || matchesAnIgnoredOverriddenEqualsField(dualKey)
+           || shouldIgnoreOverriddenEqualsOf(dualKey.key1.getClass());
   }
 
   @VisibleForTesting
@@ -415,4 +419,8 @@ private void describeTypeCheckingStrictness(StringBuilder description) {
     description.append(format(str));
   }
 
+  public void ignoreAllOverriddenEquals() {
+    ignoreAllOverriddenEquals = true;
+  }
+
 }

src/main/java/org/assertj/core/util/Lists.java

@@ -45,7 +45,7 @@ public static <T> ArrayList<T> newArrayList(T... elements) {
   }
 
   @SafeVarargs
-  public static <T> ArrayList<T> list(T... elements) {
+  public static <T> List<T> list(T... elements) {
     return newArrayList(elements);
   }
 

src/test/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_Test.java

@@ -28,6 +28,7 @@
 import static org.assertj.core.test.NeverEqualComparator.NEVER_EQUALS;
 import static org.assertj.core.test.TestFailures.failBecauseExpectedAssertionErrorWasNotThrown;
 import static org.assertj.core.util.Lists.list;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.Mockito.verify;
 
 import java.sql.Timestamp;
@@ -117,10 +118,10 @@ private static Stream<Arguments> recursivelyEqualObjects() {
     person4.name = "John";
     person4.home.address.number = 1;
 
-    return Stream.of(Arguments.of(person1, person2, "same data, same type"),
-                     Arguments.of(person2, person1, "same data, same type reversed"),
-                     Arguments.of(person3, person4, "same data, different type"),
-                     Arguments.of(person4, person3, "same data, different type"));
+    return Stream.of(arguments(person1, person2, "same data, same type"),
+                     arguments(person2, person1, "same data, same type reversed"),
+                     arguments(person3, person4, "same data, different type"),
+                     arguments(person4, person3, "same data, different type"));
   }
 
   @SuppressWarnings("unused")
@@ -190,16 +191,16 @@ private static Stream<Arguments> recursivelyEqualObjectsIgnoringNullValues() {
     person8.neighbour.neighbour = new Person("James");
     person8.neighbour.neighbour.home.address.number = 456;
 
-    return Stream.of(Arguments.of(person1, person2, "same data, same type, except for actual null fields"),
-                     Arguments.of(person3, person1, "all actual fields are null, should be equal to anything"),
-                     Arguments.of(person3, person2, "all actual fields are null, should be equal to anything"),
-                     Arguments.of(person3, person3, "all actual fields are null, should be equal to anything"),
-                     Arguments.of(person3, person4, "same data, different type, actual has null fields"),
-                     Arguments.of(person5, person2, "same data, different type, actual has null fields"),
-                     Arguments.of(person6, person2, "same data, different type, actual has only non null name"),
-                     Arguments.of(person6, person7, "same data, actual has only non null name"),
-                     Arguments.of(person6, person8, "same data, actual has only non null name"),
-                     Arguments.of(person7, person8, "same data, actual has null fields deep in its graph"));
+    return Stream.of(arguments(person1, person2, "same data, same type, except for actual null fields"),
+                     arguments(person3, person1, "all actual fields are null, should be equal to anything"),
+                     arguments(person3, person2, "all actual fields are null, should be equal to anything"),
+                     arguments(person3, person3, "all actual fields are null, should be equal to anything"),
+                     arguments(person3, person4, "same data, different type, actual has null fields"),
+                     arguments(person5, person2, "same data, different type, actual has null fields"),
+                     arguments(person6, person2, "same data, different type, actual has only non null name"),
+                     arguments(person6, person7, "same data, actual has only non null name"),
+                     arguments(person6, person8, "same data, actual has only non null name"),
+                     arguments(person7, person8, "same data, actual has null fields deep in its graph"));
   }
 
   @SuppressWarnings("unused")
@@ -252,16 +253,16 @@ private static Stream<Arguments> recursivelyEqualObjectsIgnoringGivenFields() {
     person8.neighbour.neighbour = new Person("James");
     person8.neighbour.neighbour.home.address.number = 457;
 
-    return Stream.of(Arguments.of(person1, person2, "same data and type, except for one ignored field",
+    return Stream.of(arguments(person1, person2, "same data and type, except for one ignored field",
                                   list("name")),
-                     Arguments.of(giant1, person1,
+                     arguments(giant1, person1,
                                   "different type, same data except name and height which is not even a field from person1",
                                   list("name", "height")),
-                     Arguments.of(person3, person4, "same data, different type, except for several ignored fields",
+                     arguments(person3, person4, "same data, different type, except for several ignored fields",
                                   list("name", "home.address.number")),
-                     Arguments.of(person5, person6, "same data except for one subfield of an ignored field",
+                     arguments(person5, person6, "same data except for one subfield of an ignored field",
                                   list("home")),
-                     Arguments.of(person7, person8, "same data except for one subfield of an ignored field",
+                     arguments(person7, person8, "same data except for one subfield of an ignored field",
                                   list("neighbour.neighbour.home.address.number", "neighbour.name")));
   }
 

src/test/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_ignoreOverriddenEquals_Test.java

@@ -15,6 +15,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.recursive.comparison.FieldLocation.fielLocation;
 import static org.assertj.core.util.Lists.list;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.util.Date;
 import java.util.List;
@@ -34,6 +35,36 @@
 public class RecursiveComparisonAssert_isEqualTo_ignoreOverriddenEquals_Test
     extends RecursiveComparisonAssert_isEqualTo_BaseTest {
 
+  @SuppressWarnings("unused")
+  @ParameterizedTest(name = "{2}: actual={0} / expected={1}")
+  @MethodSource("comparison_ignores_all_fields_overridden_equals_methods_data")
+  public void should_pass_when_comparison_ignores_all_fields_overridden_equals_methods(Object actual,
+                                                                                       Object expected,
+                                                                                       String testDescription) {
+    assertThat(actual).usingRecursiveComparison()
+                      .ignoringAllOverriddenEquals()
+                      .isEqualTo(expected);
+  }
+
+  @SuppressWarnings("unused")
+  private static Stream<Arguments> comparison_ignores_all_fields_overridden_equals_methods_data() {
+    Person person1 = new Person();
+    person1.neighbour = new AlwaysDifferentPerson();
+    Person person2 = new Person();
+    person2.neighbour = new AlwaysDifferentPerson();
+
+    Person person3 = new Person();
+    person3.home.address = new AlwaysDifferentAddress();
+    person3.neighbour = new AlwaysDifferentPerson();
+    Person person4 = new Person();
+    person4.home.address = new AlwaysDifferentAddress();
+    person4.neighbour = new AlwaysDifferentPerson();
+
+    return Stream.of(arguments(person1, person2, "AlwaysDifferentPerson neighbour identical field by field"),
+                     arguments(person3, person4,
+                                  "AlwaysDifferentPerson neighbour and AlwaysDifferentAddress identical field by field"));
+  }
+
   // ignoringOverriddenEqualsForFieldsMatchingRegexes tests
 
   @SuppressWarnings("unused")
@@ -62,12 +93,12 @@ private static Stream<Arguments> comparison_ignores_overridden_equals_methods_by
     person4.home.address = new AlwaysDifferentAddress();
     person4.neighbour = new AlwaysDifferentPerson();
 
-    return Stream.of(Arguments.of(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
+    return Stream.of(arguments(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
                                   list("org.assertj.core.internal.objects.data.*")),
-                     Arguments.of(person3, person4,
+                     arguments(person3, person4,
                                   "AlwaysDifferentPerson neighbour and AlwaysDifferentAddress identical field by field",
                                   list(".*AlwaysDifferent.*")),
-                     Arguments.of(person3, person4, "Several regexes",
+                     arguments(person3, person4, "Several regexes",
                                   list(".*AlwaysDifferentPerson", ".*AlwaysDifferentAddress")));
   }
 
@@ -140,9 +171,9 @@ private static Stream<Arguments> comparison_ignores_overridden_equals_methods_by
     person4.home.address = new AlwaysDifferentAddress();
     person4.neighbour = new AlwaysDifferentPerson();
 
-    return Stream.of(Arguments.of(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
+    return Stream.of(arguments(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
                                   list(AlwaysDifferentPerson.class)),
-                     Arguments.of(person3, person4,
+                     arguments(person3, person4,
                                   "AlwaysDifferentPerson neighbour and AlwaysDifferentAddress identical field by field",
                                   list(AlwaysDifferentPerson.class, AlwaysDifferentAddress.class)));
   }
@@ -212,9 +243,9 @@ private static Stream<Arguments> comparison_ignores_overridden_equals_methods_by
     person4.home.address = new AlwaysDifferentAddress();
     person4.neighbour = new AlwaysDifferentPerson();
 
-    return Stream.of(Arguments.of(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
+    return Stream.of(arguments(person1, person2, "AlwaysDifferentPerson neighbour identical field by field",
                                   list("neighbour")),
-                     Arguments.of(person3, person4,
+                     arguments(person3, person4,
                                   "AlwaysDifferentPerson neighbour and AlwaysDifferentAddress identical field by field",
                                   list("neighbour", "home.address")));
   }
@@ -248,7 +279,7 @@ public void should_fail_when_actual_differs_from_expected_as_some_overridden_equ
   }
 
   @Test
-  public void ignoring_overridden_equals_methods_only_applies_on_fields_but_not_on_the_object_under_test_itself() {
+  public void overridden_equals_is_not_used_on_the_object_under_test_itself() {
     // GIVEN
     AlwaysEqualPerson actual = new AlwaysEqualPerson();
     actual.name = "John";

src/test/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_withFieldComparators_Test.java

@@ -21,6 +21,7 @@
 import static org.assertj.core.test.AlwaysEqualComparator.alwaysEqual;
 import static org.assertj.core.test.Maps.mapOf;
 import static org.assertj.core.test.NeverEqualComparator.NEVER_EQUALS;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.sql.Timestamp;
 import java.util.Comparator;
@@ -81,11 +82,11 @@ private static Stream<Arguments> recursivelyEqualObjectsWhenUsingFieldComparator
     MapEntry<String, Comparator<?>> addressNumberComparator = entry("home.address.number", alwaysEqual());
     MapEntry<String, Comparator<?>> neighbourComparator = entry("neighbour", alwaysEqual());
 
-    return Stream.of(Arguments.of(person1, person2, mapOf(nameComparator, addressNumberComparator),
+    return Stream.of(arguments(person1, person2, mapOf(nameComparator, addressNumberComparator),
                                   "same data except int fields and case for strings"),
-                     Arguments.of(person3, person4, mapOf(addressNumberComparator), "same data except for int fields"),
+                     arguments(person3, person4, mapOf(addressNumberComparator), "same data except for int fields"),
                      // any neighbour differences should be ignored as we compare persons with AlwaysEqualComparator
-                     Arguments.of(person5, person6, mapOf(neighbourComparator),
+                     arguments(person5, person6, mapOf(neighbourComparator),
                                   "same data except for persons, person's fields should not be compared recursively except at the root level"));
   }
 

src/test/java/org/assertj/core/api/recursive/comparison/RecursiveComparisonAssert_isEqualTo_withTypeComparators_Test.java

@@ -18,6 +18,7 @@
 import static org.assertj.core.test.AlwaysEqualComparator.ALWAY_EQUALS_TIMESTAMP;
 import static org.assertj.core.test.Maps.mapOf;
 import static org.assertj.core.test.NeverEqualComparator.NEVER_EQUALS;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import java.sql.Timestamp;
 import java.util.Comparator;
@@ -81,11 +82,11 @@ private static Stream<Arguments> recursivelyEqualObjectsWhenUsingTypeComparators
     MapEntry<Class<?>, Comparator<?>> stringComparator = entry(String.class, CaseInsensitiveStringComparator.instance);
     MapEntry<Class<?>, Comparator<?>> intComparator = entry(Integer.class, new AlwaysEqualComparator<Integer>());
     MapEntry<Class<?>, Comparator<?>> personComparator = entry(Person.class, new AlwaysEqualComparator<Person>());
-    return Stream.of(Arguments.of(person1, person2, mapOf(stringComparator, intComparator),
+    return Stream.of(arguments(person1, person2, mapOf(stringComparator, intComparator),
                                   "same data except int fields and case for strings"),
-                     Arguments.of(person3, person4, mapOf(intComparator), "same data except for int fields"),
+                     arguments(person3, person4, mapOf(intComparator), "same data except for int fields"),
                      // any neighbour differences should be ignored as we compare persons with AlwaysEqualComparator
-                     Arguments.of(person5, person6, mapOf(personComparator),
+                     arguments(person5, person6, mapOf(personComparator),
                                   "same data except for persons, person's fields should not be compared recursively except at the root level"));
   }