test: remove timeout from pop_sent_msg_ex()

Arbitrary timeouts often result in flaky tests,
especially if CI runners may be paused.
In this case however timeout was not used by any tests
and only slowed down the tests in cases where
no message is expected but non-zero timeout is set.

Author: link2xt

src/chat/chat_tests.rs

@@ -1281,15 +1281,15 @@ async fn test_marknoticed_all_chats() -> Result<()> {
 
     tcm.section("bob: receive messages, accept all chats and send a reply to each messsage");
 
-    while let Some(sent_msg) = alice.pop_sent_msg_opt(Duration::default()).await {
+    while let Some(sent_msg) = alice.pop_sent_msg_opt().await {
         let bob_message = bob.recv_msg(&sent_msg).await;
         let bob_chat_id = bob_message.chat_id;
         bob_chat_id.accept(bob).await?;
         send_text_msg(bob, bob_chat_id, "reply".to_string()).await?;
     }
 
     tcm.section("alice: receive replies from bob");
-    while let Some(sent_msg) = bob.pop_sent_msg_opt(Duration::default()).await {
+    while let Some(sent_msg) = bob.pop_sent_msg_opt().await {
         alice.recv_msg(&sent_msg).await;
     }
     // ensure chats have unread messages
@@ -2816,7 +2816,7 @@ async fn test_cant_remove_nonmember() -> Result<()> {
 
     let alice_charlie_id = alice.add_or_lookup_contact_id(charlie).await;
     remove_contact_from_chat(alice, alice_broadcast_id, alice_charlie_id).await?;
-    assert!(alice.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+    assert!(alice.pop_sent_msg_opt().await.is_none());
     assert!(!remove_from_chat_contacts_table(alice, alice_broadcast_id, alice_charlie_id).await?);
     assert!(
         !remove_from_chat_contacts_table_without_trace(alice, alice_broadcast_id, alice_charlie_id)
@@ -3067,10 +3067,7 @@ async fn test_broadcast_resend_to_new_member() -> Result<()> {
     }
     for i in 0..N_MSGS_TO_NEW_BROADCAST_MEMBER {
         let rev_order = false;
-        let resent_msg = alice
-            .pop_sent_msg_ex(rev_order, Duration::ZERO)
-            .await
-            .unwrap();
+        let resent_msg = alice.pop_sent_msg_ex(rev_order).await.unwrap();
         let fiona_msg = fiona.recv_msg(&resent_msg).await;
         assert_eq!(fiona_msg.chat_id, fiona_bc_id);
         assert_eq!(fiona_msg.text, (i + 1).to_string());
@@ -3087,7 +3084,7 @@ async fn test_broadcast_resend_to_new_member() -> Result<()> {
         );
         bob.recv_msg_trash(&resent_msg).await;
     }
-    assert!(alice.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+    assert!(alice.pop_sent_msg_opt().await.is_none());
     Ok(())
 }
 
@@ -3552,12 +3549,7 @@ async fn test_chat_description(
 
     tcm.section("Alice calls set_chat_description() without actually changing the description");
     set_chat_description(alice, alice_chat_id, "ä ẟ 😂").await?;
-    assert!(
-        alice
-            .pop_sent_msg_opt(Duration::from_secs(0))
-            .await
-            .is_none()
-    );
+    assert!(alice.pop_sent_msg_opt().await.is_none());
 
     Ok(())
 }
@@ -3582,12 +3574,7 @@ async fn test_setting_empty_chat_description() -> Result<()> {
     let _hi = alice.send_text(alice_chat_id, "hi").await;
 
     set_chat_description(alice, alice_chat_id, "").await?;
-    assert!(
-        alice
-            .pop_sent_msg_opt(Duration::from_secs(0))
-            .await
-            .is_none()
-    );
+    assert!(alice.pop_sent_msg_opt().await.is_none());
 
     Ok(())
 }

src/ephemeral/ephemeral_tests.rs

@@ -171,7 +171,7 @@ async fn test_ephemeral_unpromoted() -> Result<()> {
     chat_id
         .set_ephemeral_timer(&alice, Timer::Enabled { duration: 60 })
         .await?;
-    let sent = alice.pop_sent_msg_opt(Duration::from_secs(1)).await;
+    let sent = alice.pop_sent_msg_opt().await;
     assert!(sent.is_none());
     assert_eq!(
         chat_id.get_ephemeral_timer(&alice).await?,
@@ -181,13 +181,13 @@ async fn test_ephemeral_unpromoted() -> Result<()> {
     // Promote the group.
     send_text_msg(&alice, chat_id, "hi!".to_string()).await?;
     assert!(chat_id.is_promoted(&alice).await?);
-    let sent = alice.pop_sent_msg_opt(Duration::from_secs(1)).await;
+    let sent = alice.pop_sent_msg_opt().await;
     assert!(sent.is_some());
 
     chat_id
         .set_ephemeral_timer(&alice.ctx, Timer::Disabled)
         .await?;
-    let sent = alice.pop_sent_msg_opt(Duration::from_secs(1)).await;
+    let sent = alice.pop_sent_msg_opt().await;
     assert!(sent.is_some());
     assert_eq!(chat_id.get_ephemeral_timer(&alice).await?, Timer::Disabled);
 

src/mimeparser/mimeparser_tests.rs

@@ -1,6 +1,5 @@
 use mailparse::ParsedMail;
 use std::mem;
-use std::time::Duration;
 
 use super::*;
 use crate::{
@@ -1435,7 +1434,7 @@ async fn test_intended_recipient_fingerprint() -> Result<()> {
     let chat_id = chat::create_group(t, "").await?;
 
     chat::send_text_msg(t, chat_id, "hi!".to_string()).await?;
-    assert!(t.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+    assert!(t.pop_sent_msg_opt().await.is_none());
 
     for (i, member) in members.iter().enumerate() {
         let contact = t.add_or_lookup_contact(member).await;

src/receive_imf/receive_imf_tests.rs

@@ -2030,12 +2030,12 @@ async fn test_no_smtp_job_for_self_chat() -> Result<()> {
     let chat_id = bob.get_self_chat().await.id;
     let mut msg = Message::new_text("Happy birthday to me".to_string());
     chat::send_msg(bob, chat_id, &mut msg).await?;
-    assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+    assert!(bob.pop_sent_msg_opt().await.is_none());
 
     bob.set_config_bool(Config::BccSelf, true).await?;
     let mut msg = Message::new_text("Happy birthday to me".to_string());
     chat::send_msg(bob, chat_id, &mut msg).await?;
-    assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_some());
+    assert!(bob.pop_sent_msg_opt().await.is_some());
 
     Ok(())
 }

src/securejoin/securejoin_tests.rs

@@ -950,7 +950,7 @@ async fn test_parallel_setup_contact(bob_deletes_fiona_contact: bool) -> Result<
         Contact::delete(bob, bob_fiona_contact_id).await?;
 
         bob.recv_msg_trash(&sent_fiona_vc_auth_required).await;
-        let sent = bob.pop_sent_msg_opt(Duration::ZERO).await;
+        let sent = bob.pop_sent_msg_opt().await;
         assert!(sent.is_none());
     } else {
         bob.recv_msg_trash(&sent_fiona_vc_auth_required).await;
@@ -1235,7 +1235,7 @@ async fn test_rejoin_group() -> Result<()> {
     assert_eq!(progress, 1000);
 
     // Bob does not send any more messages by scanning the QR code.
-    assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+    assert!(bob.pop_sent_msg_opt().await.is_none());
 
     Ok(())
 }

src/test_utils.rs

@@ -8,7 +8,7 @@ use std::ops::{Deref, DerefMut};
 use std::panic;
 use std::path::Path;
 use std::sync::{Arc, LazyLock};
-use std::time::{Duration, Instant};
+use std::time::Duration;
 
 use anyhow::Result;
 use async_channel::{self as channel, Receiver, Sender};
@@ -283,10 +283,10 @@ impl TestContextManager {
         inviters: &[&TestContext],
         qr: &str,
     ) -> ChatId {
-        assert!(joiner.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+        assert!(joiner.pop_sent_msg_opt().await.is_none());
         let inviter_addr = inviters[0].get_primary_self_addr().await.unwrap();
         for inviter in inviters {
-            assert!(inviter.pop_sent_msg_opt(Duration::ZERO).await.is_none());
+            assert!(inviter.pop_sent_msg_opt().await.is_none());
             assert_eq!(inviter.get_primary_self_addr().await.unwrap(), inviter_addr);
         }
 
@@ -295,14 +295,14 @@ impl TestContextManager {
         for _ in 0..2 {
             let mut something_sent = false;
             let rev_order = false;
-            if let Some(sent) = joiner.pop_sent_msg_ex(rev_order, Duration::ZERO).await {
+            if let Some(sent) = joiner.pop_sent_msg_ex(rev_order).await {
                 for inviter in inviters {
                     inviter.recv_msg_opt(&sent).await;
                 }
                 something_sent = true;
             }
             for inviter in inviters {
-                if let Some(sent) = inviter.pop_sent_msg_ex(rev_order, Duration::ZERO).await {
+                if let Some(sent) = inviter.pop_sent_msg_ex(rev_order).await {
                     if sent.recipients.split(' ').any(|addr| addr == inviter_addr) {
                         for observer in inviters {
                             // `imap::prefetch_should_download()` returns false on the sender side.
@@ -649,22 +649,17 @@ impl TestContext {
     ///
     /// Panics if there is no message or on any error.
     pub async fn pop_sent_msg(&self) -> SentMessage<'_> {
-        self.pop_sent_msg_opt(Duration::from_secs(3))
+        self.pop_sent_msg_opt()
             .await
             .expect("no sent message found in jobs table")
     }
 
-    pub async fn pop_sent_msg_opt(&self, timeout: Duration) -> Option<SentMessage<'_>> {
+    pub async fn pop_sent_msg_opt(&self) -> Option<SentMessage<'_>> {
         let rev_order = true;
-        self.pop_sent_msg_ex(rev_order, timeout).await
+        self.pop_sent_msg_ex(rev_order).await
     }
 
-    pub async fn pop_sent_msg_ex(
-        &self,
-        rev_order: bool,
-        timeout: Duration,
-    ) -> Option<SentMessage<'_>> {
-        let start = Instant::now();
+    pub async fn pop_sent_msg_ex(&self, rev_order: bool) -> Option<SentMessage<'_>> {
         let mut query = "
 SELECT id, msg_id, mime, recipients
 FROM smtp
@@ -673,28 +668,18 @@ ORDER BY id"
         if rev_order {
             query += " DESC";
         }
-        let (rowid, msg_id, payload, recipients) = loop {
-            let row = self
-                .ctx
-                .sql
-                .query_row_optional(&query, (), |row| {
-                    let rowid: i64 = row.get(0)?;
-                    let msg_id: MsgId = row.get(1)?;
-                    let mime: String = row.get(2)?;
-                    let recipients: String = row.get(3)?;
-                    Ok((rowid, msg_id, mime, recipients))
-                })
-                .await
-                .expect("query_row_optional failed");
-            if let Some(row) = row {
-                break row;
-            }
-            if start.elapsed() < timeout {
-                tokio::time::sleep(Duration::from_millis(100)).await;
-            } else {
-                return None;
-            }
-        };
+        let (rowid, msg_id, payload, recipients) = self
+            .ctx
+            .sql
+            .query_row_optional(&query, (), |row| {
+                let rowid: i64 = row.get(0)?;
+                let msg_id: MsgId = row.get(1)?;
+                let mime: String = row.get(2)?;
+                let recipients: String = row.get(3)?;
+                Ok((rowid, msg_id, mime, recipients))
+            })
+            .await
+            .expect("query_row_optional failed")?;
         self.ctx
             .sql
             .execute("DELETE FROM smtp WHERE id=?;", (rowid,))

src/tests/pre_messages/forward_and_save.rs

@@ -1,5 +1,4 @@
 //! Tests about forwarding and saving Pre-Messages
-use std::time::Duration;
 
 use anyhow::Result;
 use pretty_assertions::assert_eq;
@@ -108,12 +107,7 @@ async fn test_receive_both() -> Result<()> {
     forward_msgs(alice, &[alice_msg_id], alice_chat_id).await?;
     let rev_order = false;
     let msg = bob
-        .recv_msg(
-            &alice
-                .pop_sent_msg_ex(rev_order, Duration::ZERO)
-                .await
-                .unwrap(),
-        )
+        .recv_msg(&alice.pop_sent_msg_ex(rev_order).await.unwrap())
         .await;
     assert_eq!(msg.download_state(), DownloadState::Available);
     assert_eq!(msg.is_forwarded(), true);

src/webxdc/integration.rs

@@ -145,7 +145,6 @@ mod tests {
     use crate::message::{Message, Viewtype};
     use crate::test_utils::TestContext;
     use anyhow::Result;
-    use std::time::Duration;
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_default_integrations_are_single_device() -> Result<()> {
@@ -158,7 +157,7 @@ mod tests {
         t.set_webxdc_integration(file.to_str().unwrap()).await?;
 
         // default integrations are shipped with the apps and should not be sent over the wire
-        let sent = t.pop_sent_msg_opt(Duration::from_secs(1)).await;
+        let sent = t.pop_sent_msg_opt().await;
         assert!(sent.is_none());
 
         Ok(())