diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_maildirname.cpp | 96 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 537 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 264 |
3 files changed, 895 insertions, 2 deletions
diff --git a/tests/test_maildirname.cpp b/tests/test_maildirname.cpp index dcc8fab..a8e4c01 100644 --- a/tests/test_maildirname.cpp +++ b/tests/test_maildirname.cpp @@ -18,7 +18,10 @@ #include "maildirname.h" +#include <QDir> +#include <QFile> #include <QSet> +#include <QTemporaryDir> #include <QTest> class TestMaildirName : public QObject @@ -31,6 +34,11 @@ private slots: void anEmptyFlagSuffixIsPreserved(); void aNameWithNoSuffixGetsNone(); void theUidInfixIsNotCarriedAcross(); + void resolveRenamedReturnsAPathThatStillExists(); + void resolveRenamedFindsTheFileMbsyncRenamed(); + void resolveRenamedIsEmptyWhenTheFileIsReallyGone(); + void resolveRenamedDoesNotMatchADifferentMessage(); + void resolveRenamedRefusesAnAmbiguousMatch(); }; // Two messages written in the same second must not collide, which a @@ -89,5 +97,93 @@ void TestMaildirName::theUidInfixIsNotCarriedAcross() .arg(name))); } +namespace { + +/// One empty file, so a test can assert on which PATH is chosen rather than on +/// content. resolveRenamed() answers a filesystem question and never opens the +/// file. +bool touch(const QString &path) +{ + QFile file(path); + if (!file.open(QIODevice::WriteOnly)) + return false; + file.close(); + return true; +} + +} // namespace + +void TestMaildirName::resolveRenamedReturnsAPathThatStillExists() +{ + // The ordinary case, and the one that must stay cheap: nothing was + // renamed, so the answer is the question. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString path = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(path)); + + QCOMPARE(MaildirName::resolveRenamed(path), path); +} + +void TestMaildirName::resolveRenamedFindsTheFileMbsyncRenamed() +{ + // Item 163. mbsync uploads the file and inserts its `,U=<uid>` infix + // before the flag suffix, leaving the unique stem alone. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + const QString renamed = + dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=5:2,D")); + QVERIFY(touch(renamed)); + QVERIFY2(!QFile::exists(stale), "the stale path must not exist"); + + QCOMPARE(MaildirName::resolveRenamed(stale), renamed); +} + +void TestMaildirName::resolveRenamedIsEmptyWhenTheFileIsReallyGone() +{ + // The bounded half. A deleted file must NOT be recovered from, or a + // reportable defect becomes a wrong answer. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString gone = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(!QFile::exists(gone)); + + QVERIFY(MaildirName::resolveRenamed(gone).isEmpty()); +} + +void TestMaildirName::resolveRenamedDoesNotMatchADifferentMessage() +{ + // A neighbouring file in the same folder is not this message. Matching on + // anything looser than the whole stem would return it, and the caller + // would then open, display or MOVE the wrong mail. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q3.host,U=5:2,D")))); + QVERIFY(touch(dir.filePath(QStringLiteral("9999999999.M111Q1.host,U=6:2,D")))); + + QVERIFY(MaildirName::resolveRenamed(stale).isEmpty()); +} + +void TestMaildirName::resolveRenamedRefusesAnAmbiguousMatch() +{ + // Two files sharing one stem cannot happen in a correct Maildir, so this + // is a "the world is not what I assumed" case. Guessing between them could + // move or delete the wrong file, and the caller reports honestly instead. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + const QString stale = dir.filePath(QStringLiteral("1787647354.M369Q2.host:2,D")); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=5:2,D")))); + QVERIFY(touch(dir.filePath(QStringLiteral("1787647354.M369Q2.host,U=6:2,S")))); + + QVERIFY(MaildirName::resolveRenamed(stale).isEmpty()); +} + QTEST_MAIN(TestMaildirName) #include "test_maildirname.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index f935cac..f76ff70 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -260,6 +260,7 @@ private slots: void narrowingAnEmptyQueryBarIsAPlainSearch(); void aMalformedAccountIsReportedWithoutBlockingTheConstructor(); void aWorkerBackedWindowReturnsRealThreads(); + void aPurgeTakesTheRowsOutOfTheViewWithoutARefresh(); // Compose and send, item 123 task 12. void theMailRootComesFromTheConfigNotTheIndex(); @@ -383,6 +384,15 @@ private slots: void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList(); void markCurrentThreadReadResolvesTheThreadThroughTheIndex(); void deletingAReplyRepaintsThatReplyRow(); + void deleteIsHiddenOnMailAlreadyInTheTrash(); + void restoreIsHiddenOnMailThatWasNeverDeleted(); + void deleteAlsoMarksTheMessageRead(); + void emptyTrashAsksBeforeDestroyingAnything(); + void theUnreadLabelSaysWhichDirectionItWillGo(); + void theUnreadLabelFollowsAWriteWithoutReselecting(); + void theUnreadActionIsHiddenOnAMixedSelection(); + void markThreadUnreadReachesAMixedThread(); + void markThreadReadAndUnreadAreSeparateActions(); void toggleUnreadOnAReplyReadsTheReplysOwnState(); void toggleUnreadOnAReplyRepaintsItInBothDirections(); void taggingTheOpenReplyUpdatesTheMessagePaneStrip(); @@ -490,6 +500,7 @@ private slots: void doubleClickingADraftOpensTheComposer(); void aResumedDraftReplacesItsFileRatherThanAddingOne(); void aResumedDraftKeepsItsBlindRecipients(); + void aDraftRenamedByASyncStillReopensAndReplacesItsFile(); void theComposerSplitsItsToolbarByScope(); void ccAndBccHideBehindADisclosure(); void ccAndBccAreRevealedWhenTheyCarryAValue(); @@ -5206,6 +5217,408 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() "deleting one reply marked its whole thread deleted"); } +/// A window whose one account owns `acct/`, with its trash at `acct/trash`. +/// +/// Delete and Restore both ask about a row's PATH, so a test for either needs +/// a config that says which prefix is a trash folder. Bare-window tests carry +/// no account at all and would answer "not in the trash" for every row. +static Config configWithTrash(QTemporaryDir &dir) +{ + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + if (file.open(QIODevice::WriteOnly | QIODevice::Text)) { + QTextStream out(&file); + out << "[account.acct]\n" + << "maildir = acct\n" + << "trash = trash\n" + << "inbox = inbox\n"; + } + Config config; + config.load(path); + return config; +} + +/// One thread row whose displayed message sits at `filePath`. +static ThreadSummary threadAtPath(const QString &id, const QString &filePath, + const QStringList &tags = {}) +{ + ThreadSummary thread = makeThread(id, tags); + thread.firstMessagePath = filePath; + thread.firstMessageTags = tags; + return thread; +} + +void TestMainWindow::deleteIsHiddenOnMailAlreadyInTheTrash() +{ + // Item 168, from the user: "I noticed I can hit delete via context menu on + // a message already in the trash." + // + // It was not dangerous, which is the part that made it survive: the file + // is already in the destination, so moveMessages() takes its + // already-there branch, reports the message as moved and counts an + // unsynced change for a move that never happened. The menu claimed to + // have done something and nothing had. + // + // The question is about the PATH, never the `deleted` TAG: a message + // trashed by another client carries no such tag, which is why item 103 + // made the trash view path-based, and asking the tag would offer Delete on + // exactly the mail a trash view is full of. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *deleteAction = + window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(deleteAction); + + model->appendBatch({ + threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/inbox/cur/1:2,S")), + threadAtPath(QStringLiteral("t2"), + QStringLiteral("acct/trash/cur/2:2,S")), + }); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(deleteAction->isVisible(), + "Delete is hidden on mail that is NOT in the trash, so this test " + "cannot tell the two cases apart"); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(!deleteAction->isVisible(), + "Delete is still offered on a message already in the trash, " + "where it reports success and does nothing"); + + // A folder whose name STARTS with the trash folder's is a different + // folder. Without the trailing separator `acct/trash-old` matches + // `acct/trash` and Delete silently disappears from mail that was never + // trashed, which is the quiet half of the same mistake. + model->appendBatch({ threadAtPath(QStringLiteral("t3"), + QStringLiteral("acct/trash-old/cur/3:2,S")) }); + view->setCurrentIndex(model->index(2, 0, {})); + QVERIFY2(deleteAction->isVisible(), + "Delete vanished on mail in acct/trash-old, which is not the " + "trash: the prefix was compared without its separator"); +} + +void TestMainWindow::restoreIsHiddenOnMailThatWasNeverDeleted() +{ + // The mirror, shipped beside it: `restore` was added unconditionally to + // both menus, so it was offered on mail that was never deleted, where it + // has as little meaning as Delete has in the trash. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *restore = window.findChild<QAction *>(QStringLiteral("restore")); + QVERIFY(restore); + + model->appendBatch({ + threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/inbox/cur/1:2,S")), + threadAtPath(QStringLiteral("t2"), + QStringLiteral("acct/trash/cur/2:2,S")), + }); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(restore->isVisible(), "Restore is hidden on trashed mail"); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(!restore->isVisible(), + "Restore is still offered on mail that was never deleted"); +} + +void TestMainWindow::deleteAlsoMarksTheMessageRead() +{ + // The user's second request on the same tangent: "messages moved to the + // trash should be automatically marked -unread". Deleting is a decision + // about the message, so the unread count must not go on including what + // the user threw away. + // + // Asserted on the undo TEXT and depth rather than on the tags: the write + // is a move, which a bare window cannot complete, but the tag change it + // composes is pushed as one command either way. One command, not two, is + // the property that matters: undo has to return the folder AND the tag + // together. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + model->appendBatch({ threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/inbox/cur/1:2,S"), + { QStringLiteral("unread") }) }); + const QModelIndex row = model->index(0, 0, {}); + view->setCurrentIndex(row); + + QVERIFY2(model->threadFor(row).isUnread(), + "the fixture is already read, so this test cannot see the tag go"); + + auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(deleteAction); + deleteAction->trigger(); + + QVERIFY2(!model->threadFor(row).isUnread(), + "Delete left the message unread in the trash"); +} + +void TestMainWindow::emptyTrashAsksBeforeDestroyingAnything() +{ + // Item 118, and the one place this application asks. CLAUDE.md rules out + // confirmation dialogs for mutations because every mutation pushes its + // inverse onto the undo stack; a purge has no inverse, so the rule does + // not reach it. What the rule protects is that a user never loses work to + // a keystroke, and here the dialog is what provides that rather than + // contradicting it. + // + // Asserting the action EXISTS and is wired, not the dialog's buttons: a + // modal cannot be driven from a test without blocking it (item 84), so + // the dialog itself is a hand test. What is pinned here is that nothing + // is destroyed without going through it. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("empty_trash")); + QVERIFY2(action, "empty_trash does not exist"); + + // Reachable from a menu, which everyActionIsReachableFromAMenu() also + // enforces globally. Named here as well because an unreachable purge is + // worse than an unreachable anything else: the user cannot discover the + // action, but a stray keybinding still runs it. + bool found = false; + const QList<QMenu *> menus = window.findChildren<QMenu *>(); + for (QMenu *menu : menus) { + if (menu->actions().contains(action)) { + found = true; + break; + } + } + QVERIFY2(found, "empty_trash is in no menu"); + + // No shortcut, deliberately: this is the one irreversible action, and a + // chord is exactly how it would be run by accident. + QVERIFY2(action->shortcut().isEmpty(), + qPrintable(QStringLiteral("empty_trash carries the shortcut %1; " + "the one irreversible action must not " + "be a keystroke away") + .arg(action->shortcut().toString()))); +} + +void TestMainWindow::theUnreadLabelSaysWhichDirectionItWillGo() +{ + // The user's note: "the label for toggle unread should be dynamic. On an + // unread message it should be Mark as read, on a read message Mark as + // unread." + // + // "Toggle unread" reads the same whichever way it will go, so the only + // way to learn what it does is to press it and look. The action stays a + // toggle, because one message has a real two-valued state; what changes + // is that the label tells the truth about the direction it has chosen. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), {}) }); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(action->text().contains(QStringLiteral("read")), + qPrintable(action->text())); + QVERIFY2(!action->text().contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("an UNREAD row must offer Mark as " + "read, not: %1").arg(action->text()))); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(action->text().contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("a READ row must offer Mark as unread, " + "not: %1").arg(action->text()))); +} + +void TestMainWindow::theUnreadLabelFollowsAWriteWithoutReselecting() +{ + // The label describes the selection's STATE, and a write moves that state + // without touching the selection. Marking the current row read has to + // leave the entry offering "Mark as unread" on the same row, or the menu + // offers to do again what was just done. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }) }); + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(action->text().contains(QStringLiteral("read")) + && !action->text().contains(QStringLiteral("unread")), + qPrintable(action->text())); + + action->trigger(); + + QVERIFY2(action->text().contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("the label did not follow the write: " + "still offering %1 on a row it just " + "marked read").arg(action->text()))); +} + +void TestMainWindow::theUnreadActionIsHiddenOnAMixedSelection() +{ + // The other half of the same note: "on a thread with mixed states it + // should be hidden, we have a submenu for thread actions". + // + // A selection spanning an unread row and a read one has no single state, + // so no honest label exists for it. Hiding the entry sends the user to + // the thread submenu, whose entries are absolute and work regardless of + // the mix. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), {}) }); + + // From a row that is already current, and NOT via selectAll(): a fresh + // selectAll emits no currentRowChanged at all and leaves the current + // index invalid, so a test using it passes against a missing guard + // (CLAUDE.md). + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(action->isVisible(), "a single row already has no single state"); + + view->selectionModel()->select( + model->index(1, 0, {}), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + QCOMPARE(view->selectionModel()->selectedRows().size(), 2); + + QVERIFY2(!action->isVisible(), + qPrintable(QStringLiteral("a mixed selection still offers the " + "unread action, labelled: %1") + .arg(action->text()))); + + // ...and it comes back when the selection agrees again, or the entry + // would be gone for the rest of the session. + view->selectionModel()->select( + model->index(1, 0, {}), + QItemSelectionModel::Deselect | QItemSelectionModel::Rows); + QVERIFY2(action->isVisible(), + "the action did not return when the selection agreed again"); +} + +void TestMainWindow::markThreadUnreadReachesAMixedThread() +{ + // Item 112. The user's report: on a thread with two unread replies, asking + // to mark the whole thread unread marked it READ instead. + // + // ThreadSummary::tags is notmuch's UNION over the conversation, so a + // thread containing even one unread message answers "unread" and a toggle + // reading that predicate always picks "mark read". There was no input that + // could reach "mark thread unread" on a mixed thread: the only threads + // taking that branch were the ones already entirely read. + // + // A union is not a state. The fix is two fixed-direction actions, so this + // asserts the direction rather than the resulting tags: on a mixed thread + // BOTH directions are reachable, which is the property that was missing. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + // MIXED: the union carries `unread` because some message is unread, while + // others are not. A thread whose messages are all in one state answers + // identically whichever way the direction is computed, so a uniform + // fixture passes against the bug (CLAUDE.md, item 88's opposite-states + // requirement). + model->appendBatch({ makeThread(QStringLiteral("T1"), + { QStringLiteral("unread") }) }); + const QModelIndex thread = model->index(0, 0, {}); + QVERIFY(thread.isValid()); + QVERIFY2(model->threadFor(thread).isUnread(), + "the fixture's union does not carry unread, so this test cannot " + "reach the branch the defect lives in"); + view->setCurrentIndex(thread); + + auto *markUnread = + window.findChild<QAction *>(QStringLiteral("mark_thread_unread")); + QVERIFY2(markUnread, "mark_thread_unread does not exist: the thread toggle " + "was not split, so a mixed thread still has no way to " + "be marked unread"); + markUnread->trigger(); + + QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("wrong direction on a mixed thread: %1") + .arg(window.undoTextForTesting()))); + QVERIFY2(!window.undoTextForTesting().contains(QStringLiteral("Mark thread read")), + qPrintable(QStringLiteral("marked the thread READ when asked to " + "mark it unread: %1") + .arg(window.undoTextForTesting()))); +} + +void TestMainWindow::markThreadReadAndUnreadAreSeparateActions() +{ + // The other half: the read direction must still be reachable, and must be + // its own action rather than the same one answering differently. Both are + // asserted on the SAME mixed thread, which a toggle cannot do: whichever + // direction it picks, the other is unreachable there. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("T1"), + { QStringLiteral("unread") }) }); + const QModelIndex thread = model->index(0, 0, {}); + view->setCurrentIndex(thread); + + auto *markRead = + window.findChild<QAction *>(QStringLiteral("mark_thread_read")); + QVERIFY(markRead); + markRead->trigger(); + QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark thread read")), + qPrintable(window.undoTextForTesting())); + + // The old toggle must be gone rather than left beside its replacements, + // which would leave the defect reachable from the menu it still sat in. + QVERIFY2(!window.findChild<QAction *>(QStringLiteral("toggle_unread_thread")), + "toggle_unread_thread still exists beside the split actions"); +} + void TestMainWindow::toggleUnreadOnAReplyReadsTheReplysOwnState() { // The user's report: "read/unread still doesn't trigger a repaint of the @@ -5550,7 +5963,8 @@ void TestMainWindow::theThreadSubmenuIsReachableFromBothMenus() QStringLiteral("archive_thread"), QStringLiteral("delete_thread"), QStringLiteral("spam_thread"), - QStringLiteral("toggle_unread_thread"), + QStringLiteral("mark_thread_read"), + QStringLiteral("mark_thread_unread"), QStringLiteral("flag_thread"), }; @@ -7479,7 +7893,8 @@ void TestMainWindow::noTwoActionsShareAnIcon() QStringLiteral("archive_thread"), QStringLiteral("delete_thread"), QStringLiteral("spam_thread"), - QStringLiteral("toggle_unread_thread"), + QStringLiteral("mark_thread_read"), + QStringLiteral("mark_thread_unread"), QStringLiteral("flag_thread"), QStringLiteral("reply_no_quote"), }; @@ -8457,6 +8872,47 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); } +void TestMainWindow::aPurgeTakesTheRowsOutOfTheViewWithoutARefresh() +{ + // Found by hand: the mail was destroyed correctly and the list went on + // showing it until the user re-ran the query themselves. + // + // A purge is the one mutation with no optimistic update to apply. Every + // other one CHANGES a row, so the model can rewrite it in place; this one + // takes the row away entirely, and the only honest view afterwards is the + // one the query gives now. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/trash"), QStringLiteral("doomed@example.org"), + QStringLiteral("A subject"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.buildWithAccounts({ { QStringLiteral("acct"), + QStringLiteral("acct"), + QStringLiteral("trash"), + {}, {}, {} } }), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + queryEdit->setText(QStringLiteral("path:\"acct/trash/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + // Straight to the purge, bypassing the confirmation: a modal cannot be + // driven from a test without blocking it (item 84), and what is under + // test is what happens AFTER the user has confirmed. + window.purgeForTesting({ QStringLiteral("doomed@example.org") }); + + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); +} + namespace { /// A worker-backed window with one message in one account's maildir. @@ -12861,6 +13317,83 @@ void TestMainWindow::aResumedDraftReplacesItsFileRatherThanAddingOne() "now exists twice"); } +void TestMainWindow::aDraftRenamedByASyncStillReopensAndReplacesItsFile() +{ + // Item 163, the composer site, and the one that costs data rather than + // display. mbsync uploads a draft and renames it to add its `,U=<uid>` + // infix; the model's path was captured when the query ran, so the reopen + // is handed a name that no longer exists. + // + // The refusal happens BEFORE any composer exists, so the user composes + // again into a FRESH window whose autosave has no previous path to unlink. + // The old revision survives, each save mints a new Message-ID, and both + // files reach the server. Asserted as the file COUNT, which is the shape + // the fork actually takes. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + OutgoingMessage message; + message.accountKey = QStringLiteral("acct"); + message.to = { QStringLiteral("someone@example.org") }; + message.subject = QStringLiteral("Written before a sync"); + message.markdownBody = QStringLiteral("The first half."); + + const QString folder = fixture.mailRoot() + QStringLiteral("/acct/Drafts"); + const QString path = writeDraftFile(folder, message, + fixture.config().account( + QStringLiteral("acct"))); + QVERIFY(!path.isEmpty()); + + // mbsync's rename: same directory, same unique stem, `,U=<uid>` inserted + // before the flag suffix. Nothing reindexes, so the caller below still + // holds the pre-rename name, which is the whole precondition. + const QFileInfo before(path); + const QString base = before.fileName(); + const int suffix = base.indexOf(QStringLiteral(":2,")); + QVERIFY2(suffix > 0, "the draft fixture has no maildir flag suffix"); + const QString renamed = before.absolutePath() + QLatin1Char('/') + + base.left(suffix) + QStringLiteral(",U=7") + + base.mid(suffix); + QVERIFY2(QFile::rename(path, renamed), "could not stage the sync rename"); + + // The guard that proves this test can fail: without it, a fixture that + // quietly left the original in place would pass against the bug. + QVERIFY2(!QFile::exists(path), "the stale path should no longer exist"); + + const auto draftCount = [&folder]() { + return QDir(folder + QStringLiteral("/cur")) + .entryList(QDir::Files).size(); + }; + QCOMPARE(draftCount(), 1); + + // The STALE path, exactly as openComposerFor() passes MessageRef::filePath. + const ComposeContext context = + ComposeContextBuilder::forDraft(fixture.config(), path); + QVERIFY2(context.kind == ComposeContext::Kind::Draft, + "the reopen was refused, so the user would compose a second draft"); + // Resolved, not the caller's: seeding the stale path would let the reopen + // succeed and the unlink still miss, forking the draft one step later. + QCOMPARE(context.draftPath, renamed); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("The second half.")); + + auto *timer = window.findChild<QTimer *>(QStringLiteral("autosave")); + QVERIFY2(timer, "the composer has no autosave timer"); + QVERIFY2(timer->isActive(), "editing the body did not arm the autosave"); + timer->setInterval(0); + QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 5000); + + // Still ONE draft: the autosave replaced the renamed file rather than + // leaving it behind beside a new one. + QCOMPARE(draftCount(), 1); + QVERIFY2(!QFile::exists(renamed), + "the renamed draft survived the autosave, so the draft was forked " + "into two files and both would reach the server"); +} + void TestMainWindow::aResumedDraftKeepsItsBlindRecipients() { // MessageBuilder writes Bcc into the draft file deliberately, and says diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index d02f8bd..3f75898 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -91,7 +91,14 @@ private slots: void moveMessagesKeepsTheMessagesTags(); void moveMessagesReportsOnlyWhatMoved(); void moveMessagesGivesTheFileAFreshMaildirName(); + void purgeMessagesDeletesTheFileAndTheIndexEntry(); + void purgeMessagesReportsWhatItDestroyed(); + void purgeMessagesLeavesOtherMessagesAlone(); + void purgeMessagesDoesNotClaimAnIdItCouldNotDelete(); + void resolveQueryMessagesRefusesAnEmptyQuery(); void moveMessagesKeepsTheMaildirFlags(); + void moveMessagesRecoversWhenASyncRenamedTheFile(); + void moveMessagesStillReportsAMessageThatIsReallyGone(); void indexDraftFileMakesAFileFindable(); void indexDraftFileRemovesThePreviousFile(); @@ -102,6 +109,8 @@ private slots: void aSplitIndexListsTheMaildirsFolders(); void twoMessagesMovedTogetherGetDistinctNames(); + void aQuerySeesMailIndexedAfterTheWorkerOpened(); + private: /// Adds one read message in `folder` and reindexes, for the move tests. /// Each of those takes its own message, because a move is destructive and @@ -184,6 +193,59 @@ void TestNotmuchWorker::initTestCase() QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); } +void TestNotmuchWorker::aQuerySeesMailIndexedAfterTheWorkerOpened() +{ + // The defect this covers is item 104, and it is the reason mail arriving + // while the window is open was invisible until the application restarted. + // + // A read-only notmuch handle is a Xapian SNAPSHOT taken when it is opened. + // `notmuch new` runs in a separate process, so nothing it writes is visible + // to a handle already open, however long it is held and however many + // queries are run through it. The worker opens once and keeps that handle + // for the process lifetime, so every query after the first sync answered + // from a stale index: a refresh missed the mail, and so did a query the + // user typed by hand, which is what ruled out the model and the generation + // counter when this was diagnosed. + // + // ONE worker across both queries is the whole point. The runQuery() helper + // builds a fresh worker per call, which opens a fresh handle and therefore + // cannot reproduce this at all: a test written through it passes against + // the bug. + NotmuchWorker worker(m_fixture.configPath()); + + const QString query = QStringLiteral("subject:\"Arrived mid-session\""); + + { + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + worker.runQuery(query, 1); + QVector<ThreadSummary> before; + for (const QList<QVariant> &args : ready) + before += args.at(0).value<QVector<ThreadSummary>>(); + // Establishes that the handle is open and the query is well-formed, + // rather than leaving "found nothing" to mean either. + QCOMPARE(before.size(), 0); + } + + // A second process writes to the index, exactly as the sync script's + // `notmuch new` does while the window is open. + QVERIFY(m_fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("mid@example.org"), + QStringLiteral("Arrived mid-session"), + QStringLiteral("Carol <carol@example.org>"), + QStringLiteral("Tue, 9 Jun 2026 10:00:00 +0000"), + QStringLiteral("new mail"))); + QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); + + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + worker.runQuery(query, 2); + QVector<ThreadSummary> after; + for (const QList<QVariant> &args : ready) + after += args.at(0).value<QVector<ThreadSummary>>(); + + QCOMPARE(after.size(), 1); + QCOMPARE(after.first().subject, QStringLiteral("Arrived mid-session")); +} + QVector<ThreadSummary> TestNotmuchWorker::runQuery( const QString &query, NotmuchWorker::SortOrder sort, bool withRecipients) { @@ -1228,6 +1290,132 @@ void TestNotmuchWorker::moveMessagesRelocatesTheFile() QVERIFY(!QFile::exists(before)); } +void TestNotmuchWorker::purgeMessagesDoesNotClaimAnIdItCouldNotDelete() +{ + // The report drives what the UI tells the user, and the one number they + // will remember about an irreversible action is how much it destroyed. An + // id whose file the database names but that is not on disk contributes + // nothing: the index entry is still cleaned up, but claiming it as + // destroyed would overstate what happened. + const QString real = QStringLiteral("purge6@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), real), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy purged(&worker, &NotmuchWorker::messagesPurged); + + // A KNOWN id whose file is already gone, which is the case that reaches + // the removal loop and finds nothing to unlink. An unknown id is skipped + // far earlier and proves nothing about it. + const QString stale = QStringLiteral("purge7@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), stale), + qPrintable(m_fixture.error())); + const QString staleFile = fileOf(stale); + QVERIFY(!staleFile.isEmpty()); + QVERIFY(QFile::remove(staleFile)); + + worker.purgeMessages({ real, stale }); + + QCOMPARE(purged.size(), 1); + const QStringList reported = purged.first().at(0).toStringList(); + QVERIFY2(reported.contains(real), qPrintable(reported.join(QLatin1Char(',')))); + QVERIFY2(!reported.contains(stale), + "claimed to have destroyed a message whose file was already gone"); +} + +void TestNotmuchWorker::resolveQueryMessagesRefusesAnEmptyQuery() +{ + // An EMPTY query means "match everything" to notmuch, and this walk is + // what Empty Trash enumerates from. An account with no trash folder + // configured produces an empty query, so without this guard the dialog + // would offer to destroy the entire Maildir and say so accurately. + QVERIFY2(addMovableMessage(QStringLiteral("trash"), + QStringLiteral("empty1@example.org")), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy resolved(&worker, &NotmuchWorker::threadMessagesResolved); + + worker.resolveQueryMessages(QString(), QStringLiteral("purge")); + QCOMPARE(resolved.size(), 0); +} + +void TestNotmuchWorker::purgeMessagesDeletesTheFileAndTheIndexEntry() +{ + // Item 118. The one destructive action in this application: the file is + // removed from disk and the message from the index, with no undo. Both + // halves are asserted, because either one alone leaves a visible defect: + // a file without an index entry is invisible mail on disk, and an index + // entry without a file is a row that opens onto nothing. + const QString id = QStringLiteral("purge1@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), id), + qPrintable(m_fixture.error())); + + const QString before = fileOf(id); + QVERIFY(!before.isEmpty()); + QVERIFY(QFile::exists(before)); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.purgeMessages({ id }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QVERIFY2(!QFile::exists(before), qPrintable(before)); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(id)).size(), 0); +} + +void TestNotmuchWorker::purgeMessagesReportsWhatItDestroyed() +{ + // The count the confirmation named has to be the count that happened, and + // the UI has nothing else to report from: unlike a move, there is no new + // path to observe afterwards. + const QString first = QStringLiteral("purge2@example.org"); + const QString second = QStringLiteral("purge3@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), first), + qPrintable(m_fixture.error())); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), second), + qPrintable(m_fixture.error())); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy purged(&worker, &NotmuchWorker::messagesPurged); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.purgeMessages({ first, second }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(purged.size(), 1); + QStringList reported = purged.first().at(0).toStringList(); + reported.sort(); + QCOMPARE(reported, (QStringList{ first, second })); +} + +void TestNotmuchWorker::purgeMessagesLeavesOtherMessagesAlone() +{ + // The blast radius. A purge names ids, and nothing outside that list may + // be touched: this is the action with no undo, so an over-reach is not + // recoverable. The survivor is in the SAME folder, which is where a + // folder-wide delete would take everything with it. + const QString doomed = QStringLiteral("purge4@example.org"); + const QString survivor = QStringLiteral("purge5@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), doomed), + qPrintable(m_fixture.error())); + QVERIFY2(addMovableMessage(QStringLiteral("trash"), survivor), + qPrintable(m_fixture.error())); + + const QString survivorFile = fileOf(survivor); + QVERIFY(!survivorFile.isEmpty()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + worker.purgeMessages({ doomed }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(doomed)).size(), 0); + QCOMPARE(runQuery(QStringLiteral("id:%1").arg(survivor)).size(), 1); + QVERIFY2(QFile::exists(survivorFile), qPrintable(survivorFile)); +} + void TestNotmuchWorker::moveMessagesReindexesAtTheNewPath() { // The half a filesystem check cannot see. A moved file with a stale index @@ -1365,6 +1553,82 @@ void TestNotmuchWorker::moveMessagesKeepsTheMaildirFlags() QVERIFY(!name.contains(QStringLiteral(",U="))); } +void TestNotmuchWorker::moveMessagesRecoversWhenASyncRenamedTheFile() +{ + // Item 162. mbsync uploads a file and RENAMES it to record the server UID, + // and notmuch keeps the pre-`U=` name until that sync's `notmuch new` + // runs. moveMessages() then renames a path that no longer exists, reports + // "Cannot move <file> to <folder>", and silently does nothing. + // + // The ordinary fixture layout cannot see this: nothing renames a file + // underneath the index. Driving it means renaming the file WITHOUT + // reindexing, which is exactly the window mbsync opens. + const QString id = QStringLiteral("move-stale@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString indexed = fileOf(id); + QVERIFY(!indexed.isEmpty()); + + // mbsync's rename, and deliberately NO m_fixture.index() afterwards: the + // database must still name the old path, which is the whole precondition. + const QString renamed = QFileInfo(indexed).absolutePath() + + QStringLiteral("/move-stale.example.org,U=7:2,D"); + QVERIFY2(QFile::rename(indexed, renamed), "could not stage the sync rename"); + + // The guard that proves this test can fail: without it, a fixture that + // quietly reindexed would make the assertions below pass against the bug. + QCOMPARE(fileOf(id), indexed); + QVERIFY2(!QFile::exists(indexed), "the stale path should no longer exist"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + + // The file really moved, and the database followed it. + const QString after = fileOf(id); + QVERIFY2(!after.isEmpty(), "the message is not in the database after the move"); + QCOMPARE(QFileInfo(after).absolutePath(), + m_fixture.maildirPath() + QStringLiteral("/trash/cur")); + QVERIFY2(QFile::exists(after), qPrintable(after)); + QVERIFY(!QFile::exists(renamed)); + + // The `,U=` infix must not be carried across the folder boundary: that is + // what produced `Maildir error: duplicate UID` on real mail. + QVERIFY(!QFileInfo(after).fileName().contains(QStringLiteral(",U="))); +} + +void TestNotmuchWorker::moveMessagesStillReportsAMessageThatIsReallyGone() +{ + // The bounded half of the recovery above. A file that is genuinely absent, + // rather than merely renamed, must still be REPORTED: recovering silently + // from every missing path would turn a real defect into a move that + // claims success and does nothing. + const QString id = QStringLiteral("move-gone@example.org"); + QVERIFY2(addMovableMessage(QStringLiteral("inbox"), id), + qPrintable(m_fixture.error())); + + const QString indexed = fileOf(id); + QVERIFY(!indexed.isEmpty()); + QVERIFY2(QFile::remove(indexed), "could not remove the file"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("trash")); + + QCOMPARE(errors.size(), 1); + // Nothing is claimed to have moved. + QVERIFY(moved.isEmpty() || moved.first().at(0).toStringList().isEmpty()); +} + void TestNotmuchWorker::twoMessagesMovedTogetherGetDistinctNames() { // The generated name must be unique, since a collision is the entire class |
