From fe483c056d9516d3550519c29fbf14a4d84d06d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Tue, 13 Aug 2024 18:57:42 +0200 Subject: [PATCH] Improve performance of JUnit reporter when handling passing assertions 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 --- src/catch2/reporters/catch_reporter_junit.cpp | 4 +- .../SelfTest/Baselines/junit.sw.approved.txt | 162 ++++++++++++++++++ .../Baselines/junit.sw.multi.approved.txt | 162 ++++++++++++++++++ 3 files changed, 326 insertions(+), 2 deletions(-) diff --git a/src/catch2/reporters/catch_reporter_junit.cpp b/src/catch2/reporters/catch_reporter_junit.cpp index 4589365c..27bdfe28 100644 --- a/src/catch2/reporters/catch_reporter_junit.cpp +++ b/src/catch2/reporters/catch_reporter_junit.cpp @@ -88,7 +88,7 @@ namespace Catch { xml( m_stream ) { m_preferences.shouldRedirectStdOut = true; - m_preferences.shouldReportAllAssertions = true; + m_preferences.shouldReportAllAssertions = false; m_shouldStoreSuccesfulAssertions = false; } @@ -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" ); diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index cfe9de63..710169c4 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -12,6 +12,7 @@ + @@ -30,10 +31,12 @@ Nor would this + + @@ -56,6 +59,7 @@ failure to init at Generators.tests.cpp: + @@ -89,6 +93,7 @@ at Misc.tests.cpp: + @@ -147,6 +152,7 @@ at Condition.tests.cpp: + @@ -323,6 +329,7 @@ with expansion: at Class.tests.cpp: + @@ -341,6 +348,7 @@ to infinity and beyond at Misc.tests.cpp: + @@ -359,6 +367,7 @@ at Tricky.tests.cpp: + @@ -376,6 +385,7 @@ at Exception.tests.cpp: + @@ -383,30 +393,38 @@ at Exception.tests.cpp: + + + + + + + + @@ -424,8 +442,10 @@ at Exception.tests.cpp: + + @@ -445,6 +465,7 @@ with expansion: at Matchers.tests.cpp: + @@ -667,6 +688,7 @@ at Matchers.tests.cpp: + @@ -712,6 +734,7 @@ at Message.tests.cpp: + @@ -719,6 +742,7 @@ at Message.tests.cpp: + @@ -727,46 +751,64 @@ at Message.tests.cpp: + + + + + + + + + + + + + + + + + + @@ -876,6 +918,7 @@ at Condition.tests.cpp: + @@ -885,6 +928,7 @@ at Condition.tests.cpp: + @@ -920,6 +964,9 @@ with expansion: at Matchers.tests.cpp: + + + @@ -927,6 +974,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -934,6 +984,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -941,6 +994,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -1108,6 +1164,7 @@ at Condition.tests.cpp: + @@ -1125,9 +1182,11 @@ at Message.tests.cpp: + + @@ -1135,44 +1194,58 @@ at Message.tests.cpp: + + + + + + + + + + + + + + @@ -1252,14 +1325,27 @@ at Matchers.tests.cpp: + + + + + + + + + + + + + @@ -1271,6 +1357,8 @@ A string sent to stderr via clog + + Message from section one @@ -1294,15 +1382,18 @@ with expansion: at Matchers.tests.cpp: + + + @@ -1311,13 +1402,16 @@ at Matchers.tests.cpp: + + + @@ -1342,6 +1436,7 @@ with expansion: at Misc.tests.cpp: + @@ -1441,6 +1536,7 @@ at Misc.tests.cpp: + @@ -1464,13 +1560,17 @@ at Exception.tests.cpp: + + + + @@ -1479,76 +1579,107 @@ FAILED: at Exception.tests.cpp: + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + FAILED: @@ -1567,11 +1698,13 @@ with expansion: at Matchers.tests.cpp: + + FAILED: @@ -1703,10 +1836,12 @@ unexpected exception at Exception.tests.cpp: + + @@ -1724,6 +1859,7 @@ at Skip.tests.cpp: + @@ -1747,6 +1883,7 @@ with expansion: at Misc.tests.cpp: + @@ -1771,6 +1908,8 @@ at Skip.tests.cpp: + + @@ -1817,6 +1956,8 @@ FAILED: at Skip.tests.cpp: + + @@ -1832,7 +1973,10 @@ previous unscoped info SHOULD not be seen at Message.tests.cpp: + + + FAILED: @@ -1910,12 +2054,15 @@ at Misc.tests.cpp: + + + FAILED: @@ -1925,10 +2072,15 @@ with expansion: at Misc.tests.cpp: + + + + + SKIPPED @@ -1957,6 +2109,7 @@ at Message.tests.cpp: + @@ -1979,14 +2132,17 @@ this SHOULD be seen only ONCE at Message.tests.cpp: + + + @@ -1994,6 +2150,8 @@ at Message.tests.cpp: + + @@ -2049,11 +2207,13 @@ at Message.tests.cpp: + + @@ -2099,6 +2259,7 @@ at Exception.tests.cpp: + @@ -2120,6 +2281,7 @@ at Exception.tests.cpp: + diff --git a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt index 42db614f..80817478 100644 --- a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt @@ -11,6 +11,7 @@ + @@ -29,10 +30,12 @@ Nor would this + + @@ -55,6 +58,7 @@ failure to init at Generators.tests.cpp: + @@ -88,6 +92,7 @@ at Misc.tests.cpp: + @@ -146,6 +151,7 @@ at Condition.tests.cpp: + @@ -322,6 +328,7 @@ with expansion: at Class.tests.cpp: + @@ -340,6 +347,7 @@ to infinity and beyond at Misc.tests.cpp: + @@ -358,6 +366,7 @@ at Tricky.tests.cpp: + @@ -375,6 +384,7 @@ at Exception.tests.cpp: + @@ -382,30 +392,38 @@ at Exception.tests.cpp: + + + + + + + + @@ -423,8 +441,10 @@ at Exception.tests.cpp: + + @@ -444,6 +464,7 @@ with expansion: at Matchers.tests.cpp: + @@ -666,6 +687,7 @@ at Matchers.tests.cpp: + @@ -711,6 +733,7 @@ at Message.tests.cpp: + @@ -718,6 +741,7 @@ at Message.tests.cpp: + @@ -726,46 +750,64 @@ at Message.tests.cpp: + + + + + + + + + + + + + + + + + + @@ -875,6 +917,7 @@ at Condition.tests.cpp: + @@ -884,6 +927,7 @@ at Condition.tests.cpp: + @@ -919,6 +963,9 @@ with expansion: at Matchers.tests.cpp: + + + @@ -926,6 +973,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -933,6 +983,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -940,6 +993,9 @@ FAILED: at Condition.tests.cpp: + + + @@ -1107,6 +1163,7 @@ at Condition.tests.cpp: + @@ -1124,9 +1181,11 @@ at Message.tests.cpp: + + @@ -1134,44 +1193,58 @@ at Message.tests.cpp: + + + + + + + + + + + + + + @@ -1251,14 +1324,27 @@ at Matchers.tests.cpp: + + + + + + + + + + + + + @@ -1270,6 +1356,8 @@ A string sent to stderr via clog + + Message from section one @@ -1293,15 +1381,18 @@ with expansion: at Matchers.tests.cpp: + + + @@ -1310,13 +1401,16 @@ at Matchers.tests.cpp: + + + @@ -1341,6 +1435,7 @@ with expansion: at Misc.tests.cpp: + @@ -1440,6 +1535,7 @@ at Misc.tests.cpp: + @@ -1463,13 +1559,17 @@ at Exception.tests.cpp: + + + + @@ -1478,76 +1578,107 @@ FAILED: at Exception.tests.cpp: + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + FAILED: @@ -1566,11 +1697,13 @@ with expansion: at Matchers.tests.cpp: + + FAILED: @@ -1702,10 +1835,12 @@ unexpected exception at Exception.tests.cpp: + + @@ -1723,6 +1858,7 @@ at Skip.tests.cpp: + @@ -1746,6 +1882,7 @@ with expansion: at Misc.tests.cpp: + @@ -1770,6 +1907,8 @@ at Skip.tests.cpp: + + @@ -1816,6 +1955,8 @@ FAILED: at Skip.tests.cpp: + + @@ -1831,7 +1972,10 @@ previous unscoped info SHOULD not be seen at Message.tests.cpp: + + + FAILED: @@ -1909,12 +2053,15 @@ at Misc.tests.cpp: + + + FAILED: @@ -1924,10 +2071,15 @@ with expansion: at Misc.tests.cpp: + + + + + SKIPPED @@ -1956,6 +2108,7 @@ at Message.tests.cpp: + @@ -1978,14 +2131,17 @@ this SHOULD be seen only ONCE at Message.tests.cpp: + + + @@ -1993,6 +2149,8 @@ at Message.tests.cpp: + + @@ -2048,11 +2206,13 @@ at Message.tests.cpp: + + @@ -2098,6 +2258,7 @@ at Exception.tests.cpp: + @@ -2119,6 +2280,7 @@ at Exception.tests.cpp: +