aboutsummaryrefslogtreecommitdiffstats
path: root/tests/test_threadlistmodel.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-10 20:43:05 +0200
committerDanilo M. <danix@danix.xyz>2026-08-10 20:43:05 +0200
commited0e085377440a68cef63ade6dc2afd322c22df9 (patch)
treea8b4e5631a6eb5ff56341253f3775b45e3a4c026 /tests/test_threadlistmodel.cpp
parent39a055fef99a3ce6877829753f384843b6a19177 (diff)
downloadqtmaildir-ed0e085377440a68cef63ade6dc2afd322c22df9.tar.gz
qtmaildir-ed0e085377440a68cef63ade6dc2afd322c22df9.zip
feat(view): follow a background sync without a keystroke
The thread list now updates itself when a sync finishes, whether it is empty or populated. New threads appear where the sort puts them, threads that stopped matching leave, and threads whose state changed repaint. Refreshing used to mean re-running the query, which cleared the model, the selection, the message pane and the undo stack, so 0.8.0 declined to do it on a cron timer and asked the user to press Enter instead. The result was a list that quietly disagreed with the database: mail indexed by cron never appeared, and an Unread view read to the end sat empty in front of it. ThreadListModel::reconcile() diffs a result against the current rows by thread id instead, so a surviving thread keeps its row, its persistent index and its loaded replies. Order comes from the result and is never imposed here, which is what makes the sort dropdown authoritative. The undo constraint this was sized around did not exist: no undo entry was ever keyed on a row. ThreadTagCommand stores thread ids and MessageTagCommand stores message ids, and applyTagChange() looks its target up by id, so an entry already survived its rows leaving the view. A thread read out of the current view now leaves the list, which is correct and would otherwise strand the reader, so MessageView grows a notice saying the open thread no longer matches, with a button that re-queries it. Recovery lists the whole conversation, expands it, and restores the message that was on screen rather than reopening at the first one. Ten defects were found building this, nine of them by hand testing: - SyncMonitor::start() polls synchronously, so an idle lock file emits stateChanged(Idle) from inside buildUi() and the first handler to touch a widget segfaults before the window exists. - QTreeView sets a current index when it takes focus with none set, and current drives loading, so new mail opened itself and was marked read without the user having looked at it. Selection is now required. - The notice outlived what it described, both when the pane was blanked and when another message replaced it. - Retiring the "Background sync completed" message left the bar claiming a sync was still running: silent means saying nothing new, not leaving a stale claim on screen. - A thread root sets both the thread id and the message id, so treating the message id as the message-row case discarded it for the commonest way to open a thread. - A freshly queried root does not know its own first message until the tree loads, so recovery selected nothing and left the pane blank. - A user query mid-recovery had its result hijacked by the pending selection. - MessageView emitted the recovery signal with its own members, so a direct connection handed MainWindow references that runCurrentQuery() then cleared by blanking the pane. The ids went empty mid-slot and no recovery ever ran. Every test passed against this, because reaching a slot through invokeMethod copies its arguments. A Qt signal argument is a reference until something copies it. Emitting a member to a slot that can re-enter the emitter is a use-after-write, and it presents as a wrong value rather than as a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests/test_threadlistmodel.cpp')
-rw-r--r--tests/test_threadlistmodel.cpp248
1 files changed, 248 insertions, 0 deletions
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index aa71080..947a6b3 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -73,6 +73,14 @@ private slots:
void tagChangeForUnknownThreadIsIgnored();
void tagChangeRoundTripsForRevert();
void modelPassesQtTester();
+ void reconcileAddsNewThreadsInTheOrderGiven();
+ void reconcileRemovesThreadsThatNoLongerMatch();
+ void reconcileKeepsSurvivingRowsAndTheirExpansion();
+ void reconcileUpdatesTagsOnASurvivingThread();
+ void reconcileOnAnEmptyModelFillsIt();
+ void reconcileWithAnIdenticalResultChangesNothing();
+ void reconcileMovesAThreadBumpedByANewReply();
+ void reconcileKeepsAMovedRowsPersistentIndex();
};
static ThreadSummary makeThread(const QString &id, const QString &subject)
@@ -1172,5 +1180,245 @@ void TestThreadListModel::replySharingEveryThreadTagShowsNone()
.isEmpty());
}
+void TestThreadListModel::reconcileAddsNewThreadsInTheOrderGiven()
+{
+ // Item 35b. The auto-refresh hands the model a fresh result set and the
+ // model works out the difference, rather than being cleared and refilled.
+ //
+ // Position comes from the result, never from a rule of this model's own:
+ // the query is sorted by the worker, so with newest-first a new thread
+ // arrives at the front and with oldest-first at the back. A model that
+ // forced new rows to the top would contradict the sort the user chose.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ model.reconcile({ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Newest")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t1"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileRemovesThreadsThatNoLongerMatch()
+{
+ // A thread read out of an Unread view stops matching, and the list has to
+ // say so. Leaving it would make the list disagree with its own query, and
+ // every view-wide action (Mark all read) acts on what the list holds.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ model.reconcile({ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 1);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileKeepsSurvivingRowsAndTheirExpansion()
+{
+ // The whole point of reconciling rather than clearing. A surviving thread
+ // must keep the SAME row identity, because the view's selection, its
+ // expanded state and the open message all hang off persistent indexes: a
+ // beginResetModel drops every one of them, which is what made the old
+ // refresh close the thread being read.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ MessageNode root = makeNode(QStringLiteral("m1"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m2"), 1);
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+ QCOMPARE(model.rowCount(model.index(0, 0)), 1);
+
+ const QPersistentModelIndex survivor(model.index(0, 0));
+ QVERIFY(survivor.isValid());
+
+ // t2 leaves, t3 arrives, t1 stays put.
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ QVERIFY2(survivor.isValid(),
+ "reconciling invalidated a surviving row, so the selection and "
+ "the open thread would be lost exactly as a reset loses them");
+ QCOMPARE(model.threadAt(survivor.row()).threadId, QStringLiteral("t1"));
+
+ // Its loaded replies survive too, or the thread collapses under the reader.
+ QCOMPARE(model.rowCount(model.index(survivor.row(), 0)), 1);
+}
+
+void TestThreadListModel::reconcileUpdatesTagsOnASurvivingThread()
+{
+ // A thread that stays but changed state: read elsewhere, tagged by a
+ // filter, flagged on the phone. The row has to repaint, or the list shows
+ // stale state while claiming to be current.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")) });
+ QVERIFY(model.threadAt(0).tags.contains(QStringLiteral("unread")));
+
+ ThreadSummary readNow = makeThread(QStringLiteral("t1"),
+ QStringLiteral("One"));
+ readNow.tags = QStringList{ QStringLiteral("inbox") };
+
+ QSignalSpy changed(&model, &QAbstractItemModel::dataChanged);
+ model.reconcile({ readNow });
+
+ QCOMPARE(model.rowCount(), 1);
+ QVERIFY2(!model.threadAt(0).tags.contains(QStringLiteral("unread")),
+ "a surviving thread kept its stale tags");
+ QVERIFY2(!changed.isEmpty(),
+ "the row's new state was stored without repainting it");
+}
+
+void TestThreadListModel::reconcileOnAnEmptyModelFillsIt()
+{
+ // Item 35a's case, now reached through the same path as every other
+ // refresh rather than through a special one: read the view empty, cron
+ // indexes new mail, it appears.
+ ThreadListModel model;
+ QCOMPARE(model.rowCount(), 0);
+
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("New")) });
+
+ QCOMPARE(model.rowCount(), 1);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1"));
+}
+
+void TestThreadListModel::reconcileWithAnIdenticalResultChangesNothing()
+{
+ // The common case: the sync brought nothing this query cares about. It
+ // runs every ten minutes under a reader, so it must not churn rows, and
+ // must not emit a reset that would collapse an expanded thread.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ const QPersistentModelIndex kept(model.index(1, 0));
+ QSignalSpy reset(&model, &QAbstractItemModel::modelReset);
+ QSignalSpy inserted(&model, &QAbstractItemModel::rowsInserted);
+ QSignalSpy removed(&model, &QAbstractItemModel::rowsRemoved);
+
+ model.reconcile({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 2);
+ QVERIFY2(reset.isEmpty(), "an unchanged result reset the model");
+ QVERIFY2(inserted.isEmpty(), "an unchanged result inserted rows");
+ QVERIFY2(removed.isEmpty(), "an unchanged result removed rows");
+ QVERIFY(kept.isValid());
+ QCOMPARE(kept.row(), 1);
+}
+
+void TestThreadListModel::reconcileMovesAThreadBumpedByANewReply()
+{
+ // The case the other reconcile tests all miss, and the commonest reordering
+ // there is: an old thread gets a new reply, so under newest-first the
+ // worker returns it at the FRONT although it was already on screen. It is
+ // neither an arrival nor a departure, and a reconcile that only handles
+ // those two leaves it where it was, showing an order the query does not
+ // agree with.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ // t3 was replied to and now sorts first.
+ model.reconcile({ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t1"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t2"));
+}
+
+void TestThreadListModel::reconcileKeepsAMovedRowsPersistentIndex()
+{
+ // A reordering seen from the VIEW's side rather than the data's.
+ //
+ // reconcile() places rows with beginMoveRows, and the assertions on
+ // threadAt() cannot tell a correct move from a broken one: QVector::move
+ // reorders the storage whatever Qt was told, so the data lands right even
+ // if the signal is wrong and only a persistent index reports the
+ // difference. A real view's selection rides on exactly that.
+ //
+ // Every move reconcile() makes is upwards, which is a property of the walk
+ // and not of this data: the result is walked front to back, so rows ahead
+ // of the target are already final and a misplaced survivor is always
+ // pulled forward. The model asserts that invariant.
+ ThreadListModel model;
+
+ // Fatal, and attached BEFORE the move. The tester is what actually checks
+ // the beginMoveRows arguments against the rows that end up moving; the
+ // assertions below read m_threads, which QVector::move reorders correctly
+ // whatever destination Qt was told. Without this a wrong destination
+ // corrupts only what the VIEW is told, and every assertion here still
+ // passes while a real view's selection lands on the wrong row.
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Fatal);
+ Q_UNUSED(tester);
+
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")),
+ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")) });
+
+ const QPersistentModelIndex moved(model.index(0, 0));
+ QVERIFY(moved.isValid());
+
+ // t1 ends last. Reached by t2 and t3 each being pulled forward past it,
+ // which is what makes t1's persistent index the thing under test: it is
+ // displaced twice without ever being the row that moves.
+ model.reconcile({ makeThread(QStringLiteral("t2"),
+ QStringLiteral("Two")),
+ makeThread(QStringLiteral("t3"),
+ QStringLiteral("Three")),
+ makeThread(QStringLiteral("t1"),
+ QStringLiteral("One")) });
+
+ QCOMPARE(model.rowCount(), 3);
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t2"));
+ QCOMPARE(model.threadAt(1).threadId, QStringLiteral("t3"));
+ QCOMPARE(model.threadAt(2).threadId, QStringLiteral("t1"));
+
+ // The persistent index followed the row rather than being invalidated,
+ // which is what keeps a selection on a thread that reordered under it.
+ // This is the assertion the destination adjustment is answerable to: with
+ // the wrong destination the DATA still lands correctly (QVector::move does
+ // not care what Qt was told) and only this reports the difference.
+ QVERIFY2(moved.isValid(), "a moved row lost its persistent index");
+ QCOMPARE(moved.row(), 2);
+}
+
QTEST_MAIN(TestThreadListModel)
#include "test_threadlistmodel.moc"