diff options
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 38 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 17 | ||||
| -rw-r--r-- | src/mainwindow.h | 12 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 22 |
4 files changed, 85 insertions, 4 deletions
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 0186bfa..dd6a8ca 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 @@ -85,7 +85,7 @@ taking that too literally. | 35 | No refresh of the thread list after a sync | workflow | M | open | | 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand | | 37 | The worker stalls on a tag edit made during a background sync | correctness | S | **done** | -| 38 | `test_mainwindow` fails when a real sync holds the lock | testing | XS | open | +| 38 | `test_mainwindow` fails when a real sync holds the lock | testing | XS | **done** | | 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 | open | @@ -93,7 +93,7 @@ taking that too literally. | 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 on the main window | discoverability | XS | open | -| 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | open | +| 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1917,6 +1917,30 @@ are unaffected either way; it is only the construction-time state that leaks in. construction-time state IS the behaviour under test for this case, and a window that never polls would pass the assertion for the wrong reason. +### Outcome (done) + +`MainWindow::setLocksPathForTesting()` / `locksPath()` give the window the seam +`SyncMonitor` already had, and the test points it at an empty file in its own +`QTemporaryDir` so construction observes no sync. The constraint above is +respected: the monitor is still constructed and still started, it simply reads a +lock table the test controls. + +**A test seam, deliberately not a config key.** `/proc/locks` is not something a +user would ever set, and a wrong value fails silently by disabling background +sync detection rather than loudly. A `[general]` key was considered and rejected +for that reason. + +**The override is process-wide and is reset at the end of the test**, since the +`QTemporaryDir` holding the file is destroyed with it; leaving it set would +point every later window at a path that no longer exists. + +**Verified by reproducing the original failure rather than waiting for cron.** +Running the suite under `flock -n /tmp/mbsync.lock` fails the assertion exactly +as reported when the seam is bypassed, and passes with it in place. The first +mutation attempted was a dud worth recording: writing `MUTANT` into the injected +lock table does not fail the test, because it is not a parseable `/proc/locks` +line and `lockHeldIn()` correctly finds no lock in it. + ## 39. Thread list cannot be sorted by clicking a column header **Observed (user, 2026-08-05):** "left pane columns order by clicking on the @@ -2207,6 +2231,16 @@ the property under test (the size that went in comes back out) is exact. the machine they run on, and both were found by running the suite in a context its author had not tried rather than by reading it. +### Outcome (done) + +The asserted size is now 640x560, which fits the offscreen platform's 800x800 +screen. Nothing in `MainWindow` changed: the persistence was never broken, only +the test's choice of a window wider than the smallest screen it runs against. + +Verified both ways round, since this one passed on Wayland throughout: 45 of 45 +under offscreen where it previously failed, and still green on the real +platform. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 7c7b801..75549d3 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -83,6 +83,21 @@ QString MainWindow::uiStatePath() return base + QStringLiteral("/qtmaildir/uistate.conf"); } +namespace { +/// Overridden only by setLocksPathForTesting(); "/proc/locks" in every real run. +QString g_locksPath = QStringLiteral("/proc/locks"); +} // namespace + +void MainWindow::setLocksPathForTesting(const QString &path) +{ + g_locksPath = path; +} + +QString MainWindow::locksPath() +{ + return g_locksPath; +} + void MainWindow::restoreUiState() { QSettings state(uiStatePath(), QSettings::IniFormat); @@ -465,7 +480,7 @@ void MainWindow::buildUi() // every ten minutes, so mail arrives and tags change while the window sits // idle, and until now nothing here noticed. m_syncMonitor = new SyncMonitor(SyncMonitor::defaultLockPath(), - QStringLiteral("/proc/locks"), this); + locksPath(), this); connect(m_syncMonitor, &SyncMonitor::stateChanged, this, &MainWindow::onExternalSyncStateChanged); m_syncMonitor->start(); diff --git a/src/mainwindow.h b/src/mainwindow.h index 22c8187..96b27f4 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -104,6 +104,18 @@ public: /// preserve comments or key order). static QString uiStatePath(); + /// Kernel lock table every MainWindow's SyncMonitor watches, "/proc/locks" + /// unless a test overrides it. + /// + /// A test seam, deliberately NOT a config key: /proc/locks is not something + /// a user would ever set, and a wrong value silently disables background + /// sync detection rather than failing loudly. Without this every window a + /// test builds observes the machine's real sync state, so a test asserting + /// on the sync button fails whenever the user's cron sync happens to be + /// running (item 38). + static void setLocksPathForTesting(const QString &path); + static QString locksPath(); + protected: void closeEvent(QCloseEvent *event) override; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index c409d1a..6a4f903 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -233,7 +233,11 @@ void TestMainWindow::uiStateSurvivesARestart() QStandardPaths::setTestModeEnabled(true); QFile::remove(MainWindow::uiStatePath()); - const QSize resized(940, 620); + // Must fit the smallest screen this ever runs against: the offscreen + // platform reports 800x800, and restoreGeometry() clamps to the available + // area, so a 940px width came back as 798 and failed only under offscreen. + // The number carries no meaning beyond differing from the default size. + const QSize resized(640, 560); { const Config config; MainWindow window(config); @@ -995,6 +999,17 @@ void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() s.setValue(QStringLiteral("sync/command"), QStringLiteral("/bin/true")); } + // An empty lock table, so construction observes no sync. Against the real + // /proc/locks this assertion fails whenever the user's cron sync happens to + // be running: cron fires every ten minutes and a run lasts ~35s, so roughly + // 6% of runs landed inside one and the failure looked like flakiness. + const QString locks = dir.filePath(QStringLiteral("locks")); + { + QFile f(locks); + QVERIFY(f.open(QIODevice::WriteOnly)); + } + MainWindow::setLocksPathForTesting(locks); + Config config; config.load(conf); MainWindow window(config); @@ -1014,6 +1029,11 @@ void TestMainWindow::theSyncButtonIsDisabledWhileABackgroundSyncHoldsTheLock() SyncMonitor::State::Idle)); QVERIFY2(button->isEnabled(), "the sync button was not re-enabled after the background sync"); + + // The override is process-wide, and QTemporaryDir takes the file with it at + // the end of this scope: leaving it set would point every later window at a + // path that no longer exists. + MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); } void TestMainWindow::anUnobservableLockTableLeavesTheSyncButtonUsable() |