1852480 - mail/base/test/browser/browser_archive.js potentially kills account while copy is still running

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Ben Campbell]

(split out from https://bugzilla.mozilla.org/show_bug.cgi?id=1749276#c65)

The browser_archive.js test deletes (archives) a thread, then clicks "undo". It then waits until it sees an item appear in back the thread tree.
Buuuuut... if you augment the test to have more messages in the thread, the undo (really a message copy) operation is still running when the test end conditions are met (at least one message has been re-instated, but the rest are still in-progress).
The test then ends, and the test cleanup code is run. And the cleanup code deletes the account being used for the test - which includes the incomingserver and its msgStore. So... kaboom!

The line which checks for the reappearance of the messages is:
https://searchfox.org/comm-central/source/mail/base/test/browser/browser_archive.js#93

To increase the number of messages in the thread, change the '2's here into '5's, say:
https://searchfox.org/comm-central/source/mail/base/test/browser/browser_archive.js#50
Doing this causes the test to crash.

It seems to run OK as is - maybe with 2 messages it completes quickly enough that the timing doesn't become an issue?
In any case, I think it'd be worth seeing if that test can be changed to ensure all the messages have been copied back before finishing the test.
It seems like this'd be a prime candidate for an intermittent failure.

Comment 1

[Brendan Abolivier [:babolivier]]

Looks like you're correct. I've changed the test locally so that it waits for all messages to be copied:

diff --git a/mail/base/test/browser/browser_archive.js b/mail/base/test/browser/browser_archive.js
--- a/mail/base/test/browser/browser_archive.js
+++ b/mail/base/test/browser/browser_archive.js
@@ -11,6 +11,7 @@ const { MessageGenerator } = ChromeUtils
 const tabmail = document.getElementById("tabmail");
 const about3Pane = tabmail.currentAbout3Pane;
 const { threadTree } = about3Pane;
+const messagesInThread = 5;
 
 add_setup(async function () {
   // Create an account for the test.
@@ -47,7 +48,7 @@ add_setup(async function () {
   const generator = new MessageGenerator();
   testFolder.addMessageBatch(
     generator
-      .makeMessages({ count: 2, msgsPerThread: 2 })
+      .makeMessages({ count: messagesInThread, msgsPerThread: messagesInThread })
       .map(message => message.toMboxString())
   );
 
@@ -90,7 +91,16 @@ add_task(async function testArchiveUndo(
 
   // Make sure the thread makes it back to the thread tree.
   await TestUtils.waitForCondition(
-    () => threadTree.getRowAtIndex(0) !== null,
-    "The thread should have returned back from the archive"
+    () => {
+      goDoCommand("cmd_expandAllThreads");
+      for (let i = 0; i < messagesInThread; i++) {
+        if (threadTree.getRowAtIndex(i) === null) {
+          console.debug(`Missing message at index ${i}`)
+          return false;
+        }
+      }
+      return true;
+    },
+    "The thread should have returned back from the archive",
   );
 });

With this change the crash doesn't occur anymore.

Comment 2

[Brendan Abolivier [:babolivier]]

Attachment: Bug 1852480 - Make sure every message has been moved back when testing undoing archiving. r=#thunderbird-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a101577fd526
Make sure every message has been moved back when testing undoing archiving. r=mkmelin