Try to fix test_delete_multiple_messages(), remove some commented-out code, add some comments

deltachat-rpc-client/tests/test_something.py

@@ -1232,6 +1232,10 @@ def test_leave_and_delete_group(acfactory, log):
 
 
 def test_immediate_autodelete(acfactory, direct_imap, log):
+    """
+    `bcc_self` is off by default,
+    so that messages are supposed to be immediately autodeleted
+    """
     ac1, ac2 = acfactory.get_online_accounts(2)
 
     # "1" means delete immediately, while "0" means do not delete

python/tests/test_1_online.py

@@ -1073,6 +1073,8 @@ def test_send_receive_locations(acfactory, lp):
 
 def test_delete_multiple_messages(acfactory, lp):
     ac1, ac2 = acfactory.get_online_accounts(2)
+    # Make sure that messages are not immediately auto-deleted:
+    ac2.set_config("bcc_self", "1")
     chat12 = acfactory.get_accepted_chat(ac1, ac2)
 
     lp.sec("ac1: sending seven messages")

src/ephemeral/ephemeral_tests.rs

@@ -511,30 +511,6 @@ async fn test_delete_expired_imap_messages() -> Result<()> {
         0
     );
 
-    // t.set_config(Config::DeleteServerAfter, Some(&*(25 * HOUR).to_string()))
-    //     .await?;
-    // delete_expired_imap_messages(&t).await?;
-    // test_marked_for_deletion(&t, 1000).await?;
-
-    // MsgId::new(1000)
-    //     .update_download_state(&t, DownloadState::Available)
-    //     .await?;
-    // t.sql
-    //     .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1000'", ())
-    //     .await?;
-    // delete_expired_imap_messages(&t).await?;
-    // test_marked_for_deletion(&t, 1000).await?; // Delete downloadable anyway.
-    // remove_uid(&t, 1000).await?;
-
-    // t.set_config(Config::DeleteServerAfter, Some(&*(22 * HOUR).to_string()))
-    //     .await?;
-    // delete_expired_imap_messages(&t).await?;
-    // test_marked_for_deletion(&t, 1010).await?;
-    // t.sql
-    //     .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1010'", ())
-    //     .await?;
-    // TODO check if removing this code makes the test fail
-
     MsgId::new(1010)
         .update_download_state(&t, DownloadState::Available)
         .await?;
@@ -547,11 +523,6 @@ async fn test_delete_expired_imap_messages() -> Result<()> {
         0
     );
 
-    // TODO instead, test that setting bcc_self to 0 removes messages
-    // t.set_config(Config::DeleteServerAfter, Some("1")).await?;
-    // delete_expired_imap_messages(&t).await?;
-    // test_marked_for_deletion(&t, 3000).await?;
-
     Ok(())
 }
 

src/receive_imf/receive_imf_tests.rs

@@ -1977,17 +1977,6 @@ async fn test_no_smtp_job_for_self_chat() -> Result<()> {
     chat::send_msg(bob, chat_id, &mut msg).await?;
     assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none());
 
-    bob.set_config_bool(Config::BccSelf, true).await?;
-    // TODO instead, test setting bcc_self
-    // bob.set_config(Config::DeleteServerAfter, Some("1")).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_none());
-
-    // bob.set_config(Config::DeleteServerAfter, None).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());
     Ok(())
 }
 

src/smtp.rs

@@ -699,31 +699,6 @@ pub(crate) async fn add_self_recipients(
     recipients: &mut Vec<String>,
     encrypted: bool,
 ) -> Result<()> {
-    // Previous versions of Delta Chat did not send BCC self
-    // if DeleteServerAfter was set to immediately delete messages
-    // from the server. This is not the case anymore
-    // because BCC-self messages are also used to detect
-    // that message was sent if SMTP server is slow to respond
-    // and connection is frequently lost
-    // before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf`
-    // disabled by default is fine.
-
-    // Seems like the correct replacement is `if true`.
-
-    // Before my change, we're adding the self-recipient iff:
-    // - Messages are NOT deleted at once
-    // - OR there are recipients
-    //
-    // i.e. we skip adding the self-recipient iff:
-    // - Messages are deleted at once
-    // - AND there are no recipients
-    // probably because in this case, it's not necesary to send anything.
-    //
-    // Messages are deleted at once iff BccSelf is off in a chatmail profile now.
-    // But BccSelf is always on when this function is called.
-    // So, we can just remove the condition.
-    // TODO remove commented-out code
-    // if context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty() {
     // Avoid sending unencrypted messages to all transports, chatmail relays won't accept
     // them. Normally the user should have a non-chatmail primary transport to send unencrypted
     // messages.
@@ -732,9 +707,14 @@ pub(crate) async fn add_self_recipients(
             recipients.push(addr);
         }
     }
-    // `from` must be the last addr, see `receive_imf_inner()` why.
+    // `from` must be the last addr
+    // because `receive_imf_inner()` marks the message as 'delivered'
+    // if it arrives to the self-server via `bcc_self`.
+    // This helps with marking messages as delivered
+    // if the server is slow and we never get an `OK` response
+    // before the connection times out.
     let from = context.get_primary_self_addr().await?;
     recipients.push(from);
-    // }
+
     Ok(())
 }