diff --git a/src/catch2/catch_test_case_info.cpp b/src/catch2/catch_test_case_info.cpp index 6a60f92873..7dab6ae481 100644 --- a/src/catch2/catch_test_case_info.cpp +++ b/src/catch2/catch_test_case_info.cpp @@ -101,6 +101,10 @@ namespace Catch { const size_t extras = 3 + 3; return extractFilenamePart(filepath).size() + extras; } + } // end unnamed namespace + + bool operator<( Tag const& lhs, Tag const& rhs ) { + return lhs.original < rhs.original; } Detail::unique_ptr @@ -228,15 +232,19 @@ namespace Catch { StringRef(backingLCaseTags.c_str() + backingStart, backingEnd - backingStart)); } - - bool TestCaseHandle::operator == ( TestCaseHandle const& rhs ) const { - return m_invoker == rhs.m_invoker - && m_info->name == rhs.m_info->name - && m_info->className == rhs.m_info->className; - } - - bool TestCaseHandle::operator < ( TestCaseHandle const& rhs ) const { - return m_info->name < rhs.m_info->name; + bool operator<( TestCaseInfo const& lhs, TestCaseInfo const& rhs ) { + // We want to avoid redoing the string comparisons multiple times, + // so we store the result of a three-way comparison before using + // it in the actual comparison logic. + const auto cmpName = lhs.name.compare( rhs.name ); + if ( cmpName != 0 ) { + return cmpName < 0; + } + const auto cmpClassName = lhs.className.compare( rhs.className ); + if ( cmpClassName != 0 ) { + return cmpClassName < 0; + } + return lhs.tags < rhs.tags; } TestCaseInfo const& TestCaseHandle::getTestCaseInfo() const { diff --git a/src/catch2/catch_test_case_info.hpp b/src/catch2/catch_test_case_info.hpp index 1383f11b9e..8048bd3a30 100644 --- a/src/catch2/catch_test_case_info.hpp +++ b/src/catch2/catch_test_case_info.hpp @@ -30,6 +30,8 @@ namespace Catch { original(original_), lowerCased(lowerCased_) {} StringRef original, lowerCased; + + friend bool operator<( Tag const& lhs, Tag const& rhs ); }; struct ITestInvoker; @@ -44,7 +46,15 @@ namespace Catch { Benchmark = 1 << 6 }; - + /** + * Various metadata about the test case. + * + * A test case is uniquely identified by its (class)name and tags + * combination, with source location being ignored, and other properties + * being determined from tags. + * + * Tags are kept sorted. + */ struct TestCaseInfo : Detail::NonCopyable { TestCaseInfo(std::string const& _className, @@ -59,6 +69,10 @@ namespace Catch { // Adds the tag(s) with test's filename (for the -# flag) void addFilenameTag(); + //! Orders by name, classname and tags + friend bool operator<( TestCaseInfo const& lhs, + TestCaseInfo const& rhs ); + std::string tagsAsString() const; @@ -75,6 +89,12 @@ namespace Catch { TestCaseProperties properties = TestCaseProperties::None; }; + /** + * Wrapper over the test case information and the test case invoker + * + * Does not own either, and is specifically made to be cheap + * to copy around. + */ class TestCaseHandle { TestCaseInfo* m_info; ITestInvoker* m_invoker; @@ -87,9 +107,6 @@ namespace Catch { } TestCaseInfo const& getTestCaseInfo() const; - - bool operator== ( TestCaseHandle const& rhs ) const; - bool operator < ( TestCaseHandle const& rhs ) const; }; Detail::unique_ptr makeTestCaseInfo( std::string const& className, diff --git a/src/catch2/internal/catch_test_case_registry_impl.cpp b/src/catch2/internal/catch_test_case_registry_impl.cpp index 318156c405..dd1744a707 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.cpp +++ b/src/catch2/internal/catch_test_case_registry_impl.cpp @@ -54,20 +54,36 @@ namespace { case TestRunOrder::LexicographicallySorted: { std::vector sorted = unsortedTestCases; - std::sort(sorted.begin(), sorted.end()); + std::sort( + sorted.begin(), + sorted.end(), + []( TestCaseHandle const& lhs, TestCaseHandle const& rhs ) { + return lhs.getTestCaseInfo() < rhs.getTestCaseInfo(); + } + ); return sorted; } case TestRunOrder::Randomized: { seedRng(config); + using TestWithHash = std::pair; + TestHasher h{ config.rngSeed() }; - std::vector> indexed_tests; + std::vector indexed_tests; indexed_tests.reserve(unsortedTestCases.size()); for (auto const& handle : unsortedTestCases) { indexed_tests.emplace_back(h(handle.getTestCaseInfo()), handle); } - std::sort(indexed_tests.begin(), indexed_tests.end()); + std::sort( indexed_tests.begin(), + indexed_tests.end(), + []( TestWithHash const& lhs, TestWithHash const& rhs ) { + if ( lhs.first == rhs.first ) { + return lhs.second.getTestCaseInfo() < + rhs.second.getTestCaseInfo(); + } + return lhs.first < rhs.first; + } ); std::vector randomized; randomized.reserve(indexed_tests.size()); @@ -91,14 +107,22 @@ namespace { return testSpec.matches( testCase.getTestCaseInfo() ) && isThrowSafe( testCase, config ); } - void enforceNoDuplicateTestCases( std::vector const& functions ) { - std::set seenFunctions; - for( auto const& function : functions ) { - auto prev = seenFunctions.insert( function ); - CATCH_ENFORCE( prev.second, - "error: TEST_CASE( \"" << function.getTestCaseInfo().name << "\" ) already defined.\n" - << "\tFirst seen at " << prev.first->getTestCaseInfo().lineInfo << "\n" - << "\tRedefined at " << function.getTestCaseInfo().lineInfo ); + void + enforceNoDuplicateTestCases( std::vector const& tests ) { + auto testInfoCmp = []( TestCaseInfo const* lhs, + TestCaseInfo const* rhs ) { + return *lhs < *rhs; + }; + std::set seenTests(testInfoCmp); + for ( auto const& test : tests ) { + const auto infoPtr = &test.getTestCaseInfo(); + const auto prev = seenTests.insert( infoPtr ); + CATCH_ENFORCE( + prev.second, + "error: test case \"" << infoPtr->name << "\", with tags \"" + << infoPtr->tagsAsString() << "\" already defined.\n" + << "\tFirst seen at " << ( *prev.first )->lineInfo << "\n" + << "\tRedefined at " << infoPtr->lineInfo ); } } diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt index bd7a575c40..bce10df5b2 100644 --- a/tests/ExtraTests/CMakeLists.txt +++ b/tests/ExtraTests/CMakeLists.txt @@ -180,6 +180,55 @@ add_test( COMMAND ${PYTHON_EXECUTABLE} ${CATCH_DIR}/tests/TestScripts/testPartialTestCaseEvent.py $ ) + +add_executable(DuplicatedTestCases-SameNameAndTags ${TESTS_DIR}/X31-DuplicatedTestCases.cpp) +target_link_libraries(DuplicatedTestCases-SameNameAndTags PRIVATE Catch2::Catch2WithMain) +add_test( + NAME DuplicatedTestCases::SameNameAndTags + COMMAND $ +) +set_tests_properties( + DuplicatedTestCases::SameNameAndTags + PROPERTIES + PASS_REGULAR_EXPRESSION "error: .* already defined\\." +) + +add_executable(DuplicatedTestCases-SameNameDifferentTags ${TESTS_DIR}/X32-DuplicatedTestCasesDifferentTags.cpp) +target_link_libraries(DuplicatedTestCases-SameNameDifferentTags PRIVATE Catch2::Catch2WithMain) +add_test( + NAME DuplicatedTestCases::SameNameDifferentTags + COMMAND $ +) +set_tests_properties( + DuplicatedTestCases::SameNameDifferentTags + PROPERTIES + FAIL_REGULAR_EXPRESSION "error: .* already defined\\." +) + +add_executable(DuplicatedTestCases-DuplicatedTestCaseMethods ${TESTS_DIR}/X33-DuplicatedTestCaseMethods.cpp) +target_link_libraries(DuplicatedTestCases-DuplicatedTestCaseMethods PRIVATE Catch2::Catch2WithMain) +add_test( + NAME DuplicatedTestCases::DuplicatedTestCaseMethods + COMMAND $ +) +set_tests_properties( + DuplicatedTestCases::DuplicatedTestCaseMethods + PROPERTIES + PASS_REGULAR_EXPRESSION "error: .* already defined\\." +) + +add_executable(DuplicatedTestCases-DifferentFixtures ${TESTS_DIR}/X34-DuplicatedTestCaseMethodsDifferentFixtures.cpp) +target_link_libraries(DuplicatedTestCases-DifferentFixtures PRIVATE Catch2::Catch2WithMain) +add_test( + NAME DuplicatedTestCases::DuplicatedTestCaseMethodsDifferentFixtures + COMMAND $ +) +set_tests_properties( + DuplicatedTestCases::DuplicatedTestCaseMethodsDifferentFixtures + PROPERTIES + FAIL_REGULAR_EXPRESSION "error: .* already defined\\." +) + #add_executable(DebugBreakMacros ${TESTS_DIR}/X12-CustomDebugBreakMacro.cpp) #target_link_libraries(DebugBreakMacros Catch2) #add_test(NAME DebugBreakMacros COMMAND DebugBreakMacros --break) @@ -196,6 +245,10 @@ set( EXTRA_TEST_BINARIES DisabledExceptions-CustomHandler FallbackStringifier DisableStringification + PartialTestCaseEvents + DuplicatedTestCases-SameNameAndTags + DuplicatedTestCases-SameNameDifferentTags + DuplicatedTestCases-DuplicatedTestCaseMethods # DebugBreakMacros ) diff --git a/tests/ExtraTests/X31-DuplicatedTestCases.cpp b/tests/ExtraTests/X31-DuplicatedTestCases.cpp new file mode 100644 index 0000000000..2235ca8396 --- /dev/null +++ b/tests/ExtraTests/X31-DuplicatedTestCases.cpp @@ -0,0 +1,16 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +/**\file + * Checks that test cases with identical name and tags are reported as error + */ + +#include + +TEST_CASE("A test case with duplicated name and tags", "[tag1][tag2]") {} +TEST_CASE("A test case with duplicated name and tags", "[tag1][tag2]") {} diff --git a/tests/ExtraTests/X32-DuplicatedTestCasesDifferentTags.cpp b/tests/ExtraTests/X32-DuplicatedTestCasesDifferentTags.cpp new file mode 100644 index 0000000000..6a4246be51 --- /dev/null +++ b/tests/ExtraTests/X32-DuplicatedTestCasesDifferentTags.cpp @@ -0,0 +1,17 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +/**\file + * Checks that test cases with identical name but different tags are + * not reported as an error. + */ + +#include + +TEST_CASE("A test case with duplicated name but different tags", "[tag1]") {} +TEST_CASE("A test case with duplicated name but different tags", "[tag2]") {} diff --git a/tests/ExtraTests/X33-DuplicatedTestCaseMethods.cpp b/tests/ExtraTests/X33-DuplicatedTestCaseMethods.cpp new file mode 100644 index 0000000000..ff6660a8a1 --- /dev/null +++ b/tests/ExtraTests/X33-DuplicatedTestCaseMethods.cpp @@ -0,0 +1,22 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +/**\file + * Checks that test case methods with identical class, name and tags are + * reported as error. + */ + +#include + +class TestCaseFixture { +public: + int m_a; +}; + +TEST_CASE_METHOD(TestCaseFixture, "A test case with duplicated name and tags", "[tag1]") {} +TEST_CASE_METHOD(TestCaseFixture, "A test case with duplicated name and tags", "[tag1]") {} diff --git a/tests/ExtraTests/X34-DuplicatedTestCaseMethodsDifferentFixtures.cpp b/tests/ExtraTests/X34-DuplicatedTestCaseMethodsDifferentFixtures.cpp new file mode 100644 index 0000000000..b2021f5fbf --- /dev/null +++ b/tests/ExtraTests/X34-DuplicatedTestCaseMethodsDifferentFixtures.cpp @@ -0,0 +1,27 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 + +/**\file + * Checks that test case methods with different class, but same name and + * tags name and tags are not reported as error. + */ + +#include + +class TestCaseFixture1 { +public: + int m_a; +}; + +class TestCaseFixture2 { +public: + int m_a; +}; + +TEST_CASE_METHOD(TestCaseFixture1, "A test case with duplicated name and tags", "[tag1]") {} +TEST_CASE_METHOD(TestCaseFixture2, "A test case with duplicated name and tags", "[tag1]") {} diff --git a/tests/SelfTest/UsageTests/Misc.tests.cpp b/tests/SelfTest/UsageTests/Misc.tests.cpp index ee4444f591..e3b9bd3a23 100644 --- a/tests/SelfTest/UsageTests/Misc.tests.cpp +++ b/tests/SelfTest/UsageTests/Misc.tests.cpp @@ -519,3 +519,6 @@ TEMPLATE_TEST_CASE_SIG("#1954 - 7 arg template test case sig compiles", "[regres (1, 1, 1, 1, 1, 0, 0), (5, 1, 1, 1, 1, 0, 0), (5, 3, 1, 1, 1, 0, 0)) { SUCCEED(); } + +TEST_CASE("Same test name but with different tags is fine", "[.approvals][some-tag]") {} +TEST_CASE("Same test name but with different tags is fine", "[.approvals][other-tag]") {}