Skip to content

Commit

Permalink
Fix bad indentation calculation in the console reporter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
horenmar committed Oct 25, 2021
1 parent 0fdee1c commit 905bf43
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 16 deletions.
38 changes: 30 additions & 8 deletions src/catch2/reporters/catch_reporter_console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions tests/SelfTest/Baselines/automake.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,7 @@ Reporters.tests.cpp:<line number>: passed: listingString, ContainsSubstring( "fa
</SourceInfo>
</TestCase>
</MatchingTests>" ( contains: "fake test name" and contains: "fakeTestTag" ) with 1 message: 'Tested reporter: xml'
Reporters.tests.cpp:<line number>: passed:
Message.tests.cpp:<line number>: passed: with 1 message: 'this is a success'
Message.tests.cpp:<line number>: passed:
BDD.tests.cpp:<line number>: passed: before == 0 for: 0 == 0
Expand Down
4 changes: 2 additions & 2 deletions tests/SelfTest/Baselines/console.std.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

13 changes: 11 additions & 2 deletions tests/SelfTest/Baselines/console.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:<line number>
...............................................................................

Reporters.tests.cpp:<line number>: PASSED:

-------------------------------------------------------------------------------
SUCCEED counts as a test pass
-------------------------------------------------------------------------------
Expand Down Expand Up @@ -17030,6 +17039,6 @@ Misc.tests.cpp:<line number>
Misc.tests.cpp:<line number>: 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

3 changes: 2 additions & 1 deletion tests/SelfTest/Baselines/junit.sw.approved.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuitesloose text artifact
>
<testsuite name="<exe-name>" errors="17" failures="129" tests="2131" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
<testsuite name="<exe-name>" errors="17" failures="129" tests="2132" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
<properties>
<property name="random-seed" value="1"/>
<property name="filters" value="~[!nonportable]~[!benchmark]~[approvals] *"/>
Expand Down Expand Up @@ -1163,6 +1163,7 @@ Matchers.tests.cpp:<line number>
<testcase classname="<exe-name>.global" name="Reporter's write listings to provided stream/xml reporter lists tags" time="{duration}" status="run"/>
<testcase classname="<exe-name>.global" name="Reporter's write listings to provided stream/xml reporter lists reporters" time="{duration}" status="run"/>
<testcase classname="<exe-name>.global" name="Reporter's write listings to provided stream/xml reporter lists tests" time="{duration}" status="run"/>
<testcase classname="<exe-name>.global" name="Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla" time="{duration}" status="run"/>
<testcase classname="<exe-name>.global" name="SUCCEED counts as a test pass" time="{duration}" status="run"/>
<testcase classname="<exe-name>.global" name="SUCCEED does not require an argument" time="{duration}" status="run"/>
<testcase classname="<exe-name>.Fixture" name="Scenario: BDD tests requiring Fixtures to provide commonly-accessed data or methods/Given: No operations precede me" time="{duration}" status="run"/>
Expand Down
1 change: 1 addition & 0 deletions tests/SelfTest/Baselines/sonarqube.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
<testCase name="Reporter's write listings to provided stream/xml reporter lists tags" duration="{duration}"/>
<testCase name="Reporter's write listings to provided stream/xml reporter lists reporters" duration="{duration}"/>
<testCase name="Reporter's write listings to provided stream/xml reporter lists tests" duration="{duration}"/>
<testCase name="Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla" duration="{duration}"/>
<testCase name="The default listing implementation write to provided stream/Listing tags" duration="{duration}"/>
<testCase name="The default listing implementation write to provided stream/Listing reporters" duration="{duration}"/>
<testCase name="The default listing implementation write to provided stream/Listing tests" duration="{duration}"/>
Expand Down
4 changes: 3 additions & 1 deletion tests/SelfTest/Baselines/tap.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2590,6 +2590,8 @@ ok {test-number} - listingString, ContainsSubstring("fake reporter"s) for: "<?xm
ok {test-number} - !(factories.empty()) for: !false
# Reporter's write listings to provided stream
ok {test-number} - listingString, ContainsSubstring( "fake test name"s ) && ContainsSubstring( "fakeTestTag"s ) for: "<?xml version="1.0" encoding="UTF-8"?> <MatchingTests> <TestCase> <Name>fake test name</Name> <ClassName/> <Tags>[fakeTestTag]</Tags> <SourceInfo> <File>fake-file.cpp</File> <Line>123456789</Line> </SourceInfo> </TestCase> </MatchingTests>" ( 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
Expand Down Expand Up @@ -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

2 changes: 2 additions & 0 deletions tests/SelfTest/Baselines/teamcity.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,8 @@ Matchers.tests.cpp:<line number>|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']
Expand Down
7 changes: 5 additions & 2 deletions tests/SelfTest/Baselines/xml.sw.approved.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12248,6 +12248,9 @@ All available test cases:
</Section>
<OverallResult success="true"/>
</TestCase>
<TestCase name="Reproducer for #2309 - a very long description past 80 chars (default console width) with a late colon : blablabla" tags="[console-reporter]" filename="tests/<exe-name>/IntrospectiveTests/Reporters.tests.cpp" >
<OverallResult success="true"/>
</TestCase>
<TestCase name="SUCCEED counts as a test pass" tags="[messages]" filename="tests/<exe-name>/UsageTests/Message.tests.cpp" >
<OverallResult success="true"/>
</TestCase>
Expand Down Expand Up @@ -20027,6 +20030,6 @@ loose text artifact
</Section>
<OverallResult success="true"/>
</TestCase>
<OverallResults successes="1958" failures="146" expectedFailures="27"/>
<OverallResultsCases successes="279" failures="86" expectedFailures="7"/>
<OverallResults successes="1959" failures="146" expectedFailures="27"/>
<OverallResultsCases successes="280" failures="86" expectedFailures="7"/>
</Catch2TestRun>
5 changes: 5 additions & 0 deletions tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit 905bf43

Please sign in to comment.