Skip to content

Commit

Permalink
Improve performance of JUnit reporter when handling passing assertions
Browse files Browse the repository at this point in the history
This is done by no longer requiring all assertions to be seen
by the JUnit reporter. Since the JUnit reporter never outputs
all assertions, even with `-s`, the only difference from storing
passing assertions in the `CumulativeReporterBase` was some
bookkeeping in deciding which sections are relevant to the output.

Given `TEST_CASE` that just runs many passing assertions, e.g.

```
TEST_CASE( "PERFORMANCE_TEST_CASE", "[.]" ) {
    for (size_t i = 0; i < 1000'000'000; ++i) {
        REQUIRE( i == i );
    }
}
```

the new JUnit reporter will finish in 5:47, using up ~7.7 MB of RAM.
The old JUnit reporter would finish in 0:30, due to bad_alloc
after using up 14.5 GB of RAM (the system has 16 GB total).

If the total number of assertions is lowered to 10M, the old
implementation uses up ~7.1 GB of RAM and finishes in 12 minutes.
The new implementation still needs only ~7.7 MB of RAM, and finishes
in 4 minutes.

There is a slight downside in that the output is slightly different;
the new implementation will include the `TEST_CASE` level section
in output, even if it does not have its own assertion. In other words,
given a `TEST_CASE` like this

```
TEST_CASE( "JsonWriter", "[JSON][JsonWriter]" ) {
    std::stringstream stream;
    SECTION( "Newly constructed JsonWriter does nothing" ) {
        Catch::JsonValueWriter writer{ stream };
        REQUIRE( stream.str() == "" );
    }

    SECTION( "Calling writeObject will create an empty pair of braces" ) {
        { auto writer = Catch::JsonValueWriter{ stream }.writeObject(); }
        REQUIRE( stream.str() == "{\n}" );
    }
}
```

the new implementation will output 3 `testcase` tags, 2 for the explicit
 `SECTION`s with tests, and 1 for the top level section.

However, this can be worked-around if required, and the performance
improvement is such that it is worth changing the current output,
even if it needs to be fixed in the future.

Closes #2897
  • Loading branch information
horenmar committed Aug 13, 2024
1 parent b15158c commit fe483c0
Show file tree
Hide file tree
Showing 3 changed files with 326 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/catch2/reporters/catch_reporter_junit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ namespace Catch {
xml( m_stream )
{
m_preferences.shouldRedirectStdOut = true;
m_preferences.shouldReportAllAssertions = true;
m_preferences.shouldReportAllAssertions = false;
m_shouldStoreSuccesfulAssertions = false;
}

Expand Down Expand Up @@ -198,7 +198,7 @@ namespace Catch {
if( !rootName.empty() )
name = rootName + '/' + name;

if( sectionNode.hasAnyAssertions()
if ( sectionNode.stats.assertions.total() > 0
|| !sectionNode.stdOut.empty()
|| !sectionNode.stdErr.empty() ) {
XmlWriter::ScopedElement e = xml.scopedElement( "testcase" );
Expand Down
Loading

0 comments on commit fe483c0

Please sign in to comment.