Skip to content

Commit 68b08a7

Browse files
MaisiKolenimarcphilipp
authored andcommitted
Fix unintended exceptions in assertLinesMatch
This commit adds tests for fast-forward lines surrounded by spaces. This commit also adds a test for limited, non-terminal fast-forward exceeding actual lines. Prior to this commit, isFastForwardLine and parseFastForwardLimit currently didn't match each other in how they handle spaces on the outside of the fast-forward markers. > isFastForwardLine allows them and will detect " >> 2 >> " as a > fast-forward line, but parseFastForwardLimit does not handle the spaces > correctly. It only trims the line that substring(int, int) is invoked on > and does not do so when determining the end index based on the string > length. > This can lead to an IndexOutOfBoundsException being thrown for strings > with three or more surrounding spaces, like " >> 2 >> ". Strings with > one or two surround spaces, like " >> 2 >> ", are treated like an > unlimited fast-forward. Now, a trimmed line is passed to parseFastForwardLimit before any further usage. This implementation is analogous to the implementation in isFastForwardLine that reassigns the trimmed line to the parameter. Also, an NoSuchElementException is prevented when fast-forward exceeds the number of actual lines.
1 parent 248ac22 commit 68b08a7

File tree

3 files changed

+33
-4
lines changed

3 files changed

+33
-4
lines changed

documentation/src/docs/asciidoc/release-notes/release-notes-5.8.1.adoc

+8
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ GitHub.
3434
[[release-notes-5.8.1-junit-jupiter]]
3535
=== JUnit Jupiter
3636

37+
==== Bug Fixes
38+
39+
* `assertLinesMatch()` in `Assertions` no longer fails with a `NoSuchElementException` if
40+
a limited fast-forward followed by at least one more expected line exceeds the remaining
41+
actual lines.
42+
* `assertLinesMatch()` in `Assertions` now handles fast-forwards with leading and trailing
43+
spaces correctly and no longer throws an `IndexOutOfBoundsException`.
44+
3745
==== New Features and Improvements
3846

3947
* `JAVA_18` has been added to the `JRE` enum for use with JRE-based execution conditions.

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

+7-2
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ void assertLinesMatchWithFastForward() {
136136
// fast-forward marker found in expected line: fast-forward actual line...
137137
if (isFastForwardLine(expectedLine)) {
138138
int fastForwardLimit = parseFastForwardLimit(expectedLine);
139+
int actualRemaining = actualDeque.size();
139140

140141
// trivial case: fast-forward marker was in last expected line
141142
if (expectedDeque.isEmpty()) {
142-
int actualRemaining = actualDeque.size();
143143
// no limit given or perfect match? we're done.
144144
if (fastForwardLimit == Integer.MAX_VALUE || fastForwardLimit == actualRemaining) {
145145
return;
@@ -150,6 +150,10 @@ void assertLinesMatchWithFastForward() {
150150

151151
// fast-forward limit was given: use it
152152
if (fastForwardLimit != Integer.MAX_VALUE) {
153+
if (actualRemaining < fastForwardLimit) {
154+
fail("fast-forward(%d) error: not enough actual lines remaining (%s)", fastForwardLimit,
155+
actualRemaining);
156+
}
153157
// fast-forward now: actualDeque.pop(fastForwardLimit)
154158
for (int i = 0; i < fastForwardLimit; i++) {
155159
actualDeque.pop();
@@ -202,7 +206,8 @@ static boolean isFastForwardLine(String line) {
202206
}
203207

204208
static int parseFastForwardLimit(String fastForwardLine) {
205-
String text = fastForwardLine.trim().substring(2, fastForwardLine.length() - 2).trim();
209+
fastForwardLine = fastForwardLine.trim();
210+
String text = fastForwardLine.substring(2, fastForwardLine.length() - 2).trim();
206211
try {
207212
int limit = Integer.parseInt(text);
208213
condition(limit > 0, () -> format("fast-forward(%d) limit must be greater than zero", limit));

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,19 @@ void assertLinesMatchUsingFastForwardMarkerWithTooHighLimitFails() {
164164
assertError(error, "terminal fast-forward(100) error: fast-forward(2) expected", expected, actual);
165165
}
166166

167+
@Test
168+
void assertLinesMatchUsingFastForwardMarkerWithTooHighLimitAndFollowingLineFails() {
169+
/*
170+
* It is important here that the line counts are expected <= actual, that the
171+
* fast-forward exceeds the available actual lines and that it is not a
172+
* terminal fast-forward.
173+
*/
174+
var expected = List.of("first line", ">> 3 >>", "not present");
175+
var actual = List.of("first line", "first skipped", "second skipped");
176+
var error = assertThrows(AssertionFailedError.class, () -> assertLinesMatch(expected, actual));
177+
assertError(error, "fast-forward(3) error: not enough actual lines remaining (2)", expected, actual);
178+
}
179+
167180
@Test
168181
void assertLinesMatchUsingFastForwardMarkerWithoutMatchingNextLineFails() {
169182
var expected = List.of("first line", ">> fails, because next line is >>", "not present");
@@ -187,7 +200,8 @@ void assertLinesMatchIsFastForwardLine() {
187200
() -> assertTrue(isFastForwardLine(">> stacktrace >>")),
188201
() -> assertTrue(isFastForwardLine(">> single line, non Integer.parse()-able comment >>")),
189202
() -> assertTrue(isFastForwardLine(">>9>>")), () -> assertTrue(isFastForwardLine(">> 9 >>")),
190-
() -> assertTrue(isFastForwardLine(">> -9 >>")));
203+
() -> assertTrue(isFastForwardLine(">> -9 >>")), () -> assertTrue(isFastForwardLine(" >> 9 >> ")),
204+
() -> assertTrue(isFastForwardLine(" >> 9 >> ")));
191205
}
192206

193207
@Test
@@ -198,7 +212,9 @@ void assertLinesMatchParseFastForwardLimit() {
198212
() -> assertEquals(Integer.MAX_VALUE, parseFastForwardLimit(">> stacktrace >>")),
199213
() -> assertEquals(Integer.MAX_VALUE, parseFastForwardLimit(">> non Integer.parse()-able comment >>")),
200214
() -> assertEquals(9, parseFastForwardLimit(">>9>>")),
201-
() -> assertEquals(9, parseFastForwardLimit(">> 9 >>")));
215+
() -> assertEquals(9, parseFastForwardLimit(">> 9 >>")),
216+
() -> assertEquals(9, parseFastForwardLimit(" >> 9 >> ")),
217+
() -> assertEquals(9, parseFastForwardLimit(" >> 9 >> ")));
202218
Throwable error = assertThrows(PreconditionViolationException.class, () -> parseFastForwardLimit(">>0>>"));
203219
assertMessageEquals(error, "fast-forward(0) limit must be greater than zero");
204220
error = assertThrows(PreconditionViolationException.class, () -> parseFastForwardLimit(">>-1>>"));

0 commit comments

Comments
 (0)