Track already visited pairs of Iterables to avoid stack overflow

Resolves #2915.

Co-authored-by: Thomas Zub <thomas.zub@codecentric.de>
Co-authored-by: Marc Philipp <mail@marcphilipp.de>

junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertIterableEquals.java

@@ -19,6 +19,8 @@
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Supplier;
@@ -49,6 +51,11 @@ static void assertIterableEquals(Iterable<?> expected, Iterable<?> actual, Suppl
 
 	private static void assertIterableEquals(Iterable<?> expected, Iterable<?> actual, Deque<Integer> indexes,
 			Object messageOrSupplier) {
+		assertIterableEquals(expected, actual, indexes, messageOrSupplier, new LinkedHashMap<>());
+	}
+
+	private static void assertIterableEquals(Iterable<?> expected, Iterable<?> actual, Deque<Integer> indexes,
+			Object messageOrSupplier, Map<Pair, Status> investigatedElements) {
 
 		if (expected == actual) {
 			return;
@@ -60,28 +67,61 @@ private static void assertIterableEquals(Iterable<?> expected, Iterable<?> actua
 
 		int processed = 0;
 		while (expectedIterator.hasNext() && actualIterator.hasNext()) {
-			processed++;
 			Object expectedElement = expectedIterator.next();
 			Object actualElement = actualIterator.next();
 
-			if (Objects.equals(expectedElement, actualElement)) {
-				continue;
-			}
+			indexes.addLast(processed);
+
+			assertIterableElementsEqual(expectedElement, actualElement, indexes, messageOrSupplier,
+				investigatedElements);
 
-			indexes.addLast(processed - 1);
-			assertIterableElementsEqual(expectedElement, actualElement, indexes, messageOrSupplier);
 			indexes.removeLast();
+			processed++;
 		}
 
 		assertIteratorsAreEmpty(expectedIterator, actualIterator, processed, indexes, messageOrSupplier);
 	}
 
 	private static void assertIterableElementsEqual(Object expected, Object actual, Deque<Integer> indexes,
-			Object messageOrSupplier) {
+			Object messageOrSupplier, Map<Pair, Status> investigatedElements) {
+
+		// If both are equal, we don't need to check recursively.
+		if (Objects.equals(expected, actual)) {
+			return;
+		}
+
+		// If both are iterables, we need to check whether they contain the same elements.
 		if (expected instanceof Iterable && actual instanceof Iterable) {
-			assertIterableEquals((Iterable<?>) expected, (Iterable<?>) actual, indexes, messageOrSupplier);
+
+			Pair pair = new Pair(expected, actual);
+
+			// Before comparing their elements, we check whether we have already checked this pair.
+			Status status = investigatedElements.get(pair);
+
+			// If we've already determined that both contain the same elements, we don't need to check them again.
+			if (status == Status.CONTAIN_SAME_ELEMENTS) {
+				return;
+			}
+
+			// If the pair is already under investigation, we fail to avoid infinite recursion.
+			if (status == Status.UNDER_INVESTIGATION) {
+				indexes.removeLast();
+				failIterablesNotEqual(expected, actual, indexes, messageOrSupplier);
+			}
+
+			// Otherwise, we put the pair under investigation and recurse.
+			investigatedElements.put(pair, Status.UNDER_INVESTIGATION);
+
+			assertIterableEquals((Iterable<?>) expected, (Iterable<?>) actual, indexes, messageOrSupplier,
+				investigatedElements);
+
+			// If we reach this point, we've checked that the two iterables contain the same elements so we store this information
+			// in case we come across the same pair again.
+			investigatedElements.put(pair, Status.CONTAIN_SAME_ELEMENTS);
 		}
-		else if (!Objects.equals(expected, actual)) {
+
+		// Otherwise, they are neither equal nor iterables, so we fail.
+		else {
 			assertIterablesNotNull(expected, actual, indexes, messageOrSupplier);
 			failIterablesNotEqual(expected, actual, indexes, messageOrSupplier);
 		}
@@ -131,4 +171,34 @@ private static void failIterablesNotEqual(Object expected, Object actual, Deque<
 		fail(prefix + message);
 	}
 
+	private final static class Pair {
+		private final Object left;
+		private final Object right;
+
+		public Pair(Object left, Object right) {
+			this.left = left;
+			this.right = right;
+		}
+
+		@Override
+		public boolean equals(Object o) {
+			if (this == o)
+				return true;
+			if (o == null || getClass() != o.getClass())
+				return false;
+			Pair that = (Pair) o;
+			return Objects.equals(this.left, that.left) //
+					&& Objects.equals(this.right, that.right);
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash(left, right);
+		}
+	}
+
+	private enum Status {
+		UNDER_INVESTIGATION, CONTAIN_SAME_ELEMENTS
+	}
+
 }

junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertIterableEqualsAssertionsTests.java

@@ -83,6 +83,7 @@ void assertIterableEqualsNestedIterablesWithIntegers() {
 			listOf(listOf(1), listOf(2), listOf(listOf(3, listOf(4)))));
 		assertIterableEquals(setOf(setOf(1), setOf(2), setOf(setOf(3, setOf(4)))),
 			setOf(setOf(1), setOf(2), setOf(setOf(3, setOf(4)))));
+		assertIterableEquals(listOf(listOf(1), listOf(listOf(1))), setOf(setOf(1), setOf(setOf(1))));
 	}
 
 	@Test
@@ -460,4 +461,18 @@ void assertIterableEqualsThrowsStackOverflowErrorForInterlockedRecursiveStructur
 		assertThrows(StackOverflowError.class, () -> assertIterableEquals(expected, actual));
 	}
 
+	@Test
+	// https://github.com/junit-team/junit5/issues/2915
+	void assertIterableEqualsWithDifferentListOfPath() {
+		try {
+			var expected = listOf(Path.of("1").resolve("2"));
+			var actual = listOf(Path.of("1").resolve("3"));
+			assertIterableEquals(expected, actual);
+			expectAssertionFailedError();
+		}
+		catch (AssertionFailedError ex) {
+			assertMessageEquals(ex, "iterable contents differ at index [0][1], expected: <2> but was: <3>");
+		}
+	}
+
 }