diff options
| author | Martin Hořeňovský | 2026-08-22 23:28:11 +0200 |
|---|---|---|
| committer | Martin Hořeňovský | 2026-08-22 23:28:11 +0200 |
| commit | ef3a728265292e6a1dbec727b0099d3d8b3f6420 (patch) | |
| tree | 7c2ddb997a6a1fdbd1ad25edc18966ee63ebd4dd /src/catch2 | |
| parent | 69da66ee7370d7c249ba6f2a322a78dcdd623761 (diff) | |
| download | catch3-ef3a728265292e6a1dbec727b0099d3d8b3f6420.tar.gz | |
Don't store extra copy of all `TestCaseInfo*` in `TestRegistry`
We used to store a full vector of "views" of the registered test infos,
so that it can be retrieved externally. However, the only user of that
interface left was `Session`, specifically `Session::applyFilenameAsTags`.
By moving the adding of the filename tags into the `ITestCaseRegistry`
interface, we avoid storing the vector and make the initial test registration
faster.
The new code is ugly and shows a design/layering issue, but no worse than
the previous approach did.
Diffstat (limited to 'src/catch2')
| -rw-r--r-- | src/catch2/catch_registry_hub.cpp | 4 | ||||
| -rw-r--r-- | src/catch2/catch_session.cpp | 4 | ||||
| -rw-r--r-- | src/catch2/interfaces/catch_interfaces_registry_hub.hpp | 2 | ||||
| -rw-r--r-- | src/catch2/interfaces/catch_interfaces_testcase.hpp | 3 | ||||
| -rw-r--r-- | src/catch2/internal/catch_test_case_registry_impl.cpp | 12 | ||||
| -rw-r--r-- | src/catch2/internal/catch_test_case_registry_impl.hpp | 11 |
6 files changed, 19 insertions, 17 deletions
diff --git a/src/catch2/catch_registry_hub.cpp b/src/catch2/catch_registry_hub.cpp index 25aecdde..165d74a2 100644 --- a/src/catch2/catch_registry_hub.cpp +++ b/src/catch2/catch_registry_hub.cpp @@ -72,6 +72,10 @@ namespace Catch { #endif } + ITestCaseRegistry& getMutableTestCaseRegistry() override { + return m_testCaseRegistry; + } + private: TestRegistry m_testCaseRegistry; ReporterRegistry m_reporterRegistry; diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index 5e9c5c19..bf14d23b 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -140,9 +140,7 @@ namespace Catch { }; void applyFilenamesAsTags() { - for (auto const& testInfo : getRegistryHub().getTestCaseRegistry().getAllInfos()) { - testInfo->addFilenameTag(); - } + getMutableRegistryHub().getMutableTestCaseRegistry().enableFilenameTags(); } // Creates empty file at path. The path must be writable, we do not diff --git a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp index 3695a192..072b48bf 100644 --- a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp +++ b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp @@ -53,6 +53,8 @@ namespace Catch { virtual void registerTranslator( Detail::unique_ptr<IExceptionTranslator>&& translator ) = 0; virtual void registerTagAlias( std::string const& alias, std::string const& tag, SourceLineInfo const& lineInfo ) = 0; virtual void registerStartupException() noexcept = 0; + + virtual ITestCaseRegistry& getMutableTestCaseRegistry() = 0; }; IRegistryHub const& getRegistryHub(); diff --git a/src/catch2/interfaces/catch_interfaces_testcase.hpp b/src/catch2/interfaces/catch_interfaces_testcase.hpp index daee8482..67c4371a 100644 --- a/src/catch2/interfaces/catch_interfaces_testcase.hpp +++ b/src/catch2/interfaces/catch_interfaces_testcase.hpp @@ -19,8 +19,7 @@ namespace Catch { class ITestCaseRegistry { public: virtual ~ITestCaseRegistry(); // = default - // TODO: this exists only for adding filenames to test cases -- let's expose this in a saner way later - virtual std::vector<TestCaseInfo* > const& getAllInfos() const = 0; + virtual void enableFilenameTags() = 0; virtual std::vector<TestCaseHandle> const& getAllTests() const = 0; virtual std::vector<TestCaseHandle> const& getAllTestsSorted( IConfig const& config ) const = 0; }; diff --git a/src/catch2/internal/catch_test_case_registry_impl.cpp b/src/catch2/internal/catch_test_case_registry_impl.cpp index d23056bc..e0d7c139 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.cpp +++ b/src/catch2/internal/catch_test_case_registry_impl.cpp @@ -131,21 +131,21 @@ namespace Catch { // We pre-reserve some reasonable number of tests to avoid the // initial geometric growth churning during test registration. m_handles.reserve( kInitialTestCount ); - m_viewed_test_infos.reserve( kInitialTestCount ); - m_owned_test_infos.reserve( kInitialTestCount ); + m_test_infos.reserve( kInitialTestCount ); m_invokers.reserve( kInitialTestCount ); } TestRegistry::~TestRegistry() = default; void TestRegistry::registerTest(Detail::unique_ptr<TestCaseInfo> testInfo, Detail::unique_ptr<ITestInvoker> testInvoker) { m_handles.emplace_back(testInfo.get(), testInvoker.get()); - m_viewed_test_infos.push_back(testInfo.get()); - m_owned_test_infos.push_back(CATCH_MOVE(testInfo)); + m_test_infos.push_back(CATCH_MOVE(testInfo)); m_invokers.push_back(CATCH_MOVE(testInvoker)); } - std::vector<TestCaseInfo*> const& TestRegistry::getAllInfos() const { - return m_viewed_test_infos; + void TestRegistry::enableFilenameTags() { + for (auto& info : m_test_infos) { + info->addFilenameTag(); + } } std::vector<TestCaseHandle> const& TestRegistry::getAllTests() const { diff --git a/src/catch2/internal/catch_test_case_registry_impl.hpp b/src/catch2/internal/catch_test_case_registry_impl.hpp index 1f883109..3a9dc30c 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.hpp +++ b/src/catch2/internal/catch_test_case_registry_impl.hpp @@ -31,8 +31,8 @@ namespace Catch { class TestRegistry final : public ITestCaseRegistry { public: void registerTest( Detail::unique_ptr<TestCaseInfo> testInfo, Detail::unique_ptr<ITestInvoker> testInvoker ); + void enableFilenameTags() override; - std::vector<TestCaseInfo*> const& getAllInfos() const override; std::vector<TestCaseHandle> const& getAllTests() const override; std::vector<TestCaseHandle> const& getAllTestsSorted( IConfig const& config ) const override; @@ -40,12 +40,11 @@ namespace Catch { ~TestRegistry() override; // = default private: - std::vector<Detail::unique_ptr<TestCaseInfo>> m_owned_test_infos; - // Keeps a materialized vector for `getAllInfos`. - // We should get rid of that eventually (see interface note) - std::vector<TestCaseInfo*> m_viewed_test_infos; - + // Owns the test infos for handles + std::vector<Detail::unique_ptr<TestCaseInfo>> m_test_infos; + // Owns the test invokers for handles std::vector<Detail::unique_ptr<ITestInvoker>> m_invokers; + std::vector<TestCaseHandle> m_handles; mutable TestRunOrder m_currentSortOrder = TestRunOrder::Declared; mutable std::vector<TestCaseHandle> m_sortedFunctions; |