From 68b08a79c56557277762da0c30eb1d66ee189884 Mon Sep 17 00:00:00 2001 From: Christian Femers Date: Tue, 21 Sep 2021 12:52:17 +0200 Subject: [PATCH] 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. --- .../release-notes/release-notes-5.8.1.adoc | 8 ++++++++ .../junit/jupiter/api/AssertLinesMatch.java | 9 +++++++-- .../api/AssertLinesMatchAssertionsTests.java | 20 +++++++++++++++++-- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.8.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.8.1.adoc index e12d283c334c..8357ebab4dda 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.8.1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.8.1.adoc @@ -34,6 +34,14 @@ GitHub. [[release-notes-5.8.1-junit-jupiter]] === JUnit Jupiter +==== Bug Fixes + +* `assertLinesMatch()` in `Assertions` no longer fails with a `NoSuchElementException` if + a limited fast-forward followed by at least one more expected line exceeds the remaining + actual lines. +* `assertLinesMatch()` in `Assertions` now handles fast-forwards with leading and trailing + spaces correctly and no longer throws an `IndexOutOfBoundsException`. + ==== New Features and Improvements * `JAVA_18` has been added to the `JRE` enum for use with JRE-based execution conditions. diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertLinesMatch.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertLinesMatch.java index f6eadb91807d..eb580b75b15f 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertLinesMatch.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertLinesMatch.java @@ -136,10 +136,10 @@ void assertLinesMatchWithFastForward() { // fast-forward marker found in expected line: fast-forward actual line... if (isFastForwardLine(expectedLine)) { int fastForwardLimit = parseFastForwardLimit(expectedLine); + int actualRemaining = actualDeque.size(); // trivial case: fast-forward marker was in last expected line if (expectedDeque.isEmpty()) { - int actualRemaining = actualDeque.size(); // no limit given or perfect match? we're done. if (fastForwardLimit == Integer.MAX_VALUE || fastForwardLimit == actualRemaining) { return; @@ -150,6 +150,10 @@ void assertLinesMatchWithFastForward() { // fast-forward limit was given: use it if (fastForwardLimit != Integer.MAX_VALUE) { + if (actualRemaining < fastForwardLimit) { + fail("fast-forward(%d) error: not enough actual lines remaining (%s)", fastForwardLimit, + actualRemaining); + } // fast-forward now: actualDeque.pop(fastForwardLimit) for (int i = 0; i < fastForwardLimit; i++) { actualDeque.pop(); @@ -202,7 +206,8 @@ static boolean isFastForwardLine(String line) { } static int parseFastForwardLimit(String fastForwardLine) { - String text = fastForwardLine.trim().substring(2, fastForwardLine.length() - 2).trim(); + fastForwardLine = fastForwardLine.trim(); + String text = fastForwardLine.substring(2, fastForwardLine.length() - 2).trim(); try { int limit = Integer.parseInt(text); condition(limit > 0, () -> format("fast-forward(%d) limit must be greater than zero", limit)); diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertLinesMatchAssertionsTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertLinesMatchAssertionsTests.java index eb6228663df1..bf8f31289a7f 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertLinesMatchAssertionsTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/AssertLinesMatchAssertionsTests.java @@ -164,6 +164,19 @@ void assertLinesMatchUsingFastForwardMarkerWithTooHighLimitFails() { assertError(error, "terminal fast-forward(100) error: fast-forward(2) expected", expected, actual); } + @Test + void assertLinesMatchUsingFastForwardMarkerWithTooHighLimitAndFollowingLineFails() { + /* + * It is important here that the line counts are expected <= actual, that the + * fast-forward exceeds the available actual lines and that it is not a + * terminal fast-forward. + */ + var expected = List.of("first line", ">> 3 >>", "not present"); + var actual = List.of("first line", "first skipped", "second skipped"); + var error = assertThrows(AssertionFailedError.class, () -> assertLinesMatch(expected, actual)); + assertError(error, "fast-forward(3) error: not enough actual lines remaining (2)", expected, actual); + } + @Test void assertLinesMatchUsingFastForwardMarkerWithoutMatchingNextLineFails() { var expected = List.of("first line", ">> fails, because next line is >>", "not present"); @@ -187,7 +200,8 @@ void assertLinesMatchIsFastForwardLine() { () -> assertTrue(isFastForwardLine(">> stacktrace >>")), () -> assertTrue(isFastForwardLine(">> single line, non Integer.parse()-able comment >>")), () -> assertTrue(isFastForwardLine(">>9>>")), () -> assertTrue(isFastForwardLine(">> 9 >>")), - () -> assertTrue(isFastForwardLine(">> -9 >>"))); + () -> assertTrue(isFastForwardLine(">> -9 >>")), () -> assertTrue(isFastForwardLine(" >> 9 >> ")), + () -> assertTrue(isFastForwardLine(" >> 9 >> "))); } @Test @@ -198,7 +212,9 @@ void assertLinesMatchParseFastForwardLimit() { () -> assertEquals(Integer.MAX_VALUE, parseFastForwardLimit(">> stacktrace >>")), () -> assertEquals(Integer.MAX_VALUE, parseFastForwardLimit(">> non Integer.parse()-able comment >>")), () -> assertEquals(9, parseFastForwardLimit(">>9>>")), - () -> assertEquals(9, parseFastForwardLimit(">> 9 >>"))); + () -> assertEquals(9, parseFastForwardLimit(">> 9 >>")), + () -> assertEquals(9, parseFastForwardLimit(" >> 9 >> ")), + () -> assertEquals(9, parseFastForwardLimit(" >> 9 >> "))); Throwable error = assertThrows(PreconditionViolationException.class, () -> parseFastForwardLimit(">>0>>")); assertMessageEquals(error, "fast-forward(0) limit must be greater than zero"); error = assertThrows(PreconditionViolationException.class, () -> parseFastForwardLimit(">>-1>>"));