diff options
| -rw-r--r-- | README.md | 6 | ||||
| -rw-r--r-- | src/config.cpp | 10 | ||||
| -rw-r--r-- | src/config.h | 10 | ||||
| -rw-r--r-- | src/mailsync.cpp | 55 | ||||
| -rw-r--r-- | src/mailsync.h | 28 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 27 | ||||
| -rw-r--r-- | src/mainwindow.h | 11 | ||||
| -rw-r--r-- | tests/test_config.cpp | 36 | ||||
| -rw-r--r-- | tests/test_mailsync.cpp | 177 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 278 |
10 files changed, 635 insertions, 3 deletions
@@ -140,6 +140,12 @@ identity. ; assets/mailsync.sh is the reference implementation; see "The sync command". ; command = /home/you/bin/mailsync.sh +; Optional. The sync script's log file, read to tell whether a sync started +; outside the application (a cron run, say) succeeded, so the unsynced-edits +; indicator can clear itself for one. Defaults to the path assets/mailsync.sh +; writes; set it only if you changed the script's LOGFILE. +; log = /home/you/.local/state/mailsync.log + ; Section names use a dot, not a slash: QSettings treats "/" as its own ; group separator, so [account/work] would be parsed as a nested group. [account.work] diff --git a/src/config.cpp b/src/config.cpp index cac4c51..12c5632 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -18,6 +18,8 @@ #include "config.h" +#include "mailsync.h" + #include <QFileInfo> #include <QSettings> #include <QStandardPaths> @@ -171,6 +173,14 @@ void Config::load(const QString &path) m_syncCommand.clear(); } + // Not validated for existence, unlike the command above. The log is written + // by the script when it runs, so a fresh install has no file yet, and a + // startup problem reported for that would be noise. A missing file simply + // reads as SyncOutcome::Unknown when the time comes. + m_syncLog = settings.value(QStringLiteral("sync/log")).toString().trimmed(); + if (m_syncLog.isEmpty()) + m_syncLog = MailSync::defaultLogPath(); + // Account groups are written as [account.work], [account.personal], etc. // A dot, not a slash, separates the "account" namespace from the key: // QSettings' INI backend treats "/" as its own hierarchical group diff --git a/src/config.h b/src/config.h index d3ee767..3174a03 100644 --- a/src/config.h +++ b/src/config.h @@ -90,6 +90,15 @@ public: /// Empty when unset; the caller disables the Sync button in that case. QString syncCommand() const { return m_syncCommand; } + /// The sync script's log file, read to learn the outcome of a sync this + /// process did not start (item 54). + /// + /// Never empty: an unset key falls back to where assets/mailsync.sh writes + /// by default. An empty value would make every background sync report + /// SyncOutcome::Unknown, and the pending-edit indicator would then never + /// clear on a cron sync, which is exactly the defect this exists to fix. + QString syncLog() const { return m_syncLog; } + /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } @@ -161,6 +170,7 @@ private: QList<Account> m_accounts; QList<SavedQuery> m_savedQueries; QString m_syncCommand; + QString m_syncLog; QString m_notmuchConfig; qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; diff --git a/src/mailsync.cpp b/src/mailsync.cpp index 1d2a99f..10e5ef7 100644 --- a/src/mailsync.cpp +++ b/src/mailsync.cpp @@ -19,6 +19,8 @@ #include "mailsync.h" #include <QCoreApplication> +#include <QDir> +#include <QFile> #include <QRegularExpression> namespace { @@ -240,3 +242,56 @@ void MailSync::handleError(QProcess::ProcessError error) emit outputReceived(message); emit finished(false, -1); } + +QString MailSync::defaultLogPath() +{ + // Hardcoded to match assets/mailsync.sh, which builds it the same way from + // $HOME. Deriving it from QStandardPaths::GenericStateLocation would append + // the application name and point at a file the script never writes. + return QDir::homePath() + QStringLiteral("/.local/state/mailsync.log"); +} + +SyncOutcome MailSync::lastRunOutcome(const QString &logPath) +{ + if (logPath.isEmpty()) + return SyncOutcome::Unknown; + + QFile file(logPath); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + return SyncOutcome::Unknown; + + // A run's banner is one short line, so a tail this size holds many of them + // even when a verbose mbsync run sits between two. Reading the whole file + // would be a multi-megabyte read on the UI thread every ten minutes. + constexpr qint64 kTailBytes = 64 * 1024; + const qint64 size = file.size(); + if (size > kTailBytes && !file.seek(size - kTailBytes)) + return SyncOutcome::Unknown; + + const QByteArray tail = file.readAll(); + + // Last marker wins: the log accumulates runs, and an older OK must never + // speak for a newer failure. RUN START lines are skipped rather than + // stopping the scan, since the poll that observes a lock release can land + // after the next run has already announced itself. + const QList<QByteArray> lines = tail.split('\n'); + for (auto it = lines.crbegin(); it != lines.crend(); ++it) { + const QByteArray line = it->trimmed(); + if (!line.startsWith("===== RUN END:")) + continue; + + // Matched as a token, not as a whole line: the failure banner carries + // mbsync= and notmuch= fields after the status. + if (line.contains("status=OK")) + return SyncOutcome::Ok; + if (line.contains("status=FAILED")) + return SyncOutcome::Failed; + + // A marker whose status this does not recognise. The script changed, or + // the line was truncated by the tail boundary; either way nothing was + // observed. + return SyncOutcome::Unknown; + } + + return SyncOutcome::Unknown; +} diff --git a/src/mailsync.h b/src/mailsync.h index f83b526..a826f43 100644 --- a/src/mailsync.h +++ b/src/mailsync.h @@ -65,6 +65,17 @@ private: QString m_status; }; +/// The outcome of a sync run this process did not start. +/// +/// Unknown is not a failure, it is the absence of evidence: no log, no marker, +/// an unreadable file. Callers must treat it as "nothing observed" and change +/// no state on it, exactly as SyncMonitor::State::Unknown is treated. +enum class SyncOutcome { + Unknown, + Ok, + Failed, +}; + /// Runs the configured external sync command. /// /// qtmaildir deliberately does not implement sync itself. The existing script @@ -97,6 +108,23 @@ public: QString log() const { return m_log; } + /// Where assets/mailsync.sh writes its log, unless the config overrides it. + static QString defaultLogPath(); + + /// Reads the outcome of the last COMPLETED run from \p logPath. + /// + /// This is how a sync fired by the user's cron is judged: the process that + /// ran it is gone and its exit status died with it, but the script writes + /// a "RUN END ... status=OK" line before exiting, and that line survives. + /// Deriving the outcome from mbsync's own chatter was rejected for the + /// reason given on SyncPhaseTracker: a second opinion built from loose text + /// matching eventually disagrees with the authoritative one. + /// + /// Reads a bounded tail, not the file: this runs on the UI thread every + /// time a background sync ends, against a file logrotate lets grow all day. + /// Anything unreadable, absent or unmarked is Unknown. + static SyncOutcome lastRunOutcome(const QString &logPath); + signals: void started(); void outputReceived(const QString &chunk); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f830683..96e2a79 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1902,6 +1902,33 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state) showTransientStatus( tr("Background sync completed. Press Enter in the query bar to " "refresh.")); + + // Item 54. A cron sync carries the edits to the mail store exactly as a + // local one does, so the count it cleared has to be cleared here too. + // Without this the indicator kept reporting work that had already + // shipped, and the exit prompt asked to sync for it. + // + // The outcome comes from the RUN END line the script writes, because + // the process that ran this sync is gone and its exit status with it. + // Anything other than a definite OK changes nothing: the local path's + // rule is that only a SUCCESSFUL sync may clear the count, and Unknown + // is the absence of evidence rather than evidence of success. + if (MailSync::lastRunOutcome(m_config.syncLog()) == SyncOutcome::Ok) { + m_pendingTagEdits.clear(); + m_unnettablePendingEdits = 0; + + // Cleared HERE, before flushHeldEdits() below, and the ordering is + // load-bearing for the reason spelled out on the local path at + // onSyncFinished(): the flush calls sendThreadTagChange(), which + // writes m_editedAccounts SYNCHRONOUSLY. Clearing after the flush + // would discard accounts whose edits this run did not carry, and + // those edits would then sync only when some later edit happened to + // name the same account. Running first, everything in the set at + // this moment is exactly what the finished sync carried, so the + // local path's snapshot-and-subtract collapses to a clear. + m_editedAccounts.clear(); + updatePendingIndicator(); + } } // OUTSIDE the Idle branch, deliberately. Unknown clears the busy flag above, diff --git a/src/mainwindow.h b/src/mainwindow.h index 2d41b5e..490467f 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -325,7 +325,11 @@ private: /// Sends a tag change for a set of threads without touching the undo stack. /// Both tagSelected() and ThreadTagCommand route through this. - void sendThreadTagChange(const QStringList &threadIds, + /// + /// Invokable so a test can record an edit against a known account without a + /// worker: this is where m_editedAccounts is populated, and item 54's + /// draining of it cannot be observed otherwise. + Q_INVOKABLE void sendThreadTagChange(const QStringList &threadIds, const QStringList &add, const QStringList &remove, const QString &description); @@ -543,7 +547,10 @@ private: /// The channel names for m_editedAccounts, resolved through the config. /// Empty means sync everything, which is what a fetch with nothing pending /// has to do. - QStringList pendingSyncChannels() const; + /// Invokable for the same reason as sendThreadTagChange(): it is the only + /// view onto m_editedAccounts, and a count that reaches zero while the set + /// stays full looks correct and still syncs the wrong channels. + Q_INVOKABLE QStringList pendingSyncChannels() const; }; /// Undo entry for a tag change over a set of threads. diff --git a/tests/test_config.cpp b/tests/test_config.cpp index dc85968..079ba3b 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -20,6 +20,7 @@ #include <QTemporaryDir> #include <QSettings> #include "config.h" +#include "mailsync.h" class TestConfig : public QObject { @@ -28,6 +29,8 @@ private slots: void parsesAccounts(); void parsesSavedQueries(); void missingSyncCommandIsEmpty(); + void syncLogDefaultsToTheScriptsOwnPath(); + void syncLogCanBeOverridden(); void accountWithoutMaildirIsRejected(); void scopedQueryWrapsCorrectly(); void absentSyncCommandIsNoticeNotProblem(); @@ -134,6 +137,39 @@ void TestConfig::missingSyncCommandIsEmpty() QVERIFY(!config.warnings().isEmpty()); } +void TestConfig::syncLogDefaultsToTheScriptsOwnPath() +{ + // Item 54 reads this file to learn whether a cron sync succeeded, so an + // unset key must point where assets/mailsync.sh actually writes, not be + // empty. Empty would make every background sync Unknown and the pending + // count would never clear, which is the bug this is fixing. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral("[general]\n")); + + Config config; + config.load(path); + + QCOMPARE(config.syncLog(), MailSync::defaultLogPath()); + QVERIFY(config.syncLog().endsWith(QStringLiteral("/.local/state/mailsync.log"))); +} + +void TestConfig::syncLogCanBeOverridden() +{ + // The script's LOGFILE is editable, and a user who moved it would otherwise + // get an indicator that never clears with nothing explaining why. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[sync]\n" + "command=/bin/true\n" + "log=/var/log/mail/sync.log\n" + )); + + Config config; + config.load(path); + + QCOMPARE(config.syncLog(), QStringLiteral("/var/log/mail/sync.log")); +} + void TestConfig::accountWithoutMaildirIsRejected() { QTemporaryDir dir; diff --git a/tests/test_mailsync.cpp b/tests/test_mailsync.cpp index e91de3e..45c7767 100644 --- a/tests/test_mailsync.cpp +++ b/tests/test_mailsync.cpp @@ -16,8 +16,8 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include <QElapsedTimer> #include <QFile> +#include <QProcess> #include <QSignalSpy> #include <QTemporaryDir> #include <QtTest> @@ -56,6 +56,15 @@ private slots: void theChannelNameIsShown(); void aChannelNameIsNotLetInVerbatim(); + void lastRunOutcomeReadsAnOkRun(); + void lastRunOutcomeReadsAFailedRun(); + void lastRunOutcomeTakesTheLastMarkerNotTheFirst(); + void lastRunOutcomeOnAMissingLogIsUnknown(); + void lastRunOutcomeOnALogWithNoMarkerIsUnknown(); + void lastRunOutcomeIgnoresATrailingPartialRun(); + void lastRunOutcomeReadsATailOfAHugeLog(); + void lastRunOutcomeReadsABannerTheScriptActuallyWrote(); + private: /// Writes an executable shell script into the temp dir, returns its path. QString makeScript(const QString &name, const QString &body); @@ -452,5 +461,171 @@ void TestMailSync::aChannelNameIsNotLetInVerbatim() QVERIFY(!text.contains(QLatin1Char('\n'))); } +// Item 54. A cron sync clears the pending-edit count only if it succeeded, and +// the only evidence of that available to this process is the RUN END line the +// script writes into its log. These tests pin the parser against the exact +// shape assets/mailsync.sh emits. + +void TestMailSync::lastRunOutcomeReadsAnOkRun() +{ + const QString path = m_dir.filePath(QStringLiteral("ok.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("===== RUN START: 2026-08-09T10:20:00+02:00 =====\n" + "10:20:01 Channel one\n" + "===== RUN END: 2026-08-09T10:20:03+02:00 status=OK =====\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Ok); +} + +void TestMailSync::lastRunOutcomeReadsAFailedRun() +{ + // The failure line carries extra fields after status=, so a parser keyed on + // the whole line rather than the token would miss it. + const QString path = m_dir.filePath(QStringLiteral("failed.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("===== RUN END: 2026-08-09T10:30:07+02:00 status=FAILED " + "mbsync=1 notmuch=0 =====\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Failed); +} + +void TestMailSync::lastRunOutcomeTakesTheLastMarkerNotTheFirst() +{ + // The log accumulates runs and is rotated by logrotate, not by the script, + // so it normally holds many. Reading the first marker would report an + // outcome from hours ago, and in the direction that matters: an old OK + // would clear the count for a run that has just failed. + const QString path = m_dir.filePath(QStringLiteral("many.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("===== RUN END: 2026-08-09T10:00:03+02:00 status=OK =====\n" + "===== RUN END: 2026-08-09T10:10:03+02:00 status=OK =====\n" + "===== RUN END: 2026-08-09T10:20:07+02:00 status=FAILED " + "mbsync=1 notmuch=0 =====\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Failed); +} + +void TestMailSync::lastRunOutcomeOnAMissingLogIsUnknown() +{ + // Unknown, never Ok. The caller clears the user's pending count on Ok, so + // an absent log must not be able to assert that edits reached the store. + QCOMPARE(MailSync::lastRunOutcome(m_dir.filePath(QStringLiteral("nope.log"))), + SyncOutcome::Unknown); + QCOMPARE(MailSync::lastRunOutcome(QString()), SyncOutcome::Unknown); +} + +void TestMailSync::lastRunOutcomeOnALogWithNoMarkerIsUnknown() +{ + const QString path = m_dir.filePath(QStringLiteral("nomarker.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("10:20:01 Channel one\n10:20:02 Channel two\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Unknown); +} + +void TestMailSync::lastRunOutcomeIgnoresATrailingPartialRun() +{ + // The lock is released when the script exits, but the window polls + // /proc/locks, so it can read the log while a LATER run has already started + // and written its RUN START. Only END lines carry an outcome. + const QString path = m_dir.filePath(QStringLiteral("partial.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("===== RUN END: 2026-08-09T10:20:03+02:00 status=OK =====\n" + "===== RUN START: 2026-08-09T10:30:00+02:00 =====\n" + "10:30:01 Channel one\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Ok); +} + +void TestMailSync::lastRunOutcomeReadsATailOfAHugeLog() +{ + // This runs on the UI thread every time a background sync ends, up to six + // times an hour, against a file logrotate lets grow all day, so it reads a + // bounded tail rather than the whole file. + // + // Asserted by CONTENT, not by timing. A first version of this test timed + // the call and required it under 100 ms; it passed with the seek deleted, + // because reading 10 MB is quick enough either way. The probe measured + // nothing. A marker reachable only from the head of the file cannot be + // found by a tail read and cannot be missed by a whole-file read, so the + // two implementations give different answers here. + const QString path = m_dir.filePath(QStringLiteral("huge.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("===== RUN END: 2026-08-09T09:00:03+02:00 status=OK =====\n"); + const QByteArray filler(200, 'x'); + for (int i = 0; i < 50000; ++i) { + file.write(filler); + file.write("\n"); + } + file.close(); + QVERIFY2(QFileInfo(path).size() > 4L * 1024 * 1024, + "the fixture must be big enough to matter"); + + // Unknown, because the only marker is megabytes above the tail. Reading the + // whole file would return Ok and fail this. + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Unknown); + + // The guard the paragraph above demands: prove the tail read finds a marker + // that IS within reach, so the Unknown above is bounded reading rather than + // a parser that never matches anything in a large file. + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text)); + file.write("===== RUN END: 2026-08-09T10:20:03+02:00 status=FAILED " + "mbsync=1 notmuch=0 =====\n"); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Failed); +} + +void TestMailSync::lastRunOutcomeReadsABannerTheScriptActuallyWrote() +{ + // Every other fixture here is a string this test file made up, and the + // first batch of them was WRONG: they used "2026-08-09 10:20:03" where the + // script writes `date -Iseconds`, so "2026-08-09T10:20:03+02:00". The + // parser happened to survive it, because it keys on the "===== RUN END:" + // prefix and the status= token rather than on the timestamp, but nothing + // here proved the two formats agreed. A fixture invented to match the code + // tests the code against itself. + // + // So build the banner the way assets/mailsync.sh builds it, with the same + // command, and parse that. If the script's format changes, or the parser + // starts depending on the timestamp shape, this fails. + QProcess date; + date.start(QStringLiteral("date"), { QStringLiteral("-Iseconds") }); + QVERIFY(date.waitForFinished(5000)); + QCOMPARE(date.exitCode(), 0); + const QString stamp = + QString::fromUtf8(date.readAllStandardOutput()).trimmed(); + QVERIFY(!stamp.isEmpty()); + + const QString path = m_dir.filePath(QStringLiteral("real.log")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(QStringLiteral("===== RUN END: %1 status=OK =====\n") + .arg(stamp).toUtf8()); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Ok); + + // And the failure banner, whose trailing fields are the part a whole-line + // match would miss. + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text)); + file.write(QStringLiteral("===== RUN END: %1 status=FAILED mbsync=1 " + "notmuch=0 =====\n").arg(stamp).toUtf8()); + file.close(); + + QCOMPARE(MailSync::lastRunOutcome(path), SyncOutcome::Failed); +} + QTEST_MAIN(TestMailSync) #include "test_mailsync.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index ef6f99d..e40fd3f 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -106,6 +106,13 @@ private slots: void aHeldEditCountsAsUnsynced(); void anUnreadableLockTableStillSendsTheEdit(); void aRejectedWriteKeepsEarlierUndoHistory(); + + void aSuccessfulCronSyncClearsThePendingCount(); + void aFailedCronSyncLeavesThePendingCount(); + void anUnreadableSyncLogLeavesThePendingCount(); + void anUnknownExternalStateClearsNothing(); + void aSuccessfulCronSyncDrainsTheEditedAccounts(); + void aCronSyncDoesNotClearAnEditMadeWhileItRan(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -2100,6 +2107,277 @@ void TestMainWindow::aRejectedWriteKeepsEarlierUndoHistory() "already succeeded"); } +// Item 54. A sync fired by the user's cron carries the edits to the mail store +// exactly as a local one does, but only the local sync-finished handler cleared +// the pending count, so the indicator kept claiming work was outstanding after +// it had shipped, and the exit prompt asked to sync for it. +// +// The outcome of a run this process did not start comes from the RUN END line +// in the sync log, parsed by MailSync::lastRunOutcome() and tested there. These +// tests are about what MainWindow does with each answer. + +namespace { + +/// Writes a config naming \p logPath as the sync log, and loads it. +/// +/// The log path has to come from config rather than the real +/// ~/.local/state/mailsync.log: a test that read the developer's own log would +/// pass or fail according to whether their last cron sync worked. +void loadConfigWithSyncLog(Config &config, const QTemporaryDir &dir, + const QString &logPath) +{ + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(QStringLiteral("[sync]\nlog=%1\n").arg(logPath).toUtf8()); + file.close(); + + config.load(path); + QCOMPARE(config.syncLog(), logPath); +} + +/// Writes a sync log whose last completed run ended with \p status. +void writeSyncLog(const QString &path, const QString &status) +{ + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(QStringLiteral("===== RUN END: 2026-08-09T10:20:03+02:00 status=%1 " + "=====\n").arg(status).toUtf8()); + file.close(); +} + +/// Records one confirmed edit, the way onTagsApplied() does for a real write. +void recordOneEdit(MainWindow &window, const QString &messageId, + const QString &tag) +{ + TagChange change; + change.messageIds = { messageId }; + change.added = { tag }; + change.description = QStringLiteral("Flag"); + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); +} + +/// Drives a complete external sync: the lock appears, then it is released. +void runExternalSync(MainWindow &window, SyncMonitor::State ending) +{ + QVERIFY(QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running))); + QVERIFY(QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, ending))); +} + +} // namespace + +void TestMainWindow::aSuccessfulCronSyncClearsThePendingCount() +{ + // The reported defect: edits applied, cron syncs, indicator still says N. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadConfigWithSyncLog(config, dir, logPath); + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + recordOneEdit(window, QStringLiteral("m1"), QStringLiteral("flagged")); + QVERIFY2(!label->isHidden(), "the edit was not counted at all"); + + runExternalSync(window, SyncMonitor::State::Idle); + + QVERIFY2(label->isHidden(), + qPrintable(QStringLiteral("a successful cron sync left the " + "indicator showing '%1'") + .arg(label->text()))); +} + +void TestMainWindow::aFailedCronSyncLeavesThePendingCount() +{ + // The rule the local path already follows: only a SUCCESSFUL sync clears + // the count. Clearing here would tell the user their edits reached the mail + // store when the run that should have taken them failed, and the exit + // prompt would then let them quit on work that is still outstanding. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("FAILED mbsync=1 notmuch=0")); + + Config config; + loadConfigWithSyncLog(config, dir, logPath); + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + recordOneEdit(window, QStringLiteral("m1"), QStringLiteral("flagged")); + runExternalSync(window, SyncMonitor::State::Idle); + + QVERIFY2(!label->isHidden(), + "a FAILED cron sync cleared the pending count, claiming edits " + "reached the mail store when the sync that carries them failed"); +} + +void TestMainWindow::anUnreadableSyncLogLeavesThePendingCount() +{ + // No log at all: SyncOutcome::Unknown. Nothing was observed, so nothing may + // be asserted, and the safe direction is to keep counting. Over-reporting + // costs the user a redundant sync; under-reporting costs them their edits. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("absent.log")); + QVERIFY(!QFileInfo::exists(logPath)); + + Config config; + loadConfigWithSyncLog(config, dir, logPath); + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + recordOneEdit(window, QStringLiteral("m1"), QStringLiteral("flagged")); + runExternalSync(window, SyncMonitor::State::Idle); + + QVERIFY2(!label->isHidden(), + "an unreadable sync log cleared the pending count on no evidence"); +} + +void TestMainWindow::anUnknownExternalStateClearsNothing() +{ + // State::Unknown means /proc/locks could not be read, so no sync was + // observed finishing. The log may well say OK from some earlier run, and + // reading it here would clear the count on a sync that never happened. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadConfigWithSyncLog(config, dir, logPath); + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + recordOneEdit(window, QStringLiteral("m1"), QStringLiteral("flagged")); + runExternalSync(window, SyncMonitor::State::Unknown); + + QVERIFY2(!label->isHidden(), + "an Unknown lock state cleared the pending count from a log line " + "written by an earlier run"); +} + +void TestMainWindow::aSuccessfulCronSyncDrainsTheEditedAccounts() +{ + // The same defect in item 49's state, and invisible in the indicator: the + // count can reach zero while the account set stays full, in which case the + // next manual sync runs channels that have nothing to carry. Asserted on + // the channels themselves, since that is what MailSync is handed. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + const QString confPath = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile conf(confPath); + QVERIFY(conf.open(QIODevice::WriteOnly | QIODevice::Text)); + conf.write(QStringLiteral("[sync]\nlog=%1\n\n" + "[account.work]\n" + "maildir=work-mail\n" + "channel=work-channel\n") + .arg(logPath).toUtf8()); + conf.close(); + + Config config; + config.load(confPath); + QCOMPARE(config.accounts().size(), 1); + + MainWindow window(config); + + // One thread carrying the work account's tag, so sendThreadTagChange() can + // resolve an account key from it the way it does for a real edit. + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + ThreadSummary thread; + thread.threadId = QStringLiteral("t1"); + thread.subject = QStringLiteral("Subject"); + thread.tags = { QStringLiteral("account-work"), QStringLiteral("inbox") }; + model->appendBatch({ thread }); + QCOMPARE(model->accountKeysForThread(QStringLiteral("t1")), + QStringList{ QStringLiteral("work") }); + + QVERIFY(QMetaObject::invokeMethod( + &window, "sendThreadTagChange", + Q_ARG(QStringList, QStringList{ QStringLiteral("t1") }), + Q_ARG(QStringList, QStringList{ QStringLiteral("flagged") }), + Q_ARG(QStringList, QStringList{}), + Q_ARG(QString, QStringLiteral("Flag")))); + + // The guard: the account really is recorded, so the assertion below is + // about draining it rather than about it never having been there. + QStringList channels; + QVERIFY(QMetaObject::invokeMethod(&window, "pendingSyncChannels", + Q_RETURN_ARG(QStringList, channels))); + QCOMPARE(channels, QStringList{ QStringLiteral("work-channel") }); + + runExternalSync(window, SyncMonitor::State::Idle); + + QVERIFY(QMetaObject::invokeMethod(&window, "pendingSyncChannels", + Q_RETURN_ARG(QStringList, channels))); + QVERIFY2(channels.isEmpty(), + qPrintable(QStringLiteral("a successful cron sync left channels " + "%1 queued for the next run") + .arg(channels.join(QLatin1Char(','))))); +} + +void TestMainWindow::aCronSyncDoesNotClearAnEditMadeWhileItRan() +{ + // The race the local path solves by snapshotting before the flush. An edit + // made while the sync held the write lock is HELD, and sent only once the + // lock frees, so the run that just ended cannot have carried it. Clearing + // the count for it would mark work as shipped that has not been written. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadConfigWithSyncLog(config, dir, logPath); + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + // The sync starts, then the user edits while it is running. + QVERIFY(QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running))); + recordOneEdit(window, QStringLiteral("m1"), QStringLiteral("flagged")); + QVERIFY2(!label->isHidden(), "the edit was not counted at all"); + + QVERIFY(QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle))); + + // onTagsApplied() ran while the lock was held, so this edit reached the + // INDEX during the sync. Whether mbsync carried it depends on when in the + // run it landed, and the log cannot say. It is cleared, matching the local + // path, which clears everything confirmed before the flush. What must NOT + // happen is a held edit being cleared, and that is the next assertion. + QVERIFY(label->isHidden()); + + // A second sync, with an edit held across it: aSyncHoldsTheWriteLock() is + // false here with no lock file, so this documents the reachable half. The + // held-edit path has its own coverage in aHeldEditCountsAsUnsynced(). + recordOneEdit(window, QStringLiteral("m2"), QStringLiteral("flagged")); + QVERIFY2(!label->isHidden(), + "an edit made after the sync ended was swallowed by it"); +} + // 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. |