IterableDiff refactoring

Improve a bit containsOnly tests so that it would fail is we did not use the comparison strategy in Iterables.assertContainsOnly
assertj · Dec 27, 2017 · 7bb3911 · 7bb3911
1 parent 5d394d3
commit 7bb3911
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 42 deletions.
diff --git a/src/main/java/org/assertj/core/internal/Arrays.java b/src/main/java/org/assertj/core/internal/Arrays.java
@@ -218,12 +218,11 @@ void assertContainsOnly(AssertionInfo info, Failures failures, Object actual, Ob
       }
     }
 
-    if (notExpected.isEmpty() && notFound.isEmpty()) return;
-
-    throw failures.failure(info, shouldContainOnly(actual, values,
-                                                   notFound, notExpected,
-                                                   comparisonStrategy));
-
+    if (!notExpected.isEmpty() || !notFound.isEmpty()) {
+      throw failures.failure(info, shouldContainOnly(actual, values,
+                                                     notFound, notExpected,
+                                                     comparisonStrategy));
+    }
   }
 
   void assertContainsOnlyNulls(AssertionInfo info, Failures failures, Object[] actual) {

diff --git a/src/main/java/org/assertj/core/internal/IterableDiff.java b/src/main/java/org/assertj/core/internal/IterableDiff.java
@@ -28,8 +28,10 @@ class IterableDiff {
 
   <T> IterableDiff(Iterable<T> actual, Iterable<T> expected, ComparisonStrategy comparisonStrategy) {
     this.comparisonStrategy = comparisonStrategy;
-    this.unexpected = unexpectedElements(actual, expected);
-    this.missing = missingElements(actual, expected);
+    // return the elements in actual that are not in expected: actual - expected
+    this.unexpected = subtract(actual, expected);
+    // return the elements in expected that are not in actual: expected - actual
+    this.missing = subtract(expected, actual);
   }
 
   static <T> IterableDiff diff(Iterable<T> actual, Iterable<T> expected, ComparisonStrategy comparisonStrategy) {
@@ -40,32 +42,22 @@ boolean differencesFound() {
     return !unexpected.isEmpty() || !missing.isEmpty();
   }
 
-  private <T> List<Object> missingElements(Iterable<T> actual, Iterable<T> expected) {
-    // return the elements in expected that are not in actual
-    return subtract(actual, expected);
-  }
-
-  private <T> List<Object> unexpectedElements(Iterable<T> actual, Iterable<T> expected) {
-    // return the elements in actual that are not in expected
-    return subtract(expected, actual);
-  }
-
   /**
-   * Returns the list of elements in the second iterable that are not in the first  
-   * @param first the list we want to look for missing elements
-   * @param second the list of expected elements
-   * @return list of elements from expected missing in source
+   * Returns the list of elements in the first iterable that are not in the second, i.e. first - second  
+   * @param first the list we want to subtract from
+   * @param second the list to subtract
+   * @return the list of elements in the first iterable that are not in the second, i.e. first - second
    */
   private <T> List<Object> subtract(Iterable<T> first, Iterable<T> second) {
     List<Object> missingInFirst = new ArrayList<>();
     // use a copy to deal correctly with potential duplicates
-    List<T> copyOfFirst = newArrayList(first);
-    for (Object elementInSecond : second) {
-      if (!iterableContains(copyOfFirst, elementInSecond)) {
-        missingInFirst.add(elementInSecond);
+    List<T> copyOfSecond = newArrayList(second);
+    for (Object elementInFirst : first) {
+      if (iterableContains(copyOfSecond, elementInFirst)) {
+        // remove the element otherwise a duplicate would be found in the case if there is one in actual
+        iterablesRemoveFirst(copyOfSecond, elementInFirst);
       } else {
-        // remove the element otherwise a duplicate would be still found in the case if there was only one in actual
-        iterablesRemoveFirst(copyOfFirst, elementInSecond);
+        missingInFirst.add(elementInFirst);
       }
     }
     return unmodifiableList(missingInFirst);

diff --git a/src/main/java/org/assertj/core/internal/Iterables.java b/src/main/java/org/assertj/core/internal/Iterables.java
@@ -289,12 +289,11 @@ public void assertContainsOnly(AssertionInfo info, Iterable<?> actual, Object[]
       }
     }
 
-    if (unexpectedValues.isEmpty() && missingValues.isEmpty()) return;
-
-    throw failures.failure(info, shouldContainOnly(actual, expectedValues,
-                                                   missingValues, unexpectedValues,
-                                                   comparisonStrategy));
-
+    if (!unexpectedValues.isEmpty() || !missingValues.isEmpty()) {
+      throw failures.failure(info, shouldContainOnly(actual, expectedValues,
+                                                     missingValues, unexpectedValues,
+                                                     comparisonStrategy));
+    }
   }
 
   /**

diff --git a/src/test/java/org/assertj/core/internal/iterables/Iterables_assertContainsOnly_Test.java b/src/test/java/org/assertj/core/internal/iterables/Iterables_assertContainsOnly_Test.java
@@ -45,7 +45,6 @@ public void should_pass_if_actual_contains_given_values_only() {
 
   @Test
   public void should_pass_if_actual_contains_given_values_only_with_null_elements() {
-    iterables.assertContainsOnly(someInfo(), actual, array("Luke", "Yoda", "Leia"));
     actual.add(null);
     actual.add(null);
     iterables.assertContainsOnly(someInfo(), actual, array("Luke", null, "Yoda", "Leia", null));
@@ -139,25 +138,28 @@ public void should_fail_if_actual_contains_a_subset_of_expected_elements() {
 
   @Test
   public void should_pass_if_actual_contains_given_values_only_according_to_custom_comparison_strategy() {
-    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual, array("LUKE", "YODA", "Leia"));
+    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual,
+                                                                      array("LUKE", "YODA", "Leia"));
   }
 
   @Test
   public void should_pass_if_actual_contains_given_values_only_in_different_order_according_to_custom_comparison_strategy() {
-    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual, array("LEIA", "yoda", "LukE"));
+    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual,
+                                                                      array("LEIA", "yoda", "LukE"));
   }
 
   @Test
   public void should_pass_if_actual_contains_given_values_only_more_than_once_according_to_custom_comparison_strategy() {
     actual.addAll(newArrayList("Luke", "Luke"));
-    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual, array("luke", "YOda", "LeIA"));
+    iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual,
+                                                                      array("luke", "YOda", "LeIA"));
   }
 
   @Test
   public void should_pass_if_actual_contains_given_values_only_even_if_duplicated_according_to_custom_comparison_strategy() {
     actual.addAll(newArrayList("LUKE"));
     iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(someInfo(), actual,
-                                                                      array("LUke", "LUke", "lukE", "YOda", "Leia"));
+                                                                      array("LUke", "LUKE", "lukE", "YOda", "Leia"));
   }
 
   @Test
@@ -168,7 +170,7 @@ public void should_fail_if_actual_does_not_contain_given_values_only_according_t
       iterablesWithCaseInsensitiveComparisonStrategy.assertContainsOnly(info, actual, expected);
     } catch (AssertionError e) {
       verify(failures).failure(info, shouldContainOnly(actual, expected, newArrayList("Han"), newArrayList("Leia"),
-                                                 comparisonStrategy));
+                                                       comparisonStrategy));
       return;
     }
     failBecauseExpectedAssertionErrorWasNotThrown();

diff --git a/...est/java/org/assertj/core/internal/objectarrays/ObjectArrays_assertContainsOnly_Test.java b/...est/java/org/assertj/core/internal/objectarrays/ObjectArrays_assertContainsOnly_Test.java
@@ -27,7 +27,6 @@
 import org.assertj.core.internal.ObjectArraysBaseTest;
 import org.junit.Test;
 
-
 /**
  * Tests for <code>{@link ObjectArrays#assertContainsOnly(AssertionInfo, Object[], Object[])}</code>.
  * 
@@ -114,7 +113,8 @@ public void should_pass_if_actual_contains_given_values_only_more_than_once_acco
   @Test
   public void should_pass_if_actual_contains_given_values_only_even_if_duplicated_according_to_custom_comparison_strategy() {
     actual = array("Luke", "Yoda", "Leia", "LUke");
-    arraysWithCustomComparisonStrategy.assertContainsOnly(someInfo(), actual, array("LUKE", "LUKE", "LuKE", "YOda", "LeiA"));
+    arraysWithCustomComparisonStrategy.assertContainsOnly(someInfo(), actual,
+                                                          array("LUke", "LUKE", "LuKE", "YOda", "LeiA"));
   }
 
   @Test