From fc5552d27b57628f8faf6064ca6b305e2d3bf458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Thu, 17 Feb 2022 20:44:27 +0100 Subject: [PATCH] Push down handling of default reporter to Config's constructor This simplifies the handling of default reporter in console parsing, at the cost of making `Config`'s constructor responsible for more things. --- src/catch2/catch_config.cpp | 15 ++++++++++ src/catch2/catch_config.hpp | 10 +------ src/catch2/internal/catch_commandline.cpp | 11 ++------ .../Baselines/compact.sw.approved.txt | 8 ++++-- .../Baselines/compact.sw.multi.approved.txt | 8 ++++-- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 20 +++++++++---- .../Baselines/console.sw.multi.approved.txt | 20 +++++++++---- .../SelfTest/Baselines/junit.sw.approved.txt | 2 +- .../Baselines/junit.sw.multi.approved.txt | 2 +- tests/SelfTest/Baselines/tap.sw.approved.txt | 8 ++++-- .../Baselines/tap.sw.multi.approved.txt | 8 ++++-- tests/SelfTest/Baselines/xml.sw.approved.txt | 28 +++++++++++++++---- .../Baselines/xml.sw.multi.approved.txt | 28 +++++++++++++++---- .../IntrospectiveTests/CmdLine.tests.cpp | 16 ++++++++++- 15 files changed, 132 insertions(+), 54 deletions(-) diff --git a/src/catch2/catch_config.cpp b/src/catch2/catch_config.cpp index c507b1b6..e6a5b2ed 100644 --- a/src/catch2/catch_config.cpp +++ b/src/catch2/catch_config.cpp @@ -6,6 +6,7 @@ // SPDX-License-Identifier: BSL-1.0 #include +#include #include #include #include @@ -57,6 +58,7 @@ namespace Catch { elem = trim(elem); } + TestSpecParser parser(ITagAliasRegistry::get()); if (!m_data.testsOrTags.empty()) { m_hasTestFilters = true; @@ -66,6 +68,19 @@ namespace Catch { } m_testSpec = parser.testSpec(); + + // Insert the default reporter if user hasn't asked for a specfic one + if ( m_data.reporterSpecifications.empty() ) { + m_data.reporterSpecifications.push_back( { +#if defined( CATCH_CONFIG_DEFAULT_REPORTER ) + CATCH_CONFIG_DEFAULT_REPORTER, +#else + "console", +#endif + {} + } ); + } + m_reporterStreams.reserve( m_data.reporterSpecifications.size() ); for ( auto const& reporterAndFile : m_data.reporterSpecifications ) { if ( reporterAndFile.outputFileName.none() ) { diff --git a/src/catch2/catch_config.hpp b/src/catch2/catch_config.hpp index 1aaa9828..a8cdc48a 100644 --- a/src/catch2/catch_config.hpp +++ b/src/catch2/catch_config.hpp @@ -71,15 +71,7 @@ namespace Catch { std::string defaultOutputFilename; std::string name; std::string processName; -#ifndef CATCH_CONFIG_DEFAULT_REPORTER -#define CATCH_CONFIG_DEFAULT_REPORTER "console" -#endif - std::vector reporterSpecifications = { - {CATCH_CONFIG_DEFAULT_REPORTER, {}} - }; - // Internal: used as parser state - bool _nonDefaultReporterSpecifications = false; -#undef CATCH_CONFIG_DEFAULT_REPORTER + std::vector reporterSpecifications; std::vector testsOrTags; std::vector sectionsToRun; diff --git a/src/catch2/internal/catch_commandline.cpp b/src/catch2/internal/catch_commandline.cpp index dd7b9f6c..ca1af626 100644 --- a/src/catch2/internal/catch_commandline.cpp +++ b/src/catch2/internal/catch_commandline.cpp @@ -139,15 +139,6 @@ namespace Catch { return ParserResult::runtimeError( "Received empty reporter spec." ); } - IReporterRegistry::FactoryMap const& factories = getRegistryHub().getReporterRegistry().getFactories(); - - // clear the default reporter - if (!config._nonDefaultReporterSpecifications) { - config.reporterSpecifications.clear(); - config._nonDefaultReporterSpecifications = true; - } - - // Exactly one of the reporters may be specified without an output // file, in which case it defaults to the output specified by "-o" // (or standard output). @@ -176,6 +167,8 @@ namespace Catch { fileNameSeparatorPos + separatorSize, reporterSpec.size() ); } + IReporterRegistry::FactoryMap const& factories = + getRegistryHub().getReporterRegistry().getFactories(); auto result = factories.find( reporterName ); if( result == factories.end() ) diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 20098061..87e318dd 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -1267,10 +1267,12 @@ CmdLine.tests.cpp:: passed: config.processName == "test" for: "test CmdLine.tests.cpp:: passed: config.shouldDebugBreak == false for: false == false CmdLine.tests.cpp:: passed: config.abortAfter == -1 for: -1 == -1 CmdLine.tests.cpp:: passed: config.noThrow == false for: false == false -CmdLine.tests.cpp:: passed: config.reporterSpecifications == std::vector{ {"console", {}} } for: { { console, } } -== -{ { console, } } +CmdLine.tests.cpp:: passed: config.reporterSpecifications.empty() for: true CmdLine.tests.cpp:: passed: !(cfg.hasTestFilters()) for: !false +CmdLine.tests.cpp:: passed: cfg.getReportersAndOutputFiles().size() == 1 for: 1 == 1 +CmdLine.tests.cpp:: passed: cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } for: { console, } +== +{ console, } CmdLine.tests.cpp:: passed: result for: {?} CmdLine.tests.cpp:: passed: cfg.hasTestFilters() for: true CmdLine.tests.cpp:: passed: cfg.testSpec().matches(*fakeTestCase("notIncluded")) == false for: false == false diff --git a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt index 993a7b7c..6c7c1baf 100644 --- a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt @@ -1265,10 +1265,12 @@ CmdLine.tests.cpp:: passed: config.processName == "test" for: "test CmdLine.tests.cpp:: passed: config.shouldDebugBreak == false for: false == false CmdLine.tests.cpp:: passed: config.abortAfter == -1 for: -1 == -1 CmdLine.tests.cpp:: passed: config.noThrow == false for: false == false -CmdLine.tests.cpp:: passed: config.reporterSpecifications == std::vector{ {"console", {}} } for: { { console, } } -== -{ { console, } } +CmdLine.tests.cpp:: passed: config.reporterSpecifications.empty() for: true CmdLine.tests.cpp:: passed: !(cfg.hasTestFilters()) for: !false +CmdLine.tests.cpp:: passed: cfg.getReportersAndOutputFiles().size() == 1 for: 1 == 1 +CmdLine.tests.cpp:: passed: cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } for: { console, } +== +{ console, } CmdLine.tests.cpp:: passed: result for: {?} CmdLine.tests.cpp:: passed: cfg.hasTestFilters() for: true CmdLine.tests.cpp:: passed: cfg.testSpec().matches(*fakeTestCase("notIncluded")) == false for: false == false diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index f6f6b292..195e0ccb 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1396,5 +1396,5 @@ due to unexpected exception with message: =============================================================================== test cases: 381 | 305 passed | 69 failed | 7 failed as expected -assertions: 2209 | 2054 passed | 128 failed | 27 failed as expected +assertions: 2211 | 2056 passed | 128 failed | 27 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 8d1a25b4..d2d8a04f 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -9260,17 +9260,27 @@ with expansion: false == false CmdLine.tests.cpp:: PASSED: - CHECK( config.reporterSpecifications == std::vector{ {"console", {}} } ) + CHECK( config.reporterSpecifications.empty() ) with expansion: - { { console, } } - == - { { console, } } + true CmdLine.tests.cpp:: PASSED: CHECK_FALSE( cfg.hasTestFilters() ) with expansion: !false +CmdLine.tests.cpp:: PASSED: + CHECK( cfg.getReportersAndOutputFiles().size() == 1 ) +with expansion: + 1 == 1 + +CmdLine.tests.cpp:: PASSED: + CHECK( cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } ) +with expansion: + { console, } + == + { console, } + ------------------------------------------------------------------------------- Process can be configured on command line test lists @@ -17755,5 +17765,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 381 | 291 passed | 83 failed | 7 failed as expected -assertions: 2224 | 2054 passed | 143 failed | 27 failed as expected +assertions: 2226 | 2056 passed | 143 failed | 27 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.multi.approved.txt b/tests/SelfTest/Baselines/console.sw.multi.approved.txt index 920a1c7a..045f7e0d 100644 --- a/tests/SelfTest/Baselines/console.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.multi.approved.txt @@ -9258,17 +9258,27 @@ with expansion: false == false CmdLine.tests.cpp:: PASSED: - CHECK( config.reporterSpecifications == std::vector{ {"console", {}} } ) + CHECK( config.reporterSpecifications.empty() ) with expansion: - { { console, } } - == - { { console, } } + true CmdLine.tests.cpp:: PASSED: CHECK_FALSE( cfg.hasTestFilters() ) with expansion: !false +CmdLine.tests.cpp:: PASSED: + CHECK( cfg.getReportersAndOutputFiles().size() == 1 ) +with expansion: + 1 == 1 + +CmdLine.tests.cpp:: PASSED: + CHECK( cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } ) +with expansion: + { console, } + == + { console, } + ------------------------------------------------------------------------------- Process can be configured on command line test lists @@ -17747,5 +17757,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 381 | 291 passed | 83 failed | 7 failed as expected -assertions: 2224 | 2054 passed | 143 failed | 27 failed as expected +assertions: 2226 | 2056 passed | 143 failed | 27 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index c020fa4b..b259f75d 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + diff --git a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt index 91398339..26b2f904 100644 --- a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt @@ -1,6 +1,6 @@ - + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index bd0e9881..5c36015e 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -2453,10 +2453,14 @@ ok {test-number} - config.abortAfter == -1 for: -1 == -1 # Process can be configured on command line ok {test-number} - config.noThrow == false for: false == false # Process can be configured on command line -ok {test-number} - config.reporterSpecifications == std::vector{ {"console", {}} } for: { { console, } } == { { console, } } +ok {test-number} - config.reporterSpecifications.empty() for: true # Process can be configured on command line ok {test-number} - !(cfg.hasTestFilters()) for: !false # Process can be configured on command line +ok {test-number} - cfg.getReportersAndOutputFiles().size() == 1 for: 1 == 1 +# Process can be configured on command line +ok {test-number} - cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } for: { console, } == { console, } +# Process can be configured on command line ok {test-number} - result for: {?} # Process can be configured on command line ok {test-number} - cfg.hasTestFilters() for: true @@ -4450,5 +4454,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2224 +1..2226 diff --git a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt index 59e8d1a8..9eb4aaca 100644 --- a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt @@ -2451,10 +2451,14 @@ ok {test-number} - config.abortAfter == -1 for: -1 == -1 # Process can be configured on command line ok {test-number} - config.noThrow == false for: false == false # Process can be configured on command line -ok {test-number} - config.reporterSpecifications == std::vector{ {"console", {}} } for: { { console, } } == { { console, } } +ok {test-number} - config.reporterSpecifications.empty() for: true # Process can be configured on command line ok {test-number} - !(cfg.hasTestFilters()) for: !false # Process can be configured on command line +ok {test-number} - cfg.getReportersAndOutputFiles().size() == 1 for: 1 == 1 +# Process can be configured on command line +ok {test-number} - cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } for: { console, } == { console, } +# Process can be configured on command line ok {test-number} - result for: {?} # Process can be configured on command line ok {test-number} - cfg.hasTestFilters() for: true @@ -4442,5 +4446,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2224 +1..2226 diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index 49b11484..91dc7e3c 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -11289,12 +11289,10 @@ Nor would this - config.reporterSpecifications == std::vector<Catch::ConfigData::ReporterAndFile>{ {"console", {}} } + config.reporterSpecifications.empty() - { { console, <default-output> } } -== -{ { console, <default-output> } } + true @@ -11305,7 +11303,25 @@ Nor would this !false - + + + cfg.getReportersAndOutputFiles().size() == 1 + + + 1 == 1 + + + + + cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } + + + { console, <default-output> } +== +{ console, <default-output> } + + +
@@ -20863,6 +20879,6 @@ loose text artifact
- + diff --git a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt index f73fe121..e7dddbb0 100644 --- a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt @@ -11289,12 +11289,10 @@ Nor would this - config.reporterSpecifications == std::vector<Catch::ConfigData::ReporterAndFile>{ {"console", {}} } + config.reporterSpecifications.empty() - { { console, <default-output> } } -== -{ { console, <default-output> } } + true @@ -11305,7 +11303,25 @@ Nor would this !false - + + + cfg.getReportersAndOutputFiles().size() == 1 + + + 1 == 1 + + + + + cfg.getReportersAndOutputFiles()[0] == Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } + + + { console, <default-output> } +== +{ console, <default-output> } + + +
@@ -20862,6 +20878,6 @@ There is no extra whitespace here
- + diff --git a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index b4ec826b..97bccf49 100644 --- a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -348,10 +349,23 @@ TEST_CASE( "Process can be configured on command line", "[config][command-line]" CHECK(config.shouldDebugBreak == false); CHECK(config.abortAfter == -1); CHECK(config.noThrow == false); - CHECK(config.reporterSpecifications == std::vector{ {"console", {}} }); + CHECK( config.reporterSpecifications.empty() ); Catch::Config cfg(config); CHECK_FALSE(cfg.hasTestFilters()); + + // The Config is responsible for mixing in the default reporter + auto expectedReporter = +#if defined( CATCH_CONFIG_DEFAULT_REPORTER ) + CATCH_CONFIG_DEFAULT_REPORTER +#else + "console" +#endif + ; + + CHECK( cfg.getReportersAndOutputFiles().size() == 1 ); + CHECK( cfg.getReportersAndOutputFiles()[0] == + Catch::ConfigData::ReporterAndFile{ expectedReporter, {} } ); } SECTION("test lists") {