Introduce type to handle reporter specs with defaults filled-in

src/catch2/catch_config.cpp

@@ -18,6 +18,14 @@
 
 namespace Catch {
 
+    bool operator==( ProcessedReporterSpec const& lhs,
+                     ProcessedReporterSpec const& rhs ) {
+        return lhs.name == rhs.name &&
+               lhs.outputFilename == rhs.outputFilename &&
+               lhs.colourMode == rhs.colourMode &&
+               lhs.customOptions == rhs.customOptions;
+    }
+
     Config::Config( ConfigData const& data ):
         m_data( data ) {
         // We need to trim filter specs to avoid trouble with superfluous
@@ -80,25 +88,23 @@ namespace Catch {
         // We now fixup the reporter specs to handle default output spec,
         // default colour spec, etc
         bool defaultOutputUsed = false;
-        for ( auto& reporterSpec : m_data.reporterSpecifications ) {
+        for ( auto const& reporterSpec : m_data.reporterSpecifications ) {
+            // We do the default-output check separately, while always
+            // using the default output below to make the code simpler
+            // and avoid superfluous copies.
             if ( reporterSpec.outputFile().none() ) {
                 CATCH_ENFORCE( !defaultOutputUsed,
                                "Internal error: cannot use default output for "
                                "multiple reporters" );
                 defaultOutputUsed = true;
-
-                reporterSpec = ReporterSpec{ reporterSpec.name(),
-                                             data.defaultOutputFilename,
-                                             reporterSpec.colourMode(),
-                                             reporterSpec.customOptions() };
             }
 
-            if ( reporterSpec.colourMode().none() ) {
+            m_processedReporterSpecs.push_back( ProcessedReporterSpec{
-                reporterSpec = ReporterSpec{ reporterSpec.name(),
+                reporterSpec.name(),
-                                             reporterSpec.outputFile(),
+                reporterSpec.outputFile() ? *reporterSpec.outputFile()
-                                             data.defaultColourMode,
+                                          : data.defaultOutputFilename,
-                                             reporterSpec.customOptions() };
+                reporterSpec.colourMode().valueOr( data.defaultColourMode ),
-            }
+                reporterSpec.customOptions() } );
         }
     }
 
@@ -116,6 +122,11 @@ namespace Catch {
         return m_data.reporterSpecifications;
     }
 
+    std::vector<ProcessedReporterSpec> const&
+    Config::getProcessedReporterSpecs() const {
+        return m_processedReporterSpecs;
+    }
+
     TestSpec const& Config::testSpec() const { return m_testSpec; }
     bool Config::hasTestFilters() const { return m_hasTestFilters; }
 

src/catch2/catch_config.hpp

@@ -24,6 +24,24 @@ namespace Catch {
 
     class IStream;
 
+    /**
+     * `ReporterSpec` but with the defaults filled in.
+     *
+     * Like `ReporterSpec`, the semantics are unchecked.
+     */
+    struct ProcessedReporterSpec {
+        std::string name;
+        std::string outputFilename;
+        ColourMode colourMode;
+        std::map<std::string, std::string> customOptions;
+        friend bool operator==( ProcessedReporterSpec const& lhs,
+                                ProcessedReporterSpec const& rhs );
+        friend bool operator!=( ProcessedReporterSpec const& lhs,
+                                ProcessedReporterSpec const& rhs ) {
+            return !( lhs == rhs );
+        }
+    };
+
     struct ConfigData {
 
         bool listTests = false;
@@ -81,6 +99,8 @@ namespace Catch {
         bool listReporters() const;
 
         std::vector<ReporterSpec> const& getReporterSpecs() const;
+        std::vector<ProcessedReporterSpec> const&
+        getProcessedReporterSpecs() const;
 
         std::vector<std::string> const& getTestsOrTags() const override;
         std::vector<std::string> const& getSectionsToRun() const override;
@@ -116,6 +136,7 @@ namespace Catch {
 
     private:
         ConfigData m_data;
+        std::vector<ProcessedReporterSpec> m_processedReporterSpecs;
         TestSpec m_testSpec;
         bool m_hasTestFilters = false;
     };

src/catch2/catch_session.cpp

@@ -43,15 +43,14 @@ namespace Catch {
 
         IEventListenerPtr prepareReporters(Config const* config) {
             if (Catch::getRegistryHub().getReporterRegistry().getListeners().empty()
-                    && config->getReporterSpecs().size() == 1) {
+                    && config->getProcessedReporterSpecs().size() == 1) {
-                auto const& spec = config->getReporterSpecs()[0];
+                auto const& spec = config->getProcessedReporterSpecs()[0];
                 return createReporter(
-                    spec.name(),
+                    spec.name,
-                    ReporterConfig(
+                    ReporterConfig( config,
-                        config,
+                                    makeStream( spec.outputFilename ),
-                        makeStream(*spec.outputFile()),
+                                    spec.colourMode,
-                        *spec.colourMode(),
+                                    spec.customOptions ) );
-                        spec.customOptions() ) );
             }
 
             auto multi = Detail::make_unique<MultiReporter>(config);
@@ -62,13 +61,13 @@ namespace Catch {
             }
 
             std::size_t reporterIdx = 0;
-            for (auto const& reporterSpec : config->getReporterSpecs()) {
+            for ( auto const& reporterSpec : config->getProcessedReporterSpecs() ) {
                 multi->addReporter( createReporter(
-                    reporterSpec.name(),
+                    reporterSpec.name,
                     ReporterConfig( config,
-                                    makeStream( *reporterSpec.outputFile() ),
+                                    makeStream( reporterSpec.outputFilename ),
-                                    *reporterSpec.colourMode(),
+                                    reporterSpec.colourMode,
-                                    reporterSpec.customOptions() ) ) );
+                                    reporterSpec.customOptions ) ) );
                 reporterIdx++;
             }
 

tests/SelfTest/Baselines/compact.sw.approved.txt

@@ -1317,7 +1317,9 @@ CmdLine.tests.cpp:<line number>: passed: config.noThrow == false for: false == f
 CmdLine.tests.cpp:<line number>: passed: config.reporterSpecifications.empty() for: true
 CmdLine.tests.cpp:<line number>: passed: !(cfg.hasTestFilters()) for: !false
 CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs().size() == 1 for: 1 == 1
-CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
+CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } for: {?} == {?}
+CmdLine.tests.cpp:<line number>: passed: cfg.getProcessedReporterSpecs().size() == 1 for: 1 == 1
+CmdLine.tests.cpp:<line number>: passed: cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
 CmdLine.tests.cpp:<line number>: passed: result for: {?}
 CmdLine.tests.cpp:<line number>: passed: cfg.hasTestFilters() for: true
 CmdLine.tests.cpp:<line number>: passed: cfg.testSpec().matches(*fakeTestCase("notIncluded")) == false for: false == false

tests/SelfTest/Baselines/compact.sw.multi.approved.txt

@@ -1315,7 +1315,9 @@ CmdLine.tests.cpp:<line number>: passed: config.noThrow == false for: false == f
 CmdLine.tests.cpp:<line number>: passed: config.reporterSpecifications.empty() for: true
 CmdLine.tests.cpp:<line number>: passed: !(cfg.hasTestFilters()) for: !false
 CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs().size() == 1 for: 1 == 1
-CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
+CmdLine.tests.cpp:<line number>: passed: cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } for: {?} == {?}
+CmdLine.tests.cpp:<line number>: passed: cfg.getProcessedReporterSpecs().size() == 1 for: 1 == 1
+CmdLine.tests.cpp:<line number>: passed: cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
 CmdLine.tests.cpp:<line number>: passed: result for: {?}
 CmdLine.tests.cpp:<line number>: passed: cfg.hasTestFilters() for: true
 CmdLine.tests.cpp:<line number>: passed: cfg.testSpec().matches(*fakeTestCase("notIncluded")) == false for: false == false

tests/SelfTest/Baselines/console.std.approved.txt

@@ -1396,5 +1396,5 @@ due to unexpected exception with message:
 
 ===============================================================================
 test cases:  390 |  314 passed |  69 failed |  7 failed as expected
-assertions: 2223 | 2068 passed | 128 failed | 27 failed as expected
+assertions: 2225 | 2070 passed | 128 failed | 27 failed as expected
 

tests/SelfTest/Baselines/console.sw.approved.txt

@@ -9438,7 +9438,17 @@ with expansion:
   1 == 1
 
 CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK( cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } )
+  CHECK( cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } )
+with expansion:
+  {?} == {?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cfg.getProcessedReporterSpecs().size() == 1 )
+with expansion:
+  1 == 1
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } )
 with expansion:
   {?} == {?}
 
@@ -17917,5 +17927,5 @@ Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
 test cases:  390 |  300 passed |  83 failed |  7 failed as expected
-assertions: 2238 | 2068 passed | 143 failed | 27 failed as expected
+assertions: 2240 | 2070 passed | 143 failed | 27 failed as expected
 

tests/SelfTest/Baselines/console.sw.multi.approved.txt

@@ -9436,7 +9436,17 @@ with expansion:
   1 == 1
 
 CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK( cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } )
+  CHECK( cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } )
+with expansion:
+  {?} == {?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cfg.getProcessedReporterSpecs().size() == 1 )
+with expansion:
+  1 == 1
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } )
 with expansion:
   {?} == {?}
 
@@ -17909,5 +17919,5 @@ Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
 test cases:  390 |  300 passed |  83 failed |  7 failed as expected
-assertions: 2238 | 2068 passed | 143 failed | 27 failed as expected
+assertions: 2240 | 2070 passed | 143 failed | 27 failed as expected
 

tests/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="126" tests="2238" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="126" tests="2240" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="~[!nonportable]~[!benchmark]~[approvals] *"/>

tests/SelfTest/Baselines/junit.sw.multi.approved.txt

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="<exe-name>" errors="17" failures="126" tests="2238" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="126" tests="2240" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="~[!nonportable]~[!benchmark]~[approvals] *"/>

tests/SelfTest/Baselines/tap.sw.approved.txt

@@ -2481,7 +2481,11 @@ ok {test-number} - !(cfg.hasTestFilters()) for: !false
 # Process can be configured on command line
 ok {test-number} - cfg.getReporterSpecs().size() == 1 for: 1 == 1
 # Process can be configured on command line
-ok {test-number} - cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
+ok {test-number} - cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } for: {?} == {?}
+# Process can be configured on command line
+ok {test-number} - cfg.getProcessedReporterSpecs().size() == 1 for: 1 == 1
+# Process can be configured on command line
+ok {test-number} - cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
 # Process can be configured on command line
 ok {test-number} - result for: {?}
 # Process can be configured on command line
@@ -4478,5 +4482,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2238
+1..2240
 

tests/SelfTest/Baselines/tap.sw.multi.approved.txt

@@ -2479,7 +2479,11 @@ ok {test-number} - !(cfg.hasTestFilters()) for: !false
 # Process can be configured on command line
 ok {test-number} - cfg.getReporterSpecs().size() == 1 for: 1 == 1
 # Process can be configured on command line
-ok {test-number} - cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
+ok {test-number} - cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} } for: {?} == {?}
+# Process can be configured on command line
+ok {test-number} - cfg.getProcessedReporterSpecs().size() == 1 for: 1 == 1
+# Process can be configured on command line
+ok {test-number} - cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} } for: {?} == {?}
 # Process can be configured on command line
 ok {test-number} - result for: {?}
 # Process can be configured on command line
@@ -4470,5 +4474,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0
 ok {test-number} -
 # xmlentitycheck
 ok {test-number} -
-1..2238
+1..2240
 

tests/SelfTest/Baselines/xml.sw.approved.txt

@@ -11476,13 +11476,29 @@ C
       </Expression>
       <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
         <Original>
-          cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} }
+          cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} }
         </Original>
         <Expanded>
           {?} == {?}
         </Expanded>
       </Expression>
-      <OverallResults successes="9" failures="0" expectedFailures="0"/>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
+        <Original>
+          cfg.getProcessedReporterSpecs().size() == 1
+        </Original>
+        <Expanded>
+          1 == 1
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
+        <Original>
+          cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} }
+        </Original>
+        <Expanded>
+          {?} == {?}
+        </Expanded>
+      </Expression>
+      <OverallResults successes="11" failures="0" expectedFailures="0"/>
     </Section>
     <Section name="test lists" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
       <Section name="Specify one test case using" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
@@ -21034,6 +21050,6 @@ loose text artifact
     </Section>
     <OverallResult success="true"/>
   </TestCase>
-  <OverallResults successes="2068" failures="143" expectedFailures="27"/>
+  <OverallResults successes="2070" failures="143" expectedFailures="27"/>
   <OverallResultsCases successes="300" failures="83" expectedFailures="7"/>
 </Catch2TestRun>

tests/SelfTest/Baselines/xml.sw.multi.approved.txt

@@ -11476,13 +11476,29 @@ C
       </Expression>
       <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
         <Original>
-          cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} }
+          cfg.getReporterSpecs()[0] == Catch::ReporterSpec{ expectedReporter, {}, {}, {} }
         </Original>
         <Expanded>
           {?} == {?}
         </Expanded>
       </Expression>
-      <OverallResults successes="9" failures="0" expectedFailures="0"/>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
+        <Original>
+          cfg.getProcessedReporterSpecs().size() == 1
+        </Original>
+        <Expanded>
+          1 == 1
+        </Expanded>
+      </Expression>
+      <Expression success="true" type="CHECK" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
+        <Original>
+          cfg.getProcessedReporterSpecs()[0] == Catch::ProcessedReporterSpec{ expectedReporter, std::string{}, Catch::ColourMode::PlatformDefault, {} }
+        </Original>
+        <Expanded>
+          {?} == {?}
+        </Expanded>
+      </Expression>
+      <OverallResults successes="11" failures="0" expectedFailures="0"/>
     </Section>
     <Section name="test lists" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
       <Section name="Specify one test case using" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
@@ -21033,6 +21049,6 @@ There is no extra whitespace here
     </Section>
     <OverallResult success="true"/>
   </TestCase>
-  <OverallResults successes="2068" failures="143" expectedFailures="27"/>
+  <OverallResults successes="2070" failures="143" expectedFailures="27"/>
   <OverallResultsCases successes="300" failures="83" expectedFailures="7"/>
 </Catch2TestRun>

tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp

@@ -363,10 +363,13 @@ TEST_CASE( "Process can be configured on command line", "[config][command-line]"
 
         CHECK( cfg.getReporterSpecs().size() == 1 );
         CHECK( cfg.getReporterSpecs()[0] ==
-               Catch::ReporterSpec{ expectedReporter,
+               Catch::ReporterSpec{ expectedReporter, {}, {}, {} } );
-                                    std::string{},
+        CHECK( cfg.getProcessedReporterSpecs().size() == 1 );
-                                    Catch::ColourMode::PlatformDefault,
+        CHECK( cfg.getProcessedReporterSpecs()[0] ==
-                                    {} } );
+               Catch::ProcessedReporterSpec{ expectedReporter,
+                                             std::string{},
+                                             Catch::ColourMode::PlatformDefault,
+                                             {} } );
     }
 
     SECTION("test lists") {