From a990b89836e1e16e5b30edb40ee873233cfdbf29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Thu, 22 Feb 2024 11:45:18 -0500
Subject: [PATCH] tests: fix some sporadic failures

Use member event signal to check if a member is removed and avoid
merge id

Change-Id: Ifc18d5e12594bedf2f47bbc5d6d88a9c0dfbd6c7
---
 .../conversation/conversationMembersEvent.cpp | 56 +++++++++++--------
 test/unitTest/syncHistory/syncHistory.cpp     | 53 +++++++++++++++---
 2 files changed, 78 insertions(+), 31 deletions(-)

diff --git a/test/unitTest/conversation/conversationMembersEvent.cpp b/test/unitTest/conversation/conversationMembersEvent.cpp
index bb82a996a5..332a96939a 100644
--- a/test/unitTest/conversation/conversationMembersEvent.cpp
+++ b/test/unitTest/conversation/conversationMembersEvent.cpp
@@ -57,6 +57,7 @@ struct UserData {
     std::string payloadTrustRequest;
     std::vector<libjami::SwarmMessage> messages;
     std::vector<libjami::SwarmMessage> messagesUpdated;
+    std::map<std::string, int> members;
 };
 
 class ConversationMembersEventTest : public CppUnit::TestFixture
@@ -269,6 +270,19 @@ ConversationMembersEventTest::connectSignals()
             }
             cv.notify_one();
         }));
+    confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationMemberEvent>(
+        [&](const std::string& accountId, const std::string& conversationId, const auto& member, auto status) {
+            if (accountId == aliceId) {
+                aliceData.members[member] = status;
+            } else if (accountId == bobId) {
+                bobData.members[member] = status;
+            } else if (accountId == bob2Id) {
+                bob2Data.members[member] = status;
+            } else if (accountId == carlaId) {
+                carlaData.members[member] = status;
+            }
+            cv.notify_one();
+        }));
     confHandlers.insert(
         libjami::exportable_callback<libjami::ConfigurationSignal::IncomingTrustRequest>(
             [&](const std::string& account_id,
@@ -890,26 +904,22 @@ ConversationMembersEventTest::testMemberCannotBanOther()
     libjami::addConversationMember(aliceId, convId, bobUri);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; }));
-    auto aliceMsgSize = aliceData.messages.size();
     libjami::acceptConversationRequest(bobId, convId);
-    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !bobData.conversationId.empty() && aliceMsgSize + 1 == aliceData.messages.size(); }));
-    auto bobMsgSize = bobData.messages.size();
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !bobData.conversationId.empty() && aliceData.members[bobUri] == 1; }));
     libjami::addConversationMember(aliceId, convId, carlaUri);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, 30s, [&]() {
-            return aliceMsgSize + 2 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size() && carlaData.requestReceived; }));
-    aliceMsgSize = aliceData.messages.size();
-    bobMsgSize = bobData.messages.size();
+            return aliceData.members.find(carlaUri) != aliceData.members.end() && bobData.members.find(carlaUri) != bobData.members.end() && carlaData.requestReceived; }));
     libjami::acceptConversationRequest(carlaId, convId);
     CPPUNIT_ASSERT(
-        cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty() && aliceMsgSize + 1 == aliceData.messages.size() && bobMsgSize + 1 == bobData.messages.size(); }));
+        cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty() && aliceData.members[carlaUri] == 1 && bobData.members[carlaUri] == 1; }));
 
     // Now Carla remove Bob as a member
     // remove from member & add into banned without voting for the ban
     simulateRemoval(carlaAccount, convId, bobUri);
     CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceData.errorDetected; }));
 
-    bobMsgSize = bobData.messages.size();
+    auto bobMsgSize = bobData.messages.size();
     libjami::sendMessage(aliceId, convId, "hi"s, "");
     CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobMsgSize + 1 == bobData.messages.size(); }));
 }
@@ -939,27 +949,22 @@ ConversationMembersEventTest::testMemberCannotUnBanOther()
     CPPUNIT_ASSERT(
         cv.wait_for(lk, 30s, [&]() { return carlaData.requestReceived; }));
     aliceMsgSize = aliceData.messages.size();
-    auto bobMsgSize = bobData.messages.size();
     libjami::acceptConversationRequest(carlaId, convId);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, 30s, [&]() { return !carlaData.conversationId.empty()
-                                        && aliceMsgSize + 1 == aliceData.messages.size()
-                                        && bobMsgSize + 1 == bobData.messages.size(); }));
+                                        && aliceData.members[carlaUri] == 1
+                                        && bobData.members[carlaUri] == 1; }));
 
     // Now check that alice, has the only admin, can remove bob
-    aliceMsgSize = aliceData.messages.size();
     libjami::removeConversationMember(aliceId, convId, bobUri);
-    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 2 == aliceData.messages.size(); }));
-    auto msgId = aliceData.messages.rbegin()->id;
-    auto getMessage = [](const auto& data, const auto& mid) -> bool {
-        return std::find_if(data.messages.begin(), data.messages.end(), [&](auto& msg) { return msg.id == mid; }) != data.messages.end();
-    };
-    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return getMessage(carlaData, msgId); }));
+    CPPUNIT_ASSERT(
+        cv.wait_for(lk, 30s, [&]() { return aliceData.members[bobUri] == 3
+                                        && carlaData.members[bobUri] == 3; }));
 
-    aliceMsgSize = aliceData.messages.size();
     libjami::addConversationMember(carlaId, convId, bobUri);
     CPPUNIT_ASSERT(
-        !cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 2 == aliceData.messages.size();; }));
+        !cv.wait_for(lk, 10s, [&]() { return aliceData.members[bobUri] == 1
+                                        && carlaData.members[bobUri] == 1; }));
     auto members = libjami::getConversationMembers(aliceId, convId);
     auto bobBanned = false;
     for (auto& member : members) {
@@ -1574,9 +1579,16 @@ ConversationMembersEventTest::testBanUnbanGotFirstConv()
 
     // Alice can sends some messages now
     aliceMsgSize = aliceData.messages.size();
+    auto getMessageFromBody = [](const auto& data, const auto& body) -> std::string {
+        auto it = std::find_if(data.messages.begin(), data.messages.end(), [&](auto& msg) { return msg.body.find("body") != msg.body.end() && msg.body.at("body") == body; });
+        if (it != data.messages.end()) {
+            return it->id;
+        }
+        return {};
+    };
     libjami::sendMessage(aliceId, aliceData.conversationId, "hi"s, "");
-    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return aliceMsgSize != aliceData.messages.size(); }));
-    msgId = aliceData.messages.rbegin()->id;
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !getMessageFromBody(aliceData, "hi").empty(); }));
+    msgId = getMessageFromBody(aliceData, "hi");
     CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return getMessage(bobData, msgId) && getMessage(bob2Data, msgId); }));
 }
 
diff --git a/test/unitTest/syncHistory/syncHistory.cpp b/test/unitTest/syncHistory/syncHistory.cpp
index c63ab9ccf6..c3bbf7122f 100644
--- a/test/unitTest/syncHistory/syncHistory.cpp
+++ b/test/unitTest/syncHistory/syncHistory.cpp
@@ -57,6 +57,7 @@ struct UserData {
     std::vector<libjami::SwarmMessage> messages;
     std::vector<libjami::SwarmMessage> messagesLoaded;
     std::vector<libjami::SwarmMessage> messagesUpdated;
+    std::map<std::string, int> members;
 };
 
 class SyncHistoryTest : public CppUnit::TestFixture
@@ -210,6 +211,17 @@ SyncHistoryTest::connectSignals()
                 alice2Data.profilePath = path;
             cv.notify_one();
         }));
+    confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationMemberEvent>(
+        [&](const std::string& accountId, const std::string& conversationId, const auto& member, auto status) {
+            if (accountId == aliceId) {
+                aliceData.members[member] = status;
+            } else if (accountId == bobId) {
+                bobData.members[member] = status;
+            } else if (accountId == alice2Id) {
+                alice2Data.members[member] = status;
+            }
+            cv.notify_one();
+        }));
     confHandlers.insert(
         libjami::exportable_callback<libjami::ConfigurationSignal::AccountMessageStatusChanged>(
             [&](const std::string& accountId,
@@ -283,6 +295,8 @@ SyncHistoryTest::connectSignals()
                 aliceData.messages.emplace_back(message);
             } else if (accountId == bobId) {
                 bobData.messages.emplace_back(message);
+            } else if (accountId == alice2Id) {
+                alice2Data.messages.emplace_back(message);
             }
             cv.notify_one();
         }));
@@ -874,34 +888,55 @@ SyncHistoryTest::testLastInteractionAfterSomeMessages()
     details[ConfProperties::ARCHIVE_PIN] = "";
     details[ConfProperties::ARCHIVE_PATH] = aliceArchive;
     alice2Id = Manager::instance().addAccount(details);
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() {return alice2Data.deviceAnnounced; }));
+    auto getMessageFromBody = [](const auto& data, const auto& body) -> std::string {
+        auto it = std::find_if(data.messages.begin(), data.messages.end(), [&](auto& msg) { return msg.body.find("body") != msg.body.end() && msg.body.at("body") == body; });
+        if (it != data.messages.end()) {
+            return it->id;
+        }
+        return {};
+    };
+    auto getMessage = [](const auto& data, const auto& mid) -> bool {
+        return std::find_if(data.messages.begin(), data.messages.end(), [&](auto& msg) { return msg.id == mid; }) != data.messages.end();
+    };
 
     aliceAccount->addContact(bobUri);
     aliceAccount->sendTrustRequest(bobUri, {});
     CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived && !alice2Data.conversationId.empty(); }));
 
-    auto aliceMsgSize = aliceData.messages.size();
     CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri));
     CPPUNIT_ASSERT(
-        cv.wait_for(lk, 30s, [&]() { return aliceMsgSize + 1 == aliceData.messages.size(); }));
+        cv.wait_for(lk, 30s, [&]() { return aliceData.members[bobUri] == 1 && alice2Data.members[bobUri] == 1; }));
 
     // Start conversation
+    bobData.messages.clear();
     libjami::sendMessage(bobId, aliceData.conversationId, std::string("Message 1"), "");
-    CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return aliceMsgSize + 2 == aliceData.messages.size(); }));
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !getMessageFromBody(bobData, "Message 1").empty(); }));
+    auto msgId = getMessageFromBody(bobData, "Message 1");
+    CPPUNIT_ASSERT(cv.wait_for(lk, 20s, [&] { return getMessage(aliceData, msgId) && getMessage(alice2Data, msgId); }));
+
+    bobData.messages.clear();
     libjami::sendMessage(bobId, aliceData.conversationId, std::string("Message 2"), "");
-    CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return aliceMsgSize + 3 == aliceData.messages.size(); }));
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !getMessageFromBody(bobData, "Message 2").empty(); }));
+    msgId = getMessageFromBody(bobData, "Message 2");
+    CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return getMessage(aliceData, msgId) && getMessage(alice2Data, msgId); }));
+
+    bobData.messages.clear();
     libjami::sendMessage(bobId, aliceData.conversationId, std::string("Message 3"), "");
-    CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return aliceMsgSize + 4 == aliceData.messages.size(); }));
+    CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !getMessageFromBody(bobData, "Message 3").empty(); }));
+    msgId = getMessageFromBody(bobData, "Message 3");
+    CPPUNIT_ASSERT(cv.wait_for(lk, 10s, [&] { return getMessage(aliceData, msgId) && getMessage(alice2Data, msgId); }));
 
-    auto displayedId = aliceData.messages.rbegin()->id;
-    libjami::setMessageDisplayed(aliceId, "swarm:" + aliceData.conversationId, displayedId, 3);
-    CPPUNIT_ASSERT(cv.wait_for(lk, 20s, [&] { return aliceData.sent && alice2Data.sent; }));
+    libjami::setMessageDisplayed(aliceId, "swarm:" + aliceData.conversationId, msgId, 3);
+    CPPUNIT_ASSERT(cv.wait_for(lk, 20s, [&] {
+        return aliceData.sent && alice2Data.sent; }));
 
     auto membersInfos = libjami::getConversationMembers(alice2Id, alice2Data.conversationId);
     CPPUNIT_ASSERT(std::find_if(membersInfos.begin(),
                                 membersInfos.end(),
                                 [&](auto infos) {
                                     return infos["uri"] == aliceUri
-                                           && infos["lastDisplayed"] == displayedId;
+                                           && infos["lastDisplayed"] == msgId;
                                 })
                    != membersInfos.end());
 }
-- 
GitLab