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 );
         }
     }