UnitTests: Fix flacky tests

There were multiple reasons why the tests were flacky. First
Utils::reverseCompare had a bug. Now

std::lexicographical_compare(first.rbegin(), first.rend(),
                             second.rbegin(), second.rend())

is used.

Second the check StorageCache::checkEntries was not const. So it would
change the vector which it was iterating. So the iterator could be an
dangling.

Fixes: QDS-10197
Change-Id: I84fca6a2b24e1cae9fb85a01b6208de7e58240df
Reviewed-by: Qt CI Patch Build Bot <ci_patchbuild_bot@qt.io>
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/libs/utils/smallstringview.h

@@ -131,38 +131,6 @@ constexpr int compare(SmallStringView first, SmallStringView second) noexcept
     return first.compare(second);
 }
 
-namespace Internal {
-constexpr int reverse_memcmp(const char *first, const char *second, size_t n)
-{
-    const char *currentFirst = first + n - 1;
-    const char *currentSecond = second + n - 1;
-
-    while (n > 0) {
-        // If the current characters differ, return an appropriately signed
-        // value; otherwise, keep searching backwards
-        int difference = *currentFirst - *currentSecond;
-        if (difference != 0)
-            return difference;
-
-        --currentFirst;
-        --currentSecond;
-        --n;
-    }
-
-    return 0;
-}
-} // namespace Internal
-
-constexpr int reverseCompare(SmallStringView first, SmallStringView second) noexcept
-{
-    int difference = Internal::reverse_memcmp(first.data(), second.data(), first.size());
-
-    if (difference == 0)
-        return int(first.size()) - int(second.size());
-
-    return difference;
-}
-
 } // namespace Utils
 
 constexpr Utils::SmallStringView operator""_sv(const char *const string, size_t size)

src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h

@@ -499,7 +499,7 @@ private:
 
     static bool moduleNameLess(Utils::SmallStringView first, Utils::SmallStringView second) noexcept
     {
-        return Utils::reverseCompare(first, second) < 0;
+        return first < second;
     }
 
     using ModuleCache = StorageCache<Utils::PathString,

src/plugins/qmldesigner/designercore/projectstorage/sourcepathcache.h

@@ -143,7 +143,10 @@ private:
 
     static bool sourceContextLess(Utils::SmallStringView first, Utils::SmallStringView second) noexcept
     {
-        return Utils::reverseCompare(first, second) < 0;
+        return std::lexicographical_compare(first.rbegin(),
+                                            first.rend(),
+                                            second.rbegin(),
+                                            second.rend());
     }
 
     static bool sourceLess(Cache::SourceNameView first, Cache::SourceNameView second) noexcept

src/plugins/qmldesigner/designercore/projectstorage/sourcepathcachetypes.h

@@ -35,6 +35,7 @@ public:
 class SourceNameEntry
 {
 public:
+    SourceNameEntry() = default;
     SourceNameEntry(Utils::SmallStringView sourceName, SourceContextId sourceContextId)
         : sourceName(sourceName)
         , sourceContextId(sourceContextId)

src/plugins/qmldesigner/designercore/projectstorage/storagecache.h

@@ -296,19 +296,57 @@ private:
         }
     }
 
-    auto find(ViewType view)
+    template<typename Entries>
+    static auto find(Entries &&entries, ViewType view)
     {
-        auto found = std::lower_bound(m_entries.begin(), m_entries.end(), view, compare);
+        auto begin = entries.begin();
+        auto end = entries.end();
+        auto found = std::lower_bound(begin, end, view, compare);
 
-        if (found == m_entries.end())
+        if (found == entries.end()) {
-            return m_entries.end();
+            return entries.end();
+        }
 
-        if (*found == view)
+        auto value = *found;
+
+        if (value == view) {
             return found;
+        }
 
-        return m_entries.end();
+        return entries.end();
     }
 
+    IndexType id(ViewType view) const
+    {
+        std::shared_lock<Mutex> sharedLock(m_mutex);
+
+        auto found = find(view);
+
+        if (found != m_entries.end()) {
+            return found->id;
+        }
+
+        return IndexType();
+    }
+
+    ResultType value(IndexType id) const
+    {
+        std::shared_lock<Mutex> sharedLock(m_mutex);
+
+        if (IndexType::create(static_cast<IndexDatabaseType>(m_indices.size()) + 1) > id) {
+            if (StorageCacheIndex indirectionIndex = m_indices.at(static_cast<std::size_t>(id) - 1);
+                indirectionIndex.isValid()) {
+                return m_entries.at(static_cast<std::size_t>(indirectionIndex)).value;
+            }
+        }
+
+        return ResultType();
+    }
+
+    auto find(ViewType view) const { return find(m_entries, view); }
+
+    auto find(ViewType view) { return find(m_entries, view); }
+
     void incrementLargerOrEqualIndicesByOne(StorageCacheIndex newIndirectionIndex)
     {
         std::transform(m_indices.begin(), m_indices.end(), m_indices.begin(), [&](StorageCacheIndex index) {
@@ -337,7 +375,7 @@ private:
         return inserted;
     }
 
-    void checkEntries()
+    void checkEntries() const
     {
         for (const auto &entry : m_entries) {
             if (entry.value != value(entry.id) || entry.id != id(entry.value))

tests/unit/tests/unittests/projectstorage/sourcepathcache-test.cpp

@@ -403,7 +403,7 @@ TEST_F(SourcePathCache, get_directory_path_after_populate_if_empty)
     ASSERT_THAT(path, Eq("/path/to"));
 }
 
-TEST_F(SourcePathCache, get_file_path_after_populate_if_emptye)
+TEST_F(SourcePathCache, get_file_path_after_populate_if_empty)
 {
     cacheNotFilled.populateIfEmpty();
 

tests/unit/tests/unittests/projectstorage/storagecache-test.cpp

@@ -17,7 +17,6 @@ namespace {
 using QmlDesigner::SourceContextId;
 using QmlDesigner::StorageCacheException;
 using Utils::compare;
-using Utils::reverseCompare;
 
 class StorageAdapter
 {
@@ -33,7 +32,7 @@ public:
 
 auto less(Utils::SmallStringView first, Utils::SmallStringView second) -> bool
 {
-    return Utils::reverseCompare(first, second) < 0;
+    return Utils::compare(first, second) < 0;
 };
 
 using CacheWithMockLocking = QmlDesigner::StorageCache<Utils::PathString,
@@ -62,7 +61,7 @@ protected:
             return compare(f, l) < 0;
         });
         std::sort(reverseFilePaths.begin(), reverseFilePaths.end(), [](auto &f, auto &l) {
-            return reverseCompare(f, l) < 0;
+            return std::lexicographical_compare(f.rbegin(), f.rend(), l.rbegin(), l.rend());
         });
 
         ON_CALL(this->mockStorage, fetchSourceContextId(Eq("foo"))).WillByDefault(Return(id42));

tests/unit/tests/unittests/utils/smallstring-test.cpp

@@ -1875,17 +1875,3 @@ TEST(SmallString, compare)
     ASSERT_THAT(Utils::compare("textx", "texta"), Gt(0));
     ASSERT_THAT(Utils::compare("texta", "textx"), Le(0));
 }
-
-TEST(SmallString, reverse_compare)
-{
-    const char longText[] = "textfoo";
-
-    ASSERT_THAT(Utils::reverseCompare("", ""), Eq(0));
-    ASSERT_THAT(Utils::reverseCompare("text", "text"), Eq(0));
-    ASSERT_THAT(Utils::reverseCompare("text", Utils::SmallStringView(longText, 4)), Eq(0));
-    ASSERT_THAT(Utils::reverseCompare("", "text"), Le(0));
-    ASSERT_THAT(Utils::reverseCompare("textx", "text"), Gt(0));
-    ASSERT_THAT(Utils::reverseCompare("text", "textx"), Le(0));
-    ASSERT_THAT(Utils::reverseCompare("textx", "texta"), Gt(0));
-    ASSERT_THAT(Utils::reverseCompare("texta", "textx"), Le(0));
-}