From 000270f97e7dbe3f89755640d40c3f01bd226232 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sat, 13 Jun 2026 16:45:44 +0200 Subject: [PATCH] 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. --- src/chat/chat_tests.rs | 27 +++------- src/ephemeral/ephemeral_tests.rs | 6 +-- src/mimeparser/mimeparser_tests.rs | 3 +- src/receive_imf/receive_imf_tests.rs | 4 +- src/securejoin/securejoin_tests.rs | 4 +- src/test_utils.rs | 57 ++++++++-------------- src/tests/pre_messages/forward_and_save.rs | 8 +-- src/webxdc/integration.rs | 3 +- 8 files changed, 38 insertions(+), 74 deletions(-) diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 38c8c4af23..e617877b18 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1281,7 +1281,7 @@ 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?; @@ -1289,7 +1289,7 @@ async fn test_marknoticed_all_chats() -> Result<()> { } 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(()) } diff --git a/src/ephemeral/ephemeral_tests.rs b/src/ephemeral/ephemeral_tests.rs index 5121363aaf..5d004ffb0c 100644 --- a/src/ephemeral/ephemeral_tests.rs +++ b/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); diff --git a/src/mimeparser/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index 32c24d2388..c2246b53d0 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/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; diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index ecfbbed2f4..c582da600a 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/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(()) } diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 5423ae9f02..d25999bb1e 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/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(()) } diff --git a/src/test_utils.rs b/src/test_utils.rs index 098c294e00..b5b859ada6 100644 --- a/src/test_utils.rs +++ b/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> { + pub async fn pop_sent_msg_opt(&self) -> Option> { 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> { - let start = Instant::now(); + pub async fn pop_sent_msg_ex(&self, rev_order: bool) -> Option> { 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,)) diff --git a/src/tests/pre_messages/forward_and_save.rs b/src/tests/pre_messages/forward_and_save.rs index 1961d56674..feb3a72c89 100644 --- a/src/tests/pre_messages/forward_and_save.rs +++ b/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); diff --git a/src/webxdc/integration.rs b/src/webxdc/integration.rs index 0e3dbbd747..e757048a26 100644 --- a/src/webxdc/integration.rs +++ b/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(())