From 905bf438ae4358d1e6b9fb99be40f868589d0857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Mon, 25 Oct 2021 15:10:35 +0200 Subject: [PATCH] Fix bad indentation calculation in the console reporter The problem came from the console reporter trying to provide a fancy linebreaking (primarily for things like `SCENARIO` or the BDD macros), so that new lines start with extra indentation if the text being line broken starts as "{text}: ". The console reporter did not properly take into account cases where the ": " part would already be in a later line, in which case it would ask for non-sensical level of indentation (larger than single line length). We fixed this by also enforcing that the special indentation case only triggers if the ": " is found early enough in the line, so that we also avoid degenerate cases like this: ``` blablabla: F a n c y . . . ``` Fixes #2309 --- .../reporters/catch_reporter_console.cpp | 38 +++++++++++++++---- .../Baselines/automake.sw.approved.txt | 1 + .../Baselines/compact.sw.approved.txt | 1 + .../Baselines/console.std.approved.txt | 4 +- .../Baselines/console.sw.approved.txt | 13 ++++++- .../SelfTest/Baselines/junit.sw.approved.txt | 3 +- .../Baselines/sonarqube.sw.approved.txt | 1 + tests/SelfTest/Baselines/tap.sw.approved.txt | 4 +- .../Baselines/teamcity.sw.approved.txt | 2 + tests/SelfTest/Baselines/xml.sw.approved.txt | 7 +++- .../IntrospectiveTests/Reporters.tests.cpp | 5 +++ 11 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/catch2/reporters/catch_reporter_console.cpp b/src/catch2/reporters/catch_reporter_console.cpp index 5c8b6ec791..a18908979f 100644 --- a/src/catch2/reporters/catch_reporter_console.cpp +++ b/src/catch2/reporters/catch_reporter_console.cpp @@ -559,15 +559,37 @@ void ConsoleReporter::printOpenHeader(std::string const& _name) { } } -// if string has a : in first line will set indent to follow it on -// subsequent lines void ConsoleReporter::printHeaderString(std::string const& _string, std::size_t indent) { - std::size_t i = _string.find(": "); - if (i != std::string::npos) - i += 2; - else - i = 0; - stream << TextFlow::Column(_string).indent(indent + i).initialIndent(indent) << '\n'; + // We want to get a bit fancy with line breaking here, so that subsequent + // lines start after ":" if one is present, e.g. + // ``` + // blablabla: Fancy + // linebreaking + // ``` + // but we also want to avoid problems with overly long indentation causing + // the text to take up too many lines, e.g. + // ``` + // blablabla: F + // a + // n + // c + // y + // . + // . + // . + // ``` + // So we limit the prefix indentation check to first quarter of the possible + // width + std::size_t idx = _string.find( ": " ); + if ( idx != std::string::npos && idx < CATCH_CONFIG_CONSOLE_WIDTH / 4 ) { + idx += 2; + } else { + idx = 0; + } + stream << TextFlow::Column( _string ) + .indent( indent + idx ) + .initialIndent( indent ) + << '\n'; } struct SummaryColumn { diff --git a/tests/SelfTest/Baselines/automake.sw.approved.txt b/tests/SelfTest/Baselines/automake.sw.approved.txt index 12a47a7d06..0860a79ebb 100644 --- a/tests/SelfTest/Baselines/automake.sw.approved.txt +++ b/tests/SelfTest/Baselines/automake.sw.approved.txt @@ -191,6 +191,7 @@ Nor would this :test-result: FAIL Regex string matcher :test-result: PASS Regression test #1 :test-result: PASS Reporter's write listings to provided stream +:test-result: PASS Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla :test-result: PASS SUCCEED counts as a test pass :test-result: PASS SUCCEED does not require an argument :test-result: PASS Scenario: BDD tests requiring Fixtures to provide commonly-accessed data or methods diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 34b6272c93..dfb08230f6 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -1449,6 +1449,7 @@ Reporters.tests.cpp:: passed: listingString, ContainsSubstring( "fa " ( contains: "fake test name" and contains: "fakeTestTag" ) with 1 message: 'Tested reporter: xml' +Reporters.tests.cpp:: passed: Message.tests.cpp:: passed: with 1 message: 'this is a success' Message.tests.cpp:: passed: BDD.tests.cpp:: passed: before == 0 for: 0 == 0 diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index 24ef16c4be..f7320bd70e 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1426,6 +1426,6 @@ due to unexpected exception with message: Why would you throw a std::string? =============================================================================== -test cases: 372 | 295 passed | 70 failed | 7 failed as expected -assertions: 2114 | 1958 passed | 129 failed | 27 failed as expected +test cases: 373 | 296 passed | 70 failed | 7 failed as expected +assertions: 2115 | 1959 passed | 129 failed | 27 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 754fbff614..695a30efe9 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -10231,6 +10231,15 @@ with expansion: with message: Tested reporter: xml +------------------------------------------------------------------------------- +Reproducer for #2309 - a very long description past 80 chars (default console +width) with a late colon : blablabla +------------------------------------------------------------------------------- +Reporters.tests.cpp: +............................................................................... + +Reporters.tests.cpp:: PASSED: + ------------------------------------------------------------------------------- SUCCEED counts as a test pass ------------------------------------------------------------------------------- @@ -17030,6 +17039,6 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: =============================================================================== -test cases: 372 | 279 passed | 86 failed | 7 failed as expected -assertions: 2131 | 1958 passed | 146 failed | 27 failed as expected +test cases: 373 | 280 passed | 86 failed | 7 failed as expected +assertions: 2132 | 1959 passed | 146 failed | 27 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index 70a0c25336..c2486391ed 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -1163,6 +1163,7 @@ Matchers.tests.cpp: + diff --git a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt index f9f8c33874..18bca50717 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -181,6 +181,7 @@ + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index e7aa9eb5ae..aad246f575 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -2590,6 +2590,8 @@ ok {test-number} - listingString, ContainsSubstring("fake reporter"s) for: " fake test name [fakeTestTag] fake-file.cpp 123456789 " ( contains: "fake test name" and contains: "fakeTestTag" ) with 1 message: 'Tested reporter: xml' +# Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla +ok {test-number} - # SUCCEED counts as a test pass ok {test-number} - with 1 message: 'this is a success' # SUCCEED does not require an argument @@ -4264,5 +4266,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2131 +1..2132 diff --git a/tests/SelfTest/Baselines/teamcity.sw.approved.txt b/tests/SelfTest/Baselines/teamcity.sw.approved.txt index da0cbd086c..ef70e14c18 100644 --- a/tests/SelfTest/Baselines/teamcity.sw.approved.txt +++ b/tests/SelfTest/Baselines/teamcity.sw.approved.txt @@ -491,6 +491,8 @@ Matchers.tests.cpp:|nexpression failed|n CHECK_THAT( testStringFor ##teamcity[testFinished name='Regression test #1' duration="{duration}"] ##teamcity[testStarted name='Reporter|'s write listings to provided stream'] ##teamcity[testFinished name='Reporter|'s write listings to provided stream' duration="{duration}"] +##teamcity[testStarted name='Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla'] +##teamcity[testFinished name='Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla' duration="{duration}"] ##teamcity[testStarted name='SUCCEED counts as a test pass'] ##teamcity[testFinished name='SUCCEED counts as a test pass' duration="{duration}"] ##teamcity[testStarted name='SUCCEED does not require an argument'] diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index 3f06d48b7c..419941dee6 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -12248,6 +12248,9 @@ All available test cases: + + + @@ -20027,6 +20030,6 @@ loose text artifact - - + + diff --git a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp index 69d05eef6c..5c95657d49 100644 --- a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp @@ -107,3 +107,8 @@ TEST_CASE( "Reporter's write listings to provided stream", "[reporters]" ) { } } } + + +TEST_CASE("Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla", "[console-reporter]") { + SUCCEED(); +}