Remove elements by reference in assertSatisfiesExactlyInAnyOrder

scordio · scordio · commit d51cfd9a3217 · 2024-01-21T11:54:35.000+01:00
Fixes #3339.
diff --git a/assertj-core/src/main/java/org/assertj/core/internal/ElementsSatisfyingConsumer.java b/assertj-core/src/main/java/org/assertj/core/internal/ElementsSatisfyingConsumer.java
@@ -13,6 +13,7 @@
 package org.assertj.core.internal;
 
 import static java.util.stream.Collectors.toList;
+import static java.util.stream.IntStream.range;
 import static org.assertj.core.internal.Iterables.byPassingAssertions;
 import static org.assertj.core.util.Streams.stream;
 
@@ -28,12 +29,17 @@
  * @param <E> element type
  */
 class ElementsSatisfyingConsumer<E> {
+
   private final List<E> elements;
 
   ElementsSatisfyingConsumer(Iterable<? extends E> actual, Consumer<? super E> assertions) {
     this(filterByPassingAssertions(actual, assertions));
   }
 
+  private static <E> List<E> filterByPassingAssertions(Iterable<? extends E> actual, Consumer<? super E> assertions) {
+    return stream(actual).filter(byPassingAssertions(assertions)).collect(toList());
+  }
+
   private ElementsSatisfyingConsumer(List<E> elements) {
     this.elements = elements;
   }
@@ -43,7 +49,7 @@ List<E> getElements() {
   }
 
   /**
-   * New <code>ElementsSatisfyingConsumer</code> containing all elements except the (first occurrence of the) given element.
+   * New <code>ElementsSatisfyingConsumer</code> containing all elements except the first occurrence of the given element.
    *
    * <p> This instance is not modified.
    *
@@ -52,11 +58,14 @@ List<E> getElements() {
    */
   ElementsSatisfyingConsumer<E> withoutElement(E element) {
     ArrayList<E> listWithoutElement = new ArrayList<>(elements);
-    listWithoutElement.remove(element);
+    removeFirstReference(element, listWithoutElement);
     return new ElementsSatisfyingConsumer<>(listWithoutElement);
   }
 
-  private static <E> List<E> filterByPassingAssertions(Iterable<? extends E> actual, Consumer<? super E> assertions) {
-    return stream(actual).filter(byPassingAssertions(assertions)).collect(toList());
+  private static void removeFirstReference(Object element, List<?> elements) {
+    range(0, elements.size()).filter(i -> elements.get(i) == element)
+                             .findFirst()
+                             .ifPresent(elements::remove);
   }
+
 }
diff --git a/assertj-core/src/main/java/org/assertj/core/internal/Iterables.java b/assertj-core/src/main/java/org/assertj/core/internal/Iterables.java
@@ -461,7 +461,7 @@ public void assertContainsOnlyNulls(AssertionInfo info, Iterable<?> actual) {
     if (sizeOf(actual) == 0) throw failures.failure(info, shouldContainOnlyNulls(actual));
     // look for any non-null elements
     List<Object> nonNullElements = stream(actual).filter(java.util.Objects::nonNull).collect(toList());
-    if (nonNullElements.size() > 0) throw failures.failure(info, shouldContainOnlyNulls(actual, nonNullElements));
+    if (!nonNullElements.isEmpty()) throw failures.failure(info, shouldContainOnlyNulls(actual, nonNullElements));
   }
 
   /**
@@ -623,7 +623,7 @@ public void assertIsSubsetOf(AssertionInfo info, Iterable<?> actual, Iterable<?>
     checkIterableIsNotNull(values);
     List<Object> extra = stream(actual).filter(actualElement -> !iterableContains(values, actualElement))
                                        .collect(toList());
-    if (extra.size() > 0) throw failures.failure(info, shouldBeSubsetOf(actual, values, extra, comparisonStrategy));
+    if (!extra.isEmpty()) throw failures.failure(info, shouldBeSubsetOf(actual, values, extra, comparisonStrategy));
   }
 
   /**
@@ -1253,7 +1253,7 @@ private static <E> boolean areAllConsumersSatisfied(Queue<ElementsSatisfyingCons
     // recursively test whether we can find any specific matching permutation that can meet the requirements
     if (satisfiedElementsPerConsumer.isEmpty()) return true; // all consumers have been satisfied
 
-    // pop the head (i.e, elements satisfying the current consumer), process the tail (i.e., remaining consumers)...
+    // pop the head (i.e., elements satisfying the current consumer), process the tail (i.e., remaining consumers)...
     ElementsSatisfyingConsumer<E> head = satisfiedElementsPerConsumer.remove();
     List<E> elementsSatisfyingCurrentConsumer = head.getElements();
     if (elementsSatisfyingCurrentConsumer.isEmpty()) return false;   // no element satisfies current consumer
@@ -1335,7 +1335,7 @@ public <E> void assertNoneSatisfy(AssertionInfo info, Iterable<? extends E> actu
                                               .filter(Optional::isPresent)
                                               .map(Optional::get)
                                               .collect(toList());
-    if (erroneousElements.size() > 0) throw failures.failure(info, noElementsShouldSatisfy(actual, erroneousElements));
+    if (!erroneousElements.isEmpty()) throw failures.failure(info, noElementsShouldSatisfy(actual, erroneousElements));
   }
 
   private <E> Optional<E> failsRestrictions(E element, Consumer<? super E> restrictions) {
diff --git a/assertj-core/src/test/java/org/assertj/core/internal/iterables/Iterables_assertSatisfiesExactlyInAnyOrder_Test.java b/assertj-core/src/test/java/org/assertj/core/internal/iterables/Iterables_assertSatisfiesExactlyInAnyOrder_Test.java
@@ -25,18 +25,13 @@
 import java.util.List;
 import java.util.function.Consumer;
 
-import org.assertj.core.api.AssertionInfo;
-import org.assertj.core.internal.Iterables;
 import org.assertj.core.internal.IterablesBaseTest;
-import org.junit.jupiter.api.DisplayName;
+import org.assertj.core.test.Jedi;
 import org.junit.jupiter.api.Test;
 
 /**
- * Tests for <code>{@link Iterables#assertSatisfiesExactlyInAnyOrder(AssertionInfo, Iterable, Consumer[])}</code>.
- *
  * @author Ting Sun
  */
-@DisplayName("Iterables assertSatisfiesExactlyInAnyOrder")
 class Iterables_assertSatisfiesExactlyInAnyOrder_Test extends IterablesBaseTest {
 
   private List<String> actual = newArrayList("Luke", "Leia", "Yoda");
@@ -50,7 +45,6 @@ void should_pass_if_all_consumers_are_satisfied_by_different_elements_in_order()
       assertThat(s).hasSize(4);
       assertThat(s).doesNotContain("L");
     }; // Matches "Yoda"
-
     // WHEN/THEN
     iterables.assertSatisfiesExactlyInAnyOrder(info, actual, array(consumer1, consumer2, consumer3));
   }
@@ -61,7 +55,6 @@ void should_pass_if_all_consumers_are_satisfied_by_different_elements_in_any_ord
     Consumer<String> consumer1 = s -> assertThat(s).contains("Y"); // Matches "Yoda"
     Consumer<String> consumer2 = s -> assertThat(s).contains("L"); // Matches "Luke" and "Leia"
     Consumer<String> consumer3 = s -> assertThat(s).doesNotContain("a"); // Matches "Luke"
-
     // WHEN/THEN
     iterables.assertSatisfiesExactlyInAnyOrder(info, actual, array(consumer1, consumer2, consumer3));
     iterables.assertSatisfiesExactlyInAnyOrder(info, actual, array(consumer1, consumer3, consumer2));
@@ -82,7 +75,6 @@ void should_fail_if_one_of_the_consumer_cannot_be_satisfied() {
                                                                                                           array(consumer1,
                                                                                                                 consumer2,
                                                                                                                 consumer3)));
-
     // THEN
     then(assertionError).hasMessage(shouldSatisfyExactlyInAnyOrder(actual).create());
   }
@@ -118,12 +110,12 @@ void should_fail_if_one_of_the_requirements_cannot_be_satisfied() {
   }
 
   @Test
-  void should_pass_if_iterable_contains_multiple_equal_elements() {
+  void should_pass_if_iterable_contains_same_elements() {
     // GIVEN
-    List<String> names = newArrayList("Luke", "Luke");
+    String luke = "Luke";
+    List<String> names = newArrayList(luke, luke);
     Consumer<String> consumer1 = s -> assertThat(s).contains("L");
     Consumer<String> consumer2 = s -> assertThat(s).contains("u");
-
     // WHEN/THEN
     iterables.assertSatisfiesExactlyInAnyOrder(info, names, array(consumer1, consumer2));
   }
@@ -186,4 +178,38 @@ void should_fail_if_there_are_too_many_consumers() {
     then(assertionError).hasMessage(shouldHaveSize(actual, 3, 4).create());
   }
 
+  @Test
+  void should_pass_without_relying_on_elements_equality() {
+    // GIVEN
+    List<Jedi> actual = newArrayList(new JediOverridingEquals("Luke", "blue"),
+                                     new JediOverridingEquals("Luke", "green"),
+                                     new JediOverridingEquals("Luke", "green"));
+    Consumer<Jedi>[] consumers = array(jedi -> assertThat(jedi.lightSaberColor).isEqualTo("green"),
+                                       jedi -> assertThat(jedi.lightSaberColor).isEqualTo("blue"),
+                                       jedi -> assertThat(jedi.lightSaberColor).isEqualTo("green"));
+    // WHEN/THEN
+    iterables.assertSatisfiesExactlyInAnyOrder(info, actual, consumers);
+  }
+
+  private static class JediOverridingEquals extends Jedi {
+
+    private JediOverridingEquals(String name, String lightSaberColor) {
+      super(name, lightSaberColor);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      JediOverridingEquals jedi = (JediOverridingEquals) o;
+      return getName().equals(jedi.getName());
+    }
+
+    @Override
+    public int hashCode() {
+      return getName().hashCode();
+    }
+
+  }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,7 @@`
`13`	`13`	`package org.assertj.core.internal;`
`14`	`14`
`15`	`15`	`import static java.util.stream.Collectors.toList;`
	`16`	`+import static java.util.stream.IntStream.range;`
`16`	`17`	`import static org.assertj.core.internal.Iterables.byPassingAssertions;`
`17`	`18`	`import static org.assertj.core.util.Streams.stream;`
`18`	`19`
`@@ -28,12 +29,17 @@`
`28`	`29`	`* @param <E> element type`
`29`	`30`	`*/`
`30`	`31`	`class ElementsSatisfyingConsumer<E> {`
	`32`	`+`
`31`	`33`	`private final List<E> elements;`
`32`	`34`
`33`	`35`	`ElementsSatisfyingConsumer(Iterable<? extends E> actual, Consumer<? super E> assertions) {`
`34`	`36`	`this(filterByPassingAssertions(actual, assertions));`
`35`	`37`	`}`
`36`	`38`
	`39`	`+ private static <E> List<E> filterByPassingAssertions(Iterable<? extends E> actual, Consumer<? super E> assertions) {`
	`40`	`+ return stream(actual).filter(byPassingAssertions(assertions)).collect(toList());`
	`41`	`+ }`
	`42`	`+`
`37`	`43`	`private ElementsSatisfyingConsumer(List<E> elements) {`
`38`	`44`	`this.elements = elements;`
`39`	`45`	`}`
`@@ -43,7 +49,7 @@ List<E> getElements() {`
`43`	`49`	`}`
`44`	`50`
`45`	`51`	`/**`
`46`		`- * New <code>ElementsSatisfyingConsumer</code> containing all elements except the (first occurrence of the) given element.`
	`52`	`+ * New <code>ElementsSatisfyingConsumer</code> containing all elements except the first occurrence of the given element.`
`47`	`53`	`*`
`48`	`54`	`* <p> This instance is not modified.`
`49`	`55`	`*`
`@@ -52,11 +58,14 @@ List<E> getElements() {`
`52`	`58`	`*/`
`53`	`59`	`ElementsSatisfyingConsumer<E> withoutElement(E element) {`
`54`	`60`	`ArrayList<E> listWithoutElement = new ArrayList<>(elements);`
`55`		`- listWithoutElement.remove(element);`
	`61`	`+ removeFirstReference(element, listWithoutElement);`
`56`	`62`	`return new ElementsSatisfyingConsumer<>(listWithoutElement);`
`57`	`63`	`}`
`58`	`64`
`59`		`- private static <E> List<E> filterByPassingAssertions(Iterable<? extends E> actual, Consumer<? super E> assertions) {`
`60`		`- return stream(actual).filter(byPassingAssertions(assertions)).collect(toList());`
	`65`	`+ private static void removeFirstReference(Object element, List<?> elements) {`
	`66`	`+ range(0, elements.size()).filter(i -> elements.get(i) == element)`
	`67`	`+ .findFirst()`
	`68`	`+ .ifPresent(elements::remove);`
`61`	`69`	`}`
	`70`	`+`
`62`	`71`	`}`