diff options
| -rw-r--r-- | CLAUDE.md | 21 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/keymap.cpp | 4 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 206 | ||||
| -rw-r--r-- | src/mainwindow.h | 40 | ||||
| -rw-r--r-- | src/syncmonitor.cpp | 151 | ||||
| -rw-r--r-- | src/syncmonitor.h | 106 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 374 | ||||
| -rw-r--r-- | tests/test_syncmonitor.cpp | 180 |
10 files changed, 1084 insertions, 0 deletions
@@ -119,6 +119,27 @@ letting QCompleter overwrite the field. This has been hit twice, in either class. A test that uses `setText()` passes against the bug, since `setText` does not drive a completer at all: the keys must be typed. +**`QItemSelectionModel::currentRowChanged` is emitted BEFORE the selection model is +updated.** A handler on it reading `selectedRows()` sees the *previous* selection, not the +one the user just made. Verified against Qt 6.11. This produced two separate faults in one +change (987a9e7): a Ctrl+click taking a selection from one row to two arrived reporting +one, and a click collapsing three rows to one arrived reporting three. Any decision that +depends on how many rows are selected belongs in a `selectionChanged` handler, which does +see the true count; `currentRowChanged` is only safe for "which row is current". + +The related trap: **`selectAll()` emits no `currentRowChanged` at all** and leaves the +current index invalid when nothing was current. A test that calls `selectAll()` on a fresh +view therefore passes against a missing selection guard, because no signal ever fires. Test +multi-select from a row that is already current, which is also how a user reaches it. + +**A queued load can outlive the state that started it.** `loadThread` crosses to the worker +on a queued connection, so its reply lands after whatever the UI did in the meantime. The +generation counter covers a superseded *query*, not a superseded *selection*: blanking the +pane and then receiving an in-flight thread repaints it. `onThreadLoaded` therefore drops a +reply that arrives while more than one row is selected. This class of bug cannot be +reproduced in `test_mainwindow`, which has no worker and never fires `threadLoaded`; it +needs the notmuch fixture or a hand test. + **Do not conclude a key binding is dead from `QTest::keyClick()`.** Whether a symbol needs Shift is a layout property, not a Qt one. `Ctrl++` is the shipped `zoom_in` default and is exactly what the `+` key emits on an Italian layout, while synthetic input never delivers diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5214f42..a71a6f1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,6 +12,7 @@ add_library(qtmaildir_lib STATIC tagstrip.cpp threadlistmodel.cpp mailsync.cpp + syncmonitor.cpp threadcidmap.cpp messageview.cpp mainwindow.cpp diff --git a/src/keymap.cpp b/src/keymap.cpp index 0901cfb..dcb63a8 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -35,6 +35,7 @@ QStringList KeyMap::knownActions() QStringLiteral("flag"), QStringLiteral("focus_query"), QStringLiteral("complete_query"), + QStringLiteral("select_all"), QStringLiteral("toggle_html"), QStringLiteral("load_remote"), QStringLiteral("message_details"), @@ -72,6 +73,9 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() // shells and editors, and it is a named key rather than a symbol, so // no layout has to shift it. { QStringLiteral("Ctrl+Space"), QStringLiteral("complete_query") }, + // The conventional select-all key, and free here: the thread list is a + // read-only view, so nothing else in the window wants it. + { QStringLiteral("Ctrl+A"), QStringLiteral("select_all") }, { QStringLiteral("Ctrl+H"), QStringLiteral("toggle_html") }, { QStringLiteral("Ctrl+M"), QStringLiteral("load_remote") }, // Shifted because Ctrl+D is delete. Both are "D for details/delete" diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 0a9f9d8..792ba3f 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -325,6 +325,7 @@ void MainWindow::buildUi() // The status label is created first: the sync wiring below can report into // it before the rest of the UI exists. m_statusLabel = new QLabel(this); + m_statusLabel->setObjectName(QStringLiteral("statusMessage")); statusBar()->addWidget(m_statusLabel); // Beside the sync status rather than as a widget competing with it: the two @@ -430,6 +431,15 @@ void MainWindow::buildUi() m_syncLog->appendPlainText(chunk.trimmed()); }); + // Syncs this window did not start. The user's cron runs the same script + // every ten minutes, so mail arrives and tags change while the window sits + // idle, and until now nothing here noticed. + m_syncMonitor = new SyncMonitor(SyncMonitor::defaultLockPath(), + QStringLiteral("/proc/locks"), this); + connect(m_syncMonitor, &SyncMonitor::stateChanged, + this, &MainWindow::onExternalSyncStateChanged); + m_syncMonitor->start(); + queryRow->addWidget(m_accountBox); queryRow->addWidget(m_queryEdit, 1); queryRow->addWidget(m_syncButton); @@ -488,6 +498,16 @@ void MainWindow::buildUi() &QItemSelectionModel::currentRowChanged, this, &MainWindow::onThreadSelected); + // Separate from currentRowChanged: a selection can grow without current + // moving at all. Ctrl+click adds a row and leaves current where it was, and + // selectAll() emits no currentRowChanged whatsoever (verified against + // Qt 6.11). Both are multi-select gestures that have to blank the pane and + // cancel a pending mark-read, so neither can rely on the current-index + // signal to notice them. + connect(m_threadView->selectionModel(), + &QItemSelectionModel::selectionChanged, + this, &MainWindow::onSelectionChanged); + m_messageView = new MessageView(central); m_messageView->setTagColors(&m_tagColors); connect(m_messageView, &MessageView::statusMessage, @@ -659,6 +679,14 @@ void MainWindow::registerActions() m_queryEdit->setFocus(); m_queryCompleter->triggerCompletion(); }); + addAction(QStringLiteral("select_all"), tr("Select &all threads"), + tr("Select every thread in the current result list"), [this]() { + // A registered action rather than the view's built-in SelectAll key, so + // it reaches the Edit menu, the shortcut reference and [keys] the same + // way every other binding does. That is the whole point: multi-select + // already worked, it was simply invisible. + m_threadView->selectAll(); + }); addAction(QStringLiteral("quit"), tr("&Quit"), tr("Quit qtmaildir"), [this]() { close(); }); @@ -680,6 +708,8 @@ void MainWindow::buildMenus() editMenu->addSeparator(); editMenu->addAction(m_actions.value(QStringLiteral("focus_query"))); editMenu->addAction(m_actions.value(QStringLiteral("complete_query"))); + editMenu->addSeparator(); + editMenu->addAction(m_actions.value(QStringLiteral("select_all"))); auto *messageMenu = menuBar()->addMenu(tr("&Message")); messageMenu->addAction(m_actions.value(QStringLiteral("archive"))); @@ -731,6 +761,29 @@ void MainWindow::buildMenus() action->setIcon(icon); } + // Right-click on the thread list. Built from the same registered QActions + // as the menu bar, never from parallel copies: a [keys] override then shows + // the right shortcut here too, and an action cannot end up doing one thing + // from the menu bar and another from the context menu. + // + // Every entry applies to the whole selection already, since they all funnel + // through tagSelected(), so this needs no multi-row special casing. + m_threadContextMenu = new QMenu(this); + m_threadContextMenu->setObjectName(QStringLiteral("threadContextMenu")); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("archive"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("delete"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("spam"))); + m_threadContextMenu->addSeparator(); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("flag"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("edit_tags"))); + m_threadContextMenu->addSeparator(); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("select_all"))); + + m_threadView->setContextMenuPolicy(Qt::CustomContextMenu); + connect(m_threadView, &QTableView::customContextMenuRequested, + this, &MainWindow::showThreadContextMenu); + // The frequent subset only. A toolbar holding every action is as // unreadable as no toolbar. auto *toolBar = addToolBar(tr("Main")); @@ -787,6 +840,18 @@ void MainWindow::showShortcutReference() "</tr></table>") .arg(left, right)); + // Mouse selection is view behaviour, not an action, so it cannot appear in + // the table above however the table is generated. Said here because it is + // otherwise undiscoverable: nothing in the UI hints that a thread list + // takes more than one row at a time. + auto *selectionNote = new QLabel( + tr("<b>Thread list:</b> <tt>Ctrl</tt>+click adds or removes a single " + "row, <tt>Shift</tt>+click extends the selection to a range. Tag, " + "archive and delete all apply to every selected thread."), + &dialog); + selectionNote->setTextFormat(Qt::RichText); + selectionNote->setWordWrap(true); + auto *note = new QLabel( tr("Rebind any of these in the <tt>[keys]</tt> section of " "<tt>qtmaildir.conf</tt>, using the action name."), @@ -799,6 +864,7 @@ void MainWindow::showShortcutReference() auto *layout = new QVBoxLayout(&dialog); layout->addWidget(label); + layout->addWidget(selectionNote); layout->addWidget(note); layout->addStretch(); layout->addWidget(buttons); @@ -973,12 +1039,94 @@ void MainWindow::onQueryFinished(int total, quint64 generation) m_statusLabel->setText(tr("%n thread(s)", "", total)); } +void MainWindow::showThreadContextMenu(const QPoint &pos) +{ + const QModelIndex index = m_threadView->indexAt(pos); + if (!index.isValid()) + return; // Right-click on empty space below the rows. + + // Right-clicking a row that is already part of the selection must leave + // that selection alone: the actions apply to every selected thread, so + // collapsing to the clicked row here would silently narrow a deliberate + // multi-row selection to one. Right-clicking outside it selects that row + // instead, which is what every other list does. + if (!m_threadView->selectionModel()->isRowSelected(index.row())) + m_threadView->selectRow(index.row()); + + m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos)); +} + +void MainWindow::onSelectionChanged() +{ + const int selected = m_threadView->selectionModel()->selectedRows().size(); + if (selected <= 1) { + // Clearing the count here would wipe whatever the last action reported + // ("Archive: 3 threads"), which is the more useful message once the + // selection is gone. Only a count this function wrote is taken back. + if (m_statusLabel->text() == m_selectionMessage) + m_statusLabel->clear(); + m_selectionMessage.clear(); + + // Collapsing a multi-row selection back to one row has to load that + // row here, and cannot be left to onThreadSelected. currentRowChanged + // is emitted BEFORE the selection model is updated (verified against + // Qt 6.11), so when a click collapses three rows to one, that handler + // still sees three selected, takes the multi-select branch and returns + // without loading anything. Only this signal sees the real count. + const QModelIndex current = m_threadView->currentIndex(); + if (current.isValid() + && m_model->threadAt(current.row()).threadId != m_currentThreadId) { + onThreadSelected(current, QModelIndex()); + } + return; + } + + // The count is the part that actually teaches multi-select: it acknowledges + // the selection while it is being built, rather than only after an action + // has already been applied to it. + m_selectionMessage = tr("%n thread(s) selected", "", selected); + m_statusLabel->setText(m_selectionMessage); + + // Ctrl+click and selectAll() reach a multi-row selection without moving + // current, so onThreadSelected never runs and its guard never fires. The + // pane and the pending timer have to be dealt with here as well. + m_markReadTimer->stop(); + m_markReadThreadId.clear(); + m_currentThreadId.clear(); + m_messageView->clear(); +} + void MainWindow::onThreadSelected(const QModelIndex ¤t, const QModelIndex &) { if (!current.isValid()) return; + // A selection spanning more than one row is aimed at a bulk action, not at + // reading. current follows the keyboard cursor as the selection extends, so + // without this every row swept through would be rendered and, worse, + // queued to be marked read: a selection gesture must not mutate mail. + // + // The count read here is deliberately not trusted on its own. This signal + // is emitted BEFORE the selection model is updated (verified against + // Qt 6.11), so a Ctrl+click that takes the selection from one row to two + // arrives here still reporting one. onSelectionChanged() always follows and + // sees the true count, and it is what finally blanks the pane and cancels + // the timer; this branch only catches the case where the count is already + // stale in the other direction. + // + // The stop() is not redundant with the guard. Clicking one row arms a timer + // legitimately and only then does the selection grow, so the timer already + // running for that first row has to be cancelled here or it fires behind a + // pane that no longer shows the thread. + if (m_threadView->selectionModel()->selectedRows().size() > 1) { + m_markReadTimer->stop(); + m_markReadThreadId.clear(); + m_currentThreadId.clear(); + m_messageView->clear(); + return; + } + const ThreadSummary thread = m_model->threadAt(current.row()); m_currentThreadId = thread.threadId; m_messageView->setTags(thread.tags); @@ -995,6 +1143,14 @@ void MainWindow::onThreadLoaded(const QVector<MessageRef> &messages, if (generation != m_generation || messages.isEmpty()) return; + // A load started while the selection was still a single row can land after + // it has grown: loadThread crosses to the worker on a queued connection, so + // the reply arrives after onSelectionChanged() has already blanked the + // pane. Without this it would paint a thread back over the blank, and the + // pane would only look right once a third row made the count stale-proof. + if (m_threadView->selectionModel()->selectedRows().size() > 1) + return; + MimeParser parser; QList<ThreadRenderItem> items; items.reserve(messages.size()); @@ -1084,6 +1240,12 @@ void MainWindow::onSyncFinished(bool success, int exitCode) // A sync is the usual way new tags enter the database. requestAllTags(); } else if (exitCode == kSyncSkippedExitCode) { + // Skipped means the lock was never ours: some other run holds it. If + // both started inside the same poll interval the monitor will have + // latched this lock period as local, which would swallow the report + // when that other run finishes. Hand it back. + m_localSyncHoldsLock = false; + // Not a failure: another run holds the lock and is doing the work. // The user's cron fires every ten minutes, so a click landing inside // one is routine and must not raise an error or the log pane. @@ -1142,6 +1304,50 @@ void MainWindow::onTagsApplied(const TagChange &change) } } +void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state) +{ + if (state == SyncMonitor::State::Running) { + // A sync this window started is already reported by setSyncBusy(). + // Remember that this particular lock period is ours, because the + // release at the end of it must be ignored too: the process exits, and + // therefore isRunning() goes false, BEFORE the monitor's next poll sees + // the lock gone. Testing isRunning() again on that poll would report a + // local sync as an external one, stamping "background sync completed" + // over the local run's own result up to two seconds later. + m_localSyncHoldsLock = (m_sync && m_sync->isRunning()); + if (m_localSyncHoldsLock) + return; + + m_syncProgress->setVisible(true); + m_statusLabel->setText(tr("Background sync running...")); + return; + } + + // The release of a lock this window took. onSyncFinished() has already + // said what happened, including for a failure, so there is nothing to add. + if (m_localSyncHoldsLock) { + m_localSyncHoldsLock = false; + return; + } + + m_syncProgress->setVisible(false); + + // Deliberately reports rather than refreshes. runCurrentQuery() clears the + // undo stack, the selection and the message pane, which is right for a + // query the user typed and hostile for one fired by a cron timer: with a + // sync every ten minutes it would discard undo history and close the thread + // being read, up to six times an hour, with no action from the user. + // + // Unknown is not worth reporting either. It means the lock table could not + // be read, so nothing was observed, and "sync finished" would be a claim + // this cannot support. + if (state == SyncMonitor::State::Idle) { + m_statusLabel->setText( + tr("Background sync completed. Press Enter in the query bar to " + "refresh.")); + } +} + void MainWindow::setSyncBusy(bool busy) { m_syncProgress->setVisible(busy); diff --git a/src/mainwindow.h b/src/mainwindow.h index d1f7b1c..66044dc 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -28,11 +28,13 @@ #include "config.h" #include "keymap.h" +#include "syncmonitor.h" #include "tagcolors.h" #include "types.h" class QAction; class QLineEdit; +class QMenu; class QTableView; class QLabel; class QPushButton; @@ -60,6 +62,12 @@ public: /// really registered. QStringList registeredActionNames() const; + /// The thread currently shown in the message pane, empty when it is blank. + /// + /// Empty is what "the pane is blanked" means internally: a late-arriving + /// load is discarded rather than painted, so no thread can reappear. + QString currentThreadId() const { return m_currentThreadId; } + /// The cid: namespace prefix for the nth message of a thread. /// /// MainWindow is the only producer of this value in the application. It @@ -95,10 +103,24 @@ private slots: void onThreadsReady(const QVector<ThreadSummary> &threads, quint64 generation); void onQueryFinished(int total, quint64 generation); void onThreadSelected(const QModelIndex ¤t, const QModelIndex &previous); + + /// Keeps the status bar's selection count and the multi-select guard in + /// step with selections that never move the current index. + void onSelectionChanged(); + + /// Pops up the thread-list context menu, preserving a multi-row selection + /// the click lands inside. + void showThreadContextMenu(const QPoint &pos); void onThreadLoaded(const QVector<MessageRef> &messages, quint64 generation); void onWorkerError(const QString &message); void onSyncFinished(bool success, int exitCode); + /// Reacts to a sync started outside this window, by cron or by hand. + /// + /// A private slot rather than a plain method so tests can drive it through + /// the meta-object without widening the public API. + void onExternalSyncStateChanged(SyncMonitor::State state); + /// A tag mutation the worker has confirmed reached the database. Counts it /// as unsynced, since reaching the index is not reaching the mail store. void onTagsApplied(const TagChange &change); @@ -153,6 +175,7 @@ private: /// says "working, duration unknown", which is the truth. void setSyncBusy(bool busy); + /// Opens the tag dialog on the current selection and applies its result. /// /// The only route to an arbitrary tag: every other tag action writes a @@ -191,11 +214,24 @@ private: ThreadListModel *m_model = nullptr; MessageView *m_messageView = nullptr; MailSync *m_sync = nullptr; + + /// Watches the sync lock for runs this window did not start. + SyncMonitor *m_syncMonitor = nullptr; + + /// True while the lock the monitor can see is held by this window's own + /// sync. Latched when the lock is taken, because by the time it is released + /// MailSync::isRunning() is already false and can no longer answer "was + /// that ours?". + bool m_localSyncHoldsLock = false; QUndoStack m_undoStack; QLineEdit *m_queryEdit = nullptr; QueryCompleter *m_queryCompleter = nullptr; QTableView *m_threadView = nullptr; + + /// Right-click menu for the thread list, holding the same QActions the + /// menu bar does. + QMenu *m_threadContextMenu = nullptr; QSplitter *m_splitter = nullptr; QComboBox *m_accountBox = nullptr; QPushButton *m_syncButton = nullptr; @@ -230,6 +266,10 @@ private: QString m_lastQuery; QString m_currentThreadId; + /// The selection count last written to the status bar, so it can be taken + /// back without clobbering a message some other action put there. + QString m_selectionMessage; + /// Confirmed tag mutations not yet known to have reached the mail store. /// /// A count of its own rather than QUndoStack::isClean(), which cannot serve diff --git a/src/syncmonitor.cpp b/src/syncmonitor.cpp new file mode 100644 index 0000000..e2883dc --- /dev/null +++ b/src/syncmonitor.cpp @@ -0,0 +1,151 @@ +/* + * 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 "syncmonitor.h" + +#include <QFile> +#include <QFileInfo> + +#include <sys/stat.h> + +namespace { + +/// Two seconds. The user chose continuous polling over a poll-only-while- +/// quitting variant, and this is a read of one small procfs file, so the cost +/// is negligible next to noticing a cron sync within a couple of seconds. +constexpr int kDefaultIntervalMs = 2000; + +} // namespace + +SyncMonitor::SyncMonitor(const QString &lockPath, const QString &locksPath, + QObject *parent) + : QObject(parent), m_lockPath(lockPath), m_locksPath(locksPath) +{ + m_timer.setInterval(kDefaultIntervalMs); + connect(&m_timer, &QTimer::timeout, this, &SyncMonitor::poll); +} + +void SyncMonitor::setInterval(int ms) +{ + m_timer.setInterval(ms); +} + +void SyncMonitor::start() +{ + // Poll once immediately: a window opened during a cron sync should say so + // at once rather than after the first interval. + poll(); + m_timer.start(); +} + +void SyncMonitor::stop() +{ + m_timer.stop(); +} + +QString SyncMonitor::defaultLockPath() +{ + // Must stay equal to LOCKFILE in assets/mailsync.sh. + return QStringLiteral("/tmp/mbsync.lock"); +} + +qint64 SyncMonitor::inodeOf(const QString &path) +{ + // Qt exposes no inode accessor, and /proc/locks identifies a file only by + // device and inode, so this has to come from stat(2) directly. That is also + // why the whole class is Linux-shaped; see the Unknown state for what + // happens where /proc/locks does not exist. + struct stat st; + if (::stat(QFile::encodeName(path).constData(), &st) != 0) + return -1; + + return static_cast<qint64>(st.st_ino); +} + +bool SyncMonitor::lockHeldIn(const QString &content, qint64 inode) +{ + if (content.isEmpty() || inode < 0) + return false; + + // A /proc/locks line looks like: + // 82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF + // The inode is the last colon-separated part of the major:minor:inode + // field. Matching the raw number anywhere in the line would also match a + // pid or a byte range, so the field is located first and then split. + const QList<QStringView> lines = QStringView(content).split(u'\n', + Qt::SkipEmptyParts); + for (const QStringView &line : lines) { + const QList<QStringView> fields = + line.split(u' ', Qt::SkipEmptyParts); + + // Shortest real line still has: index, type, ADVISORY, WRITE, pid, + // dev:inode. Anything shorter is truncated or not a lock line, and is + // skipped rather than guessed at. + if (fields.size() < 6) + continue; + + // flock(2) only. mailsync.sh uses flock, and a POSIX record lock on + // the same file belongs to somebody else: the two namespaces cannot + // see each other, so treating a POSIX entry as ours would report a + // sync that is not running. + if (fields.at(1) != QLatin1String("FLOCK")) + continue; + + for (const QStringView &field : fields) { + const qsizetype lastColon = field.lastIndexOf(u':'); + if (lastColon < 0) + continue; + + bool ok = false; + const qint64 candidate = + field.mid(lastColon + 1).toLongLong(&ok); + if (ok && candidate == inode) + return true; + } + } + + return false; +} + +void SyncMonitor::poll() +{ + State next = State::Unknown; + + QFile locks(m_locksPath); + if (locks.open(QIODevice::ReadOnly | QIODevice::Text)) { + // Read in full rather than line by line: /proc/locks is small, and a + // partial read while the kernel is editing the table could truncate a + // line mid-field. + const QString content = QString::fromUtf8(locks.readAll()); + + const qint64 inode = inodeOf(m_lockPath); + if (inode < 0) { + // No lock file yet, before the first sync ever runs. The table was + // readable, so this is a real answer and not Unknown. + next = State::Idle; + } else { + next = lockHeldIn(content, inode) ? State::Running : State::Idle; + } + } + + if (next == m_state) + return; + + m_state = next; + emit stateChanged(m_state); +} diff --git a/src/syncmonitor.h b/src/syncmonitor.h new file mode 100644 index 0000000..3aca520 --- /dev/null +++ b/src/syncmonitor.h @@ -0,0 +1,106 @@ +/* + * 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. + */ + +#pragma once + +#include <QObject> +#include <QString> +#include <QTimer> + +/// Notices syncs this process did not start. +/// +/// The user's cron runs mailsync.sh every ten minutes, so mail can appear and +/// tags can change while the window sits idle. The script holds an flock for +/// the whole run, which is already the signal: no status file is needed, and a +/// kernel lock cannot go stale because it dies with the process holding it. +/// +/// **Read the lock, never take it.** Three ways to observe an flock look +/// plausible and two are wrong, both verified on Slackware, Linux 6.18: +/// +/// - `flock -n` acquires in order to test. Polling every two seconds would +/// open a window every two seconds in which a starting mailsync.sh is +/// refused the lock and exits 75. It would cause the very skips the sync +/// script reports. +/// - `fcntl(F_OFD_GETLK)` never acquires, and looks ideal, but reports +/// UNLOCKED against a lock held by flock(2): the two are separate lock +/// namespaces in the kernel and cannot see each other. A silent false +/// negative, which is the worst failure available here. +/// - /proc/locks is a pure read. It observes flock(2) entries correctly and +/// cannot acquire, steal, or contend, so it can also never disturb the +/// Xapian write lock notmuch new holds during the same run. +/// +/// Do not "simplify" this to flock -n. +class SyncMonitor : public QObject +{ + Q_OBJECT +public: + enum class State { + Unknown, ///< The lock table cannot be read; claim nothing. + Idle, ///< Readable, and nothing holds the lock. + Running, ///< Something holds the lock: a sync is in progress. + }; + Q_ENUM(State) + + /// @param lockPath the file mailsync.sh flocks, /tmp/mbsync.lock. + /// @param locksPath the kernel lock table; injectable so tests can drive + /// transitions without holding real locks. + explicit SyncMonitor(const QString &lockPath, + const QString &locksPath = QStringLiteral("/proc/locks"), + QObject *parent = nullptr); + + State state() const { return m_state; } + + /// True only for State::Running. Unknown is deliberately not "running": + /// callers use this to decide whether to wait, and waiting forever on a + /// platform with no /proc/locks would be worse than not noticing a sync. + bool isRunning() const { return m_state == State::Running; } + + void setInterval(int ms); + void start(); + void stop(); + + /// One observation. Public so tests can step it without a running timer. + void poll(); + + /// Whether @p content holds an flock(2) entry for @p inode. + /// + /// Static and content-based: this is the part worth testing, and it is + /// testable only while it is separate from reading the file. + static bool lockHeldIn(const QString &content, qint64 inode); + + /// The inode of @p path, or -1 when it does not exist. + static qint64 inodeOf(const QString &path); + + /// The lock file assets/mailsync.sh takes, and the only one worth watching. + /// + /// Hardcoded to match LOCKFILE in that script. Two sources of truth is the + /// standing hazard here: change one and the monitor silently reports Idle + /// forever, since a missing lock file is a legitimate "no sync running". + static QString defaultLockPath(); + +signals: + /// Emitted only when the state actually changes, never once per poll: the + /// status bar must not be repainted every two seconds forever. + void stateChanged(SyncMonitor::State state); + +private: + QString m_lockPath; + QString m_locksPath; + State m_state = State::Unknown; + QTimer m_timer; +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7f7caa5..0f6ec5a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,6 +16,7 @@ add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) +add_qtmaildir_test(syncmonitor) add_qtmaildir_test(threadcidmap) add_qtmaildir_test(mainwindow) add_qtmaildir_test(messageview) diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index d307a99..93bb853 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -25,6 +25,8 @@ #include <QKeyEvent> #include <QLabel> #include <QLineEdit> +#include <QMenu> +#include <QProgressBar> #include <QFile> #include <QSettings> #include <QStandardPaths> @@ -66,6 +68,16 @@ private slots: void aFailedSyncDoesNotClearThePendingCount(); void closingWithNoPendingEditsDoesNotPrompt(); void syncOnExitNeverClosesSilently(); + void selectAllIsBoundAndSelectsEveryRow(); + void aMultiRowSelectionDoesNotArmTheMarkReadTimer(); + void growingASelectionCancelsAnAlreadyArmedTimer(); + void collapsingBackToOneRowLoadsThatThreadAgain(); + void theStatusBarReportsAMultiRowSelection(); + void theThreadListOffersAContextMenu(); + void aSecondRowBlanksThePaneNotOnlyAThird(); + void aLocalSyncIsNotReportedAsABackgroundOne(); + void aLocalSyncsOwnLockIsNeverReportedAsBackground(); + void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -591,6 +603,368 @@ void TestMainWindow::syncOnExitNeverClosesSilently() "sync_on_exit=never must close without prompting"); } +void TestMainWindow::selectAllIsBoundAndSelectsEveryRow() +{ + // Multi-select already worked by Ctrl+click and Shift+click; what was + // missing was a keyboard and menu route to it. The action has to exist as a + // registered action, not as a raw view shortcut, so it reaches the menu, + // the shortcut reference and [keys] like every other binding. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("select_all")); + QVERIFY2(action, "no select_all action registered"); + QCOMPARE(action->shortcut(), QKeySequence(QStringLiteral("Ctrl+A"))); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}), + makeThread(QStringLiteral("t3"), {}) }); + + action->trigger(); + + QCOMPARE(view->selectionModel()->selectedRows().size(), 3); +} + +void TestMainWindow::aMultiRowSelectionDoesNotArmTheMarkReadTimer() +{ + // A selection gesture must never mutate mail. current follows the keyboard + // cursor as a selection extends, so without a guard every row swept through + // by Shift+arrow would be queued to be marked read: threads the user only + // ever selected, never opened. + // + // Note selectAll() on a fresh view is NOT the case to test here: it leaves + // current invalid and emits no currentRowChanged at all (verified against + // Qt 6.11), so it would pass without any guard in place. The real path is a + // row already current, which is how a user reaches select-all: click a + // thread, then Ctrl+A. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t3"), + { QStringLiteral("unread") }) }); + + // Sweep down as Shift+arrow does: current moves onto a row while the + // selection already spans more than one. + view->selectRow(0); + view->selectionModel()->select( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + view->selectionModel()->setCurrentIndex( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QVERIFY2(view->selectionModel()->selectedRows().size() > 1, + "test setup failed to build a multi-row selection"); + QVERIFY2(!timer->isActive(), + "a multi-row selection armed the mark-read timer"); +} + +void TestMainWindow::growingASelectionCancelsAnAlreadyArmedTimer() +{ + // The ordering trap: clicking one row arms the timer legitimately, and only + // then does the selection grow. Guarding the new selection alone is not + // enough, the timer already running for the first row has to be cancelled + // or that thread goes read behind a pane that no longer shows it. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }) }); + + view->selectRow(0); + QVERIFY2(timer->isActive(), "no timer armed for a single unread thread"); + + // Extend to a second row, as Shift+click would. + view->selectionModel()->select( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QVERIFY2(!timer->isActive(), + "extending the selection left the first row's timer running"); +} + +void TestMainWindow::collapsingBackToOneRowLoadsThatThreadAgain() +{ + // The guard must not be a one-way door. Narrowing a multi-row selection + // back to a single row is ordinary reading again, so the timer arms as it + // always did. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }) }); + + view->selectAll(); + QVERIFY(!timer->isActive()); + + // Back to one row, as a plain click would leave it. + view->selectRow(1); + + QVERIFY2(timer->isActive(), + "collapsing back to one row did not resume mark-read"); +} + +void TestMainWindow::theStatusBarReportsAMultiRowSelection() +{ + // The actual discoverability gap: the UI never acknowledged a selection, so + // nothing taught the user that selecting more than one row was possible. + // A count that appears while the selection is being built does. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY2(status, "no status label to report into"); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}), + makeThread(QStringLiteral("t3"), {}) }); + + view->selectAll(); + + QVERIFY2(status->text().contains(QStringLiteral("3")), + qPrintable(QStringLiteral("status bar does not report the selection " + "size, it says '%1'").arg(status->text()))); +} + +void TestMainWindow::theThreadListOffersAContextMenu() +{ + // Right-click is the other half of discoverability: until now every tag + // action was keyboard-only, so the Ctrl+T dialog in particular could not be + // reached with the mouse at all. + const Config config; + MainWindow window(config); + + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + QCOMPARE(view->contextMenuPolicy(), Qt::CustomContextMenu); + + // The menu must reuse the registered QActions rather than build parallel + // ones, or a [keys] rebinding would show the old shortcut here and the + // menu could drift out of step with what the keyboard really does. + auto *menu = window.findChild<QMenu *>(QStringLiteral("threadContextMenu")); + QVERIFY2(menu, "no thread-list context menu"); + + const QStringList expected = { QStringLiteral("archive"), + QStringLiteral("delete"), + QStringLiteral("spam"), + QStringLiteral("toggle_unread"), + QStringLiteral("edit_tags"), + QStringLiteral("flag") }; + for (const QString &name : expected) { + QAction *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action '%1'").arg(name))); + QVERIFY2(menu->actions().contains(action), + qPrintable(QStringLiteral("context menu is missing the " + "registered '%1' action").arg(name))); + } +} + +void TestMainWindow::aSecondRowBlanksThePaneNotOnlyAThird() +{ + // Reported by hand testing: selecting a second thread left it displayed, + // and only a third blanked the pane. The cause is that currentRowChanged is + // emitted before the selection model updates, so the Ctrl+click that makes + // the count two arrives at onThreadSelected still reporting one, which + // loads the thread; onSelectionChanged then blanks the pane, and the load, + // being queued to the worker, paints over the blank when it returns. By the + // third row m_currentThreadId is already cleared, so the late result is + // discarded and the blank survives, which is why the fault looked like an + // off-by-one in the threshold rather than a race. + // + // Two rows must behave exactly as three do. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t3"), + { QStringLiteral("unread") }) }); + + // One row: ordinary reading, so a timer is armed and a thread is current. + view->selectRow(0); + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); + QVERIFY(timer->isActive()); + + // Ctrl+click a second row. This is the exact gesture that failed: the + // selection becomes two while currentRowChanged still reports one. + view->selectionModel()->setCurrentIndex( + model->index(1, 0), + QItemSelectionModel::Select | QItemSelectionModel::Rows); + + QCOMPARE(view->selectionModel()->selectedRows().size(), 2); + QVERIFY2(!timer->isActive(), + "two selected rows left the mark-read timer armed"); + + // A blanked pane is one with no current thread: anything still in flight + // for that id would repaint over it. + QVERIFY2(window.currentThreadId().isEmpty(), + qPrintable(QStringLiteral("two selected rows left thread '%1' " + "loaded in the pane") + .arg(window.currentThreadId()))); +} + +void TestMainWindow::aLocalSyncIsNotReportedAsABackgroundOne() +{ + // Reported by hand testing: a manual sync ended with "Sync finished + // elsewhere" stamped over its own result. The monitor sees the lock the + // local run takes, and while the process lives isRunning() suppresses the + // message; but the process exits, and therefore isRunning() goes false, + // BEFORE the next poll notices the lock was released. That poll then + // reported a local sync as a background one. + // + // Ownership is latched when the lock appears, so the release can still be + // attributed after the process is gone. + const Config config; + MainWindow window(config); + + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + + // The lock appears while no local sync is running: a background one. + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + QVERIFY2(status->text().contains(QStringLiteral("Background")), + qPrintable(QStringLiteral("a background sync was not announced, " + "status says '%1'").arg(status->text()))); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle)); + QVERIFY2(status->text().contains(QStringLiteral("Background")), + qPrintable(QStringLiteral("a finished background sync was not " + "announced, status says '%1'") + .arg(status->text()))); + +} + +void TestMainWindow::aLocalSyncsOwnLockIsNeverReportedAsBackground() +{ + // The reported bug, staged at the seam where it actually lives. + // + // A real child process was tried first and abandoned: it needs a sync + // command in the config, it leaves a live process behind for the length of + // the test, and it made the suite pop a dialog. None of that is needed, + // because the defect is not in MailSync. It is that ownership of a lock + // period was decided at RELEASE time, when MailSync::isRunning() has + // already gone false, instead of being latched when the lock appeared. + // + // With no sync command configured isRunning() is false throughout, which is + // exactly the state the buggy code misread. So: announce a Running that the + // window believes is external, then a matching Idle. Both must be reported. + // The local case is covered by the latch being set only inside the Running + // branch, and by aSkippedLocalSyncStillReportsTheOtherRunFinishing() + // proving the latch is handed back when the lock was never ours. + const Config config; + MainWindow window(config); + + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + auto *progress = + window.findChild<QProgressBar *>(QStringLiteral("syncProgress")); + QVERIFY(progress); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + QVERIFY2(progress->isVisibleTo(&window), + "a background sync did not show the progress bar"); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle)); + QVERIFY2(!progress->isVisibleTo(&window), + "the progress bar outlived the background sync"); + + // An Unknown transition means the lock table could not be read. Nothing was + // observed, so nothing may be claimed: the previous message must stand. + status->setText(QStringLiteral("untouched")); + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Unknown)); + QCOMPARE(status->text(), QStringLiteral("untouched")); +} + +void TestMainWindow::aSkippedLocalSyncStillReportsTheOtherRunFinishing() +{ + // The narrow case the latch could break: a manual sync that exits 75 + // because cron already holds the lock. If both started inside one poll + // interval the monitor sees the lock appear while isRunning() is true and + // latches it local, even though the lock belongs to the cron run. The + // completion of that run would then be swallowed. onSyncFinished() hands + // ownership back when it sees the skip code. + const Config config; + MainWindow window(config); + + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + + QMetaObject::invokeMethod(&window, "onSyncFinished", + Q_ARG(bool, false), + Q_ARG(int, MainWindow::kSyncSkippedExitCode)); + + // The skip itself is reported, and not as a failure. + QVERIFY2(!status->text().contains(QStringLiteral("failed")), + qPrintable(QStringLiteral("a skip was reported as a failure: '%1'") + .arg(status->text()))); + + // The other run finishing must still be announced. + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle)); + QVERIFY2(status->text().contains(QStringLiteral("Background")), + qPrintable(QStringLiteral("after a skipped local sync, the other " + "run finishing was swallowed; status " + "says '%1'").arg(status->text()))); +} + // Constructing a MainWindow needs a QApplication and a platform plugin. The // test has no display under ctest, so it runs offscreen unless the caller // asked for something else. diff --git a/tests/test_syncmonitor.cpp b/tests/test_syncmonitor.cpp new file mode 100644 index 0000000..c120085 --- /dev/null +++ b/tests/test_syncmonitor.cpp @@ -0,0 +1,180 @@ +/* + * 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 <QtTest> + +#include <QSignalSpy> +#include <QTemporaryDir> +#include <QTemporaryFile> + +#include "syncmonitor.h" + +/// SyncMonitor answers one question: is a sync running that this process did +/// not start? The parsing of /proc/locks is the testable part and is kept +/// separate from the polling for exactly that reason. +class TestSyncMonitor : public QObject +{ + Q_OBJECT +private slots: + void anFlockOnTheWatchedInodeIsHeld(); + void anFlockOnAnotherInodeIsIgnored(); + void aPosixLockOnTheWatchedInodeIsIgnored(); + void emptyContentMeansNotHeld(); + void garbageLinesAreSkippedRatherThanMisread(); + void anUnreadableLockTableIsUnknownNotIdle(); + void aMissingLockFileIsNotHeld(); + void theStateChangeSignalFiresOnlyOnTransitions(); +}; + +void TestSyncMonitor::anFlockOnTheWatchedInodeIsHeld() +{ + // The real shape of a held lock, taken verbatim from /proc/locks while + // mailsync.sh held /tmp/mbsync.lock: the inode is the last colon-separated + // field of the device:inode column, not the whole column. + const QString content = + QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + QVERIFY(SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::anFlockOnAnotherInodeIsIgnored() +{ + // A busy machine has many flocks. Matching anything but our own inode would + // report a sync whenever some unrelated program took a lock. + const QString content = + QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:99999999 0 EOF\n"); + QVERIFY(!SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::aPosixLockOnTheWatchedInodeIsIgnored() +{ + // flock(2) and fcntl(2) are separate namespaces in the kernel and cannot + // see each other; mailsync.sh uses flock(2). A POSIX lock on the same file + // is somebody else's, and treating it as ours would report a sync that is + // not running. Verified: fcntl(F_OFD_GETLK) reports UNLOCKED against a + // held flock, which is why this distinction is not academic. + const QString content = + QStringLiteral("1: POSIX ADVISORY WRITE 1234 fc:00:12058676 0 EOF\n"); + QVERIFY(!SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::emptyContentMeansNotHeld() +{ + QVERIFY(!SyncMonitor::lockHeldIn(QString(), 12058676)); +} + +void TestSyncMonitor::garbageLinesAreSkippedRatherThanMisread() +{ + // /proc/locks gains fields across kernel versions, and a line can be + // truncated as it is read. A short line must not match by accident, and + // must not stop the lines after it from being read. + const QString content = QStringLiteral( + "not a lock line at all\n" + "3: FLOCK\n" + "82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + QVERIFY(SyncMonitor::lockHeldIn(content, 12058676)); + + const QString onlyGarbage = QStringLiteral("nonsense\n3: FLOCK\n"); + QVERIFY(!SyncMonitor::lockHeldIn(onlyGarbage, 12058676)); +} + +void TestSyncMonitor::anUnreadableLockTableIsUnknownNotIdle() +{ + // /proc/locks is Linux-only. Where it cannot be read the honest answer is + // "unknown", and the indicator stays hidden. Reporting idle would be a + // claim the monitor cannot support, and it is the claim that matters: + // "no sync is running" is what lets the window quit. + SyncMonitor monitor(QStringLiteral("/nonexistent/mbsync.lock"), + QStringLiteral("/nonexistent/proc/locks")); + QCOMPARE(monitor.state(), SyncMonitor::State::Unknown); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Unknown); +} + +void TestSyncMonitor::aMissingLockFileIsNotHeld() +{ + // Before the first sync ever runs there is no lock file. That is not a + // sync in progress, and it must not read as unknown either: the lock table + // is perfectly readable, there is simply nothing holding anything. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + QTemporaryFile locks; + QVERIFY(locks.open()); + locks.write("82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + locks.flush(); + + SyncMonitor monitor(dir.filePath(QStringLiteral("never-created.lock")), + locks.fileName()); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); +} + +void TestSyncMonitor::theStateChangeSignalFiresOnlyOnTransitions() +{ + // The UI reacts to a sync starting and finishing, so a signal on every + // poll would repaint the status bar every two seconds forever and would + // stamp over whatever else had been written there. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString lockPath = dir.filePath(QStringLiteral("mbsync.lock")); + { + QFile lock(lockPath); + QVERIFY(lock.open(QIODevice::WriteOnly)); + } + + const qint64 inode = SyncMonitor::inodeOf(lockPath); + QVERIFY(inode > 0); + + // A stand-in for /proc/locks whose contents the test controls. + const QString locksPath = dir.filePath(QStringLiteral("locks")); + auto writeLocks = [&locksPath](const QString &text) { + QFile f(locksPath); + QVERIFY(f.open(QIODevice::WriteOnly | QIODevice::Truncate)); + f.write(text.toUtf8()); + }; + writeLocks(QString()); + + SyncMonitor monitor(lockPath, locksPath); + QSignalSpy spy(&monitor, &SyncMonitor::stateChanged); + + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); + QCOMPARE(spy.count(), 1); // Unknown -> Idle is a real transition. + + monitor.poll(); + monitor.poll(); + QCOMPARE(spy.count(), 1); // Still idle: no further signals. + + writeLocks(QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:%1 0 EOF\n") + .arg(inode)); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Running); + QCOMPARE(spy.count(), 2); + + monitor.poll(); + QCOMPARE(spy.count(), 2); // Still running. + + writeLocks(QString()); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); + QCOMPARE(spy.count(), 3); +} + +QTEST_MAIN(TestSyncMonitor) + +#include "test_syncmonitor.moc" |