diff --git a/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java b/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java index 0e82d6dae93..0ab37e3e666 100644 --- a/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java +++ b/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java @@ -60,12 +60,15 @@ private static String describeDifference(Difference difference, Representation r ? representation.unambiguousToStringOf(difference.getOther()) : otherFieldValue; + String additionalInfo = difference.getDescription() + .map(desc -> format("%n- reason : %s", escapePercent(desc))) + .orElse(""); return format("%nPath to difference: <%s>%n" + - "- expected: <%s>%n" + - "- actual : <%s>", + "- actual : <%s>%n" + + "- expected: <%s>" + additionalInfo, join(difference.getPath()).with("."), - escapePercent(otherFieldValueRepresentation), - escapePercent(actualFieldValueRepresentation)); + escapePercent(actualFieldValueRepresentation), + escapePercent(otherFieldValueRepresentation)); } protected static String escapePercent(String value) { diff --git a/src/main/java/org/assertj/core/internal/DeepDifference.java b/src/main/java/org/assertj/core/internal/DeepDifference.java index fafd71a71ad..fd03dc5a3de 100644 --- a/src/main/java/org/assertj/core/internal/DeepDifference.java +++ b/src/main/java/org/assertj/core/internal/DeepDifference.java @@ -12,9 +12,11 @@ */ package org.assertj.core.internal; +import static java.lang.String.format; import static org.assertj.core.internal.Objects.getDeclaredFieldsIncludingInherited; import static org.assertj.core.internal.Objects.propertyOrFieldValuesAreEqual; import static org.assertj.core.internal.TypeComparators.defaultTypeComparators; +import static org.assertj.core.util.Sets.newHashSet; import static org.assertj.core.util.Strings.join; import static org.assertj.core.util.introspection.PropertyOrFieldSupport.COMPARISON; @@ -31,6 +33,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.SortedMap; import java.util.SortedSet; @@ -47,6 +50,7 @@ */ public class DeepDifference { + private static final String MISSING_FIELDS = "%s can't be compared to %s as %s does not declare all %s fields, it lacks these:%s"; private static final Map, Boolean> customEquals = new ConcurrentHashMap<>(); private static final Map, Boolean> customHash = new ConcurrentHashMap<>(); @@ -98,11 +102,17 @@ public static class Difference { List path; Object actual; Object other; + Optional description; public Difference(List path, Object actual, Object other) { + this(path, actual, other, null); + } + + public Difference(List path, Object actual, Object other, String description) { this.path = path; this.actual = actual; this.other = other; + this.description = Optional.ofNullable(description); } public List getPath() { @@ -117,9 +127,15 @@ public Object getOther() { return other; } + public Optional getDescription() { + return description; + } + @Override public String toString() { - return "Difference [path=" + path + ", actual=" + actual + ", other=" + other + "]"; + return description.isPresent() + ? format("Difference [path=%s, actual=%s, other=%s, description=%s]", path, actual, other, description.get()) + : format("Difference [path=%s, actual=%s, other=%s]", path, actual, other); } } @@ -150,7 +166,7 @@ public static List determineDifferences(Object a, Object b, TypeComparators comparatorByType) { // replace null comparators groups by empty one to simplify code afterwards comparatorByPropertyOrField = comparatorByPropertyOrField == null - ? new TreeMap>() + ? new TreeMap<>() : comparatorByPropertyOrField; comparatorByType = comparatorByType == null ? defaultTypeComparators() : comparatorByType; return determineDifferences(a, b, null, comparatorByPropertyOrField, comparatorByType); @@ -298,7 +314,14 @@ private static List determineDifferences(Object a, Object b, List key1FieldsNames = getFieldsNames(getDeclaredFieldsIncludingInherited(key1.getClass())); Set key2FieldsNames = getFieldsNames(getDeclaredFieldsIncludingInherited(key2.getClass())); if (!key2FieldsNames.containsAll(key1FieldsNames)) { - differences.add(new Difference(currentPath, key1, key2)); + Set key1FieldsNamesNotInKey2 = newHashSet(key1FieldsNames); + key1FieldsNamesNotInKey2.removeAll(key2FieldsNames); + String missingFields = key1FieldsNamesNotInKey2.toString(); + String key2ClassName = key2.getClass().getName(); + String key1ClassName = key1.getClass().getName(); + String missingFieldsDescription = format(MISSING_FIELDS, key1ClassName, key2ClassName, key2.getClass().getSimpleName(), + key1.getClass().getSimpleName(), missingFields); + differences.add(new Difference(currentPath, key1, key2, missingFieldsDescription)); } else { for (String fieldName : key1FieldsNames) { List path = new ArrayList<>(currentPath); @@ -330,7 +353,7 @@ private static Deque initStack(Object a, Object b, List parentP TypeComparators comparatorByType) { Deque stack = new LinkedList<>(); boolean isRootObject = parentPath == null; - List currentPath = isRootObject ? new ArrayList() : parentPath; + List currentPath = isRootObject ? new ArrayList<>() : parentPath; DualKey basicDualKey = new DualKey(currentPath, a, b); if (a != null && b != null && !isContainerType(a) && !isContainerType(b) && (isRootObject || !hasCustomComparator(basicDualKey, comparatorByPropertyOrField, comparatorByType))) { diff --git a/src/test/java/org/assertj/core/error/Person.java b/src/test/java/org/assertj/core/error/Person.java new file mode 100644 index 00000000000..a9fb0320f33 --- /dev/null +++ b/src/test/java/org/assertj/core/error/Person.java @@ -0,0 +1,31 @@ +/* + * 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. + * + * Copyright 2012-2018 the original author or authors. + */ +package org.assertj.core.error; + +class Person { + public String getFirstName() { + return firstName; + } + + public String getLastName() { + return lastName; + } + + private String firstName, lastName; + + public Person(String firstName, String lastName) { + this.firstName = firstName; + this.lastName = lastName; + } + +} \ No newline at end of file diff --git a/src/test/java/org/assertj/core/error/PersonDAO.java b/src/test/java/org/assertj/core/error/PersonDAO.java new file mode 100644 index 00000000000..9b8a83288b8 --- /dev/null +++ b/src/test/java/org/assertj/core/error/PersonDAO.java @@ -0,0 +1,43 @@ +/* + * 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. + * + * Copyright 2012-2018 the original author or authors. + */ +package org.assertj.core.error; + +class PersonDAO { + + public String getFirstname() { + return firstname; + } + + public String getLastname() { + return lastname; + } + + public Long getId() { + return id; + } + + public Integer getAge() { + return age; + } + + private String firstname, lastname; + private Long id; + private Integer age; + + public PersonDAO(String firstname, String lastname, Long id, Integer age) { + this.firstname = firstname; + this.lastname = lastname; + this.id = id; + this.age = age; + } +} \ No newline at end of file diff --git a/src/test/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively_create_Test.java b/src/test/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively_create_Test.java index 0c83e817002..602cc727bf1 100644 --- a/src/test/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively_create_Test.java +++ b/src/test/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively_create_Test.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.catchThrowable; import static org.assertj.core.configuration.ConfigurationProvider.CONFIGURATION_PROVIDER; import static org.assertj.core.error.ShouldBeEqualByComparingFieldByFieldRecursively.shouldBeEqualByComparingFieldByFieldRecursive; +import static org.assertj.core.internal.DeepDifference.determineDifferences; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -26,15 +27,17 @@ import java.util.TreeSet; import org.assertj.core.description.TextDescription; -import org.assertj.core.internal.DeepDifference; import org.assertj.core.internal.DeepDifference.Difference; import org.assertj.core.internal.objects.Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.WithCollection; import org.assertj.core.internal.objects.Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.WithMap; +import org.assertj.core.presentation.Representation; import org.assertj.core.test.Jedi; import org.junit.Test; public class ShouldBeEqualByComparingFieldByFieldRecursively_create_Test { + private static final Representation REPRESENTATION = CONFIGURATION_PROVIDER.representation(); + @Test public void should_throw_assertion_error_rather_than_null_pointer_when_one_nested_member_is_null() { // GIVEN @@ -45,7 +48,7 @@ public void should_throw_assertion_error_rather_than_null_pointer_when_one_neste Throwable throwable2 = catchThrowable(() -> assertThat(noname).isEqualToComparingFieldByFieldRecursively(yoda)); // THEN assertThat(throwable1).isInstanceOf(AssertionError.class) - .isNotInstanceOf(NullPointerException.class); + .isNotInstanceOf(NullPointerException.class); assertThat(throwable2).isInstanceOf(AssertionError.class) .isNotInstanceOf(NullPointerException.class); } @@ -58,14 +61,14 @@ public void should_use_unambiguous_fields_description_when_standard_description_ withHashSet.collection.add("bar"); withHashSet.collection.add("foo"); withSortedSet.collection.addAll(withHashSet.collection); - List differences = DeepDifference.determineDifferences(withHashSet, withSortedSet, null, null); + List differences = determineDifferences(withHashSet, withSortedSet, null, null); // WHEN // @format:off String message = shouldBeEqualByComparingFieldByFieldRecursive(withSortedSet, withHashSet, differences, - CONFIGURATION_PROVIDER.representation()) - .create(new TextDescription("Test"), CONFIGURATION_PROVIDER.representation()); + REPRESENTATION) + .create(new TextDescription("Test"), REPRESENTATION); // @format:on // THEN assertThat(message).isEqualTo(format("[Test] %n" + @@ -76,8 +79,8 @@ public void should_use_unambiguous_fields_description_when_standard_description_ "when recursively comparing field by field, but found the following difference(s):%n" + "%n" + "Path to difference: %n" + - "- expected: <[\"bar\", \"foo\"] (TreeSet@%s)>%n" + - "- actual : <[\"bar\", \"foo\"] (LinkedHashSet@%s)>", + "- actual : <[\"bar\", \"foo\"] (LinkedHashSet@%s)>%n" + + "- expected: <[\"bar\", \"foo\"] (TreeSet@%s)>", toHexString(withSortedSet.collection.hashCode()), toHexString(withHashSet.collection.hashCode()))); } @@ -90,14 +93,14 @@ public void should_use_unambiguous_fields_description_when_standard_description_ withLinkedHashMap.map.put(1L, true); withLinkedHashMap.map.put(2L, false); withTreeMap.map.putAll(withLinkedHashMap.map); - List differences = DeepDifference.determineDifferences(withLinkedHashMap, withTreeMap, null, null); + List differences = determineDifferences(withLinkedHashMap, withTreeMap, null, null); // WHEN // @format:off String message = shouldBeEqualByComparingFieldByFieldRecursive(withTreeMap, withLinkedHashMap, differences, - CONFIGURATION_PROVIDER.representation()) - .create(new TextDescription("Test"), CONFIGURATION_PROVIDER.representation()); + REPRESENTATION) + .create(new TextDescription("Test"), REPRESENTATION); // @format:on // THEN assertThat(message).isEqualTo(format("[Test] %n" + @@ -105,13 +108,46 @@ public void should_use_unambiguous_fields_description_when_standard_description_ " %n" + "to be equal to:%n" + " %n" + - "when recursively comparing field by field, but found the following difference(s):%n" - + "%n" + + "when recursively comparing field by field, but found the following difference(s):%n" + + "%n" + "Path to difference: %n" + - "- expected: <{1L=true, 2L=false} (TreeMap@%s)>%n" + - "- actual : <{1L=true, 2L=false} (LinkedHashMap@%s)>", + "- actual : <{1L=true, 2L=false} (LinkedHashMap@%s)>%n" + + "- expected: <{1L=true, 2L=false} (TreeMap@%s)>", toHexString(withTreeMap.map.hashCode()), toHexString(withLinkedHashMap.map.hashCode()))); } + @Test + public void should_precise_missing_fields_when_actual_does_not_declare_all_expected_fields() { + // GIVEN + Person person = new Person("John", "Doe"); + PersonDAO personDAO = new PersonDAO("John", "Doe", 1L, 23); + // THEN + List differences = determineDifferences(person, personDAO, null, null); + // WHEN + // @format:off + String message = shouldBeEqualByComparingFieldByFieldRecursive(person, + personDAO, + differences, + REPRESENTATION) + .create(new TextDescription("Test"), REPRESENTATION); + // @format:on + // THEN + String personHash = toHexString(person.hashCode()); + String personDAOHash = toHexString(personDAO.hashCode()); + assertThat(message).isEqualTo(format("[Test] %n" + + "Expecting:%n" + + " %n" + + "to be equal to:%n" + + " %n" + + "when recursively comparing field by field, but found the following difference(s):%n" + + "%n" + + "Path to difference: <>%n" + + "- actual : %n" + + "- expected: %n" + + "- reason : org.assertj.core.error.Person can't be compared to org.assertj.core.error.PersonDAO as PersonDAO does not declare all Person fields, it lacks these:[firstName, lastName]", + personHash, personDAOHash, + personHash, personDAOHash)); + } + } diff --git a/src/test/java/org/assertj/core/internal/ObjectsBaseTest.java b/src/test/java/org/assertj/core/internal/ObjectsBaseTest.java index 15afa06ace8..168de90efc5 100644 --- a/src/test/java/org/assertj/core/internal/ObjectsBaseTest.java +++ b/src/test/java/org/assertj/core/internal/ObjectsBaseTest.java @@ -35,6 +35,10 @@ */ public class ObjectsBaseTest { + { + Assertions.setRemoveAssertJRelatedElementsFromStackTrace(false); + } + @Rule public ExpectedException thrown = none(); diff --git a/src/test/java/org/assertj/core/internal/objects/Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.java b/src/test/java/org/assertj/core/internal/objects/Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.java index c19789837da..8c2c7c2c955 100644 --- a/src/test/java/org/assertj/core/internal/objects/Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.java +++ b/src/test/java/org/assertj/core/internal/objects/Objects_assertIsEqualToComparingFieldByFieldRecursive_Test.java @@ -15,6 +15,7 @@ import static java.lang.String.format; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.assertj.core.configuration.ConfigurationProvider.CONFIGURATION_PROVIDER; import static org.assertj.core.error.ShouldBeEqualByComparingFieldByFieldRecursively.shouldBeEqualByComparingFieldByFieldRecursive; import static org.assertj.core.internal.TypeComparators.defaultTypeComparators; @@ -215,11 +216,11 @@ public void should_have_error_message_with_differences_and_path_to_differences() "when recursively comparing field by field, but found the following difference(s):%n%n" + "Path to difference: %n" + - "- expected: <2>%n" + - "- actual : <1>%n%n" + + "- actual : <1>%n" + + "- expected: <2>%n%n" + "Path to difference: %n" + - "- expected: <\"John\">%n" + - "- actual : <\"Jack\">"); + "- actual : <\"Jack\">%n" + + "- expected: <\"John\">"); assertThat(actual).isEqualToComparingFieldByFieldRecursively(other); } @@ -240,8 +241,8 @@ public void should_have_error_message_with_path_to_difference_when_difference_is "when recursively comparing field by field, but found the following difference(s):%n%n" + "Path to difference: %n" + - "- expected: <10>%n" + - "- actual : <99>"); + "- actual : <99>%n" + + "- expected: <10>"); assertThat(actual).isEqualToComparingFieldByFieldRecursively(other); } @@ -452,6 +453,21 @@ public void should_be_able_to_compare_objects_with_percentages() { failBecauseExpectedAssertionErrorWasNotThrown(); } + @Test + public void should_report_missing_property() { + // GIVEN + Human joe = new Human(); + joe.name = "joe"; + Giant goliath = new Giant(); + goliath.name = "joe"; + goliath.height = 3.0; + // WHEN + Throwable error = catchThrowable(() -> assertThat(goliath).isEqualToComparingFieldByFieldRecursively(joe)); + // THEN + assertThat(error).hasMessageContaining("Human does not declare all Giant fields") + .hasMessageContaining("[height]"); + } + public static class WithMap { public Map map; @@ -461,7 +477,7 @@ public WithMap(Map map) { @Override public String toString() { - return String.format("WithMap [map=%s]", map); + return format("WithMap [map=%s]", map); } } @@ -475,7 +491,7 @@ public WithCollection(Collection collection) { @Override public String toString() { - return String.format("WithCollection [collection=%s]", collection); + return format("WithCollection [collection=%s]", collection); } } @@ -515,6 +531,11 @@ public static class Human extends Person { public static class Giant extends Person { public double height = 3.0; + + @Override + public String toString() { + return "Giant [name=" + name + ", home=" + home + ", " + "height " + height + "]"; + } } public static class EqualPerson extends Person {