From d090074da7da0bf8976c6c82cb7ae0bbb61c6af8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Tue, 28 Jul 2020 09:24:57 +0200 Subject: [PATCH] Devirtualize handling of ReporterPreferences The new scheme is that there is one protected member instance of `ReporterPreferences` in the `IStreamingReporter` base class, and derived classes can modify it to express their own preferences. Retrieving the preferences is now a non-virtual operation, which makes it much cheaper to read them frequently. Previously, we avoided doing so by caching the preferences in another variable, but we still read them at least once per test case run. --- .../interfaces/catch_interfaces_reporter.hpp | 26 ++++++++++++++----- src/catch2/reporters/catch_reporter_bases.hpp | 24 +++-------------- src/catch2/reporters/catch_reporter_junit.cpp | 4 +-- .../reporters/catch_reporter_listening.cpp | 4 --- .../reporters/catch_reporter_listening.hpp | 3 --- .../reporters/catch_reporter_sonarqube.hpp | 4 +-- src/catch2/reporters/catch_reporter_tap.hpp | 2 +- .../reporters/catch_reporter_teamcity.hpp | 2 +- src/catch2/reporters/catch_reporter_xml.cpp | 4 +-- 9 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/catch2/interfaces/catch_interfaces_reporter.hpp b/src/catch2/interfaces/catch_interfaces_reporter.hpp index 5f0bcfc6..a411b897 100644 --- a/src/catch2/interfaces/catch_interfaces_reporter.hpp +++ b/src/catch2/interfaces/catch_interfaces_reporter.hpp @@ -46,11 +46,6 @@ namespace Catch { IConfig const* m_fullConfig; }; - struct ReporterPreferences { - bool shouldRedirectStdOut = false; - bool shouldReportAllAssertions = false; - }; - template struct LazyStat : Option { LazyStat& operator=( T const& _value ) { @@ -178,13 +173,32 @@ namespace Catch { } }; + //! By setting up its preferences, a reporter can modify Catch2's behaviour + //! in some regards, e.g. it can request Catch2 to capture writes to + //! stdout/stderr during test execution, and pass them to the reporter. + struct ReporterPreferences { + //! Catch2 should redirect writes to stdout and pass them to the + //! reporter + bool shouldRedirectStdOut = false; + //! Catch2 should call `Reporter::assertionEnded` even for passing + //! assertions + bool shouldReportAllAssertions = false; + }; + + struct IStreamingReporter { + protected: + //! Derived classes can set up their preferences here + ReporterPreferences m_preferences; + public: virtual ~IStreamingReporter() = default; // Implementing class must also provide the following static methods: // static std::string getDescription(); - virtual ReporterPreferences getPreferences() const = 0; + ReporterPreferences const& getPreferences() const { + return m_preferences; + } virtual void noMatchingTestCases( std::string const& spec ) = 0; diff --git a/src/catch2/reporters/catch_reporter_bases.hpp b/src/catch2/reporters/catch_reporter_bases.hpp index 76aea01f..d51d9702 100644 --- a/src/catch2/reporters/catch_reporter_bases.hpp +++ b/src/catch2/reporters/catch_reporter_bases.hpp @@ -29,16 +29,10 @@ namespace Catch { struct StreamingReporterBase : IStreamingReporter { - StreamingReporterBase( ReporterConfig const& _config ) - : m_config( _config.fullConfig() ), - stream( _config.stream() ) - { - m_reporterPrefs.shouldRedirectStdOut = false; + StreamingReporterBase( ReporterConfig const& _config ): + m_config( _config.fullConfig() ), stream( _config.stream() ) { } - ReporterPreferences getPreferences() const override { - return m_reporterPrefs; - } ~StreamingReporterBase() override; @@ -83,7 +77,6 @@ namespace Catch { TestCaseInfo const* currentTestCaseInfo; std::vector m_sectionStack; - ReporterPreferences m_reporterPrefs; }; struct CumulativeReporterBase : IStreamingReporter { @@ -117,18 +110,10 @@ namespace Catch { using TestGroupNode = Node; using TestRunNode = Node; - CumulativeReporterBase( ReporterConfig const& _config ) - : m_config( _config.fullConfig() ), - stream( _config.stream() ) - { - m_reporterPrefs.shouldRedirectStdOut = false; - } + CumulativeReporterBase( ReporterConfig const& _config ): + m_config( _config.fullConfig() ), stream( _config.stream() ) {} ~CumulativeReporterBase() override; - ReporterPreferences getPreferences() const override { - return m_reporterPrefs; - } - void testRunStarting( TestRunInfo const& ) override {} void testGroupStarting( GroupInfo const& ) override {} @@ -159,7 +144,6 @@ namespace Catch { std::shared_ptr m_rootSection; std::shared_ptr m_deepestSection; std::vector> m_sectionStack; - ReporterPreferences m_reporterPrefs; }; struct lineOfChars { diff --git a/src/catch2/reporters/catch_reporter_junit.cpp b/src/catch2/reporters/catch_reporter_junit.cpp index 7e1d6829..fb826088 100644 --- a/src/catch2/reporters/catch_reporter_junit.cpp +++ b/src/catch2/reporters/catch_reporter_junit.cpp @@ -67,8 +67,8 @@ namespace Catch { : CumulativeReporterBase( _config ), xml( _config.stream() ) { - m_reporterPrefs.shouldRedirectStdOut = true; - m_reporterPrefs.shouldReportAllAssertions = true; + m_preferences.shouldRedirectStdOut = true; + m_preferences.shouldReportAllAssertions = true; } JunitReporter::~JunitReporter() {} diff --git a/src/catch2/reporters/catch_reporter_listening.cpp b/src/catch2/reporters/catch_reporter_listening.cpp index 87aae74d..860ee82b 100644 --- a/src/catch2/reporters/catch_reporter_listening.cpp +++ b/src/catch2/reporters/catch_reporter_listening.cpp @@ -26,10 +26,6 @@ namespace Catch { m_preferences.shouldRedirectStdOut = m_reporter->getPreferences().shouldRedirectStdOut; } - ReporterPreferences ListeningReporter::getPreferences() const { - return m_preferences; - } - void ListeningReporter::noMatchingTestCases( std::string const& spec ) { for ( auto const& listener : m_listeners ) { listener->noMatchingTestCases( spec ); diff --git a/src/catch2/reporters/catch_reporter_listening.hpp b/src/catch2/reporters/catch_reporter_listening.hpp index da2a86b2..8e3a3a52 100644 --- a/src/catch2/reporters/catch_reporter_listening.hpp +++ b/src/catch2/reporters/catch_reporter_listening.hpp @@ -15,7 +15,6 @@ namespace Catch { using Reporters = std::vector; Reporters m_listeners; IStreamingReporterPtr m_reporter = nullptr; - ReporterPreferences m_preferences; public: ListeningReporter(); @@ -25,8 +24,6 @@ namespace Catch { public: // IStreamingReporter - ReporterPreferences getPreferences() const override; - void noMatchingTestCases( std::string const& spec ) override; void reportInvalidArguments(std::string const&arg) override; diff --git a/src/catch2/reporters/catch_reporter_sonarqube.hpp b/src/catch2/reporters/catch_reporter_sonarqube.hpp index 4bad942a..a5f174f1 100644 --- a/src/catch2/reporters/catch_reporter_sonarqube.hpp +++ b/src/catch2/reporters/catch_reporter_sonarqube.hpp @@ -16,8 +16,8 @@ namespace Catch { SonarQubeReporter(ReporterConfig const& config) : CumulativeReporterBase(config) , xml(config.stream()) { - m_reporterPrefs.shouldRedirectStdOut = true; - m_reporterPrefs.shouldReportAllAssertions = true; + m_preferences.shouldRedirectStdOut = true; + m_preferences.shouldReportAllAssertions = true; } ~SonarQubeReporter() override; diff --git a/src/catch2/reporters/catch_reporter_tap.hpp b/src/catch2/reporters/catch_reporter_tap.hpp index 651d2b20..63042481 100644 --- a/src/catch2/reporters/catch_reporter_tap.hpp +++ b/src/catch2/reporters/catch_reporter_tap.hpp @@ -13,7 +13,7 @@ namespace Catch { TAPReporter( ReporterConfig const& config ): StreamingReporterBase( config ) { - m_reporterPrefs.shouldReportAllAssertions = true; + m_preferences.shouldReportAllAssertions = true; } ~TAPReporter() override; diff --git a/src/catch2/reporters/catch_reporter_teamcity.hpp b/src/catch2/reporters/catch_reporter_teamcity.hpp index 42f23c9c..a1d6e2fe 100644 --- a/src/catch2/reporters/catch_reporter_teamcity.hpp +++ b/src/catch2/reporters/catch_reporter_teamcity.hpp @@ -21,7 +21,7 @@ namespace Catch { TeamCityReporter( ReporterConfig const& _config ) : StreamingReporterBase( _config ) { - m_reporterPrefs.shouldRedirectStdOut = true; + m_preferences.shouldRedirectStdOut = true; } ~TeamCityReporter() override; diff --git a/src/catch2/reporters/catch_reporter_xml.cpp b/src/catch2/reporters/catch_reporter_xml.cpp index b7712ec7..137efc66 100644 --- a/src/catch2/reporters/catch_reporter_xml.cpp +++ b/src/catch2/reporters/catch_reporter_xml.cpp @@ -23,8 +23,8 @@ namespace Catch { : StreamingReporterBase( _config ), m_xml(_config.stream()) { - m_reporterPrefs.shouldRedirectStdOut = true; - m_reporterPrefs.shouldReportAllAssertions = true; + m_preferences.shouldRedirectStdOut = true; + m_preferences.shouldReportAllAssertions = true; } XmlReporter::~XmlReporter() = default;