Multiple tests can have same name as long as their tags differ

This change also changes it so that test case macros using a
class name can have same name **and** tags as long as the
used class name differs.

Closes #1915
Closes #1999

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<TestCaseInfo>
@@ -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 {

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<TestCaseInfo> makeTestCaseInfo(  std::string const& className,

src/catch2/internal/catch_test_case_registry_impl.cpp

@@ -54,20 +54,36 @@ namespace {
 
         case TestRunOrder::LexicographicallySorted: {
             std::vector<TestCaseHandle> 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::hash_t, TestCaseHandle>;
+
             TestHasher h{ config.rngSeed() };
-            std::vector<std::pair<TestHasher::hash_t, TestCaseHandle>> indexed_tests;
+            std::vector<TestWithHash> 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<TestCaseHandle> randomized;
             randomized.reserve(indexed_tests.size());
@@ -91,14 +107,22 @@ namespace {
         return testSpec.matches( testCase.getTestCaseInfo() ) && isThrowSafe( testCase, config );
     }
 
-    void enforceNoDuplicateTestCases( std::vector<TestCaseHandle> const& functions ) {
-        std::set<TestCaseHandle> 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<TestCaseHandle> const& tests ) {
+        auto testInfoCmp = []( TestCaseInfo const* lhs,
+                               TestCaseInfo const* rhs ) {
+            return *lhs < *rhs;
+        };
+        std::set<TestCaseInfo const*, decltype(testInfoCmp)> 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 );
         }
     }
 

tests/ExtraTests/CMakeLists.txt

@@ -180,6 +180,55 @@ add_test(
   COMMAND ${PYTHON_EXECUTABLE} ${CATCH_DIR}/tests/TestScripts/testPartialTestCaseEvent.py $<TARGET_FILE:PartialTestCaseEvents>
 )
 
+
+add_executable(DuplicatedTestCases-SameNameAndTags ${TESTS_DIR}/X31-DuplicatedTestCases.cpp)
+target_link_libraries(DuplicatedTestCases-SameNameAndTags PRIVATE Catch2::Catch2WithMain)
+add_test(
+  NAME DuplicatedTestCases::SameNameAndTags
+  COMMAND $<TARGET_FILE:DuplicatedTestCases-SameNameAndTags>
+)
+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 $<TARGET_FILE:DuplicatedTestCases-SameNameDifferentTags>
+)
+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 $<TARGET_FILE:DuplicatedTestCases-DuplicatedTestCaseMethods>
+)
+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 $<TARGET_FILE:DuplicatedTestCases-DifferentFixtures>
+)
+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
 )
 

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 <catch2/catch_test_macros.hpp>
+
+TEST_CASE("A test case with duplicated name and tags", "[tag1][tag2]") {}
+TEST_CASE("A test case with duplicated name and tags", "[tag1][tag2]") {}

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 <catch2/catch_test_macros.hpp>
+
+TEST_CASE("A test case with duplicated name but different tags", "[tag1]") {}
+TEST_CASE("A test case with duplicated name but different tags", "[tag2]") {}

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 <catch2/catch_test_macros.hpp>
+
+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]") {}

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 <catch2/catch_test_macros.hpp>
+
+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]") {}

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]") {}