From 69e777cc72bd846c250d68656a8d558c9401fcd5 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 4 Aug 2026 19:16:07 +0200 Subject: feat(ui): disable Sync during a background sync, and blank the pane on Esc Items 29 and 32. 29 was a constraint item 27 specified and that shipped unbuilt: while a cron sync held the lock the Sync button stayed clickable, and pressing it could only produce the EX_TEMPFAIL skip. The progress bar and the button are now written by one updateSyncControls() taking both sync sources, which the item asked for by name: two independent assignments, one per path, means whichever finishes second wins, so a background sync ending would re-enable the button in the middle of a local run. Unknown re-enables the button, deliberately. It means /proc/locks could not be read and nothing was observed, so leaving the button disabled would strand it permanently wherever the lock cannot be seen. 32 adds a clear_pane action on Esc. It clears m_currentThreadId with the pane, not merely alongside it, or a threadLoaded still in flight would paint the thread straight back; and it cancels any pending mark-read, since a thread blanked from view must not be marked read two seconds later. The selection, the query and the undo stack are untouched. The one real risk in 32 was Escape being stolen from the query completer, the way Return was once lost to a window shortcut. Probed rather than reasoned about: a popup consumes the key before a window-level shortcut sees it, so the completer still dismisses. Every test here was verified by reverting the code it covers. Co-Authored-By: Claude Opus 5 --- tests/test_mainwindow.cpp | 105 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) (limited to 'tests') diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 93bb853..8a1dde3 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -78,6 +79,9 @@ private slots: void aLocalSyncIsNotReportedAsABackgroundOne(); void aLocalSyncsOwnLockIsNeverReportedAsBackground(); void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); + void theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock(); + void anUnobservableLockTableLeavesTheSyncButtonUsable(); + void escapeBlanksTheMessagePane(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -965,6 +969,107 @@ void TestMainWindow::aSkippedLocalSyncStillReportsTheOtherRunFinishing() "says '%1'").arg(status->text()))); } +void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() +{ + // Item 27 specified this and it shipped unbuilt: while a cron sync holds + // the lock the button stayed clickable, and pressing it could only produce + // the EX_TEMPFAIL skip. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + const QString conf = dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + { + QSettings s(conf, QSettings::IniFormat); + s.setValue(QStringLiteral("sync/command"), QStringLiteral("/bin/true")); + } + + Config config; + config.load(conf); + MainWindow window(config); + + auto *button = window.findChild(QStringLiteral("syncButton")); + QVERIFY2(button, "no sync button to check"); + QVERIFY2(button->isEnabled(), "the button starts disabled with a command set"); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + QVERIFY2(!button->isEnabled(), + "the sync button stayed enabled during a background sync"); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle)); + QVERIFY2(button->isEnabled(), + "the sync button was not re-enabled after the background sync"); +} + +void TestMainWindow::anUnobservableLockTableLeavesTheSyncButtonUsable() +{ + // Unknown means /proc/locks could not be read, so nothing was observed. A + // button left permanently disabled on a platform that cannot see the lock + // is worse than one that occasionally offers a run that gets skipped. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + const QString conf = dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + { + QSettings s(conf, QSettings::IniFormat); + s.setValue(QStringLiteral("sync/command"), QStringLiteral("/bin/true")); + } + + Config config; + config.load(conf); + MainWindow window(config); + + auto *button = window.findChild(QStringLiteral("syncButton")); + QVERIFY(button); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + QVERIFY(!button->isEnabled()); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Unknown)); + QVERIFY2(button->isEnabled(), + "an unobservable lock table left the sync button disabled"); +} + +void TestMainWindow::escapeBlanksTheMessagePane() +{ + // A registered action like any other, so it reaches the menus, the shortcut + // reference and [keys]. Clearing m_currentThreadId with the pane is the + // part that matters: a late threadLoaded would otherwise paint the thread + // straight back, which is the race fixed in 0.8.0. + const Config config; + MainWindow window(config); + + auto *action = window.findChild(QStringLiteral("clear_pane")); + QVERIFY2(action, "no clear_pane action registered"); + QCOMPARE(action->shortcut(), QKeySequence(Qt::Key_Escape)); + + auto *model = window.findChild(); + QVERIFY(model); + auto *view = window.findChild(); + QVERIFY(view); + + model->appendBatch({ makeThread(QStringLiteral("t1"), {}), + makeThread(QStringLiteral("t2"), {}) }); + + view->selectRow(0); + QVERIFY2(!window.currentThreadId().isEmpty(), + "no thread was opened to blank"); + + action->trigger(); + QVERIFY2(window.currentThreadId().isEmpty(), + "Escape left the thread loaded in the pane"); + + // Blanking is a view change, not a mail change: the selection stays. + QCOMPARE(view->selectionModel()->selectedRows().size(), 1); +} + // 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. -- cgit v1.2.3