Store IStream instance owning ptrs in reporter instances

This ended up being a surprisingly large refactoring, motivated by removing a `const_cast` from `Config`'s handling of reporter streams, forced by previous commit.
2025-08-03 13:55:39 +02:00 · 2022-04-12 21:22:43 +02:00
parent 4f09f1120b
commit 79d1e82381
38 changed files with 124 additions and 109 deletions
--- a/src/catch2/catch_config.cpp
+++ b/src/catch2/catch_config.cpp
@@ -76,20 +76,28 @@ namespace Catch {
        }
 #endif

+
+        // We now fixup the reporter specs to handle default output spec,
+        // default colour spec, etc
        bool defaultOutputUsed = false;
-        m_reporterStreams.reserve( m_data.reporterSpecifications.size() );
-        for ( auto const& reporterSpec : m_data.reporterSpecifications ) {
+        for ( auto& reporterSpec : m_data.reporterSpecifications ) {
            if ( reporterSpec.outputFile().none() ) {
                CATCH_ENFORCE( !defaultOutputUsed,
                               "Internal error: cannot use default output for "
                               "multiple reporters" );
                defaultOutputUsed = true;

-                m_reporterStreams.push_back(
-                    makeStream( data.defaultOutputFilename ) );
-            } else {
-                m_reporterStreams.push_back(
-                    makeStream( *reporterSpec.outputFile() ) );
+                reporterSpec = ReporterSpec{ reporterSpec.name(),
+                                             data.defaultOutputFilename,
+                                             reporterSpec.colourMode(),
+                                             reporterSpec.customOptions() };
+            }
+
+            if ( reporterSpec.colourMode().none() ) {
+                reporterSpec = ReporterSpec{ reporterSpec.name(),
+                                             reporterSpec.outputFile(),
+                                             data.defaultColourMode,
+                                             reporterSpec.customOptions() };
            }
        }
    }
@@ -108,10 +116,6 @@ namespace Catch {
        return m_data.reporterSpecifications;
    }

-    IStream* Config::getReporterOutputStream(std::size_t reporterIdx) const {
-        return const_cast<IStream*>(m_reporterStreams.at(reporterIdx).get());
-    }
-
    TestSpec const& Config::testSpec() const { return m_testSpec; }
    bool Config::hasTestFilters() const { return m_hasTestFilters; }

--- a/src/catch2/catch_config.hpp
+++ b/src/catch2/catch_config.hpp
@@ -81,7 +81,6 @@ namespace Catch {
        bool listReporters() const;

        std::vector<ReporterSpec> const& getReporterSpecs() const;
-        IStream* getReporterOutputStream(std::size_t reporterIdx) const;

        std::vector<std::string> const& getTestsOrTags() const override;
        std::vector<std::string> const& getSectionsToRun() const override;
@@ -117,8 +116,6 @@ namespace Catch {

    private:
        ConfigData m_data;
-
-        std::vector<Detail::unique_ptr<IStream>> m_reporterStreams;
        TestSpec m_testSpec;
        bool m_hasTestFilters = false;
    };
--- a/src/catch2/catch_session.cpp
+++ b/src/catch2/catch_session.cpp
@@ -34,8 +34,8 @@ namespace Catch {
    namespace {
        const int MaxExitCode = 255;

-        IEventListenerPtr createReporter(std::string const& reporterName, ReporterConfig const& config) {
-            auto reporter = Catch::getRegistryHub().getReporterRegistry().create(reporterName, config);
+        IEventListenerPtr createReporter(std::string const& reporterName, ReporterConfig&& config) {
+            auto reporter = Catch::getRegistryHub().getReporterRegistry().create(reporterName, CATCH_MOVE(config));
            CATCH_ENFORCE(reporter, "No reporter registered with name: '" << reporterName << '\'');

            return reporter;
@@ -45,13 +45,12 @@ namespace Catch {
            if (Catch::getRegistryHub().getReporterRegistry().getListeners().empty()
                    && config->getReporterSpecs().size() == 1) {
                auto const& spec = config->getReporterSpecs()[0];
-                auto stream = config->getReporterOutputStream(0);
                return createReporter(
-                    config->getReporterSpecs()[0].name(),
+                    spec.name(),
                    ReporterConfig(
                        config,
-                        stream,
-                        spec.colourMode().valueOr( config->defaultColourMode() ),
+                        makeStream(*spec.outputFile()),
+                        *spec.colourMode(),
                        spec.customOptions() ) );
            }

@@ -64,13 +63,11 @@ namespace Catch {

            std::size_t reporterIdx = 0;
            for (auto const& reporterSpec : config->getReporterSpecs()) {
-                auto stream = config->getReporterOutputStream(reporterIdx);
                multi->addReporter( createReporter(
                    reporterSpec.name(),
                    ReporterConfig( config,
-                                    stream,
-                                    reporterSpec.colourMode().valueOr(
-                                        config->defaultColourMode() ),
+                                    makeStream( *reporterSpec.outputFile() ),
+                                    *reporterSpec.colourMode(),
                                    reporterSpec.customOptions() ) ) );
                reporterIdx++;
            }
--- a/src/catch2/interfaces/catch_interfaces_reporter.cpp
+++ b/src/catch2/interfaces/catch_interfaces_reporter.cpp
@@ -17,21 +17,25 @@
 #include <catch2/internal/catch_move_and_forward.hpp>

 #include <algorithm>
+#include <cassert>
 #include <iomanip>

 namespace Catch {

    ReporterConfig::ReporterConfig(
        IConfig const* _fullConfig,
-        IStream* _stream,
+        Detail::unique_ptr<IStream> _stream,
        ColourMode colourMode,
        std::map<std::string, std::string> customOptions ):
-        m_stream( _stream ),
+        m_stream( CATCH_MOVE(_stream) ),
        m_fullConfig( _fullConfig ),
        m_colourMode( colourMode ),
        m_customOptions( CATCH_MOVE( customOptions ) ) {}

-    IStream* ReporterConfig::stream() const { return m_stream; }
+    Detail::unique_ptr<IStream> ReporterConfig::takeStream() && {
+        assert( m_stream );
+        return CATCH_MOVE( m_stream );
+    }
    IConfig const * ReporterConfig::fullConfig() const { return m_fullConfig; }
    ColourMode ReporterConfig::colourMode() const { return m_colourMode; }

--- a/src/catch2/interfaces/catch_interfaces_reporter.hpp
+++ b/src/catch2/interfaces/catch_interfaces_reporter.hpp
@@ -36,17 +36,17 @@ namespace Catch {

    struct ReporterConfig {
        ReporterConfig( IConfig const* _fullConfig,
-                        IStream* _stream,
+                        Detail::unique_ptr<IStream> _stream,
                        ColourMode colourMode,
                        std::map<std::string, std::string> customOptions );

-        IStream* stream() const;
+        Detail::unique_ptr<IStream> takeStream() &&;
        IConfig const* fullConfig() const;
        ColourMode colourMode() const;
        std::map<std::string, std::string> const& customOptions() const;

    private:
-        IStream* m_stream;
+        Detail::unique_ptr<IStream> m_stream;
        IConfig const* m_fullConfig;
        ColourMode m_colourMode;
        std::map<std::string, std::string> m_customOptions;
--- a/src/catch2/interfaces/catch_interfaces_reporter_factory.hpp
+++ b/src/catch2/interfaces/catch_interfaces_reporter_factory.hpp
@@ -25,7 +25,7 @@ namespace Catch {
        virtual ~IReporterFactory(); // = default

        virtual IEventListenerPtr
-        create( ReporterConfig const& config ) const = 0;
+        create( ReporterConfig&& config ) const = 0;
        virtual std::string getDescription() const = 0;
    };
    using IReporterFactoryPtr = Detail::unique_ptr<IReporterFactory>;
--- a/src/catch2/interfaces/catch_interfaces_reporter_registry.hpp
+++ b/src/catch2/interfaces/catch_interfaces_reporter_registry.hpp
@@ -32,7 +32,7 @@ namespace Catch {
        using Listeners = std::vector<Detail::unique_ptr<EventListenerFactory>>;

        virtual ~IReporterRegistry(); // = default
-        virtual IEventListenerPtr create( std::string const& name, ReporterConfig const& config ) const = 0;
+        virtual IEventListenerPtr create( std::string const& name, ReporterConfig&& config ) const = 0;
        virtual FactoryMap const& getFactories() const = 0;
        virtual Listeners const& getListeners() const = 0;
    };
--- a/src/catch2/internal/catch_reporter_registry.cpp
+++ b/src/catch2/internal/catch_reporter_registry.cpp
@@ -37,11 +37,11 @@ namespace Catch {
    ReporterRegistry::~ReporterRegistry() = default;


-    IEventListenerPtr ReporterRegistry::create( std::string const& name, ReporterConfig const& config ) const {
+    IEventListenerPtr ReporterRegistry::create( std::string const& name, ReporterConfig&& config ) const {
        auto it =  m_factories.find( name );
        if( it == m_factories.end() )
            return nullptr;
-        return it->second->create( config );
+        return it->second->create( CATCH_MOVE(config) );
    }

    void ReporterRegistry::registerReporter( std::string const& name, IReporterFactoryPtr factory ) {
--- a/src/catch2/internal/catch_reporter_registry.hpp
+++ b/src/catch2/internal/catch_reporter_registry.hpp
@@ -21,7 +21,7 @@ namespace Catch {
        ReporterRegistry();
        ~ReporterRegistry() override; // = default, out of line to allow fwd decl

-        IEventListenerPtr create( std::string const& name, ReporterConfig const& config ) const override;
+        IEventListenerPtr create( std::string const& name, ReporterConfig&& config ) const override;

        void registerReporter( std::string const& name, IReporterFactoryPtr factory );
        void registerListener( Detail::unique_ptr<EventListenerFactory> factory );
--- a/src/catch2/reporters/catch_reporter_automake.hpp
+++ b/src/catch2/reporters/catch_reporter_automake.hpp
@@ -15,7 +15,6 @@ namespace Catch {
    class AutomakeReporter final : public StreamingReporterBase {
    public:
        using StreamingReporterBase::StreamingReporterBase;
-
        ~AutomakeReporter() override;

        static std::string getDescription() {
--- a/src/catch2/reporters/catch_reporter_common_base.cpp
+++ b/src/catch2/reporters/catch_reporter_common_base.cpp
@@ -14,11 +14,11 @@


 namespace Catch {
-    ReporterBase::ReporterBase( ReporterConfig const& config ):
+    ReporterBase::ReporterBase( ReporterConfig&& config ):
        IEventListener( config.fullConfig() ),
-        m_wrapped_stream( config.stream() ),
+        m_wrapped_stream( CATCH_MOVE(config).takeStream() ),
        m_stream( m_wrapped_stream->stream() ),
-        m_colour( makeColourImpl( config.colourMode(), m_wrapped_stream ) ),
+        m_colour( makeColourImpl( config.colourMode(), m_wrapped_stream.get() ) ),
        m_customOptions( config.customOptions() )
    {}

--- a/src/catch2/reporters/catch_reporter_common_base.hpp
+++ b/src/catch2/reporters/catch_reporter_common_base.hpp
@@ -29,7 +29,7 @@ namespace Catch {
    class ReporterBase : public IEventListener {
    protected:
        //! The stream wrapper as passed to us by outside code
-        IStream* m_wrapped_stream;
+        Detail::unique_ptr<IStream> m_wrapped_stream;
        //! Cached output stream from `m_wrapped_stream` to reduce
        //! number of indirect calls needed to write output.
        std::ostream& m_stream;
@@ -39,7 +39,7 @@ namespace Catch {
        std::map<std::string, std::string> m_customOptions;

    public:
-        ReporterBase( ReporterConfig const& config );
+        ReporterBase( ReporterConfig&& config );
        ~ReporterBase() override; // = default;

        /**
--- a/src/catch2/reporters/catch_reporter_console.cpp
+++ b/src/catch2/reporters/catch_reporter_console.cpp
@@ -354,8 +354,8 @@ public:
    }
 };

-ConsoleReporter::ConsoleReporter(ReporterConfig const& config)
-    : StreamingReporterBase(config),
+ConsoleReporter::ConsoleReporter(ReporterConfig&& config):
+    StreamingReporterBase( CATCH_MOVE( config ) ),
    m_tablePrinter(Detail::make_unique<TablePrinter>(m_stream,
        [&config]() -> std::vector<ColumnInfo> {
        if (config.fullConfig()->benchmarkNoAnalysis())
--- a/src/catch2/reporters/catch_reporter_console.hpp
+++ b/src/catch2/reporters/catch_reporter_console.hpp
@@ -20,7 +20,7 @@ namespace Catch {
        Detail::unique_ptr<TablePrinter> m_tablePrinter;

    public:
-        ConsoleReporter(ReporterConfig const& config);
+        ConsoleReporter(ReporterConfig&& config);
        ~ConsoleReporter() override;
        static std::string getDescription();

--- a/src/catch2/reporters/catch_reporter_junit.cpp
+++ b/src/catch2/reporters/catch_reporter_junit.cpp
@@ -13,6 +13,7 @@
 #include <catch2/internal/catch_textflow.hpp>
 #include <catch2/interfaces/catch_interfaces_config.hpp>
 #include <catch2/catch_test_case_info.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>

 #include <cassert>
 #include <ctime>
@@ -68,8 +69,8 @@ namespace Catch {

    } // anonymous namespace

-    JunitReporter::JunitReporter( ReporterConfig const& _config )
-        :   CumulativeReporterBase( _config ),
+    JunitReporter::JunitReporter( ReporterConfig&& _config )
+        :   CumulativeReporterBase( CATCH_MOVE(_config) ),
            xml( m_stream )
        {
            m_preferences.shouldRedirectStdOut = true;
--- a/src/catch2/reporters/catch_reporter_junit.hpp
+++ b/src/catch2/reporters/catch_reporter_junit.hpp
@@ -17,7 +17,7 @@ namespace Catch {

    class JunitReporter final : public CumulativeReporterBase {
    public:
-        JunitReporter(ReporterConfig const& _config);
+        JunitReporter(ReporterConfig&& _config);

        ~JunitReporter() override = default;

--- a/src/catch2/reporters/catch_reporter_registrars.hpp
+++ b/src/catch2/reporters/catch_reporter_registrars.hpp
@@ -13,6 +13,7 @@
 #include <catch2/interfaces/catch_interfaces_reporter_factory.hpp>
 #include <catch2/internal/catch_compiler_capabilities.hpp>
 #include <catch2/internal/catch_unique_ptr.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>

 namespace Catch {

@@ -22,8 +23,8 @@ namespace Catch {
    template <typename T>
    class ReporterFactory : public IReporterFactory {

-        IEventListenerPtr create( ReporterConfig const& config ) const override {
-            return Detail::make_unique<T>( config );
+        IEventListenerPtr create( ReporterConfig&& config ) const override {
+            return Detail::make_unique<T>( CATCH_MOVE(config) );
        }

        std::string getDescription() const override {
--- a/src/catch2/reporters/catch_reporter_sonarqube.hpp
+++ b/src/catch2/reporters/catch_reporter_sonarqube.hpp
@@ -11,13 +11,14 @@
 #include <catch2/reporters/catch_reporter_cumulative_base.hpp>

 #include <catch2/internal/catch_xmlwriter.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>

 namespace Catch {

    class SonarQubeReporter final : public CumulativeReporterBase {
    public:
-        SonarQubeReporter(ReporterConfig const& config)
-        : CumulativeReporterBase(config)
+        SonarQubeReporter(ReporterConfig&& config)
+        : CumulativeReporterBase(CATCH_MOVE(config))
        , xml(m_stream) {
            m_preferences.shouldRedirectStdOut = true;
            m_preferences.shouldReportAllAssertions = true;
--- a/src/catch2/reporters/catch_reporter_tap.hpp
+++ b/src/catch2/reporters/catch_reporter_tap.hpp
@@ -9,13 +9,14 @@
 #define CATCH_REPORTER_TAP_HPP_INCLUDED

 #include <catch2/reporters/catch_reporter_streaming_base.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>

 namespace Catch {

    class TAPReporter final : public StreamingReporterBase {
    public:
-        TAPReporter( ReporterConfig const& config ):
-            StreamingReporterBase( config ) {
+        TAPReporter( ReporterConfig&& config ):
+            StreamingReporterBase( CATCH_MOVE(config) ) {
            m_preferences.shouldReportAllAssertions = true;
        }
        ~TAPReporter() override = default;
--- a/src/catch2/reporters/catch_reporter_teamcity.hpp
+++ b/src/catch2/reporters/catch_reporter_teamcity.hpp
@@ -22,8 +22,8 @@ namespace Catch {

    class TeamCityReporter final : public StreamingReporterBase {
    public:
-        TeamCityReporter( ReporterConfig const& _config )
-        :   StreamingReporterBase( _config )
+        TeamCityReporter( ReporterConfig&& _config )
+        :   StreamingReporterBase( CATCH_MOVE(_config) )
        {
            m_preferences.shouldRedirectStdOut = true;
        }
--- a/src/catch2/reporters/catch_reporter_xml.cpp
+++ b/src/catch2/reporters/catch_reporter_xml.cpp
@@ -13,6 +13,7 @@
 #include <catch2/internal/catch_string_manip.hpp>
 #include <catch2/internal/catch_list.hpp>
 #include <catch2/catch_test_case_info.hpp>
+#include <catch2/internal/catch_move_and_forward.hpp>

 #if defined(_MSC_VER)
 #pragma warning(push)
@@ -22,8 +23,8 @@
 #endif

 namespace Catch {
-    XmlReporter::XmlReporter( ReporterConfig const& _config )
-    :   StreamingReporterBase( _config ),
+    XmlReporter::XmlReporter( ReporterConfig&& _config )
+    :   StreamingReporterBase( CATCH_MOVE(_config) ),
        m_xml(m_stream)
    {
        m_preferences.shouldRedirectStdOut = true;
--- a/src/catch2/reporters/catch_reporter_xml.hpp
+++ b/src/catch2/reporters/catch_reporter_xml.hpp
@@ -17,7 +17,7 @@
 namespace Catch {
    class XmlReporter : public StreamingReporterBase {
    public:
-        XmlReporter(ReporterConfig const& _config);
+        XmlReporter(ReporterConfig&& _config);

        ~XmlReporter() override;