From c29e198eab0ccdb190495397854b937677385e2e 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 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 --- 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 4589365ce8..27bdfe28f7 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 cfe9de6346..710169c44f 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 42db614f69..80817478f1 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: +