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
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 ~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 8898cc6 commit c29e198
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 c29e198

Please sign in to comment.