From 0a9ef3c77c7c593f3568f25761aad5d1f55e0e33 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 7 Aug 2026 11:39:31 +0200 Subject: feat(sync): the status bar says which account is syncing "Syncing..." was set once and never updated, so a run that takes over a minute reported nothing about what it was doing. The original diagnosis in the backlog was half wrong, and two further wrong ones were made and discarded before the real cause: plain "mbsync -a" prints NOTHING until it exits, then a single summary line. Measured on a real run, one line at 11:11:08 then 73 within the second 11:11:33, at the end of a 46-second run. So there was no stream to read for the part of a sync that takes time. It is not buffering, so stdbuf changes nothing, and the account name is not unavailable either, which was the second wrong conclusion. mbsync -V is what changes both: it announces each channel as it reaches it, which is at once the progress and the account name originally asked for. The shipped script now passes it. SyncPhaseTracker derives a short status from the output as it streams: the channel being synced, the summary counts when mbsync ends, then the notmuch reindex. It lives beside MailSync rather than in the window so the matching rules are one testable thing, and it holds no widget. Matching is loose and case-insensitive, since the wording varies by version, and nothing in it decides success or failure: the exit status remains the only authority on that. Lines are reassembled in MainWindow before being fed, because QProcess::readAll() splits wherever it happens to and a half-line would match nothing. Every status is sanitised and truncated: the channel name comes from a config file this app does not own, and a long one must not stretch the status bar. Two defects in existing code, fixed with it. setSyncBusy(true) ran after start(), so a fast run's output arrived before the per-run reset and wiped its own phase. And a first draft deferred phases while a transient message showed, which let a "Background sync completed" message armed before the sync began suppress the whole run: a running sync's state outranks an expiring event message. Verified by replaying real captured mbsync -V output through the tracker, not only against fixtures. The MainWindow test paces its script with sleeps, since a script that prints everything at once arrives in one readyRead and makes every intermediate phase unobservable. Closes item 42. Co-Authored-By: Claude Opus 5 --- assets/mailsync.sh | 10 +- .../plans/2026-08-03-post-0.1.0-usability.md | 52 +++++++- src/mailsync.cpp | 129 ++++++++++++++++++ src/mailsync.h | 42 ++++++ src/mainwindow.cpp | 52 +++++++- src/mainwindow.h | 26 +++- tests/test_mailsync.cpp | 147 +++++++++++++++++++++ tests/test_mainwindow.cpp | 117 ++++++++++++++++ 8 files changed, 560 insertions(+), 15 deletions(-) diff --git a/assets/mailsync.sh b/assets/mailsync.sh index 2134439..0e52ddd 100755 --- a/assets/mailsync.sh +++ b/assets/mailsync.sh @@ -72,7 +72,15 @@ START_TS="$(date -Iseconds)" # Timestamp every line of mbsync/notmuch output as it streams, # rather than only marking run boundaries, this is what actually # lets you tell which errors are from which run at a glance. - mbsync -a 2>&1 | while IFS= read -r line; do + # + # -V, deliberately. Without it mbsync prints NOTHING until it exits, then + # one summary line: a 100-second run is silent for all of it, so qtmaildir + # has nothing to report and its status bar can only say "Syncing...". With + # it, mbsync announces each channel as it reaches it ("Channel work"), + # which is both the progress and the account name the status bar shows. + # This is not a buffering problem and stdbuf does not help: the output + # streams fine, there simply is none to stream. + mbsync -V -a 2>&1 | while IFS= read -r line; do echo "$(date '+%H:%M:%S') $line" done echo "${PIPESTATUS[0]}" > "$STATUS_DIR/mbsync" diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index d98c91e..9424ea4 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -89,7 +89,7 @@ taking that too literally. | 39 | Thread list cannot be sorted by clicking a column header | workflow | S | open | | 40 | No live filter over the current view | workflow | M | open | | 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | **done** | -| 42 | "Syncing..." says nothing about what is being synced | feedback | S | open | +| 42 | "Syncing..." says nothing about what is being synced | feedback | S | **done** | | 43 | No "Mark all read" for the current view | workflow | S | open | | 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified | | 45 | Two Sync buttons, and only one of them works properly | correctness | S | **done** | @@ -2093,18 +2093,34 @@ away.** `assets/mailsync.sh` streams every mbsync and `notmuch new` line, timestamped, through `tee` (`assets/mailsync.sh:75-96`), and `MailSync` emits each chunk as `outputReceived` (`src/mailsync.cpp:65-74`), which fills the sync log pane. The status label is set once to `tr("Syncing...")` -(`src/mainwindow.cpp:1569`) and never updated until the run finishes. So this -needs no change to the script and no new channel; it needs the existing stream -read for state. +(`src/mainwindow.cpp:1569`) and never updated until the run finishes. + +**Correction (2026-08-07, measured): the paragraph above was half wrong, and +the "no script change needed" claim with it.** `notmuch new` does stream, but +plain `mbsync -a` prints **nothing at all** until it exits, then one summary +line. Measured on a real run: one line at 11:11:08, then 73 lines within the +single second 11:11:33, at the end of a 46-second run. So for the part of a +sync that actually takes time there was no output to read, and no parsing of +the existing stream could have fixed that. + +Two wrong diagnoses were made and discarded before the real one. It is **not** +buffering, so `stdbuf` does nothing: the output streams fine, there simply is +none. And the account name **is** available, contrary to the first reading of +this item, which concluded it was not and proposed shipping phases only. + +`mbsync -V` is what changes both: it announces each channel as it reaches it +(`Channel `), which is at once the progress indication and the account +name the user asked for. The shipped script now passes it. **Approach.** Derive a short status from the output already being received. - Recognise the phase from the stream: lines before `notmuch new` starts are mbsync's, and `notmuch new` announces itself. Show "Syncing mail (mbsync)" then "Reindexing (notmuch)". -- mbsync prints the channel it is working on, which is the account name the user - wants to see. Take it from the output rather than from config, so what is shown - is what is actually happening. +- mbsync prints the channel it is working on **only under `-V`**, which is the + account name the user wants to see. Take it from the output rather than from + config, so what is shown is what is actually happening, and in the order it + actually happens. **Constraints.** @@ -2120,6 +2136,28 @@ read for state. - Match loosely. mbsync's exact wording varies by version, and a status line that goes blank because a string moved is worse than the current fixed one. +**Two defects in existing code, found while building this and fixed with it.** + +- `startSync()` called `setSyncBusy(true)` **after** `m_sync->start()`, so any + per-run state reset there happened after the process had already produced + output. A short run delivers everything before control returns, which wiped + the phase those lines had produced. The reset now happens before the launch. +- The first draft deferred a phase while a transient message was still showing, + reading the constraint above as "never clobber a message the user is + reading". That let a `Background sync completed` message armed **before** the + sync started suppress the entire run's phases, which is how the first hand + test came back red. A running sync's state outranks an expiring event + message, so the deferral was removed. The constraint it was serving is + satisfied the other way round: a phase is written directly rather than + through `showTransientStatus()`, so the timer never reclaims it. + +**Verification note.** A test script that prints its lines at once is delivered +in a single `readyRead`, so the tracker sees the whole run in one call and only +the final phase is ever painted, which makes every intermediate one +unobservable. `test_mainwindow`'s script therefore paces itself with `sleep`, +standing in for a real sync's tens of seconds. The parser itself was checked by +replaying real captured `mbsync -V` output through it. + ## 43. No "Mark all read" for the current view **Observed (user, 2026-08-05):** a "Mark All Read" button next to Sync, Archive, diff --git a/src/mailsync.cpp b/src/mailsync.cpp index b1a5208..005580c 100644 --- a/src/mailsync.cpp +++ b/src/mailsync.cpp @@ -18,6 +18,135 @@ #include "mailsync.h" +#include +#include + +namespace { + +/// Longest status text put into the label. Sync output is unstructured and +/// arrives from a script rather than from this code, so a line is truncated +/// rather than trusted to be a sensible length: an unbounded one would resize +/// the status bar and push the permanent widgets beside it off. +constexpr int kMaxStatusChars = 120; + +/// Strips the script's "HH:MM:SS " prefix and anything that could disturb a +/// single-line label. Returns plain text, never markup. +QString sanitiseLine(const QString &line) +{ + QString text = line.trimmed(); + + static const QRegularExpression timestamp( + QStringLiteral("^\\d{2}:\\d{2}:\\d{2}\\s+")); + text.remove(timestamp); + + // Collapse every control character, not just newlines: a stray \r would + // otherwise leave the label showing the tail of the line only. + static const QRegularExpression controls(QStringLiteral("[\\x00-\\x1f\\x7f]+")); + text.replace(controls, QStringLiteral(" ")); + text = text.simplified(); + + if (text.size() > kMaxStatusChars) + text = text.left(kMaxStatusChars - 1) + QStringLiteral("…"); + + return text; +} + +} // namespace + +void SyncPhaseTracker::reset() +{ + m_phase = SyncPhase::Starting; + m_status.clear(); +} + +bool SyncPhaseTracker::feed(const QString &line) +{ + const QString text = sanitiseLine(line); + if (text.isEmpty()) + return false; + + // The script's own banners. Skipped before anything else: RUN START would + // otherwise read as mbsync output, and RUN END carries a status= field that + // must not be parsed, since the exit code is the authority on the outcome. + if (text.startsWith(QLatin1String("====="))) + return false; + + const QString before = m_status; + + // notmuch new announces itself by what it reports, since it prints no + // banner. Any of these means mbsync is done and the reindex is running. + // Matched loosely and case-insensitively: the wording varies by version. + static const QRegularExpression notmuchLine( + QStringLiteral("^(processed \\d|added \\d|no new mail|found \\d)"), + QRegularExpression::CaseInsensitiveOption); + + if (notmuchLine.match(text).hasMatch()) { + m_phase = SyncPhase::Notmuch; + m_status = QCoreApplication::translate("SyncPhaseTracker", + "Reindexing (notmuch)..."); + return m_status != before; + } + + // The channel mbsync is working on, which is the account name the user + // wants to see. Only printed under -V, which is why the shipped script + // passes it: without -V mbsync is silent until it exits. + // + // Taken from the output rather than from config, so what is shown is what + // is actually happening, and in the order it actually happens. + static const QRegularExpression channel( + QStringLiteral("^Channel\\s+(\\S.*)$"), + QRegularExpression::CaseInsensitiveOption); + + if (const auto match = channel.match(text); match.hasMatch()) { + m_phase = SyncPhase::Mbsync; + // The name comes from a config file this app does not own and lands in + // a label, so it is truncated on its own before being interpolated: + // bounding only the finished string would let a long name push the + // wording out instead of itself. + QString name = match.captured(1).trimmed(); + constexpr int kMaxNameChars = 60; + if (name.size() > kMaxNameChars) + name = name.left(kMaxNameChars - 1) + QStringLiteral("…"); + + m_status = QCoreApplication::translate("SyncPhaseTracker", + "Syncing %1...").arg(name); + return m_status != before; + } + + // mbsync's end-of-run summary, printed with or without -V. It arrives after + // every channel is done, so it reports rather than progresses; the + // "Far:/Near:" tail is dropped as unreadable at a glance. + static const QRegularExpression summary( + QStringLiteral("^Channels:\\s*(\\d+)\\s+Boxes:\\s*(\\d+)"), + QRegularExpression::CaseInsensitiveOption); + + if (const auto match = summary.match(text); match.hasMatch()) { + m_phase = SyncPhase::Mbsync; + m_status = QCoreApplication::translate( + "SyncPhaseTracker", "Syncing mail: %1 channels, %2 boxes") + .arg(match.captured(1), match.captured(2)); + if (m_status.size() > kMaxStatusChars) + m_status = m_status.left(kMaxStatusChars - 1) + QStringLiteral("…"); + return m_status != before; + } + + // Everything else while mbsync runs. The bulk of a real run is one + // "Ignoring non-mail file" line per Maildir, so individual lines are never + // shown: only the fact that mbsync is the phase. + // + // Only ever an upgrade from Starting, never a downgrade. Once the summary + // has given real counts, a later noise line must not overwrite them with + // the generic wording: the label would flicker back to saying less than it + // already said, for every one of thousands of ignored files. + if (m_phase == SyncPhase::Starting) { + m_phase = SyncPhase::Mbsync; + m_status = QCoreApplication::translate("SyncPhaseTracker", + "Syncing mail (mbsync)..."); + } + + return m_status != before; +} + MailSync::MailSync(const QString &command, QObject *parent) : QObject(parent), m_command(command) { diff --git a/src/mailsync.h b/src/mailsync.h index 8123ea9..3c0d616 100644 --- a/src/mailsync.h +++ b/src/mailsync.h @@ -22,6 +22,48 @@ #include #include +/// Which half of the sync script is running. +/// +/// The script runs mbsync and then `notmuch new`, so the phase is derived from +/// the output rather than announced: there is no side channel, and adding one +/// would mean the script and the app had to agree on a protocol. +enum class SyncPhase { + Starting, ///< Launched, nothing recognised yet. + Mbsync, ///< Fetching mail. + Notmuch, ///< Reindexing. +}; + +/// Derives a short status line from the sync script's output as it streams. +/// +/// Kept separate from MailSync so it can be tested against captured output +/// without running a process, and free of any widget so the matching rules stay +/// one thing rather than being spread through a UI handler. +/// +/// **Matching is deliberately loose.** mbsync's and notmuch's exact wording +/// varies by version, and a status line that goes blank because a string moved +/// is worse than the fixed "Syncing..." this replaces. Nothing here decides +/// whether the run succeeded: the exit status is the only authority on that, and +/// a second opinion derived from text would eventually disagree with it. +class SyncPhaseTracker +{ +public: + /// Feeds one line. Returns true when the status text changed as a result, + /// so the caller can avoid rewriting the label for every line of noise. + bool feed(const QString &line); + + /// Clears back to Starting for a new run. + void reset(); + + SyncPhase phase() const { return m_phase; } + + /// Plain text, already truncated, safe to put straight into a label. + QString statusText() const { return m_status; } + +private: + SyncPhase m_phase = SyncPhase::Starting; + QString m_status; +}; + /// Runs the configured external sync command. /// /// qtmaildir deliberately does not implement sync itself. The existing script diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index a2174a2..a3b5496 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -465,6 +465,7 @@ void MainWindow::buildUi() connect(m_sync, &MailSync::finished, this, &MainWindow::onSyncFinished); connect(m_sync, &MailSync::outputReceived, this, [this](const QString &chunk) { m_syncLog->appendPlainText(chunk.trimmed()); + feedSyncPhase(chunk); }); // Syncs this window did not start. The user's cron runs the same script @@ -1576,12 +1577,46 @@ void MainWindow::showTransientStatus(const QString &text) m_statusTimer->start(); } +void MainWindow::feedSyncPhase(const QString &chunk) +{ + // readAll() returns whatever happened to be buffered, which splits mid-line + // as often as not, so lines are reassembled here rather than in the tracker: + // a half-line fed to it would match nothing and the phase would stall. + m_syncLineBuffer += chunk; + + int newline; + bool changed = false; + while ((newline = m_syncLineBuffer.indexOf(QLatin1Char('\n'))) >= 0) { + const QString line = m_syncLineBuffer.left(newline); + m_syncLineBuffer.remove(0, newline + 1); + if (m_syncPhase.feed(line)) + changed = true; + } + + // The tail without a newline is deliberately left in the buffer: mbsync can + // sit on a line for a while, and feeding a partial one would report a phase + // from half a word. + + if (!changed) + return; + + // Not showTransientStatus(): a phase is state, not an event, and must not + // expire out from under a sync that is still running. Writing the label + // directly also leaves m_transientMessage alone, so the timer will not + // reclaim a phase it did not arm. + m_statusLabel->setText(m_syncPhase.statusText()); +} + void MainWindow::setSyncBusy(bool busy) { m_localSyncBusy = busy; updateSyncControls(); - if (busy) + // The phase tracker is reset in startSync(), before the process launches, + // not here: this runs after start() and a fast run has already produced + // output by then. Setting the label is still right, since the tracker has + // nothing to say until a line it recognises arrives. + if (busy && m_syncPhase.statusText().isEmpty()) m_statusLabel->setText(tr("Syncing...")); } @@ -1623,13 +1658,22 @@ void MainWindow::startSync() tr("No sync command configured ([sync] command in qtmaildir.conf)")); return; } + + // Fresh run, fresh output: leaving the previous run's lines in place + // makes a stale failure look like the current one. + m_syncLog->clear(); + + // BEFORE start(), not after. A short run can deliver its whole output + // before control returns here, and resetting afterwards would wipe the + // phase those lines had already produced, leaving a fast sync showing + // nothing between "Syncing..." and "Sync complete". + m_syncPhase.reset(); + m_syncLineBuffer.clear(); + if (!m_sync->start()) { showTransientStatus(tr("Sync already running")); return; } - // Fresh run, fresh output: leaving the previous run's lines in place - // makes a stale failure look like the current one. - m_syncLog->clear(); setSyncBusy(true); } diff --git a/src/mainwindow.h b/src/mainwindow.h index e673181..3984e78 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -28,6 +28,9 @@ #include "config.h" #include "keymap.h" +// Included rather than forward-declared: SyncPhaseTracker is held by value, so +// its size must be known here. MailSync itself stays a forward declaration. +#include "mailsync.h" #include "syncmonitor.h" #include "tagcolors.h" #include "types.h" @@ -155,6 +158,14 @@ private slots: /// the meta-object without widening the public API. void onExternalSyncStateChanged(SyncMonitor::State state); + /// Starts a sync and shows that it started. Every route in goes through + /// here: the toolbar, the menu, the shortcut and the button. + /// + /// A private slot for the same reason as the two above: a test needs to + /// start a real run through the meta-object to exercise the output + /// handling, without this becoming public API. + void startSync(); + /// 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); @@ -217,9 +228,9 @@ private: /// says "working, duration unknown", which is the truth. void setSyncBusy(bool busy); - /// Starts a sync and shows that it started. Every route in goes through - /// here: the toolbar, the menu, the shortcut and the button. - void startSync(); + /// Reassembles lines from a sync output chunk and updates the status label + /// when the phase or its detail changes. + void feedSyncPhase(const QString &chunk); /// Applies the sync progress bar and button state from BOTH sync sources. /// @@ -298,6 +309,15 @@ private: MessageView *m_messageView = nullptr; MailSync *m_sync = nullptr; + /// Derives "which half of the sync is running" from the output stream, so + /// the status bar says more than "Syncing...". Reset at the start of each + /// local run. + SyncPhaseTracker m_syncPhase; + + /// Holds the tail of a chunk that did not end on a newline, since + /// QProcess::readAll() splits wherever it happens to. + QString m_syncLineBuffer; + /// Watches the sync lock for runs this window did not start. SyncMonitor *m_syncMonitor = nullptr; diff --git a/tests/test_mailsync.cpp b/tests/test_mailsync.cpp index a1d193f..6c93e8d 100644 --- a/tests/test_mailsync.cpp +++ b/tests/test_mailsync.cpp @@ -44,6 +44,15 @@ private slots: void startDoesNotBlock(); void argumentsAreNotShellInterpreted(); + void phaseStartsAsMbsync(); + void notmuchLineSwitchesPhase(); + void mbsyncSummaryIsReported(); + void noiseLeavesThePhaseAlone(); + void aHostileLineCannotGrowTheStatus(); + void runMarkersAreNotAPhase(); + void theChannelNameIsShown(); + void aChannelNameIsNotLetInVerbatim(); + private: /// Writes an executable shell script into the temp dir, returns its path. QString makeScript(const QString &name, const QString &body); @@ -253,5 +262,143 @@ void TestMailSync::argumentsAreNotShellInterpreted() QVERIFY(sync.log().contains(QStringLiteral("; touch"))); } +void TestMailSync::phaseStartsAsMbsync() +{ + // A fresh tracker has nothing to report until it is fed, and a run is + // mbsync's until notmuch announces itself. + SyncPhaseTracker tracker; + QCOMPARE(tracker.phase(), SyncPhase::Starting); + + // A timestamped mbsync line, as the script emits it. + QVERIFY(tracker.feed(QStringLiteral("10:44:11 Socket error on imap.example.org (192.0.2.1:993): timeout."))); + QCOMPARE(tracker.phase(), SyncPhase::Mbsync); + QVERIFY(!tracker.statusText().isEmpty()); +} + +void TestMailSync::notmuchLineSwitchesPhase() +{ + // "notmuch new" announces itself with its own progress wording. Matching is + // loose on purpose: the exact phrasing varies by version, and a status that + // goes blank because a string moved is worse than a fixed one. + SyncPhaseTracker tracker; + tracker.feed(QStringLiteral("10:44:32 Channels: 5 Boxes: 39 Far: +0 *15 #0 -0 Near: +1 *0 #0 -0")); + QCOMPARE(tracker.phase(), SyncPhase::Mbsync); + + QVERIFY(tracker.feed(QStringLiteral("10:44:33 Processed 77 total files in almost no time."))); + QCOMPARE(tracker.phase(), SyncPhase::Notmuch); + + // Both spellings notmuch uses when it finishes. + SyncPhaseTracker other; + other.feed(QStringLiteral("11:00:33 Added 1 new message to the database.")); + QCOMPARE(other.phase(), SyncPhase::Notmuch); + + SyncPhaseTracker third; + third.feed(QStringLiteral("11:11:33 No new mail.")); + QCOMPARE(third.phase(), SyncPhase::Notmuch); +} + +void TestMailSync::mbsyncSummaryIsReported() +{ + // mbsync prints one summary at the end of its run and nothing per channel, + // so this line is the only concrete thing there is to show. The counts are + // worth surfacing; the raw "Far: +0 *15 #0 -0" tail is not. + SyncPhaseTracker tracker; + QVERIFY(tracker.feed(QStringLiteral("10:44:32 Channels: 5 Boxes: 39 Far: +0 *15 #0 -0 Near: +1 *0 #0 -0"))); + + const QString text = tracker.statusText(); + QVERIFY2(text.contains(QStringLiteral("5")), qPrintable(text)); + QVERIFY2(text.contains(QStringLiteral("39")), qPrintable(text)); +} + +void TestMailSync::noiseLeavesThePhaseAlone() +{ + // The overwhelming majority of a real run is this one line repeated, and it + // must not be shown or counted as a phase change. + SyncPhaseTracker tracker; + tracker.feed(QStringLiteral("10:44:32 Channels: 5 Boxes: 39 Far: +0 *0 #0 -0 Near: +0 *0 #0 -0")); + const QString before = tracker.statusText(); + + QVERIFY(!tracker.feed(QStringLiteral( + "11:11:33 Note: Ignoring non-mail file: /home/you/Mail/example/Inbox/.uidvalidity"))); + QCOMPARE(tracker.statusText(), before); + QCOMPARE(tracker.phase(), SyncPhase::Mbsync); +} + +void TestMailSync::aHostileLineCannotGrowTheStatus() +{ + // Sync output is local but unstructured, and it lands in a status label. + // A long line must be truncated rather than resizing the status bar, and + // control characters must not survive into it. + SyncPhaseTracker tracker; + tracker.feed(QStringLiteral("10:00:00 Channels: %1 Boxes: 2") + .arg(QString(500, QLatin1Char('9')))); + + const QString text = tracker.statusText(); + QVERIFY2(text.size() <= 120, qPrintable(QString::number(text.size()))); + QVERIFY(!text.contains(QLatin1Char('\n'))); + QVERIFY(!text.contains(QLatin1Char('\r'))); +} + +void TestMailSync::runMarkersAreNotAPhase() +{ + // The script's own banners bracket the run. RUN START must not read as + // mbsync output, and RUN END must not leave a phase claiming work is still + // going: the exit status decides the outcome, deliberately, so nothing here + // may be parsed into success or failure. + SyncPhaseTracker tracker; + QVERIFY(!tracker.feed(QStringLiteral("===== RUN START: 2026-08-07T11:10:47+02:00 ====="))); + QCOMPARE(tracker.phase(), SyncPhase::Starting); + + tracker.feed(QStringLiteral("11:11:33 No new mail.")); + QCOMPARE(tracker.phase(), SyncPhase::Notmuch); + + QVERIFY(!tracker.feed(QStringLiteral( + "===== RUN END: 2026-08-07T11:11:33+02:00 status=FAILED mbsync=1 notmuch=0 ====="))); + // Unchanged: the banner says nothing the status bar should repeat, and the + // caller reports the outcome from the exit code. + QCOMPARE(tracker.phase(), SyncPhase::Notmuch); +} + +void TestMailSync::theChannelNameIsShown() +{ + // What the user actually asked for: which account is being synced right + // now. mbsync -V announces each channel as it reaches it, and the channel + // name is the account name. + SyncPhaseTracker tracker; + + QVERIFY(tracker.feed(QStringLiteral("11:31:16 Channel provider-work"))); + QCOMPARE(tracker.phase(), SyncPhase::Mbsync); + QVERIFY2(tracker.statusText().contains(QStringLiteral("provider-work")), + qPrintable(tracker.statusText())); + + // The per-box chatter between channels must not displace it: the account + // is the useful thing, and a box name changing several times a second + // would make the status bar unreadable. + const QString onChannel = tracker.statusText(); + QVERIFY(!tracker.feed(QStringLiteral("11:31:16 Opening far side box INBOX..."))); + QCOMPARE(tracker.statusText(), onChannel); + QVERIFY(!tracker.feed(QStringLiteral("11:32:20 near side: 14758 messages, 0 recent"))); + QCOMPARE(tracker.statusText(), onChannel); + + // The next channel does replace it. + QVERIFY(tracker.feed(QStringLiteral("11:33:04 Channel provider-personal"))); + QVERIFY(tracker.statusText().contains(QStringLiteral("provider-personal"))); + QVERIFY(!tracker.statusText().contains(QStringLiteral("provider-work"))); +} + +void TestMailSync::aChannelNameIsNotLetInVerbatim() +{ + // The channel name comes from a config file this app does not own, and it + // lands in a status label. A long one must not be able to stretch the + // status bar, whatever mbsync was told to call it. + SyncPhaseTracker tracker; + tracker.feed(QStringLiteral("11:31:16 Channel %1") + .arg(QString(400, QLatin1Char('x')))); + + const QString text = tracker.statusText(); + QVERIFY2(text.size() <= 120, qPrintable(QString::number(text.size()))); + QVERIFY(!text.contains(QLatin1Char('\n'))); +} + QTEST_MAIN(TestMailSync) #include "test_mailsync.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 480be39..02490a5 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -80,6 +80,7 @@ private slots: void aLocalSyncsOwnLockIsNeverReportedAsBackground(); void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); void anUnobservableLockTableLeavesTheSyncButtonUsable(); + void theStatusBarFollowsTheSyncPhase(); void theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock(); void escapeBlanksTheMessagePane(); void deleteTogglesOnAnAlreadyDeletedThread(); @@ -1031,6 +1032,122 @@ void TestMainWindow::theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock() MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); } +void TestMainWindow::theStatusBarFollowsTheSyncPhase() +{ + // Item 42: "Syncing..." said nothing about what was happening, while the + // script was already streaming its phase into the log pane and the app was + // throwing it away. + // + // Driven through a real script rather than by calling the tracker directly, + // because the defect this guards is in the wiring: the chunks QProcess + // hands over split mid-line, so a handler that fed them straight to the + // tracker would stall on the first partial line. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + QVERIFY(QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir")))); + + const QString script = dir.filePath(QStringLiteral("fakesync.sh")); + { + QFile f(script); + QVERIFY(f.open(QIODevice::WriteOnly)); + // Shaped like the real thing: timestamped lines, the noise that makes + // up the bulk of a run, mbsync's one summary, then notmuch's output. + // Paced, not dumped. A script that prints everything at once is + // delivered in a single readyRead, so the tracker sees the whole run in + // one call and only its final phase is ever painted: the intermediate + // ones would be unobservable and the test would assert nothing. A real + // sync takes tens of seconds and arrives in separate chunks, which the + // sleeps stand in for. + f.write("#!/bin/sh\n" + "echo '===== RUN START: 2026-08-07T11:00:00+02:00 ====='\n" + "echo '11:00:01 Note: Ignoring non-mail file: /home/you/Mail/x/.uidvalidity'\n" + "sleep 0.2\n" + "echo '11:00:02 Channels: 5 Boxes: 39 Far: +0 *1 #0 -0 Near: +1 *0 #0 -0'\n" + "sleep 0.2\n" + "echo '11:00:03 Processed 79 total files in almost no time.'\n" + "echo '11:00:03 Added 1 new message to the database.'\n" + "sleep 0.2\n" + "echo '===== RUN END: 2026-08-07T11:00:03+02:00 status=OK ====='\n"); + f.close(); + QVERIFY(QFile::setPermissions(script, + QFile::ReadOwner | QFile::WriteOwner + | QFile::ExeOwner)); + } + + const QString conf = dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + { + QSettings s(conf, QSettings::IniFormat); + s.setValue(QStringLiteral("sync/command"), script); + } + + const QString locks = dir.filePath(QStringLiteral("locks")); + { + QFile f(locks); + QVERIFY(f.open(QIODevice::WriteOnly)); + } + MainWindow::setLocksPathForTesting(locks); + + Config config; + config.load(conf); + QCOMPARE(config.syncCommand(), script); + MainWindow window(config); + + auto *status = window.findChild(QStringLiteral("statusMessage")); + QVERIFY(status); + + // Every value the label takes, recorded as it changes. A run this small + // finishes in well under a second, so polling for an intermediate phase + // races the process and usually sees only "Sync complete": the sequence has + // to be captured, not sampled. + // QLabel has no textChanged signal, so the label is sampled on a fast timer + // rather than watched. Each distinct value is recorded once. + QStringList seen; + QTimer sampler; + sampler.setInterval(1); + connect(&sampler, &QTimer::timeout, &sampler, [&seen, status]() { + const QString text = status->text(); + if (seen.isEmpty() || seen.constLast() != text) + seen.append(text); + }); + sampler.start(); + + QVERIFY(QMetaObject::invokeMethod(&window, "startSync")); + + QTRY_VERIFY_WITH_TIMEOUT( + std::any_of(seen.cbegin(), seen.cend(), [](const QString &s) { + return s.contains(QStringLiteral("Sync complete")); + }), + 10000); + + const QString trace = seen.join(QStringLiteral(" | ")); + + // mbsync's summary is the only concrete thing the stream carries, since it + // names no channel unless run verbose. The counts must reach the label. + QVERIFY2(std::any_of(seen.cbegin(), seen.cend(), [](const QString &s) { + return s.contains(QStringLiteral("39")) + && s.contains(QStringLiteral("5")); + }), + qPrintable(QStringLiteral("the mbsync summary never reached the " + "status bar. Saw: ") + trace)); + + // Then the reindex phase, which is a different message entirely. Without + // the wiring the label went from "Syncing..." straight to "Sync complete", + // which is exactly what the defect looked like. + QVERIFY2(std::any_of(seen.cbegin(), seen.cend(), [](const QString &s) { + return s.contains(QStringLiteral("notmuch")); + }), + qPrintable(QStringLiteral("the notmuch phase never reached the " + "status bar. Saw: ") + trace)); + + // The banners are not a phase and must never appear in the status bar. + for (const QString &s : seen) { + QVERIFY2(!s.contains(QStringLiteral("RUN ")), qPrintable(s)); + QVERIFY2(!s.contains(QStringLiteral("status=")), qPrintable(s)); + } + + MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); +} + void TestMainWindow::anUnobservableLockTableLeavesTheSyncButtonUsable() { // Unknown means /proc/locks could not be read, so nothing was observed. A -- cgit v1.2.3