diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_cardlayout.cpp | 383 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 1059 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 140 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 681 |
5 files changed, 2021 insertions, 243 deletions
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a7bb670..4ecc04d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -41,6 +41,7 @@ add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) +add_qtmaildir_test(cardlayout) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(syncmonitor) diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp new file mode 100644 index 0000000..fdc18bf --- /dev/null +++ b/tests/test_cardlayout.cpp @@ -0,0 +1,383 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "cardlayout.h" + +#include <QFont> +#include <QLocale> +#include <QTest> + +class TestCardLayout : public QObject +{ + Q_OBJECT + +private slots: + void everyCardIsTheSameHeight(); + void threeLinesStackWithoutOverlapping(); + void replyIndentsByDepth(); + void aDepthZeroReplyStillIndents(); + void indentStopsAtTheCap(); + void expanderSitsOnTheSecondLine(); + void expanderIsEmptyWithoutReplies(); + void theExpanderReadsAsAPillWithAWord(); + void dateIsFlushRight(); + void threadCardCarriesAnAccentBar(); + void replyCardCarriesNoAccentBar(); + void theDateFitsWhenTheCardIsBold(); + void theDateFollowsTheSystemLocale(); +}; + +namespace { + +CardLayout::Input threadInput() +{ + CardLayout::Input in; + in.isMessage = false; + in.depth = 0; + in.replyCount = 3; + return in; +} + +CardLayout::Input replyInput(int depth) +{ + CardLayout::Input in; + in.isMessage = true; + in.depth = depth; + in.replyCount = 0; + return in; +} + +} // namespace + +void TestCardLayout::everyCardIsTheSameHeight() +{ + const QFont font; + const int thread = CardLayout::heightFor(font); + + // The uniform height is the whole reason setUniformRowHeights(true) + // survives this design, so it is asserted directly rather than inferred + // from two cards happening to look alike. + // + // Note what is NOT varied here: the tag list. CardLayout reserves line 3 + // unconditionally and never sees the tags, which is exactly the property + // being asserted. A version of this test that passed a tag list in would + // be testing a parameter that does not exist. + const CardLayout threadCard = + CardLayout::compute(threadInput(), QRect(0, 0, 400, thread), font); + const CardLayout deepReply = + CardLayout::compute(replyInput(3), QRect(0, 0, 400, thread), font); + CardLayout::Input noRepliesIn = threadInput(); + noRepliesIn.replyCount = 0; + const CardLayout noReplies = + CardLayout::compute(noRepliesIn, QRect(0, 0, 400, thread), font); + + QCOMPARE(threadCard.totalHeight, thread); + QCOMPARE(deepReply.totalHeight, thread); + QCOMPARE(noReplies.totalHeight, thread); + + // The third line exists on every card, including one with nothing to put + // there. That blank band is the cost the uniform height was bought with. + QCOMPARE(noReplies.tagRect.height(), threadCard.tagRect.height()); +} + +void TestCardLayout::threeLinesStackWithoutOverlapping() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + + QVERIFY(card.senderRect.height() > 0); + QVERIFY(card.subjectRect.height() > 0); + QVERIFY(card.tagRect.height() > 0); + + // Guard: these must actually be three stacked bands. A layout that + // collapsed them all to the same rect would satisfy any assertion that + // only checked they exist. + QVERIFY(card.senderRect.bottom() <= card.subjectRect.top()); + QVERIFY(card.subjectRect.bottom() <= card.tagRect.top()); + QVERIFY(card.tagRect.bottom() <= h); +} + +void TestCardLayout::replyIndentsByDepth() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + const CardLayout root = CardLayout::compute(threadInput(), rect, font); + const CardLayout d1 = CardLayout::compute(replyInput(1), rect, font); + const CardLayout d2 = CardLayout::compute(replyInput(2), rect, font); + + QVERIFY(d1.contentLeft > root.contentLeft); + QVERIFY(d2.contentLeft > d1.contentLeft); + + // One spine per depth level, so the count is the depth itself. + QCOMPARE(root.spines.size(), 0); + QCOMPARE(d1.spines.size(), 1); + QCOMPARE(d2.spines.size(), 2); + + // Each spine runs the full height of the card, which is what makes an + // expansion read as one continuous block rather than as dashes. + for (const QRect &spine : d2.spines) { + QCOMPARE(spine.top(), rect.top()); + QCOMPARE(spine.bottom(), rect.bottom()); + } +} + +void TestCardLayout::aDepthZeroReplyStillIndents() +{ + // A reply in a FLAT thread carries depth 0, because notmuch reports every + // message of a thread with no usable In-Reply-To as a top-level message. + // It is still a reply: it is a child row under the root card, and it has + // to read as one. + // + // Treating depth 0 as "no nesting" left those replies flush against their + // thread with no spine, while a nested thread's replies indented normally, + // so the list showed two different shapes for the same relationship. + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + CardLayout::Input flatReply; + flatReply.isMessage = true; + flatReply.depth = 0; + + const CardLayout root = CardLayout::compute(threadInput(), rect, font); + const CardLayout reply = CardLayout::compute(flatReply, rect, font); + + QVERIFY2(reply.contentLeft > root.contentLeft, + "a depth-0 reply sits flush with its thread, so a flat thread's " + "replies look like more threads"); + QVERIFY2(!reply.spines.isEmpty(), + "a depth-0 reply has no spine, so nothing joins it to the thread " + "above it"); + + // And it lands at the same place a depth-1 reply does: the two are the + // same relationship and notmuch's numbering is the only difference. + const CardLayout nested = CardLayout::compute(replyInput(1), rect, font); + QCOMPARE(reply.contentLeft, nested.contentLeft); + QCOMPARE(reply.spines.size(), nested.spines.size()); +} + +void TestCardLayout::indentStopsAtTheCap() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + const CardLayout d4 = CardLayout::compute(replyInput(4), rect, font); + const CardLayout d5 = CardLayout::compute(replyInput(5), rect, font); + const CardLayout d9 = CardLayout::compute(replyInput(9), rect, font); + + QCOMPARE(d5.contentLeft, d4.contentLeft); + QCOMPARE(d9.contentLeft, d4.contentLeft); + QCOMPARE(d5.spines.size(), d4.spines.size()); + QCOMPARE(d9.spines.size(), d4.spines.size()); + + // Guard: the cap must not be so low that it has already bitten at depth 3, + // which would make the three assertions above true for the wrong reason. + const CardLayout d3 = CardLayout::compute(replyInput(3), rect, font); + QVERIFY(d3.contentLeft < d4.contentLeft); +} + +void TestCardLayout::expanderSitsOnTheSecondLine() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + + QVERIFY(!card.expanderRect.isEmpty()); + // It is the reply count, so it belongs on the line the reply count is on. + QVERIFY(card.expanderRect.top() >= card.subjectRect.top()); + QVERIFY(card.expanderRect.bottom() <= card.subjectRect.bottom()); + // And it is on the right, where the count is drawn, not in a left gutter. + QVERIFY(card.expanderRect.left() > 400 / 2); +} + +void TestCardLayout::expanderIsEmptyWithoutReplies() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + CardLayout::Input in = threadInput(); + in.replyCount = 0; + + const CardLayout card = CardLayout::compute(in, QRect(0, 0, 400, h), font); + QVERIFY(card.expanderRect.isEmpty()); +} + +void TestCardLayout::theExpanderReadsAsAPillWithAWord() +{ + // A bare "3" beside the subject reads as an unexplained number and gives + // no hint that it can be clicked. The label carries the word, and the rect + // carries padding for the pill drawn behind it. + QCOMPARE(CardLayout::expanderLabel(3, false), + QStringLiteral("\u25b8 3 replies")); + QCOMPARE(CardLayout::expanderLabel(3, true), + QStringLiteral("\u25be 3 replies")); + + // Singular, because "1 replies" is the kind of detail that makes an + // interface look unfinished. + QCOMPARE(CardLayout::expanderLabel(1, false), + QStringLiteral("\u25b8 1 reply")); + + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + const QFontMetrics small(CardLayout::smallFont(font)); + + // The rect must hold the label AND its padding, or the pill's background + // is narrower than the text sitting on it. + QVERIFY2(card.expanderRect.width() + >= small.horizontalAdvance(CardLayout::expanderLabel(3, false)) + + CardLayout::kPillPaddingX * 2, + "the expander rect is too narrow for its own label and padding"); + + // And it must NOT change width when the card opens: a pill that resized on + // click would shift the subject's elision under the pointer. + CardLayout::Input open = threadInput(); + const CardLayout expanded = + CardLayout::compute(open, QRect(0, 0, 400, h), font); + QCOMPARE(expanded.expanderRect.width(), card.expanderRect.width()); +} + +void TestCardLayout::dateIsFlushRight() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + const CardLayout card = CardLayout::compute(threadInput(), rect, font); + + // Compared as exclusive edges. QRect::right() is inclusive (left + width - + // 1), so asserting card.dateRect.right() == rect.right() - kPaddingX + // demands a gap of kPaddingX - 1 pixels and is off by one against the + // padding the constant names. + QCOMPARE(card.dateRect.right() + 1, rect.right() + 1 - CardLayout::kPaddingX); + // The sender must stop before the date starts, or a long sender overwrites + // it. This is the assertion that fails if the two are laid out + // independently. + QVERIFY(card.senderRect.right() <= card.dateRect.left()); +} + +void TestCardLayout::threadCardCarriesAnAccentBar() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + const CardLayout card = CardLayout::compute(threadInput(), rect, font); + + QCOMPARE(card.accentRect.left(), rect.left()); + QCOMPARE(card.accentRect.width(), CardLayout::kAccentWidth); + // Full height, so a run of cards from one account reads as a continuous + // edge rather than as dashes. + QCOMPARE(card.accentRect.top(), rect.top()); + QCOMPARE(card.accentRect.bottom(), rect.bottom()); + + // Nothing may be drawn on top of the colour. + QVERIFY(card.contentLeft >= card.accentRect.right()); +} + +void TestCardLayout::replyCardCarriesNoAccentBar() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout reply = + CardLayout::compute(replyInput(1), QRect(0, 0, 400, h), font); + + // A reply's account is its thread's, stated once at the head. The spine + // carries the accent instead, so the gutter never holds two lines. + QVERIFY(reply.accentRect.isEmpty()); + QCOMPARE(reply.spines.size(), 1); +} + +void TestCardLayout::theDateFollowsTheSystemLocale() +{ + const QDateTime when(QDate(2025, 8, 10), QTime(6, 26)); + + // The system locale's own rendering, whatever it is. Asserting a specific + // string would only restate the hardcoded pattern this replaced, and would + // fail on any machine but the one that wrote it. + QCOMPARE(CardLayout::formatDate(when), + QLocale::system().toString(when, QLocale::ShortFormat)); + + // The specific fault: an ISO-looking pattern on a desktop that does not + // use one. Guarded so this test says nothing on a locale that genuinely + // formats that way. + if (QLocale::system().toString(when, QLocale::ShortFormat) + != QStringLiteral("2025-08-10 06:26")) { + QVERIFY2(CardLayout::formatDate(when) + != QStringLiteral("2025-08-10 06:26"), + "the date is hardcoded to yyyy-MM-dd hh:mm rather than " + "following the desktop's locale"); + } + + // And the reserved width has to follow the same formatter, or a locale + // whose dates are longer clips them exactly as the bold font did. + QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + QFont bold = font; + bold.setBold(true); + QVERIFY2(card.dateRect.width() + >= QFontMetrics(bold).horizontalAdvance( + CardLayout::formatDate(when)), + "the reserved date width is narrower than this locale's own " + "formatting of a date"); +} + +void TestCardLayout::theDateFitsWhenTheCardIsBold() +{ + // An UNREAD card draws BOLD, and bold is wider. The layout is computed from + // option.font, which is the view's regular font, while the text is painted + // with the font initStyleOption resolved from the model's Qt::FontRole. So + // a date measured regular and drawn bold overflows its rect: measured at + // 154px reserved against 170px needed, which clipped the leading digit of + // the year off every unread card. + // + // The fix is in the layout rather than in the delegate: it reserves the + // BOLD width whatever font it is handed, so the two can never disagree. + QFont regular; + regular.setBold(false); + QFont bold = regular; + bold.setBold(true); + + const QString sample = QStringLiteral("2025-08-10 06:26"); + const int boldWidth = QFontMetrics(bold).horizontalAdvance(sample); + + // Guard: bold must actually be wider here, or this asserts nothing. + QVERIFY2(boldWidth > QFontMetrics(regular).horizontalAdvance(sample), + "bold is not wider than regular in this environment, so this test " + "cannot detect the overflow it exists for"); + + const int h = CardLayout::heightFor(regular); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), regular); + + QVERIFY2(card.dateRect.width() >= boldWidth, + qPrintable(QStringLiteral("a layout computed from the REGULAR " + "font reserves %1px, and the date needs " + "%2px when the card draws bold") + .arg(card.dateRect.width()) + .arg(boldWidth))); +} + +QTEST_MAIN(TestCardLayout) +#include "test_cardlayout.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 5031ead..922705c 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -38,6 +38,7 @@ #include <QSplitter> #include <QTableView> #include <QToolBar> +#include <QTreeView> #include <QTimer> #include "config.h" @@ -45,8 +46,16 @@ #include "mainwindow.h" #include "messageview.h" #include "notmuchworker.h" +#include "carddelegate.h" +#include "cardlayout.h" + +#include <QImage> +#include <QPainter> +#include <QComboBox> +#include <QScrollBar> #include "tagchip.h" #include "threadlistmodel.h" +#include "threadlistview.h" /// MainWindow is mostly wiring, and the parts that need a real database are /// still verified manually. What is checked here is the action registry: the @@ -93,7 +102,22 @@ private slots: void anUnobservableLockTableLeavesTheSyncButtonUsable(); void theStatusBarFollowsTheSyncPhase(); void aSelectedReadThreadIsNotDimmedIntoTheHighlight(); - void thePillRowSpansTheWholeWidthNotOneColumn(); + void childRowsAreIndentedUnderTheirThread(); + void aThreadWithRepliesDrawsAVisibleExpander(); + void cardsNeverScrollSideways(); + void selectingARootCardKeepsItsThreadForMarkRead(); + void nextThreadLeavesTheLastReply(); + void altDownSkipsReplies(); + void bothThreadStepBindingsReachTheAction(); + void sortChoiceSurvivesRestart(); + void accountEntriesCarryTheirColour(); + void replyRowsKeepTheirTextUnderTheThreadLine(); + void clickingTheExpanderTogglesTheThread(); + void selectingAMessageRowTargetsThatMessageNotItsThread(); + void selectingAThreadRowNamesHowManyMessagesItStandsFor(); + void selectingAMessageRowReportsNoBulkCount(); + void anActionOnAThreadRowSaysItHitTheWholeThread(); + void anActionOnAMessageRowTagsThatMessageNotTheThread(); void markAllReadIsDisabledUntilTheQueryFinishes(); void markAllReadActsOnEveryRowAndUndoesInOneStep(); void markAllReadDoesNothingWhenNothingIsUnread(); @@ -439,11 +463,12 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() window.close(); } - // Forge a state file from an older layout: same blob, wrong column count. + // Forge a state file from the five-column layout. Nothing reads these keys + // any more, and that is exactly what must be verified: a blob saved by an + // older version has to be ignored rather than applied to a one-column view. { QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat); - state.setValue(QStringLiteral("threadlist/columns"), - int(ThreadListModel::ColumnCount) - 1); + state.setValue(QStringLiteral("threadlist/columns"), 5); state.setValue(QStringLiteral("threadlist/header"), QByteArray("not a header this model could have saved")); } @@ -452,11 +477,9 @@ void TestMainWindow::headerStateFromADifferentColumnLayoutIsDiscarded() const Config config; MainWindow reopened(config); - auto *view = reopened.findChild<QTableView *>(); + auto *view = reopened.findChild<QTreeView *>(); QVERIFY(view); - QCOMPARE(view->columnWidth(ThreadListModel::AttachmentColumn), 28); - QCOMPARE(view->columnWidth(ThreadListModel::DateColumn), 130); - QCOMPARE(view->columnWidth(ThreadListModel::SubjectColumn), 520); + QCOMPARE(view->model()->columnCount(), 1); QFile::remove(MainWindow::uiStatePath()); QStandardPaths::setTestModeEnabled(false); @@ -503,6 +526,27 @@ void TestMainWindow::returnInTheQueryBarRunsTheQueryNotOpenThread() QVERIFY(!actionFired); } +/// Selects a top-level THREAD row, replacing QTableView::selectRow which a +/// QTreeView does not have. +/// +/// Not merely a rename: setCurrentIndex alone leaves the selection model empty, +/// and select() alone leaves current invalid, so every test asserting on either +/// would break in a different way. Both are set here, exactly as +/// QTableView::selectRow did. +static void selectThreadRow(QTreeView *view, int row) +{ + const QModelIndex index = view->model()->index(row, 0, QModelIndex()); + view->selectionModel()->select( + index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(index); +} + +/// The height of a top-level row, replacing QTableView::rowHeight(int). +static int threadRowHeight(QTreeView *view, int row) +{ + return view->visualRect(view->model()->index(row, 0, QModelIndex())).height(); +} + /// A thread summary carrying the tags a test needs. Enough to drive selection; /// nothing here touches a database. static ThreadSummary makeThread(const QString &id, const QStringList &tags) @@ -515,81 +559,870 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags) return thread; } -void TestMainWindow::thePillRowSpansTheWholeWidthNotOneColumn() +void TestMainWindow::childRowsAreIndentedUnderTheirThread() +{ + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY2(view, "the thread list is not a QTreeView, so it cannot indent"); + + // With an account tag, so the thread row draws the chip that a reply row + // does not. That asymmetry is the whole reason the indent has to be wide, + // and a test against an untagged thread never sees it. + model->appendBatch({ makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }) }); + + MessageNode first; + first.messageId = QStringLiteral("m0@example.org"); + first.threadId = QStringLiteral("t1"); + first.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.from = QStringLiteral("A Replier <replier@example.org>"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { first, reply }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + view->expand(root); + QApplication::processEvents(); + + // One column, and setIndentation(0): Qt indents nothing, CardLayout draws + // the indent itself. So visualRect reports the SAME rect for a thread and + // its reply, and the indent has to be read off the layout rather than off + // the geometry. That is the trap CLAUDE.md records in reverse: there, + // visualRect reported an indent the text did not have; here it reports + // none while the text is indented. + const QModelIndex rootCell = model->index(0, 0, QModelIndex()); + const QModelIndex child = model->index(0, 0, root); + QVERIFY(child.isValid()); + + // Guards before the claim: a probe that cannot see both rows can report + // anything it likes about their relative position. + QVERIFY2(view->visualRect(rootCell).height() > 0, + "the thread row has no height, so nothing about it is measurable"); + QVERIFY2(view->visualRect(child).height() > 0, + "the reply row has no height: it is collapsed or off-screen, and " + "an indent test against it would pass without drawing anything"); + + // The indent is NOT in the geometry. setIndentation(0) means visualRect + // reports the same left edge for both rows, deliberately: CardLayout draws + // the indent inside the card's own rect. Asserting on visualRect here + // would fail against a perfectly indented list, which is the mirror of the + // trap CLAUDE.md records for item 20, where visualRect reported an indent + // the text did not have. + // + // So the real property, as before: where the TEXT lands. It is read off + // the layout, which is what the delegate paints from. + CardLayout::Input threadIn; + threadIn.isMessage = false; + threadIn.depth = 0; + CardLayout::Input replyIn; + replyIn.isMessage = true; + replyIn.depth = + model->data(child, ThreadListModel::MessageDepthRole).toInt(); + QVERIFY2(replyIn.depth > 0, + "the reply reports depth 0, so there is no nesting to measure"); + + const QRect rect = view->visualRect(rootCell); + const CardLayout threadCard = + CardLayout::compute(threadIn, rect, view->font()); + const CardLayout replyCard = + CardLayout::compute(replyIn, rect, view->font()); + + QVERIFY2(replyCard.contentLeft > threadCard.contentLeft, + qPrintable(QStringLiteral("the reply's text starts at x=%1, not " + "right of the thread's at x=%2: the " + "nesting is invisible") + .arg(replyCard.contentLeft) + .arg(threadCard.contentLeft))); + + // And the spine that makes the nesting read as one block rather than as an + // arbitrary offset. + QCOMPARE(replyCard.spines.size(), replyIn.depth); +} + +namespace { + +/// Two threads, the first with one reply, expanded. The shared fixture for the +/// two navigation tests below. +struct NavFixture +{ + QTreeView *view = nullptr; + ThreadListModel *model = nullptr; + QModelIndex root; + QModelIndex reply; +}; + +NavFixture buildNavFixture(MainWindow &window) +{ + NavFixture f; + f.view = window.findChild<QTreeView *>(); + f.model = window.findChild<ThreadListModel *>(); + + // unread, so a selection arms the mark-read timer: scheduleMarkRead() + // returns early for a thread that is already read, and a fixture without + // it would make a mark-read assertion pass for the wrong reason. + ThreadSummary first = makeThread(QStringLiteral("T1"), + QStringList{ QStringLiteral("inbox"), + QStringLiteral("unread") }); + first.totalCount = 2; + ThreadSummary second = makeThread(QStringLiteral("T2"), + QStringList{ QStringLiteral("inbox") }); + second.totalCount = 1; + f.model->appendBatch({ first, second }); + + MessageNode rootNode; + rootNode.messageId = QStringLiteral("M1"); + rootNode.threadId = QStringLiteral("T1"); + rootNode.depth = 0; + MessageNode replyNode; + replyNode.messageId = QStringLiteral("M2"); + replyNode.threadId = QStringLiteral("T1"); + replyNode.depth = 1; + f.model->setThreadMessages(QStringLiteral("T1"), { rootNode, replyNode }); + + f.root = f.model->index(0, 0); + f.view->expand(f.root); + f.reply = f.model->index(0, 0, f.root); + return f; +} + +} // namespace + +void TestMainWindow::selectingARootCardKeepsItsThreadForMarkRead() +{ + // A root card is BOTH a message and a thread: it renders the thread's + // first message, and it is still the thread that gets marked read and + // repainted on a tag change. The message-row path deliberately clears the + // current thread id; doing that here too would silently disable mark-read + // and the tag-change repaint for every thread root in the list. + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const NavFixture f = buildNavFixture(window); + f.view->setCurrentIndex(f.root); + QApplication::processEvents(); + + // Guard: the fixture loads replies, so the root knows its own message and + // the branch under test is the one that runs. + QVERIFY2(!f.model->data(f.root, ThreadListModel::MessageIdRole) + .toString().isEmpty(), + "the root card does not know its first message, so this exercises " + "the fallback rather than the path it is written for"); + + // A mark-read timer armed for the thread is what proves the thread id + // survived: scheduleMarkRead() is only reached on the thread-row path. + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + QVERIFY2(timer->isActive(), + "no mark-read timer for a selected root card: its thread id was " + "cleared along with the switch to rendering one message"); +} + +void TestMainWindow::nextThreadLeavesTheLastReply() +{ + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const NavFixture f = buildNavFixture(window); + QVERIFY(f.reply.isValid()); + QVERIFY2(f.view->isExpanded(f.root), + "the thread is collapsed, so this test would arrow down a flat " + "list and pass against the bug it exists to catch"); + + f.view->setCurrentIndex(f.reply); + + // The defect (item 60): selectRow(current.row() + 1) asked for row 1 UNDER + // T1, which does not exist, so the action did nothing at all. + window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger(); + + QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole) + .toString(), + QStringLiteral("T2")); +} + +void TestMainWindow::altDownSkipsReplies() +{ + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const NavFixture f = buildNavFixture(window); + QVERIFY(f.view->isExpanded(f.root)); + + // From the thread ROOT with its replies showing: one step must land on the + // next THREAD, not on the first reply. That is what makes the action mean + // thread-to-thread while plain Up/Down still steps message-to-message. + f.view->setCurrentIndex(f.root); + window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger(); + + QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole) + .toString(), + QStringLiteral("T2")); + QVERIFY(!f.view->currentIndex().data(ThreadListModel::IsMessageRole) + .toBool()); + + // And back, which is the mirror case the old arithmetic also failed. + window.findChild<QAction *>(QStringLiteral("prev_thread"))->trigger(); + QCOMPARE(f.view->currentIndex().data(ThreadListModel::ThreadIdRole) + .toString(), + QStringLiteral("T1")); + QVERIFY(!f.view->currentIndex().data(ThreadListModel::IsMessageRole) + .toBool()); +} + +void TestMainWindow::bothThreadStepBindingsReachTheAction() +{ + const Config config; + MainWindow window(config); + + // Two bindings per action, which needs setShortcuts rather than + // setShortcut: Ctrl+J/K for a neomutt hand, Alt+Up/Down for a mouse one. + // Alt because Shift+arrows is QTreeView's built-in extend-selection that + // multi-row tagging depends on, and a bare arrow cannot be a window + // shortcut without breaking every text field in the window. + for (const auto &pair : { std::pair<const char *, const char *>{ + "next_thread", "Alt+Down" }, + { "prev_thread", "Alt+Up" } }) { + auto *action = + window.findChild<QAction *>(QString::fromLatin1(pair.first)); + QVERIFY2(action, pair.first); + const QList<QKeySequence> shortcuts = action->shortcuts(); + QVERIFY2(shortcuts.size() >= 2, + qPrintable(QStringLiteral("%1 carries %2 shortcut(s), so the " + "second binding is unreachable") + .arg(QString::fromLatin1(pair.first)) + .arg(shortcuts.size()))); + QVERIFY2(shortcuts.contains( + QKeySequence(QString::fromLatin1(pair.second))), + pair.second); + } +} + +void TestMainWindow::sortChoiceSurvivesRestart() +{ + QStandardPaths::setTestModeEnabled(true); + QFile::remove(MainWindow::uiStatePath()); + + { + const Config config; + MainWindow window(config); + auto *sort = window.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QVERIFY(sort); + QCOMPARE(sort->count(), 2); + QCOMPARE(sort->currentIndex(), 0); // Newest first by default. + sort->setCurrentIndex(1); + window.close(); + } + + const Config config; + MainWindow second(config); + auto *sort = second.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QVERIFY(sort); + QCOMPARE(sort->currentIndex(), 1); + + // A stale or hand-edited file can hold anything, which is the lesson item + // 58 recorded: an out-of-range value must fall back rather than select a + // row that does not exist. + { + QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat); + state.setValue(QStringLiteral("threadlist/sortOrder"), 47); + } + MainWindow third(config); + auto *thirdSort = third.findChild<QComboBox *>(QStringLiteral("sortOrder")); + QCOMPARE(thirdSort->currentIndex(), 0); + + QFile::remove(MainWindow::uiStatePath()); + QStandardPaths::setTestModeEnabled(false); +} + +void TestMainWindow::accountEntriesCarryTheirColour() +{ + // Its own config, not the environment's. Reading the real one made this + // SKIP wherever no accounts are configured, which is a test that asserts + // nothing while reporting success. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QSettings s(path, QSettings::IniFormat); + s.beginGroup(QStringLiteral("account.work")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("work")); + s.setValue(QStringLiteral("color"), QStringLiteral("#3d7fd1")); + s.endGroup(); + s.beginGroup(QStringLiteral("account.personal")); + s.setValue(QStringLiteral("maildir"), QStringLiteral("personal")); + s.endGroup(); + } + + Config config; + config.load(path); + QCOMPARE(config.accounts().size(), 2); + + MainWindow window(config); + auto *box = window.findChild<QComboBox *>(QStringLiteral("accountBox")); + QVERIFY(box); + QCOMPARE(box->count(), 3); + + // "All accounts" is not an account and carries no swatch. + QVERIFY(!box->itemData(0, Qt::DecorationRole).isValid()); + + // Every real account does, including the one with no color= key: + // colourFor() never fails, deriving a stable colour from the tag name, so + // adding an account and forgetting to colour it degrades to something + // usable rather than to nothing. + QSet<QRgb> seen; + for (int i = 1; i < box->count(); ++i) { + const QVariant swatch = box->itemData(i, Qt::DecorationRole); + QVERIFY2(swatch.isValid(), + qPrintable(QStringLiteral("account %1 carries no swatch") + .arg(box->itemText(i)))); + const QColor colour = swatch.value<QColor>(); + QVERIFY(colour.isValid()); + seen.insert(colour.rgb()); + } + + // Guard: two accounts sharing one colour would make the swatches useless + // as a key to the accent bars, and would let a broken lookup pass. + QCOMPARE(seen.size(), 2); +} + +void TestMainWindow::cardsNeverScrollSideways() +{ + const Config config; + MainWindow window(config); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + auto *view = window.findChild<ThreadListView *>(); + QVERIFY(view); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + // A long subject, so the guard below is not vacuous: this is exactly the + // content that used to make the subject column wider than the viewport. + model->appendBatch({ makeThread( + QStringLiteral("t1"), + QStringList{ QStringLiteral("inbox") }) }); + QApplication::processEvents(); + + // Item 51: clicking a row used to scroll the list sideways, because the + // subject column was wider than the viewport and auto-scroll brought the + // clicked index fully into view. A card is exactly viewport width, so + // there is nowhere to scroll to. + QVERIFY2(view->visualRect(model->index(0, 0)).height() > 0, + "no card is drawn, so there is no layout to assert about"); + QCOMPARE(view->horizontalScrollBar()->minimum(), + view->horizontalScrollBar()->maximum()); +} + +void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() { - // The pills are a row-wide strip under the cells, not content of the - // subject cell. Drawn from the subject column's delegate they stop at that - // column's edge, so a thread with several tags loses the last of them; and - // they inherit the column's left edge, which puts them under the subject - // rather than under the row. + // The expander is the ONLY thing saying a thread can be opened, and it took + // four wrong attempts to get on screen before item 53, each of which looked + // correct in code and none of which a geometry or role assertion could see. + // So this counts painted pixels. // - // The property: pills appear to the LEFT of where the subject column - // starts, which no per-cell delegate on that column could produce. + // Painted through the DELEGATE rather than through viewport()->render(). + // The viewport render returns a blank image here: CLAUDE.md records that it + // does so in several ordinary situations, and this test proved it again, + // reporting zero ink over a card the delegate demonstrably paints 2183 + // pixels into. A probe that sees nothing cannot report on anything. const Config config; MainWindow window(config); auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); - thread.tags = QStringList{ QStringLiteral("mailing-list/SBo"), - QStringLiteral("signed") }; - model->appendBatch({ thread }); + // Two threads: one with replies, one without. The second is the control, + // and without it a test that counts ink would pass on any card. + ThreadSummary withReplies = makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + withReplies.totalCount = 3; + ThreadSummary lone = makeThread( + QStringLiteral("t2"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + lone.totalCount = 1; + model->appendBatch({ withReplies, lone }); + + const QModelIndex first = model->index(0, 0, QModelIndex()); + const QModelIndex second = model->index(1, 0, QModelIndex()); + + // Guards: the model agrees about which thread has replies, and only that + // one is offered an expander at all. + QCOMPARE(model->data(first, ThreadListModel::ReplyCountRole).toInt(), 2); + QCOMPARE(model->data(second, ThreadListModel::ReplyCountRole).toInt(), 0); + + const QFont font = view->font(); + const int height = CardLayout::heightFor(font); + + const auto inkInExpander = [&](const QModelIndex &index) { + QImage shot(400, height, QImage::Format_ARGB32); + shot.fill(Qt::white); + QPainter painter(&shot); + QStyleOptionViewItem option; + option.rect = QRect(0, 0, 400, height); + option.font = font; + option.palette = QApplication::palette(); + option.state = QStyle::State_Enabled; + CardDelegate delegate; + delegate.paint(&painter, option, index); + painter.end(); + + const QRect rect = CardDelegate::expanderRectFor(option, index); + int found = 0; + for (int y = rect.top(); y <= rect.bottom() && y < shot.height(); ++y) { + for (int x = rect.left(); x <= rect.right() && x < shot.width(); + ++x) { + if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu) + ++found; + } + } + + // Guard on the probe itself: prove it can see the card's own text + // before trusting it about the expander. A probe that finds no ink + // anywhere reports "nothing was drawn" whatever the delegate did. + int anyInk = 0; + for (int y = 0; y < shot.height(); ++y) + for (int x = 0; x < shot.width(); ++x) + if ((shot.pixel(x, y) | 0xff000000) != 0xffffffffu) + ++anyInk; + return std::pair<int, int>(found, anyInk); + }; + + const auto [drawn, drawnAnywhere] = inkInExpander(first); + const auto [control, controlAnywhere] = inkInExpander(second); + + QVERIFY2(drawnAnywhere > 0 && controlAnywhere > 0, + "the probe finds no ink on either card, so it cannot report on " + "the expander either"); + + QVERIFY2(drawn > 12, + qPrintable(QStringLiteral("only %1 pixels in the expander's rect: " + "the reply count is clipped or painted " + "over").arg(drawn))); + QVERIFY2(control == 0, + qPrintable(QStringLiteral("a thread with no replies drew %1 " + "pixels where an expander would go") + .arg(control))); +} + +void TestMainWindow::selectingAThreadRowNamesHowManyMessagesItStandsFor() +{ + // With two kinds of row selectable, one selected row no longer says how + // much an action will touch. CLAUDE.md forbids a confirmation dialog for + // tag mutations, so the scope is made visible instead: this is the "before" + // half of that, and the count has to come from the thread's own total, not + // from whatever happens to be expanded. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + + ThreadSummary t = makeThread(QStringLiteral("t1"), {}); + t.totalCount = 7; + model->appendBatch({ t }); + + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + // Guard: nothing is expanded, so a count taken from the loaded children + // would read 0 and this test would be measuring the wrong source. + QCOMPARE(model->rowCount(model->index(0, 0, QModelIndex())), 0); + + selectThreadRow(view, 0); + QApplication::processEvents(); + + QVERIFY2(status->text().contains(QStringLiteral("7")), + qPrintable(QStringLiteral("the status bar says '%1', which does " + "not name the 7 messages the thread " + "stands for") + .arg(status->text()))); +} + +void TestMainWindow::selectingAMessageRowReportsNoBulkCount() +{ + // Reading one message is not a bulk action, so it gets no count. A message + // row reporting "1 thread selected" would be actively wrong about what an + // action would touch. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + + ThreadSummary t = makeThread(QStringLiteral("t1"), {}); + t.totalCount = 3; + model->appendBatch({ t }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t1"); + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { root, reply }); + + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex threadRow = model->index(0, 0, QModelIndex()); + view->expand(threadRow); + QApplication::processEvents(); + + const QModelIndex messageRow = model->index(0, 0, threadRow); + QVERIFY(model->isMessageRow(messageRow)); + + view->selectionModel()->select( + messageRow, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(messageRow); + QApplication::processEvents(); + + QVERIFY2(!status->text().contains(QStringLiteral("thread")), + qPrintable(QStringLiteral("a single message row reports '%1', " + "which claims a thread-wide scope it " + "does not have") + .arg(status->text()))); +} + +void TestMainWindow::anActionOnAThreadRowSaysItHitTheWholeThread() +{ + // The "after" half. Undo is the safety net this project chose over a + // confirmation dialog, and undo is only usable if the user can tell that + // something bigger than they intended just happened. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + + ThreadSummary t = makeThread(QStringLiteral("t1"), {}); + t.totalCount = 7; + model->appendBatch({ t }); + + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + selectThreadRow(view, 0); + QApplication::processEvents(); + + auto *archive = window.findChild<QAction *>(QStringLiteral("archive")); + QVERIFY2(archive, "no archive action to trigger"); + archive->trigger(); + + // Read BEFORE processEvents, deliberately. This binary has no worker + // (backlog item 36), so the queued applyTagsToThreads reaches a throwaway + // database that has never heard of thread t1 and answers with + // errorOccurred, which overwrites the status bar. Draining the event loop + // here would assert on that error rather than on the scope message, and + // the test would fail against correct code. + const QString message = status->text(); + + QVERIFY2(message.contains(QStringLiteral("7")), + qPrintable(QStringLiteral("after archiving a 7-message thread the " + "status bar says '%1', which does not " + "say how much was touched") + .arg(message))); + + // And it must say the whole thread went, not merely how many messages: the + // count alone does not distinguish "7 messages you picked" from "7 messages + // because you picked their thread". + QVERIFY2(message.contains(QStringLiteral("whole thread")), + qPrintable(QStringLiteral("the status bar says '%1', which does " + "not say the action took the whole " + "thread") + .arg(message))); +} + +void TestMainWindow::anActionOnAMessageRowTagsThatMessageNotTheThread() +{ + // The routing itself, which nothing else here can see. A message row sent + // down the THREAD path produces the same undo depth and the same status + // text while tagging every sibling in the conversation: a mutation that did + // exactly that passed the entire suite, so this test exists because that + // gap was found rather than because the path looked risky. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread(QStringLiteral("t1"), {}); + t.totalCount = 3; + model->appendBatch({ t }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t1"); + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { root, reply }); + + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex threadRow = model->index(0, 0, QModelIndex()); + view->expand(threadRow); + QApplication::processEvents(); + + const QModelIndex messageRow = model->index(0, 0, threadRow); + QVERIFY(model->isMessageRow(messageRow)); + + view->selectionModel()->select( + messageRow, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(messageRow); + QApplication::processEvents(); + + auto *archive = window.findChild<QAction *>(QStringLiteral("archive")); + QVERIFY(archive); + archive->trigger(); + + // The change must carry the MESSAGE id and no thread id. Sent as a thread + // id it would archive the root and every other reply along with it. + QCOMPARE(window.pendingMessageIdsForTesting(), + QStringList{ QStringLiteral("m1@example.org") }); + QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(), + qPrintable(QStringLiteral("the action was sent for thread(s) %1: a " + "message row must not tag its siblings") + .arg(window.pendingThreadIdsForTesting() + .join(QStringLiteral(", "))))); + + // And it is undoable, on its own terms rather than the thread's. + QCOMPARE(window.undoDepthForTesting(), 1); +} + +void TestMainWindow::selectingAMessageRowTargetsThatMessageNotItsThread() +{ + // test_mainwindow has no worker (backlog item 36), so this cannot assert on + // what the pane renders. What it CAN assert is the decision the UI makes: + // a message row must stop tracking a current thread, or a reply arriving + // for either kind of selection cannot tell which one it belongs to. + // + // The trap this covers is specific. threadAt() takes a TOP-LEVEL row + // number, and a child's row number indexes its siblings, so handing a + // message row's number to it loads whichever thread happens to sit at that + // position in the list. Row 0 under a thread is a plausible-looking wrong + // answer, which is why the fixture puts the reply under the SECOND thread. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary first = makeThread(QStringLiteral("t1"), {}); + ThreadSummary second = makeThread(QStringLiteral("t2"), {}); + second.totalCount = 2; + model->appendBatch({ first, second }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t2"); + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t2"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t2"), { root, reply }); window.resize(1400, 300); window.show(); QVERIFY(QTest::qWaitForWindowExposed(&window)); + + // Start on a thread row, so the transition to a message row is what is + // being observed rather than the initial state. + const QModelIndex threadRow = model->index(1, 0, QModelIndex()); + selectThreadRow(view, 1); QApplication::processEvents(); + QCOMPARE(window.currentThreadId(), QStringLiteral("t2")); - const int subjectLeft = - view->columnViewportPosition(ThreadListModel::SubjectColumn); - QVERIFY2(subjectLeft > 40, - qPrintable(QStringLiteral("the subject column starts at x=%1, too " - "close to the left edge to tell a " - "row-wide strip from a subject-cell one") - .arg(subjectLeft))); - // The strip must have somewhere to paint that the subject cell does not - // reach, or this test cannot fail. - QVERIFY2(subjectLeft < view->viewport()->width(), - qPrintable(QStringLiteral("the subject column is off-screen " - "(x=%1, viewport %2), so nothing it " - "draws is measurable") - .arg(subjectLeft) - .arg(view->viewport()->width()))); + view->expand(threadRow); + QApplication::processEvents(); + + const QModelIndex messageRow = model->index(0, 0, threadRow); + QVERIFY(messageRow.isValid()); + QVERIFY2(model->isMessageRow(messageRow), + "the fixture did not produce a message row, so this test would " + "assert nothing about one"); + + view->selectionModel()->select( + messageRow, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(messageRow); + QApplication::processEvents(); + + // The thread is no longer what the pane is about. Left set, a late + // loadThread reply would repaint the whole conversation over the single + // message the user asked for. + QVERIFY2(window.currentThreadId().isEmpty(), + qPrintable(QStringLiteral("selecting a reply left the current " + "thread set to '%1': the pane is still " + "tracking the conversation") + .arg(window.currentThreadId()))); +} + +void TestMainWindow::clickingTheExpanderTogglesTheThread() +{ + // The glyph being VISIBLE and the glyph being CLICKABLE are separate + // properties, and the pixel test for the first passes happily against a + // triangle nothing can hit. Turning off rootIsDecorated to stop the style + // drawing its own dot under ours also removed the style's hit area, so the + // expander rendered perfectly and did nothing. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + t.totalCount = 3; + model->appendBatch({ t }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + QApplication::processEvents(); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + const QModelIndex subject = + model->index(0, 0, QModelIndex()); + const QRect rect = view->visualRect(subject); + + // Guards: the row is drawn, it claims to have replies, and it starts + // collapsed. Without the last one a toggle test can pass by doing nothing. + QVERIFY2(rect.height() > 0, "the thread row is not on screen"); + QVERIFY(model->data(subject, ThreadListModel::HasRepliesRole).toBool()); + QVERIFY(!view->isExpanded(root)); + + // Aimed at the rect the delegate reports, not at one reconstructed here: + // the drawn target and the clickable one cannot drift if both come from + // the same call. + QStyleOptionViewItem option; + option.rect = rect; + option.font = view->font(); + const QRect expander = CardDelegate::expanderRectFor(option, subject); + QVERIFY2(!expander.isEmpty(), "the card offers no expander to click"); + const QPoint hit = expander.center(); + + QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit); + QApplication::processEvents(); + QVERIFY2(view->isExpanded(root), + "clicking the expander did not open the thread"); + + QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, hit); + QApplication::processEvents(); + QVERIFY2(!view->isExpanded(root), + "clicking the expander again did not close the thread"); +} + +void TestMainWindow::replyRowsKeepTheirTextUnderTheThreadLine() +{ + // paintEvent runs AFTER the cells, so anything it fills across a reply row + // covers the text the delegate just drew. The tint and the thread line are + // both painted there, which makes this the obvious way to ship a block of + // blank rows. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread( + QStringLiteral("t1"), + QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }); + t.totalCount = 2; + model->appendBatch({ t }); + + MessageNode first; + first.messageId = QStringLiteral("m0@example.org"); + first.threadId = QStringLiteral("t1"); + first.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t1"); + reply.from = QStringLiteral("A Replier <replier@example.org>"); + reply.subject = QStringLiteral("Re: a subject"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { first, reply }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + view->expand(root); + QApplication::processEvents(); + + const QModelIndex child = + model->index(0, 0, root); + const QRect rect = view->visualRect(child); + QVERIFY2(rect.height() > 0, "the reply row is not on screen"); QImage shot(view->viewport()->size(), QImage::Format_ARGB32); shot.fill(Qt::transparent); view->viewport()->render(&shot); - // Count pixels matching the tag colours EXACTLY, not "saturated" pixels. - // A looser test counts the antialiased edge of the selection highlight - // blending into the background, which is several hundred distinct - // near-background colours and passes whatever the strip does. Both earlier - // versions of this test did precisely that. - QSet<QRgb> pillColours; - const QVariantList colours = - model->index(0, ThreadListModel::SubjectColumn) - .data(ThreadListModel::PillColoursRole).toList(); - QVERIFY2(!colours.isEmpty(), "the model supplied no pill colours"); - for (const QVariant &colour : colours) - pillColours.insert(colour.value<QColor>().rgb()); - - const int rowHeight = view->rowHeight(0); - QVERIFY(rowHeight > 0); - - int chipPixels = 0; - for (int y = 0; y < qMin(rowHeight, shot.height()); ++y) { - for (int x = 0; x < qMin(subjectLeft, shot.width()); ++x) { - if (pillColours.contains(shot.pixel(x, y) | 0xff000000)) - ++chipPixels; + // Count pixels in the sender cell that differ from the row's own tint. + // Text is the only thing that can produce them. + const QRgb tint = ThreadListModel::replyBackground().rgb() | 0xff000000; + int textPixels = 0; + for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) { + for (int x = rect.left(); x < qMin(rect.right(), shot.width()); ++x) { + if ((shot.pixel(x, y) | 0xff000000) != tint) + ++textPixels; } } - QVERIFY2(chipPixels > 0, - "no pill-coloured pixels left of the subject column: the strip is " - "still confined to that cell rather than spanning the row"); + QVERIFY2(textPixels > 20, + qPrintable(QStringLiteral("only %1 non-background pixels in the " + "reply's sender cell: the row was " + "painted over after its text was drawn") + .arg(textPixels))); } void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() @@ -607,7 +1440,7 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); // Both rows READ, so both are dimmed and neither is bold: the only thing @@ -633,10 +1466,10 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() // dimmed row switches it to the highlight's own text colour, so the two // rows MUST differ; comparing two identically-styled rows would pass // against a delegate that did nothing at all. - view->selectRow(0); + selectThreadRow(view, 0); QApplication::processEvents(); - const int rowHeight = view->rowHeight(0); + const int rowHeight = threadRowHeight(view, 0); QVERIFY(rowHeight > 0); QImage shot(view->viewport()->size(), QImage::Format_ARGB32); @@ -657,15 +1490,15 @@ void TestMainWindow::aSelectedReadThreadIsNotDimmedIntoTheHighlight() QVERIFY2(delegate, "the thread view has no styled delegate"); const QModelIndex index = - model->index(0, ThreadListModel::SubjectColumn); + model->index(0, 0); // initStyleOption is protected, so the resolved palette is reached the way // the painter does: through a subclass that exposes it. - struct Probe : SubjectDelegate { - using SubjectDelegate::initStyleOption; + struct Probe : CardDelegate { + using CardDelegate::initStyleOption; }; const auto *probe = static_cast<const Probe *>( - static_cast<const SubjectDelegate *>(delegate)); + static_cast<const CardDelegate *>(delegate)); probe->initStyleOption(&selected, index); probe->initStyleOption(&unselected, index); @@ -724,7 +1557,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read")); QVERIFY(action); @@ -746,7 +1579,7 @@ void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep() // One row selected, to prove the action ignores the selection rather than // acting on it. - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); @@ -831,7 +1664,7 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), @@ -841,13 +1674,13 @@ void TestMainWindow::markReadTimerRestartsRatherThanStacking() makeThread(QStringLiteral("t3"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(timer->isActive(), "no timer armed for an unread thread"); // Move on before it can fire. One timer stays armed, not three. - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY(timer->isActive()); - view->selectRow(2); + selectThreadRow(view, 2); QVERIFY(timer->isActive()); // Exactly one timer exists at all, which is what "restarted, not stacked" @@ -867,7 +1700,7 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("read"), @@ -875,16 +1708,16 @@ void TestMainWindow::markReadTimerIsNotArmedForAReadThread() makeThread(QStringLiteral("unread"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!timer->isActive(), "armed a timer for an already-read thread"); // And the unread one still arms, so this is not "never arms". - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY(timer->isActive()); // Moving back to a read thread disarms it again, rather than leaving the // previous thread's timer running to fire against the wrong row. - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY(!timer->isActive()); } @@ -909,12 +1742,12 @@ void TestMainWindow::markReadCanBeDisabled() QVERIFY(model); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!timer->isActive(), "a negative mark_read_delay_ms must disable the timer"); @@ -1091,7 +1924,7 @@ void TestMainWindow::selectAllIsBoundAndSelectsEveryRow() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), @@ -1120,7 +1953,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1134,7 +1967,7 @@ void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() // Sweep down as Shift+arrow does: current moves onto a row while the // selection already spans more than one. - view->selectRow(0); + selectThreadRow(view, 0); view->selectionModel()->select( model->index(1, 0), QItemSelectionModel::Select | QItemSelectionModel::Rows); @@ -1159,7 +1992,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1169,7 +2002,7 @@ void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() makeThread(QStringLiteral("t2"), { QStringLiteral("unread") }) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(timer->isActive(), "no timer armed for a single unread thread"); // Extend to a second row, as Shift+click would. @@ -1191,7 +2024,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1205,7 +2038,7 @@ void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() QVERIFY(!timer->isActive()); // Back to one row, as a plain click would leave it. - view->selectRow(1); + selectThreadRow(view, 1); QVERIFY2(timer->isActive(), "collapsing back to one row did not resume mark-read"); @@ -1221,7 +2054,7 @@ void TestMainWindow::theStatusBarReportsAMultiRowSelection() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); QVERIFY2(status, "no status label to report into"); @@ -1246,7 +2079,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), @@ -1256,7 +2089,7 @@ void TestMainWindow::clearSelectionBlanksThePaneAndDeselects() // and what CLAUDE.md requires: selectAll() on a fresh view emits no // currentRowChanged at all, so a test starting there passes against a // missing guard. - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); auto *action = window.findChild<QAction *>(QStringLiteral("clear_selection")); @@ -1302,13 +2135,13 @@ void TestMainWindow::clearPaneLeavesTheSelectionAlone() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), makeThread(QStringLiteral("t2"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); auto *action = window.findChild<QAction *>(QStringLiteral("clear_pane")); @@ -1421,7 +2254,7 @@ void TestMainWindow::theThreadListOffersAContextMenu() const Config config; MainWindow window(config); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); QCOMPARE(view->contextMenuPolicy(), Qt::CustomContextMenu); @@ -1464,7 +2297,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); QVERIFY(timer); @@ -1477,7 +2310,7 @@ void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() { QStringLiteral("unread") }) }); // One row: ordinary reading, so a timer is armed and a thread is current. - view->selectRow(0); + selectThreadRow(view, 0); QCOMPARE(view->selectionModel()->selectedRows().size(), 1); QVERIFY(timer->isActive()); @@ -1829,13 +2662,13 @@ void TestMainWindow::escapeBlanksTheMessagePane() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), {}), makeThread(QStringLiteral("t2"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QVERIFY2(!window.currentThreadId().isEmpty(), "no thread was opened to blank"); @@ -1856,14 +2689,14 @@ void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("delete")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("deleted") }) }); - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); @@ -1884,7 +2717,7 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("delete")); QVERIFY(action); @@ -1935,7 +2768,7 @@ void TestMainWindow::theSelectionCountIsStateAndDoesNotExpire() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); QVERIFY(status); @@ -2059,13 +2892,13 @@ void TestMainWindow::anEditDuringABackgroundSyncIsNotSentYet() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY2(action, "no flag action registered"); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); // A cron sync takes the lock. QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", @@ -2090,13 +2923,13 @@ void TestMainWindow::aHeldEditIsSentWhenTheBackgroundSyncEnds() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, @@ -2125,7 +2958,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); @@ -2134,7 +2967,7 @@ void TestMainWindow::aHeldEditCountsAsUnsynced() QVERIFY2(label->isHidden(), "the indicator starts hidden at zero"); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); - view->selectRow(0); + selectThreadRow(view, 0); QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, @@ -2162,7 +2995,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); @@ -2173,7 +3006,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", Q_ARG(SyncMonitor::State, SyncMonitor::State::Running)); - view->selectRow(0); + selectThreadRow(view, 0); action->trigger(); QVERIFY2(window.hasEditAwaitingSend(), "the edit was not held during a running sync, so this test is not " @@ -2189,7 +3022,7 @@ void TestMainWindow::anUnreadableLockTableStillSendsTheEdit() "platform without /proc/locks"); // And a NEW edit is sent rather than held. - view->selectRow(1); + selectThreadRow(view, 1); action->trigger(); QVERIFY2(!window.hasEditAwaitingSend(), "an unreadable lock table held a new edit, so writes never resume"); @@ -2206,7 +3039,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); auto *flag = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(flag); @@ -2215,7 +3048,7 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory() model->appendBatch({ makeThread(QStringLiteral("t1"), { QStringLiteral("inbox") }) }); - view->selectRow(0); + selectThreadRow(view, 0); // One edit that succeeds, so there is history worth keeping. archive->trigger(); @@ -2609,7 +3442,7 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag() auto *model = window.findChild<ThreadListModel *>(); QVERIFY(model); - auto *view = window.findChild<QTableView *>(); + auto *view = window.findChild<QTreeView *>(); QVERIFY(view); model->appendBatch({ makeThread(QStringLiteral("t1"), @@ -2619,7 +3452,7 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag() // below would pass against an action that did nothing at all. QVERIFY(!model->threadAt(0).isFlagged()); - view->selectRow(0); + selectThreadRow(view, 0); auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index b419915..88dcf0b 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -39,6 +39,8 @@ private slots: void malformedQueryYieldsNoThreads(); void unreadableConfigEmitsError(); void queryPassesGenerationThrough(); + void oldestFirstReversesTheOrder(); + void theSortOrderCrossesAQueuedCall(); void loadThreadReturnsMessagesOldestFirst(); void loadThreadMarksMatchedMessages(); @@ -58,6 +60,11 @@ private slots: void requestAllTagsReturnsSortedTags(); void requestAllTagsOnUnreadableConfigEmitsError(); + void loadMessageReturnsOnlyThatMessage(); + void loadMessageOnAnUnknownIdReturnsNothing(); + void loadThreadTreeReportsReplyDepth(); + void loadThreadTreeCarriesTheFactsARowNeeds(); + void requestCountsAnswersOneCountPerQuery(); void requestCountsKeepsPositionOnAnInvalidQuery(); void requestDatabaseStatsCountsMessagesNotThreads(); @@ -68,7 +75,9 @@ private: QStringList tagsOf(const QString &messageId); QVector<MessageRef> messagesOfThread(const QString &threadId, const QString &matchQuery = QString()); - QVector<ThreadSummary> runQuery(const QString &query); + QVector<ThreadSummary> runQuery( + const QString &query, + NotmuchWorker::SortOrder sort = NotmuchWorker::NewestFirst); QString threadIdOf(const QString &subject); NotmuchFixture m_fixture; @@ -108,13 +117,14 @@ void TestNotmuchWorker::initTestCase() QVERIFY2(m_fixture.index(), qPrintable(m_fixture.error())); } -QVector<ThreadSummary> TestNotmuchWorker::runQuery(const QString &query) +QVector<ThreadSummary> TestNotmuchWorker::runQuery( + const QString &query, NotmuchWorker::SortOrder sort) { NotmuchWorker worker(m_fixture.configPath()); QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); QSignalSpy finished(&worker, &NotmuchWorker::queryFinished); - worker.runQuery(query, 1); + worker.runQuery(query, 1, sort); QVector<ThreadSummary> all; for (const QList<QVariant> &args : ready) @@ -158,6 +168,91 @@ QStringList TestNotmuchWorker::tagsOf(const QString &messageId) return {}; } +void TestNotmuchWorker::loadMessageReturnsOnlyThatMessage() +{ + // a2 is a reply in a two-message thread. Selecting a reply row must render + // that message alone; loadThread would hand back the whole thread and the + // pane would show the conversation the user was trying to look inside. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded); + worker.loadMessage(QStringLiteral("a2@example.org"), 1); + + QCOMPARE(loaded.count(), 1); + const auto messages = loaded.first().at(0).value<QVector<MessageRef>>(); + + QCOMPARE(messages.size(), 1); + QCOMPARE(messages.first().messageId, QStringLiteral("a2@example.org")); + QVERIFY(!messages.first().filePath.isEmpty()); + + // matched, so the pane renders it expanded rather than as a stub. The user + // asked for this message by clicking it, which is as matched as it gets. + QVERIFY(messages.first().matched); +} + +void TestNotmuchWorker::loadMessageOnAnUnknownIdReturnsNothing() +{ + // Empty rather than an error: a stale row after a reindex is an ordinary + // race, not a failure worth a message in the status bar. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.loadMessage(QStringLiteral("nonexistent@example.org"), 1); + + QCOMPARE(loaded.count(), 1); + QVERIFY(loaded.first().at(0).value<QVector<MessageRef>>().isEmpty()); + QCOMPARE(errors.count(), 0); +} + +void TestNotmuchWorker::loadThreadTreeReportsReplyDepth() +{ + // Thread A is a root plus one reply carrying In-Reply-To, which is what + // notmuch threads on. Without that header the two would be separate threads + // and this test would assert nothing about depth. + const QString threadId = threadIdOf(QStringLiteral("Release notes")); + QVERIFY(!threadId.isEmpty()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::threadTreeLoaded); + worker.loadThreadTree(threadId, QString(), 1); + + QCOMPARE(loaded.count(), 1); + const auto nodes = loaded.first().at(0).value<QVector<MessageNode>>(); + + QCOMPARE(nodes.size(), 2); + QCOMPARE(nodes.at(0).messageId, QStringLiteral("a1@example.org")); + QCOMPARE(nodes.at(0).depth, 0); + QCOMPARE(nodes.at(1).messageId, QStringLiteral("a2@example.org")); + QCOMPARE(nodes.at(1).depth, 1); +} + +void TestNotmuchWorker::loadThreadTreeCarriesTheFactsARowNeeds() +{ + // A row is drawn without opening the message, so the walk has to read the + // headers. loadThread does not, which is why a separate signal exists. + const QString threadId = threadIdOf(QStringLiteral("Release notes")); + QVERIFY(!threadId.isEmpty()); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy loaded(&worker, &NotmuchWorker::threadTreeLoaded); + worker.loadThreadTree(threadId, QString(), 1); + + QCOMPARE(loaded.count(), 1); + const auto nodes = loaded.first().at(0).value<QVector<MessageNode>>(); + QCOMPARE(nodes.size(), 2); + + const MessageNode &reply = nodes.at(1); + QVERIFY(reply.from.contains(QStringLiteral("bob@example.org"))); + QCOMPARE(reply.subject, QStringLiteral("Re: Release notes")); + QVERIFY(reply.date.isValid()); + QVERIFY(!reply.filePath.isEmpty()); + + // Every node names its thread, so a batch does not need the caller to keep + // track of which thread it asked about. + QCOMPARE(reply.threadId, threadId); + QCOMPARE(nodes.at(0).threadId, threadId); +} + void TestNotmuchWorker::queryReturnsAllThreads() { const QVector<ThreadSummary> threads = runQuery(QStringLiteral("*")); @@ -234,6 +329,45 @@ void TestNotmuchWorker::queryPassesGenerationThrough() QCOMPARE(finished.first().at(1).value<quint64>(), quint64(42)); } +void TestNotmuchWorker::oldestFirstReversesTheOrder() +{ + const QVector<ThreadSummary> newest = runQuery(QStringLiteral("*")); + const QVector<ThreadSummary> oldest = + runQuery(QStringLiteral("*"), NotmuchWorker::OldestFirst); + + QCOMPARE(oldest.size(), newest.size()); + + // The guard: with fewer than two threads, or with every thread carrying + // the same date, a reversal is indistinguishable from no sorting at all + // and every assertion below would pass against a hardcoded order. + QVERIFY(newest.size() >= 2); + QVERIFY(newest.first().date != newest.last().date); + + QCOMPARE(oldest.first().threadId, newest.last().threadId); + QCOMPARE(oldest.last().threadId, newest.first().threadId); +} + +void TestNotmuchWorker::theSortOrderCrossesAQueuedCall() +{ + // MainWindow reaches the worker with invokeMethod(..., QueuedConnection) + // across a thread boundary, and a Q_ARG whose type the meta-object system + // does not know FAILS AT RUNTIME with a warning, not at compile time. So + // the enum's registration is asserted here rather than assumed from Q_ENUM. + QVERIFY2(QMetaType::fromName("NotmuchWorker::SortOrder").isValid(), + "SortOrder is not a registered metatype, so the queued runQuery " + "call will drop its sort argument at runtime"); + + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy ready(&worker, &NotmuchWorker::threadsReady); + + // The real call shape, invoked by NAME exactly as MainWindow does. + QVERIFY(QMetaObject::invokeMethod( + &worker, "runQuery", Qt::DirectConnection, + Q_ARG(QString, QStringLiteral("*")), Q_ARG(quint64, 1), + Q_ARG(NotmuchWorker::SortOrder, NotmuchWorker::OldestFirst))); + QCOMPARE(ready.size(), 1); +} + void TestNotmuchWorker::loadThreadReturnsMessagesOldestFirst() { const QString threadId = threadIdOf(QStringLiteral("Release notes")); diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index f84bbab..aa71080 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -27,6 +27,20 @@ class TestThreadListModel : public QObject { Q_OBJECT private slots: + void messageNodeHoldsDisplayFacts(); + void rootRowsSurviveTheTreeConversion(); + void repliesBecomeChildRowsUnderTheirThread(); + void messageRowsShowTheirOwnSenderAndSubject(); + void modelHasOneColumn(); + void aFlatThreadStillListsItsReplies(); + void theRootCardKnowsItsOwnMessage(); + void replyShowsOnlyItsOwnTags(); + void replySharingEveryThreadTagShowsNone(); + void reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem(); + void anUnexpandedMultiMessageThreadOffersAnExpander(); + void scopeFollowsTheSelectedRowKind(); + void scopeCountsEveryMessageOfAnUnexpandedThread(); + void scopeHonoursAMixedSelectionWithoutEscalating(); void startsEmpty(); void accountKeysComeFromTheAccountTags(); void accountKeysCoverAThreadSpanningTwoAccounts(); @@ -35,22 +49,20 @@ private slots: void appendingEmptyBatchIsNoOp(); void clearResetsModel(); void reportsSubjectAndAuthors(); - void subjectShowsMessageCountOnlyForRealThreads(); + void theReplyCountExcludesTheRootMessage(); void unreadThreadsRenderBold(); void readThreadsAreDimmedAndUnreadAreNot(); void flaggedThreadsShowAStar(); void pillTagsExcludeWhatTheRowAlreadyShows(); - void theStarColumnIsNarrowAndCarriesNoText(); void theUnreadCueDoesNotDependOnFontWeight(); void aDoomedThreadKeepsItsContrastEvenWhenRead(); - void tagsAreTheFirstColumnAndSubjectTheLast(); void accountTagBecomesAChipLabel(); void unreadStylingSurvivesAnAccountChip(); void accountChipUsesTheConfiguredColour(); void deletedThreadsAreRedAndStruckThrough(); - void attachmentColumnIsFirstAndMarksOnlyTaggedThreads(); + void attachmentIsMarkedOnlyOnTaggedThreads(); void spamThreadsAreOrangeAndStruckThrough(); - void doomedStylingCoversEveryColumn(); + void doomedStylingCoversTheWholeCard(); void ordinaryThreadsCarryNoRowColour(); void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); @@ -76,6 +88,238 @@ static ThreadSummary makeThread(const QString &id, const QString &subject) return t; } +static MessageNode makeNode(const QString &id, int depth, + const QString &from = QStringLiteral("Alice"), + const QString &subject = QStringLiteral("Re: Hi")) +{ + MessageNode n; + n.messageId = id; + n.threadId = QStringLiteral("t1"); + n.from = from; + n.subject = subject; + n.date = QDateTime::fromSecsSinceEpoch(1750000000); + n.depth = depth; + return n; +} + +void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread() +{ + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")) }); + + // Depth 0 is the thread's FIRST message and belongs on the root row, not in + // the children: the user's model is "N replies", so a thread of three shows + // one root and two children. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1), + makeNode(QStringLiteral("m2@example.org"), 2) }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + QCOMPARE(model.rowCount(root), 2); + + const QModelIndex child = + model.index(0, 0, root); + QVERIFY(child.isValid()); + QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex())); + + QVERIFY(model.data(child, ThreadListModel::IsMessageRole).toBool()); + QCOMPARE(model.data(child, ThreadListModel::MessageIdRole).toString(), + QStringLiteral("m1@example.org")); + + // A message row still belongs to a thread, so a caller that only needs the + // containing thread does not have to walk up itself. + QCOMPARE(model.data(child, ThreadListModel::ThreadIdRole).toString(), + QStringLiteral("t1")); + + // A thread root is not a message ROW, but it does carry a message id: the + // root card is the thread's first message, and selecting it renders that + // message alone. It used to answer nothing here, which is what made the + // first message of every thread unreachable. + QVERIFY(!model.data(root, ThreadListModel::IsMessageRole).toBool()); + QCOMPARE(model.data(root, ThreadListModel::MessageIdRole).toString(), + QStringLiteral("m0@example.org")); + + QAbstractItemModelTester tester( + &model, QAbstractItemModelTester::FailureReportingMode::Warning); + Q_UNUSED(tester); +} + +void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject() +{ + // A reply's row shows the REPLY's sender, not the thread's author summary. + // Reading the thread's fields for a child row is the obvious mistake and + // would look almost right, since the first sender is usually in both. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")) }); + model.setThreadMessages( + QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1, + QStringLiteral("Bob <bob@example.org>"), + QStringLiteral("Re: A subject")) }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + const QModelIndex reply = model.index(0, 0, root); + + QCOMPARE(model.data(reply, ThreadListModel::SendersRole).toString(), + QStringLiteral("Bob <bob@example.org>")); + QCOMPARE(model.data(reply, ThreadListModel::SubjectRole).toString(), + QStringLiteral("Re: A subject")); + + // No tag strip under a child row. The strip is a row-wide band carrying the + // THREAD's tags; one under every reply would stripe the list and repeat the + // same tags down the whole expansion. + QVERIFY(model.data(reply, ThreadListModel::PillTagsRole) + .toStringList().isEmpty()); +} + +void TestThreadListModel::reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem() +{ + // A thread reloaded after a sync must not end up listing its replies twice. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")) }); + + const QVector<MessageNode> nodes{ + makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) + }; + + model.setThreadMessages(QStringLiteral("t1"), nodes); + const QModelIndex root = model.index(0, 0, QModelIndex()); + QCOMPARE(model.rowCount(root), 1); + + model.setThreadMessages(QStringLiteral("t1"), nodes); + QCOMPARE(model.rowCount(root), 1); + + QAbstractItemModelTester tester( + &model, QAbstractItemModelTester::FailureReportingMode::Warning); + Q_UNUSED(tester); +} + +void TestThreadListModel::anUnexpandedMultiMessageThreadOffersAnExpander() +{ + // This is what makes lazy loading work at all. rowCount is 0 until the + // worker has walked the thread, so a view inferring the expander from + // rowCount alone draws none, the user can never expand, and the replies are + // never requested. hasChildren answers from the summary's count instead. + ThreadListModel model; + ThreadSummary many = makeThread(QStringLiteral("t1"), + QStringLiteral("Has replies")); + many.totalCount = 4; + ThreadSummary lone = makeThread(QStringLiteral("t2"), + QStringLiteral("Single message")); + lone.totalCount = 1; + model.appendBatch({ many, lone }); + + const QModelIndex withReplies = model.index(0, 0, QModelIndex()); + const QModelIndex single = model.index(1, 0, QModelIndex()); + + // Guard: neither is expanded, so this really is the unloaded case. + QCOMPARE(model.rowCount(withReplies), 0); + QCOMPARE(model.rowCount(single), 0); + + QVERIFY(model.hasChildren(withReplies)); + QVERIFY(!model.hasChildren(single)); + + // Once loaded the children are the truth, including "there are none": a + // thread whose count included duplicates must stop offering an expander + // that opens onto nothing. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0) }); + QVERIFY(!model.hasChildren(withReplies)); + + // A message row is always a leaf. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + QVERIFY(model.hasChildren(withReplies)); + QVERIFY(!model.hasChildren(model.index(0, 0, withReplies))); +} + +void TestThreadListModel::scopeFollowsTheSelectedRowKind() +{ + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")); + t.totalCount = 3; + model.appendBatch({ t }); + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + const QModelIndex child = model.index(0, 0, root); + + // A thread root acts on the whole thread, and reports every message it + // stands for so the status bar can say so. + const ActionScope threadScope = model.scopeFor({ root }); + QCOMPARE(threadScope.threadIds, QStringList{ QStringLiteral("t1") }); + QVERIFY(threadScope.messageIds.isEmpty()); + QCOMPARE(threadScope.messageCount, 3); + QVERIFY(threadScope.wholeThread); + + // A message row acts on that message alone. + const ActionScope messageScope = model.scopeFor({ child }); + QVERIFY(messageScope.threadIds.isEmpty()); + QCOMPARE(messageScope.messageIds, + QStringList{ QStringLiteral("m1@example.org") }); + QCOMPARE(messageScope.messageCount, 1); + QVERIFY(!messageScope.wholeThread); +} + +void TestThreadListModel::scopeCountsEveryMessageOfAnUnexpandedThread() +{ + // totalCount, not the loaded children. A thread that was never expanded + // still has all of its messages, and counting only what happens to be on + // screen would understate what the action is about to do. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")); + t.totalCount = 7; + model.appendBatch({ t }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + QCOMPARE(model.rowCount(root), 0); // guard: nothing expanded + + const ActionScope scope = model.scopeFor({ root }); + QCOMPARE(scope.messageCount, 7); +} + +void TestThreadListModel::scopeHonoursAMixedSelectionWithoutEscalating() +{ + // Selecting a thread root and an unrelated reply acts on that whole thread + // AND that one message. Nothing is escalated to thread scope or narrowed to + // message scope silently, which is the point of the scope being visible. + ThreadListModel model; + ThreadSummary t1 = makeThread(QStringLiteral("t1"), QStringLiteral("One")); + t1.totalCount = 2; + ThreadSummary t2 = makeThread(QStringLiteral("t2"), QStringLiteral("Two")); + t2.totalCount = 5; + model.appendBatch({ t1, t2 }); + + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + reply.threadId = QStringLiteral("t2"); + model.setThreadMessages(QStringLiteral("t2"), + { makeNode(QStringLiteral("m0@example.org"), 0), + reply }); + + const QModelIndex firstRoot = model.index(0, 0, QModelIndex()); + const QModelIndex secondRoot = model.index(1, 0, QModelIndex()); + const QModelIndex reply1 = model.index(0, 0, secondRoot); + + const ActionScope scope = model.scopeFor({ firstRoot, reply1 }); + QCOMPARE(scope.threadIds, QStringList{ QStringLiteral("t1") }); + QCOMPARE(scope.messageIds, QStringList{ QStringLiteral("m1@example.org") }); + + // 2 from the whole thread plus 1 for the lone message. + QCOMPARE(scope.messageCount, 3); + QVERIFY(scope.wholeThread); +} + void TestThreadListModel::accountKeysComeFromTheAccountTags() { // Item 49 reads this to decide which mbsync channels a sync needs. Only @@ -116,11 +360,69 @@ void TestThreadListModel::accountKeysAreEmptyForAnUnknownThread() QVERIFY(model.accountKeysForThread(QStringLiteral("nope")).isEmpty()); } +void TestThreadListModel::messageNodeHoldsDisplayFacts() +{ + // A message ROW has to be drawn without opening the message, so the display + // facts live on the node itself. MessageRef, which exists for rendering a + // thread into the pane, carries none of them. + MessageNode node; + node.messageId = QStringLiteral("id@example.org"); + node.from = QStringLiteral("A Sender <sender@example.org>"); + node.subject = QStringLiteral("Re: a subject"); + node.date = QDateTime::fromSecsSinceEpoch(1000); + node.depth = 2; + node.tags = QStringList{ QStringLiteral("unread") }; + + QCOMPARE(node.depth, 2); + QVERIFY(node.isUnread()); + QCOMPARE(node.from, QStringLiteral("A Sender <sender@example.org>")); + QCOMPARE(node.subject, QStringLiteral("Re: a subject")); + + // Depth 0 is the thread's first message, which the ROOT row stands for. + // Defaulting to 0 rather than 1 keeps "is this the root" a plain check. + const MessageNode fresh; + QCOMPARE(fresh.depth, 0); + QVERIFY(!fresh.isUnread()); +} + +void TestThreadListModel::rootRowsSurviveTheTreeConversion() +{ + // The point of this test is NOT the tree. It is that converting the base + // class from QAbstractTableModel changed nothing a thread row does: a table + // answers index() and parent() too, just trivially, and every existing test + // in this file is the real regression net beside it. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")) }); + + // A tree model reports its roots under an INVALID parent. + QCOMPARE(model.rowCount(QModelIndex()), 1); + QCOMPARE(model.columnCount(QModelIndex()), 1); + + const QModelIndex root = + model.index(0, 0, QModelIndex()); + QVERIFY(root.isValid()); + QVERIFY(!model.parent(root).isValid()); + QCOMPARE(model.data(root, ThreadListModel::ThreadIdRole).toString(), + QStringLiteral("t1")); + + // No children until a thread's messages are asked for. An expander drawn + // over a thread whose replies were never loaded would open onto nothing. + QCOMPARE(model.rowCount(root), 0); + + // Qt's own conformance check. It walks index/parent/rowCount for + // consistency and catches the classic tree-model faults, such as a parent() + // that does not round-trip, which a hand-written assertion misses. + QAbstractItemModelTester tester( + &model, QAbstractItemModelTester::FailureReportingMode::Warning); + Q_UNUSED(tester); +} + void TestThreadListModel::startsEmpty() { ThreadListModel model; QCOMPARE(model.rowCount(), 0); - QCOMPARE(model.columnCount(), ThreadListModel::ColumnCount); + QCOMPARE(model.columnCount(), 1); } void TestThreadListModel::appendsBatches() @@ -165,21 +467,21 @@ void TestThreadListModel::reportsSubjectAndAuthors() ThreadListModel model; model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); - const QModelIndex authors = model.index(0, ThreadListModel::AuthorsColumn); - QCOMPARE(model.data(authors, Qt::DisplayRole).toString(), + // One index, every field, by role. The card draws them all at once, so + // reading them through Qt::DisplayRole as five columns did is no longer + // possible: DisplayRole answers the subject alone. + const QModelIndex card = model.index(0, 0); + QCOMPARE(model.data(card, ThreadListModel::SendersRole).toString(), QStringLiteral("Alice")); + QVERIFY(model.data(card, ThreadListModel::DateRole).toDateTime().isValid()); + QCOMPARE(model.data(card, ThreadListModel::SubjectRole).toString(), + QStringLiteral("hello")); - const QModelIndex date = model.index(0, ThreadListModel::DateColumn); - QVERIFY(!model.data(date, Qt::DisplayRole).toString().isEmpty()); - - // Tags are no longer a column; they reach the strip under the message - // pane through a role instead. - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); - QCOMPARE(model.data(subject, ThreadListModel::TagsRole).toStringList(), + QCOMPARE(model.data(card, ThreadListModel::TagsRole).toStringList(), QStringList({ QStringLiteral("inbox"), QStringLiteral("unread") })); } -void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() +void TestThreadListModel::theReplyCountExcludesTheRootMessage() { ThreadListModel model; @@ -189,12 +491,18 @@ void TestThreadListModel::subjectShowsMessageCountOnlyForRealThreads() multi.totalCount = 4; model.appendBatch({ single, multi }); - QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), - Qt::DisplayRole).toString(), - QStringLiteral("alone")); - QCOMPARE(model.data(model.index(1, ThreadListModel::SubjectColumn), - Qt::DisplayRole).toString(), - QStringLiteral("group (4)")); + // The count used to be a "(4)" suffix on the subject. It is the expander + // on the card's second line now, and it counts REPLIES: totalCount + // includes the root message, which is the card itself. + QCOMPARE(model.data(model.index(0, 0), + ThreadListModel::ReplyCountRole).toInt(), 0); + QCOMPARE(model.data(model.index(1, 0), + ThreadListModel::ReplyCountRole).toInt(), 3); + + // And the subject is bare, with no count spliced into it. + QCOMPARE(model.data(model.index(1, 0), + ThreadListModel::SubjectRole).toString(), + QStringLiteral("group")); } void TestThreadListModel::unreadThreadsRenderBold() @@ -205,11 +513,11 @@ void TestThreadListModel::unreadThreadsRenderBold() model.appendBatch({ read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); const QVariant readFont = - model.data(model.index(0, ThreadListModel::SubjectColumn), Qt::FontRole); + model.data(model.index(0, 0), Qt::FontRole); QVERIFY(!readFont.isValid()); const QVariant unreadFont = - model.data(model.index(1, ThreadListModel::SubjectColumn), Qt::FontRole); + model.data(model.index(1, 0), Qt::FontRole); QVERIFY(unreadFont.isValid()); QVERIFY(unreadFont.value<QFont>().bold()); } @@ -232,10 +540,10 @@ void TestThreadListModel::readThreadsAreDimmedAndUnreadAreNot() { read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); const QVariant readFg = - model.data(model.index(0, ThreadListModel::SubjectColumn), + model.data(model.index(0, 0), Qt::ForegroundRole); const QVariant unreadFg = - model.data(model.index(1, ThreadListModel::SubjectColumn), + model.data(model.index(1, 0), Qt::ForegroundRole); QVERIFY2(readFg.isValid(), "a read thread carries no dimming"); @@ -257,16 +565,17 @@ void TestThreadListModel::flaggedThreadsShowAStar() QStringLiteral("flagged") }; model.appendBatch({ plain, starred }); - const QString none = - model.data(model.index(0, ThreadListModel::FlagColumn), - Qt::DisplayRole).toString(); - const QString star = - model.data(model.index(1, ThreadListModel::FlagColumn), - Qt::DisplayRole).toString(); - - QVERIFY2(none.isEmpty(), "an unflagged thread shows something in the column"); - QVERIFY2(!star.isEmpty(), "a flagged thread shows nothing"); - QCOMPARE(star, ThreadListModel::flagGlyph()); + QVERIFY2(!model.data(model.index(0, 0), + ThreadListModel::IsFlaggedRole).toBool(), + "an unflagged thread reports itself flagged"); + QVERIFY2(model.data(model.index(1, 0), + ThreadListModel::IsFlaggedRole).toBool(), + "a flagged thread does not report itself flagged"); + + // The glyph the delegate draws from that flag must be something a font can + // render: an unrenderable codepoint shows as tofu, which reads as + // breakage rather than as a mark. + QVERIFY(!ThreadListModel::flagGlyph().isEmpty()); } void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() @@ -290,7 +599,7 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() model.appendBatch({ thread }); const QStringList pills = - model.data(model.index(0, ThreadListModel::SubjectColumn), + model.data(model.index(0, 0), ThreadListModel::PillTagsRole).toStringList(); QVERIFY2(pills.contains(QStringLiteral("SBo")), qPrintable(pills.join(','))); @@ -320,29 +629,6 @@ void TestThreadListModel::pillTagsExcludeWhatTheRowAlreadyShows() QCOMPARE(pills, sorted); } -void TestThreadListModel::theStarColumnIsNarrowAndCarriesNoText() -{ - // A marker column, like the paperclip beside it: centred, and never - // carrying the subject or anything else that would want width. - ThreadListModel model; - ThreadSummary starred = makeThread(QStringLiteral("t1"), - QStringLiteral("starred")); - starred.tags = QStringList{ QStringLiteral("flagged") }; - model.appendBatch({ starred }); - - const QModelIndex index = model.index(0, ThreadListModel::FlagColumn); - QCOMPARE(model.data(index, Qt::TextAlignmentRole).toInt(), - int(Qt::AlignCenter)); - - // The glyph is one character, whether it is the star or its fallback: a - // column sized for a marker cannot hold a word. - QCOMPARE(ThreadListModel::flagGlyph().size(), 1); - - // And it says what it means, for anyone who cannot tell the glyph apart - // from the paperclip beside it. - QVERIFY(!model.data(index, Qt::ToolTipRole).toString().isEmpty()); -} - void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight() { // The property that matters, stated directly: strip every font from the @@ -355,17 +641,14 @@ void TestThreadListModel::theUnreadCueDoesNotDependOnFontWeight() model.appendBatch( { read, makeThread(QStringLiteral("t2"), QStringLiteral("unread")) }); - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - const QVariant readFg = - model.data(model.index(0, column), Qt::ForegroundRole); - const QVariant unreadFg = - model.data(model.index(1, column), Qt::ForegroundRole); + const QVariant readFg = + model.data(model.index(0, 0), Qt::ForegroundRole); + const QVariant unreadFg = + model.data(model.index(1, 0), Qt::ForegroundRole); - QVERIFY2(readFg != unreadFg, - qPrintable(QStringLiteral("column %1 renders read and unread " - "identically once the font is " - "ignored").arg(column))); - } + QVERIFY2(readFg != unreadFg, + "read and unread cards render identically once the font is " + "ignored"); } void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead() @@ -382,32 +665,11 @@ void TestThreadListModel::aDoomedThreadKeepsItsContrastEvenWhenRead() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, Qt::ForegroundRole).value<QBrush>().color(), QColor(Qt::white)); } -void TestThreadListModel::tagsAreTheFirstColumnAndSubjectTheLast() -{ - // Subject stretches to fill the view, so whatever sits after it is pushed - // off-screen. Tags used to be there, which is why acting on a thread - // looked like it did nothing: the only column that changed was invisible. - QCOMPARE(ThreadListModel::SubjectColumn, ThreadListModel::ColumnCount - 1); - - ThreadListModel model; - model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("hello")) }); - QCOMPARE(model.headerData(ThreadListModel::SubjectColumn, Qt::Horizontal, - Qt::DisplayRole).toString(), - QStringLiteral("Subject")); - - // No tags column at all: spelling out a dozen tags per row consumed most - // of the list's width and was unreadable. - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - QVERIFY(model.headerData(column, Qt::Horizontal, Qt::DisplayRole) - .toString() != QStringLiteral("Tags")); - } -} - void TestThreadListModel::accountTagBecomesAChipLabel() { // The account tag is a different taxonomy from a functional one: which @@ -419,7 +681,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel() QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, ThreadListModel::AccountLabelRole).toString(), QStringLiteral("webmail-personal")); QVERIFY(model.data(subject, ThreadListModel::AccountColourRole) @@ -430,7 +692,7 @@ void TestThreadListModel::accountTagBecomesAChipLabel() ThreadSummary untagged = makeThread(QStringLiteral("t2"), QStringLiteral("hi")); untagged.tags = QStringList{ QStringLiteral("inbox") }; plain.appendBatch({ untagged }); - QVERIFY(plain.data(plain.index(0, ThreadListModel::SubjectColumn), + QVERIFY(plain.data(plain.index(0, 0), ThreadListModel::AccountLabelRole).toString().isEmpty()); } @@ -446,7 +708,7 @@ void TestThreadListModel::unreadStylingSurvivesAnAccountChip() QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, ThreadListModel::AccountLabelRole) .toString().isEmpty()); @@ -469,7 +731,7 @@ void TestThreadListModel::accountChipUsesTheConfiguredColour() thread.tags = QStringList{ QStringLiteral("account-webmail-personal") }; model.appendBatch({ thread }); - QCOMPARE(model.data(model.index(0, ThreadListModel::SubjectColumn), + QCOMPARE(model.data(model.index(0, 0), ThreadListModel::AccountColourRole).value<QColor>(), QColor(QStringLiteral("#cc0000"))); } @@ -481,7 +743,7 @@ void TestThreadListModel::deletedThreadsAreRedAndStruckThrough() thread.tags = QStringList{ QStringLiteral("inbox") }; model.appendBatch({ thread }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); @@ -506,7 +768,7 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("spam") }, {}); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QCOMPARE(model.data(subject, Qt::BackgroundRole).value<QBrush>().color(), ThreadListModel::spamColour()); QVERIFY(model.data(subject, Qt::FontRole).value<QFont>().strikeOut()); @@ -515,10 +777,12 @@ void TestThreadListModel::spamThreadsAreOrangeAndStruckThrough() QVERIFY(ThreadListModel::spamColour() != ThreadListModel::deletedColour()); } -void TestThreadListModel::doomedStylingCoversEveryColumn() +void TestThreadListModel::doomedStylingCoversTheWholeCard() { - // A cue on one column would vanish the moment that column scrolled out of - // view, which is the bug this whole change exists to fix. + // The cue is on the card itself. It used to be asserted per column, + // because a cue on one column vanished the moment that column scrolled out + // of view; one column cannot scroll away, but the roles still have to be + // answered or a deleted card looks untouched. ThreadListModel model; ThreadSummary thread = makeThread(QStringLiteral("t1"), QStringLiteral("doomed")); thread.tags = QStringList{ QStringLiteral("inbox") }; @@ -526,13 +790,11 @@ void TestThreadListModel::doomedStylingCoversEveryColumn() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); - for (int column = 0; column < ThreadListModel::ColumnCount; ++column) { - const QModelIndex index = model.index(0, column); - QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), - qPrintable(QStringLiteral("column %1 has no background").arg(column))); - QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), - qPrintable(QStringLiteral("column %1 is not struck through").arg(column))); - } + const QModelIndex index = model.index(0, 0); + QVERIFY2(model.data(index, Qt::BackgroundRole).isValid(), + "a deleted card has no background"); + QVERIFY2(model.data(index, Qt::FontRole).value<QFont>().strikeOut(), + "a deleted card is not struck through"); } void TestThreadListModel::ordinaryThreadsCarryNoRowColour() @@ -546,7 +808,7 @@ void TestThreadListModel::ordinaryThreadsCarryNoRowColour() model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("deleted") }, {}); model.applyTagChange(QStringLiteral("t1"), {}, { QStringLiteral("deleted") }); - const QModelIndex subject = model.index(0, ThreadListModel::SubjectColumn); + const QModelIndex subject = model.index(0, 0); QVERIFY(!model.data(subject, Qt::BackgroundRole).isValid()); const QVariant font = model.data(subject, Qt::FontRole); QVERIFY(!font.isValid() || !font.value<QFont>().strikeOut()); @@ -569,7 +831,7 @@ void TestThreadListModel::threadIdIsReachableFromAnIndex() model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")), makeThread(QStringLiteral("t2"), QStringLiteral("two")) }); - const QModelIndex index = model.index(1, ThreadListModel::SubjectColumn); + const QModelIndex index = model.index(1, 0); QCOMPARE(model.data(index, ThreadListModel::ThreadIdRole).toString(), QStringLiteral("t2")); } @@ -587,7 +849,7 @@ void TestThreadListModel::invalidIndexesReturnNothing() // the reset in clear() before data() ever sees it. data() still checks its // own bounds, but that guard is unreachable defence, not something these // assertions can falsify. - QVERIFY(!model.index(0, ThreadListModel::ColumnCount).isValid()); + QVERIFY(!model.index(0, 1).isValid()); QVERIFY(!model.index(5, 0).isValid()); QVERIFY(!model.index(-1, 0).isValid()); @@ -651,9 +913,9 @@ void TestThreadListModel::tagChangeSignalsExactlyTheChangedRow() const QModelIndex bottomRight = changed.first().at(1).value<QModelIndex>(); QCOMPARE(topLeft.row(), 1); QCOMPARE(bottomRight.row(), 1); + // One column, so the range is a single index: the card repaints whole. QCOMPARE(topLeft.column(), 0); - // The whole row repaints: unread state changes the font of every column. - QCOMPARE(bottomRight.column(), ThreadListModel::ColumnCount - 1); + QCOMPARE(bottomRight.column(), 0); } void TestThreadListModel::tagChangeForUnknownThreadIsIgnored() @@ -704,12 +966,11 @@ void TestThreadListModel::modelPassesQtTester() model.clear(); } -void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() +void TestThreadListModel::attachmentIsMarkedOnlyOnTaggedThreads() { - // Leftmost, and narrow: the point is to see an attachment without opening - // the thread, which only works if the column is never scrolled away. - QCOMPARE(ThreadListModel::AttachmentColumn, 0); - + // The mark is drawn on the card's second line by CardDelegate. What the + // model owes it is the flag and the glyph, which is what this asserts: + // the column that used to carry it is gone. ThreadSummary plain = makeThread(QStringLiteral("t1"), QStringLiteral("no attachment")); ThreadSummary withFile = makeThread(QStringLiteral("t2"), @@ -722,13 +983,12 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() model.appendBatch({ plain, withFile }); const QModelIndex plainCell = - model.index(0, ThreadListModel::AttachmentColumn); + model.index(0, 0); const QModelIndex fileCell = - model.index(1, ThreadListModel::AttachmentColumn); + model.index(1, 0); - QVERIFY(model.data(plainCell, Qt::DisplayRole).toString().isEmpty()); - QCOMPARE(model.data(fileCell, Qt::DisplayRole).toString(), - ThreadListModel::attachmentGlyph()); + QVERIFY(!model.data(plainCell, ThreadListModel::HasAttachmentRole).toBool()); + QVERIFY(model.data(fileCell, ThreadListModel::HasAttachmentRole).toBool()); // The glyph must be something a font can draw. An unrenderable codepoint // shows as a tofu box, which reads as breakage rather than as a marker. @@ -738,11 +998,178 @@ void TestThreadListModel::attachmentColumnIsFirstAndMarksOnlyTaggedThreads() // have an attachment on hover. QVERIFY(model.data(plainCell, Qt::ToolTipRole).toString().isEmpty()); QVERIFY(!model.data(fileCell, Qt::ToolTipRole).toString().isEmpty()); +} + +void TestThreadListModel::modelHasOneColumn() +{ + ThreadListModel model; + ThreadSummary thread; + thread.threadId = QStringLiteral("T1"); + thread.subject = QStringLiteral("Build fails"); + thread.authors = QStringLiteral("alice@example.org"); + thread.date = QDateTime::currentDateTime(); + thread.totalCount = 1; + model.appendBatch({ thread }); + + QCOMPARE(model.columnCount(), 1); + + // Every field the five columns used to answer is still reachable, by role + // rather than by column, because the card draws them all. + const QModelIndex index = model.index(0, 0); + QCOMPARE(index.data(ThreadListModel::SubjectRole).toString(), + QStringLiteral("Build fails")); + QCOMPARE(index.data(ThreadListModel::SendersRole).toString(), + QStringLiteral("alice@example.org")); + QVERIFY(index.data(ThreadListModel::DateRole).toDateTime().isValid()); + + // A single-message thread offers no expander: totalCount includes the root + // message, which is the card itself. + QCOMPARE(index.data(ThreadListModel::ReplyCountRole).toInt(), 0); +} + +void TestThreadListModel::aFlatThreadStillListsItsReplies() +{ + // A thread whose messages carry no reply structure: notmuch returns them + // all from get_toplevel_messages at depth 0, which is what happens when the + // mail has no usable In-Reply-To. Measured in the user's own database: + // of 396 inbox threads, three are like this, one of them nine messages + // deep, and every one of them showed a reply count that expanded to + // nothing because the model kept only nodes with depth > 0. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), + QStringLiteral("flat thread")); + thread.totalCount = 3; + model.appendBatch({ thread }); + + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 0), + makeNode(QStringLiteral("m2@example.org"), 0) }); + + const QModelIndex root = model.index(0, 0); + + // Two children, not zero: the FIRST message is the root card itself, and + // the rest are its replies however flat the thread is. + QCOMPARE(model.rowCount(root), 2); + QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole) + .toString(), + QStringLiteral("m1@example.org")); + + // And the count the card advertises must agree with the rows beneath it, + // or the expander opens onto nothing. + QCOMPARE(root.data(ThreadListModel::ReplyCountRole).toInt(), + model.rowCount(root)); +} + +void TestThreadListModel::theRootCardKnowsItsOwnMessage() +{ + // The root card IS the thread's first message, so it has to be able to say + // which message that is. Without this the pane renders the whole thread + // when the root is selected, and the first message is unreachable: the + // only rows offering it are the replies, and it is not one of them. + ThreadListModel model; + ThreadSummary thread = makeThread(QStringLiteral("t1"), + QStringLiteral("a subject")); + thread.totalCount = 2; + model.appendBatch({ thread }); + + const QModelIndex root = model.index(0, 0); + + // Before the replies are loaded there is nothing to report, and the caller + // must fall back to loading the whole thread rather than a wrong message. + QVERIFY(root.data(ThreadListModel::MessageIdRole).toString().isEmpty()); + + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + + QCOMPARE(root.data(ThreadListModel::MessageIdRole).toString(), + QStringLiteral("m0@example.org")); + + // And it is the FIRST message, not just any of them: the reply must still + // report its own. + QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole) + .toString(), + QStringLiteral("m1@example.org")); +} + +void TestThreadListModel::replyShowsOnlyItsOwnTags() +{ + ThreadListModel model; + ThreadSummary thread; + thread.threadId = QStringLiteral("T1"); + thread.subject = QStringLiteral("Build fails"); + thread.totalCount = 2; + thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") }; + model.appendBatch({ thread }); + + MessageNode reply; + reply.messageId = QStringLiteral("M2"); + reply.threadId = QStringLiteral("T1"); + reply.from = QStringLiteral("bob@example.org"); + reply.depth = 1; + // Two the thread already has, one it does not. + reply.tags = { QStringLiteral("inbox"), QStringLiteral("work"), + QStringLiteral("todo") }; + + // Led by the thread's FIRST message, which is what the worker sends and + // what the root card draws. setThreadMessages drops it by position. + MessageNode root; + root.messageId = QStringLiteral("M1"); + root.threadId = QStringLiteral("T1"); + root.depth = 0; + model.setThreadMessages(QStringLiteral("T1"), { root, reply }); + + const QModelIndex threadIndex = model.index(0, 0); + QVERIFY(model.hasChildren(threadIndex)); + const QModelIndex replyIndex = model.index(0, 0, threadIndex); + QVERIFY(replyIndex.isValid()); + + const QStringList own = + replyIndex.data(ThreadListModel::MessageOwnTagsRole).toStringList(); + QCOMPARE(own, QStringList{ QStringLiteral("todo") }); + + // The colours must line up with the names one for one, or the delegate + // walks the two lists together and paints a chip in another tag's colour. + const QVariantList colours = + replyIndex.data(ThreadListModel::MessageOwnColoursRole).toList(); + QCOMPARE(colours.size(), own.size()); + QVERIFY(colours.first().value<QColor>().isValid()); +} + +void TestThreadListModel::replySharingEveryThreadTagShowsNone() +{ + ThreadListModel model; + ThreadSummary thread; + thread.threadId = QStringLiteral("T1"); + thread.totalCount = 2; + thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") }; + model.appendBatch({ thread }); - // The header carries no text: a label would set a minimum width far wider - // than the icon and defeat the narrow column. - QVERIFY(model.headerData(ThreadListModel::AttachmentColumn, Qt::Horizontal, - Qt::DisplayRole).toString().isEmpty()); + MessageNode reply; + reply.messageId = QStringLiteral("M2"); + reply.threadId = QStringLiteral("T1"); + reply.depth = 1; + reply.tags = { QStringLiteral("inbox"), QStringLiteral("work") }; + + MessageNode root; + root.messageId = QStringLiteral("M1"); + root.threadId = QStringLiteral("T1"); + root.depth = 0; + model.setThreadMessages(QStringLiteral("T1"), { root, reply }); + + const QModelIndex replyIndex = model.index(0, 0, model.index(0, 0)); + QVERIFY(replyIndex.isValid()); + QVERIFY(replyIndex.data(ThreadListModel::MessageOwnTagsRole) + .toStringList() + .isEmpty()); + + // A thread row has no thread to differ from, so it never answers these: + // its own chips come from PillTagsRole. + QVERIFY(model.index(0, 0) + .data(ThreadListModel::MessageOwnTagsRole) + .toStringList() + .isEmpty()); } QTEST_MAIN(TestThreadListModel) |