diff options
| -rwxr-xr-x | assets/mailsync.sh | 10 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 52 | ||||
| -rw-r--r-- | src/mailsync.cpp | 129 | ||||
| -rw-r--r-- | src/mailsync.h | 42 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 52 | ||||
| -rw-r--r-- | src/mainwindow.h | 26 | ||||
| -rw-r--r-- | tests/test_mailsync.cpp | 147 | ||||
| -rw-r--r-- | 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 <name>`), 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 <QCoreApplication> +#include <QRegularExpression> + +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 <QProcess> #include <QString> +/// 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<QLabel *>(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 |