diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/notmuchfixture.h | 7 | ||||
| -rw-r--r-- | tests/test_config.cpp | 161 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 147 | ||||
| -rw-r--r-- | tests/test_mimeparser.cpp | 76 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 164 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 104 |
6 files changed, 644 insertions, 15 deletions
diff --git a/tests/notmuchfixture.h b/tests/notmuchfixture.h index 4d2e59b..bb25526 100644 --- a/tests/notmuchfixture.h +++ b/tests/notmuchfixture.h @@ -47,10 +47,13 @@ public: /// Writes one message into <folder>/cur (or new/ when unread). /// /// Returns false if the file could not be written. Call index() afterwards. + /// `to` defaults to a single generic recipient. Pass one explicitly to + /// exercise the recipient summary, which is the only thing that reads it. bool addMessage(const QString &folder, const QString &messageId, const QString &subject, const QString &from, const QString &date, const QString &body, - bool unread = true, const QString &inReplyTo = QString()) + bool unread = true, const QString &inReplyTo = QString(), + const QString &to = QStringLiteral("you@example.org")) { // Unread messages must not carry the maildir "S" flag, so they go to // new/ where no flags exist at all. @@ -77,7 +80,7 @@ public: QTextStream out(&file); out << "From: " << from << "\n" - << "To: you@example.org\n" + << "To: " << to << "\n" << "Subject: " << subject << "\n" << "Message-ID: <" << messageId << ">\n" << "Date: " << date << "\n"; diff --git a/tests/test_config.cpp b/tests/test_config.cpp index b9321e0..60ed071 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -64,6 +64,13 @@ private slots: void malformedExtraMimetypeIsSkipped(); void syncChannelDefaultsToTheAccountKey(); void syncChannelIsActuallyRead(); + void sentQueryIsEmptyWithoutTheKey(); + void sentQueryComposesThePath(); + void sentQuerySurvivesABracketedPath(); + void sentQueryComposesWithScopedQuery(); + void allSentQueryIsEmptyWhenNoAccountHasOne(); + void allSentQuerySkipsAccountsWithoutTheKey(); + void allSentQueryJoinsEveryConfiguredAccount(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -755,5 +762,159 @@ void TestConfig::syncChannelIsActuallyRead() QStringLiteral("mail-firstlast")); } +void TestConfig::sentQueryIsEmptyWithoutTheKey() +{ + // Optional exactly as drafts is. A real account can legitimately have no + // sent folder at all, and the Sent view omits it silently rather than + // reporting a config problem on every launch about nothing. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.provider-c]\n" + "maildir = provider-c\n"))); + + QCOMPARE(config.accounts().size(), 1); + QVERIFY(config.accounts().at(0).sentQuery().isEmpty()); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::sentQueryComposesThePath() +{ + // Relative to maildir, the same way the account's own scope is, so the two + // cannot disagree about where the account lives. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.webmail-primary]\n" + "maildir = webmail-primary\n" + "sent = Sent\n"))); + + QCOMPARE(config.accounts().at(0).sentQuery(), + QStringLiteral("path:\"webmail-primary/Sent/**\"")); +} + +void TestConfig::sentQuerySurvivesABracketedPath() +{ + // The load-bearing case, and the reason this is a config key rather than a + // <maildir>/Sent convention. A real provider nests its sent folder under a + // BRACKETED parent and localises the name: "[Provider]/Posta inviata". + // + // "[" and "]" are Xapian syntax. The quotes around the whole path are what + // make the query work at all, and an implementation that built this without + // them returns nothing while looking entirely plausible. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.provider-a]\n" + "maildir = provider-a\n" + "sent = [Provider]/Posta inviata\n"))); + + const QString query = config.accounts().at(0).sentQuery(); + QCOMPARE(query, + QStringLiteral("path:\"provider-a/[Provider]/Posta inviata/**\"")); + + // Stated separately from the QCOMPARE above: the quoting is the property + // that matters, and a later change to the surrounding syntax must not be + // able to drop it while still matching a rewritten expected string. + QVERIFY2(query.contains(QStringLiteral("\"provider-a/[Provider]")), + "the composed path is not quoted, so Xapian will read the " + "brackets as syntax and the query will match nothing"); +} + +void TestConfig::sentQueryComposesWithScopedQuery() +{ + // A Sent view under one account must not show another account's sent mail. + // The account selector wraps whatever query runs, so the composed sent + // query has to survive being scoped rather than bypassing it. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.webmail-primary]\n" + "maildir = webmail-primary\n" + "sent = Sent\n"))); + + const Account account = config.accounts().at(0); + const QString scoped = account.scopedQuery(account.sentQuery()); + + QCOMPARE(scoped, + QStringLiteral("path:\"webmail-primary/**\" and " + "(path:\"webmail-primary/Sent/**\")")); +} + +void TestConfig::allSentQueryIsEmptyWhenNoAccountHasOne() +{ + // Empty rather than a query matching nothing, so the caller can hide the + // Sent button entirely instead of offering one that finds no mail. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.provider-c]\n" + "maildir = provider-c\n"))); + + QVERIFY(config.allSentQuery().isEmpty()); +} + +void TestConfig::allSentQuerySkipsAccountsWithoutTheKey() +{ + // Joining an account with no `sent` key would leave a bare "or" in the + // query, and notmuch does not reject that: it silently returns a DIFFERENT + // result. Measured directly against a real database, `A or or B` returns + // 190 where the correct pair returns 211. + // + // A malformed query that still returns plausible mail is the failure that + // ships, so this asserts the shape of the string rather than a count. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.webmail-primary]\n" + "maildir = webmail-primary\n" + "sent = Sent\n" + "\n" + "[account.provider-c]\n" + "maildir = provider-c\n" + "\n" + "[account.webmail-secondary]\n" + "maildir = webmail-secondary\n" + "sent = Sent\n"))); + + const QString all = config.allSentQuery(); + + QVERIFY2(!all.contains(QStringLiteral("or or")), + "an account without a sent key left a bare 'or' in the query"); + QVERIFY2(!all.trimmed().endsWith(QStringLiteral("or")), + "the query ends in a dangling 'or'"); + QVERIFY2(!all.trimmed().startsWith(QStringLiteral("or")), + "the query starts with a dangling 'or'"); + QVERIFY(!all.contains(QStringLiteral("provider-c"))); + + // Exactly two terms joined, one per account that configures the key. + QCOMPARE(all.count(QStringLiteral("path:")), 2); + QCOMPARE(all.count(QStringLiteral(" or ")), 1); +} + +void TestConfig::allSentQueryJoinsEveryConfiguredAccount() +{ + // Including a bracketed provider path, which is the case the quoting + // exists for and the one most likely to be broken by a later rewrite of + // this composition. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.webmail-primary]\n" + "maildir = webmail-primary\n" + "sent = Sent\n" + "\n" + "[account.provider-a]\n" + "maildir = provider-a\n" + "sent = [Provider]/Posta inviata\n"))); + + const QString all = config.allSentQuery(); + + QVERIFY(all.contains(QStringLiteral("path:\"webmail-primary/Sent/**\""))); + QVERIFY(all.contains( + QStringLiteral("path:\"provider-a/[Provider]/Posta inviata/**\""))); + QCOMPARE(all.count(QStringLiteral(" or ")), 1); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 504c10c..faa8481 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -170,6 +170,10 @@ private slots: void theImportantActionIsLabelledImportant(); void theImportantActionStillWritesTheFlaggedTag(); void theToolbarUsesTheConfiguredIconSize(); + void thereIsNoSentButtonWithoutASentKey(); + void theSentButtonRunsEveryConfiguredAccount(); + void theSentButtonSurvivesABracketedPath(); + void flatModeDoesNotSurviveTheNextQuery(); void noTwoActionsShareAnIcon(); }; @@ -4540,6 +4544,149 @@ void TestMainWindow::theToolbarUsesTheConfiguredIconSize() QCOMPARE(toolBar->iconSize(), QSize(40, 40)); } +namespace { + +/// A config whose accounts carry the given maildir/sent pairs. An empty `sent` +/// writes no key at all, which is the account-without-a-sent-folder case. +QString writeSentConfig(const QTemporaryDir &dir, + const QList<QPair<QString, QString>> &accounts) +{ + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QSettings s(path, QSettings::IniFormat); + for (const auto &account : accounts) { + s.beginGroup(QStringLiteral("account.") + account.first); + s.setValue(QStringLiteral("maildir"), account.first); + if (!account.second.isEmpty()) + s.setValue(QStringLiteral("sent"), account.second); + s.endGroup(); + } + s.sync(); + return path; +} + +} // namespace + +void TestMainWindow::thereIsNoSentButtonWithoutASentKey() +{ + // Hidden entirely rather than present and finding nothing. An account may + // legitimately keep no sent mail locally, and a button that always returns + // an empty list reads as a broken feature rather than an absent one. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, {{QStringLiteral("provider-c"), {}}})); + QVERIFY(config.allSentQuery().isEmpty()); + + MainWindow window(config); + QVERIFY(!window.findChild<QPushButton *>(QStringLiteral("sentButton"))); +} + +void TestMainWindow::theSentButtonRunsEveryConfiguredAccount() +{ + // The button composes its query rather than storing one, which is the whole + // reason it is not a [queries] entry: a saved query is a fixed string and + // would not gain the third account here without the user editing it. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, { + {QStringLiteral("webmail-primary"), QStringLiteral("Sent")}, + {QStringLiteral("provider-c"), {}}, + {QStringLiteral("webmail-secondary"), QStringLiteral("Sent")}, + })); + + MainWindow window(config); + auto *button = window.findChild<QPushButton *>(QStringLiteral("sentButton")); + QVERIFY(button); + + auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + button->click(); + const QString query = queryEdit->text(); + + QVERIFY(query.contains(QStringLiteral("webmail-primary/Sent"))); + QVERIFY(query.contains(QStringLiteral("webmail-secondary/Sent"))); + + // The account with no key contributes nothing, and leaves no bare "or" + // behind: notmuch accepts that and silently returns a different result. + QVERIFY(!query.contains(QStringLiteral("provider-c"))); + QVERIFY(!query.contains(QStringLiteral("or or"))); + QCOMPARE(query.count(QStringLiteral(" or ")), 1); +} + +void TestMainWindow::theSentButtonSurvivesABracketedPath() +{ + // A real provider nests its sent folder under a bracketed parent, and "[" + // and "]" are Xapian syntax. The quoting has to survive the trip from the + // config through Account::sentQuery() into the query bar; unquoted, the + // query looks plausible and matches nothing. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, { + {QStringLiteral("provider-a"), QStringLiteral("[Provider]/Posta inviata")}, + })); + + MainWindow window(config); + auto *button = window.findChild<QPushButton *>(QStringLiteral("sentButton")); + QVERIFY(button); + auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + button->click(); + QCOMPARE(queryEdit->text(), + QStringLiteral("path:\"provider-a/[Provider]/Posta inviata/**\"")); +} + +void TestMainWindow::flatModeDoesNotSurviveTheNextQuery() +{ + // The condition the user set for this feature: a flat Sent list is fine, a + // flat anything-else is not. Asserted at the window rather than the model, + // because the leak this guards against is in the WIRING, not in the model: + // setFlatMode(true) from the button with no matching false anywhere else + // passes every model test and flattens the app from the first Sent click + // until it restarts. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSentConfig(dir, { + {QStringLiteral("webmail-primary"), QStringLiteral("Sent")}, + })); + + MainWindow window(config); + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *button = window.findChild<QPushButton *>(QStringLiteral("sentButton")); + QVERIFY(button); + auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + QVERIFY2(!model->flatMode(), "the model starts flat"); + + button->click(); + QVERIFY2(model->flatMode(), "the Sent button did not flatten the list"); + + // Any other query restores the tree. Typed by hand rather than through a + // saved-query button, since that is the route with no flag of its own and + // therefore the one most likely to be forgotten. + queryEdit->setText(QStringLiteral("tag:inbox")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + QVERIFY2(!model->flatMode(), + "flat mode survived into an ordinary query, so every view after " + "one Sent click lost its replies"); + + // And back, so the button still works after the round trip. + button->click(); + QVERIFY(model->flatMode()); + + // Even the SAME query typed by hand comes back as a tree: the flag follows + // the button, not the text, which is the rule the user chose. + queryEdit->setText(config.allSentQuery()); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + QVERIFY(!model->flatMode()); +} + void TestMainWindow::noTwoActionsShareAnIcon() { // Reported by the user against the icons shipped in 0.12.0: Archive and diff --git a/tests/test_mimeparser.cpp b/tests/test_mimeparser.cpp index d16735f..bac71f6 100644 --- a/tests/test_mimeparser.cpp +++ b/tests/test_mimeparser.cpp @@ -41,6 +41,10 @@ private slots: void safeFilenameStripsPathComponents(); void pathInsideDirectoryRejectsSiblingPrefix(); void attachmentFolderNameIsASinglePlainComponent(); + void recipientSummaryPrefersDisplayNames(); + void recipientSummaryKeepsACommaInsideADisplayName(); + void recipientSummaryCollapsesTheOverflow(); + void recipientSummarySurvivesUnusableInput(); void folderNameSurvivesATimezoneComment(); void savingABatchNeverOverwrites(); @@ -409,5 +413,77 @@ void TestMimeParser::savingABatchNeverOverwrites() qPrintable(second_tar)); } +void TestMimeParser::recipientSummaryPrefersDisplayNames() +{ + // A display name where there is one, the address where there is not, so a + // list does not mix "Mario Rossi" with a bare address for no reason the + // reader can see. + QCOMPARE(recipientSummary( + QStringLiteral("Mario Rossi <mario@example.org>")), + QStringLiteral("Mario Rossi")); + + QCOMPARE(recipientSummary(QStringLiteral("info@example.net")), + QStringLiteral("info@example.net")); + + QCOMPARE(recipientSummary(QStringLiteral( + "Mario Rossi <mario@example.org>, info@example.net")), + QStringLiteral("Mario Rossi, info@example.net")); +} + +void TestMimeParser::recipientSummaryKeepsACommaInsideADisplayName() +{ + // The reason this uses GMime rather than QString::split(','). A quoted + // display name may CONTAIN a comma, and splitting reports three recipients + // where there are two, with "Mario" alone as one of them. + const QString summary = recipientSummary(QStringLiteral( + "\"Rossi, Mario\" <mario@example.org>, info@example.net")); + + QCOMPARE(summary, QStringLiteral("Rossi, Mario, info@example.net")); + + // Stated separately, because the QCOMPARE above would also pass a naive + // implementation that happened to rejoin the pieces in the same order. + QVERIFY2(!summary.contains(QStringLiteral("+")), + "a comma inside a display name was counted as another recipient"); +} + +void TestMimeParser::recipientSummaryCollapsesTheOverflow() +{ + // "+N" rather than eliding mid-name, mirroring the tag strip's overflow + // chip: a card has one line for this and a truncated name is worse than an + // honest count. + QCOMPARE(recipientSummary(QStringLiteral( + "a@example.org, b@example.org, c@example.org, " + "d@example.org")), + QStringLiteral("a@example.org, b@example.org +2")); + + // Exactly at the limit does not collapse: a "+0" would be absurd. + QCOMPARE(recipientSummary( + QStringLiteral("a@example.org, b@example.org")), + QStringLiteral("a@example.org, b@example.org")); +} + +void TestMimeParser::recipientSummarySurvivesUnusableInput() +{ + // The header is untrusted and every one of these is real mail. + // + // The empty string is the one that crashes if unguarded: + // internet_address_list_parse returns NULL for it rather than an empty + // list, verified against GMime directly. + QVERIFY(recipientSummary(QString()).isEmpty()); + QVERIFY(recipientSummary(QStringLiteral("")).isEmpty()); + QVERIFY(recipientSummary(QStringLiteral(" ")).isEmpty()); + + // Not a crash and not a lie: a group with no members has no names to show. + const QString group = + recipientSummary(QStringLiteral("undisclosed-recipients:;")); + QVERIFY2(!group.contains(QStringLiteral("@")), + qPrintable(QStringLiteral("a memberless group produced an " + "address: %1").arg(group))); + + // Garbage parses to something or to nothing, but never to a crash. + recipientSummary(QStringLiteral("<<<>>>")); + recipientSummary(QStringLiteral("\"unterminated <a@example.org>")); +} + QTEST_MAIN(TestMimeParser) #include "test_mimeparser.moc" diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 88dcf0b..fe0247c 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -65,6 +65,13 @@ private slots: void loadThreadTreeReportsReplyDepth(); void loadThreadTreeCarriesTheFactsARowNeeds(); + void loadThreadMatchedOnlyDropsTheRest(); + void loadThreadMatchedOnlyWithNoQueryKeepsEverything(); + + void recipientsAreAbsentUnlessAskedFor(); + void recipientsAreFoldedWhenAskedFor(); + void recipientsCrossAQueuedCall(); + void requestCountsAnswersOneCountPerQuery(); void requestCountsKeepsPositionOnAnInvalidQuery(); void requestDatabaseStatsCountsMessagesNotThreads(); @@ -74,10 +81,12 @@ private: /// Tags of one message, read back through a fresh worker query. QStringList tagsOf(const QString &messageId); QVector<MessageRef> messagesOfThread(const QString &threadId, - const QString &matchQuery = QString()); + const QString &matchQuery = QString(), + bool matchedOnly = false); QVector<ThreadSummary> runQuery( const QString &query, - NotmuchWorker::SortOrder sort = NotmuchWorker::NewestFirst); + NotmuchWorker::SortOrder sort = NotmuchWorker::NewestFirst, + bool withRecipients = false); QString threadIdOf(const QString &subject); NotmuchFixture m_fixture; @@ -114,17 +123,38 @@ void TestNotmuchWorker::initTestCase() QStringLiteral("Thu, 4 Jun 2026 10:00:00 +0000"), QStringLiteral("fourth message"), false)); + // Thread D: in a "sent" folder, with real recipients. The To header is the + // only thing that distinguishes these from the threads above, and it is + // what the recipient fold reads. + QVERIFY(m_fixture.addMessage(QStringLiteral("sent"), QStringLiteral("d1@example.org"), + QStringLiteral("Preventivo"), + QStringLiteral("You <you@example.org>"), + QStringLiteral("Fri, 5 Jun 2026 10:00:00 +0000"), + QStringLiteral("fifth message"), false, QString(), + QStringLiteral("Mario Rossi <mario@example.org>"))); + + // Thread E: several recipients, one of them with a comma inside a quoted + // display name, which is what defeats splitting on commas. + QVERIFY(m_fixture.addMessage(QStringLiteral("sent"), QStringLiteral("e1@example.org"), + QStringLiteral("Riunione"), + QStringLiteral("You <you@example.org>"), + QStringLiteral("Sat, 6 Jun 2026 10:00:00 +0000"), + QStringLiteral("sixth message"), false, QString(), + QStringLiteral("\"Rossi, Mario\" <mario@example.org>, " + "info@example.net, " + "third@example.org"))); + QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); } QVector<ThreadSummary> TestNotmuchWorker::runQuery( - const QString &query, NotmuchWorker::SortOrder sort) + const QString &query, NotmuchWorker::SortOrder sort, bool withRecipients) { NotmuchWorker worker(m_fixture.configPath()); QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); QSignalSpy finished(&worker, &NotmuchWorker::queryFinished); - worker.runQuery(query, 1, sort); + worker.runQuery(query, 1, sort, withRecipients); QVector<ThreadSummary> all; for (const QList<QVariant> &args : ready) @@ -143,11 +173,12 @@ QString TestNotmuchWorker::threadIdOf(const QString &subject) } QVector<MessageRef> TestNotmuchWorker::messagesOfThread(const QString &threadId, - const QString &matchQuery) + const QString &matchQuery, + bool matchedOnly) { NotmuchWorker worker(m_fixture.configPath()); QSignalSpy loaded(&worker, &NotmuchWorker::threadLoaded); - worker.loadThread(threadId, matchQuery, 1); + worker.loadThread(threadId, matchQuery, 1, matchedOnly); if (loaded.isEmpty()) return {}; return loaded.first().at(0).value<QVector<MessageRef>>(); @@ -256,7 +287,7 @@ void TestNotmuchWorker::loadThreadTreeCarriesTheFactsARowNeeds() void TestNotmuchWorker::queryReturnsAllThreads() { const QVector<ThreadSummary> threads = runQuery(QStringLiteral("*")); - QCOMPARE(threads.size(), 3); + QCOMPARE(threads.size(), 5); } void TestNotmuchWorker::queryFiltersByTag() @@ -325,7 +356,7 @@ void TestNotmuchWorker::queryPassesGenerationThrough() QCOMPARE(ready.size(), 1); QCOMPARE(ready.first().at(1).value<quint64>(), quint64(42)); QCOMPARE(finished.size(), 1); - QCOMPARE(finished.first().at(0).toInt(), 3); + QCOMPARE(finished.first().at(0).toInt(), 5); QCOMPARE(finished.first().at(1).value<quint64>(), quint64(42)); } @@ -513,7 +544,7 @@ void TestNotmuchWorker::queryStillWorksAfterWrite() worker.runQuery(QStringLiteral("*"), 2); QCOMPARE(ready.size(), 2); - QCOMPARE(ready.at(1).at(0).value<QVector<ThreadSummary>>().size(), 3); + QCOMPARE(ready.at(1).at(0).value<QVector<ThreadSummary>>().size(), 5); worker.applyTags(change.inverted()); } @@ -611,6 +642,113 @@ void TestNotmuchWorker::requestAllTagsOnUnreadableConfigEmitsError() QVERIFY(ready.isEmpty()); } +void TestNotmuchWorker::loadThreadMatchedOnlyDropsTheRest() +{ + // Thread A is two messages, and only the reply carries "hamsterwheel". + const QString threadId = threadIdOf(QStringLiteral("Release notes")); + QVERIFY(!threadId.isEmpty()); + + // Without the flag: both messages, the non-matching one marked as a stub. + // This is the reading pane's normal behaviour and must not change. + const QVector<MessageRef> whole = + messagesOfThread(threadId, QStringLiteral("hamsterwheel")); + QCOMPARE(whole.size(), 2); + + // With it: only the message that matched. The pane in a Sent view shows + // what the user sent, not the conversation their message started. + const QVector<MessageRef> matched = + messagesOfThread(threadId, QStringLiteral("hamsterwheel"), true); + QCOMPARE(matched.size(), 1); + QVERIFY(matched.at(0).matched); + QCOMPARE(matched.at(0).messageId, QStringLiteral("a2@example.org")); +} + +void TestNotmuchWorker::loadThreadMatchedOnlyWithNoQueryKeepsEverything() +{ + // No query means nothing was filtered, so every message counts as matched + // and the flag has nothing to drop. + // + // This does NOT prove the haveMatchSet guard in loadThread: ref.matched is + // already true for every message in this case, so removing that guard + // leaves this passing, confirmed by mutation. It pins the BEHAVIOUR, which + // is what a caller depends on, and the guard is a stated invariant rather + // than a branch a test can reach. + const QString threadId = threadIdOf(QStringLiteral("Release notes")); + QVERIFY(!threadId.isEmpty()); + + const QVector<MessageRef> all = messagesOfThread(threadId, QString(), true); + QCOMPARE(all.size(), 2); +} + +void TestNotmuchWorker::recipientsAreAbsentUnlessAskedFor() +{ + // Opt-in, and this is a PERFORMANCE contract rather than a preference. + // notmuch_message_get_header(m, "To") is not served from the index, it + // reads the message file: measured 2026-08-11 against a real database, + // folding every thread of a 4411-thread inbox took 38.2 seconds, 8.7 ms + // per thread, against 1.1 ms per thread over the 601-thread sent view. + // + // A version that always folds is correct in every other respect, which is + // exactly why it needs a test: nothing else here would notice. + const QVector<ThreadSummary> threads = + runQuery(QStringLiteral("subject:Preventivo")); + + QCOMPARE(threads.size(), 1); + QVERIFY2(threads.at(0).recipients.isEmpty(), + "the To header was read for a query that never asked for it"); +} + +void TestNotmuchWorker::recipientsAreFoldedWhenAskedFor() +{ + const QVector<ThreadSummary> one = + runQuery(QStringLiteral("subject:Preventivo"), + NotmuchWorker::NewestFirst, true); + QCOMPARE(one.size(), 1); + QCOMPARE(one.at(0).recipients, QStringLiteral("Mario Rossi")); + + // The comma-inside-a-display-name case, end to end through the worker + // rather than only against recipientSummary(): the header survives being + // written to a real maildir, indexed, and read back out of notmuch. + const QVector<ThreadSummary> many = + runQuery(QStringLiteral("subject:Riunione"), + NotmuchWorker::NewestFirst, true); + QCOMPARE(many.size(), 1); + + const QString summary = many.at(0).recipients; + QVERIFY2(summary.startsWith(QStringLiteral("Rossi, Mario")), + qPrintable(QStringLiteral("lost the quoted display name: %1") + .arg(summary))); + QVERIFY2(summary.endsWith(QStringLiteral("+1")), + qPrintable(QStringLiteral("three recipients did not collapse to " + "two plus one: %1").arg(summary))); +} + +void TestNotmuchWorker::recipientsCrossAQueuedCall() +{ + // The trap CLAUDE.md records for SortOrder, in the shape it takes for this + // argument. A bool is a registered metatype already, so this cannot fail + // the way an unregistered enum would, and the test exists to prove that + // rather than to assume it: the flag arriving as a default-constructed + // false would silently give an empty recipients column and nothing else. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + + QVERIFY(QMetaObject::invokeMethod( + &worker, "runQuery", Qt::DirectConnection, + Q_ARG(QString, QStringLiteral("subject:Preventivo")), + Q_ARG(quint64, 1), + Q_ARG(NotmuchWorker::SortOrder, NotmuchWorker::NewestFirst), + Q_ARG(bool, true))); + + QVector<ThreadSummary> all; + for (const QList<QVariant> &args : ready) + all += args.at(0).value<QVector<ThreadSummary>>(); + + QCOMPARE(all.size(), 1); + QVERIFY2(!all.at(0).recipients.isEmpty(), + "the recipients flag was dropped crossing invokeMethod"); +} + void TestNotmuchWorker::requestCountsAnswersOneCountPerQuery() { NotmuchWorker worker(m_fixture.configPath()); @@ -626,7 +764,7 @@ void TestNotmuchWorker::requestCountsAnswersOneCountPerQuery() // Threads, not messages: thread A holds two messages and must count once, // which is the number the pane's "N in inbox" line claims to be showing. const QVector<int> counts = spy.at(0).at(0).value<QVector<int>>(); - QCOMPARE(counts, QVector<int>({ 1, 3, 0 })); + QCOMPARE(counts, QVector<int>({ 1, 5, 0 })); } void TestNotmuchWorker::requestCountsKeepsPositionOnAnInvalidQuery() @@ -656,7 +794,7 @@ void TestNotmuchWorker::requestCountsKeepsPositionOnAnInvalidQuery() // The queries either side keep their own answers, which is the property // the pane depends on. QCOMPARE(counts.at(0), 1); - QCOMPARE(counts.at(2), 3); + QCOMPARE(counts.at(2), 5); } void TestNotmuchWorker::requestDatabaseStatsCountsMessagesNotThreads() @@ -676,8 +814,8 @@ void TestNotmuchWorker::requestDatabaseStatsCountsMessagesNotThreads() // one counts messages, which is what a user means by "how much mail". A // reimplementation that reused the thread count would report 3 here and be // confidently wrong under the label "messages". - QCOMPARE(stats.messages, 4); - QCOMPARE(stats.threads, 3); + QCOMPARE(stats.messages, 6); + QCOMPARE(stats.threads, 5); QVERIFY2(stats.messages != stats.threads, "messages and threads are equal, so this fixture cannot prove the " "two counts are distinct: add a reply to it"); diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 947a6b3..1fb8a1f 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -81,6 +81,9 @@ private slots: void reconcileWithAnIdenticalResultChangesNothing(); void reconcileMovesAThreadBumpedByANewReply(); void reconcileKeepsAMovedRowsPersistentIndex(); + void flatModeOffersNoExpanderAndNoReplyCount(); + void flatModeIsOffByDefaultAndReversible(); + void recipientsReplaceTheSenderWhenPresent(); }; static ThreadSummary makeThread(const QString &id, const QString &subject) @@ -1420,5 +1423,106 @@ void TestThreadListModel::reconcileKeepsAMovedRowsPersistentIndex() QCOMPARE(moved.row(), 2); } +void TestThreadListModel::flatModeOffersNoExpanderAndNoReplyCount() +{ + // A sent message lives on its own: the user's mental model of "what I sent" + // is a list, not a set of conversations, and a thread pulled in whole shows + // the replies they received under a view that claims to be their outbox. + // + // Deliberately not a second model or a filtered query. The expander is + // driven by hasChildren() and the card's count by ReplyCountRole, both + // already here, so flat mode is those two answering differently. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("Subject")) }); + + const QModelIndex thread = model.index(0, 0); + QVERIFY(thread.isValid()); + + // The tree shape, before anything is turned off. Guards the assertions + // below: a test whose subject was already flat would pass either way. + QVERIFY2(model.hasChildren(thread), + "the fixture thread is not expandable, so this proves nothing"); + QCOMPARE(model.data(thread, ThreadListModel::ReplyCountRole).toInt(), 1); + + model.setFlatMode(true); + + QVERIFY2(!model.hasChildren(thread), + "a flat list still offered an expander"); + QCOMPARE(model.data(thread, ThreadListModel::ReplyCountRole).toInt(), 0); + + // rowCount has to agree, or the view draws an expander it cannot open, or + // opens onto rows the card said were not there. + QCOMPARE(model.rowCount(thread), 0); + + // The thread itself is still a row. Flat means one row per thread, not + // fewer threads. + QCOMPARE(model.rowCount(), 1); +} + +void TestThreadListModel::flatModeIsOffByDefaultAndReversible() +{ + // The whole condition the user set for this feature: it must not leak into + // any other view. Off by default is what guarantees that, and returning to + // false has to restore the tree rather than leaving the model flattened + // for the next query. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("Subject")) }); + + const QModelIndex thread = model.index(0, 0); + QVERIFY2(model.hasChildren(thread), + "a fresh model is flat, so every ordinary view lost its replies"); + + model.setFlatMode(true); + QVERIFY(!model.hasChildren(thread)); + + model.setFlatMode(false); + QVERIFY2(model.hasChildren(thread), + "leaving flat mode did not restore the tree"); + QCOMPARE(model.data(thread, ThreadListModel::ReplyCountRole).toInt(), 1); +} + +void TestThreadListModel::recipientsReplaceTheSenderWhenPresent() +{ + // In a Sent view the sender is the user on every row, so the card shows + // who it went TO instead. One role, so the delegate needs no branch and + // cannot disagree with the model about which name a row is showing. + ThreadListModel model; + + ThreadSummary sent = makeThread(QStringLiteral("t1"), + QStringLiteral("Preventivo")); + sent.authors = QStringLiteral("You"); + sent.recipients = QStringLiteral("Mario Rossi"); + + // No recipients: an ordinary view, where authors is the answer. Same + // fixture otherwise, so the difference is the field and nothing else. + ThreadSummary received = makeThread(QStringLiteral("t2"), + QStringLiteral("Newsletter")); + received.authors = QStringLiteral("Carol"); + + model.appendBatch({ sent, received }); + + QCOMPARE(model.data(model.index(0, 0), + ThreadListModel::SendersRole).toString(), + QStringLiteral("Mario Rossi")); + QCOMPARE(model.data(model.index(1, 0), + ThreadListModel::SendersRole).toString(), + QStringLiteral("Carol")); + + // A thread whose To could not be parsed falls back rather than showing an + // empty name. The fold returns an empty string for a malformed or absent + // header, and a blank where a sender belongs reads as a rendering fault. + ThreadSummary unparseable = makeThread(QStringLiteral("t3"), + QStringLiteral("Broken")); + unparseable.authors = QStringLiteral("You"); + unparseable.recipients = QString(); + model.appendBatch({ unparseable }); + + QCOMPARE(model.data(model.index(2, 0), + ThreadListModel::SendersRole).toString(), + QStringLiteral("You")); +} + QTEST_MAIN(TestThreadListModel) #include "test_threadlistmodel.moc" |
