diff options
Diffstat (limited to 'docs/superpowers/specs')
| -rw-r--r-- | docs/superpowers/specs/2026-08-14-mainwindow-worker-fixture-design.md | 145 |
1 files changed, 145 insertions, 0 deletions
diff --git a/docs/superpowers/specs/2026-08-14-mainwindow-worker-fixture-design.md b/docs/superpowers/specs/2026-08-14-mainwindow-worker-fixture-design.md new file mode 100644 index 0000000..bb310d2 --- /dev/null +++ b/docs/superpowers/specs/2026-08-14-mainwindow-worker-fixture-design.md @@ -0,0 +1,145 @@ +# Giving `test_mainwindow` a real worker + +Resolves backlog item **36**, and produces a reproduction for item **66**. +Depends on item 84, which is already fixed: the constructor no longer raises a +modal, so a `MainWindow` built from a written config file can be constructed at +all. + +## What this is + +`test_mainwindow` gains the ability to build a `MainWindow` whose +`NotmuchWorker` is pointed at a throwaway notmuch database, so a test can select +a thread and let a real `threadLoaded` arrive. It is then used to write a +failing test for item 66. + +**The deliverable is a RED test for item 66.** Fixing that defect is not in +scope. See "Why the fix is excluded" below. + +## Why this is smaller than the backlog entry suggests + +The entry says the machinery exists and `test_mainwindow` does not use it, which +is true, and implies a hook has to be added to `MainWindow`, which is not. + +`MainWindow::wireWorker()` (`src/mainwindow.cpp:1440`) already constructs the +worker from `m_config.notmuchConfig()`, and the constructor already calls it. +`notmuch_config` is an ordinary config key read by `Config::load` +(`src/config.cpp:162`). So a test that writes a `qtmaildir.conf` carrying + +```ini +[general] +notmuch_config=/tmp/.../config +``` + +gets a `MainWindow` whose worker is already pointed at the fixture, through the +shipping code path, with no test-only setter and no `#ifdef`. Nothing in `src/` +changes. + +**Note the `[general]` trap** recorded in `CLAUDE.md`: QSettings treats a +section literally named `[general]` as its own fallback section and strips the +prefix, so the key is read as `notmuch_config`, not `general/notmuch_config`. +This is how that key went unnoticed as broken once already. Write the section as +above and read back with `Config::notmuchConfig()` to confirm before building +anything on it. + +## The helper + +Opt-in, never suite-wide. Roughly fifty existing cases construct a bare +`MainWindow` and must keep costing nothing; a suite-wide `initTestCase` would +make every one of them pay for a `notmuch new`, and shared mutable state between +cases is exactly how the `/proc/locks` bug (item 61) reached the whole suite. + +A test that wants a database calls a helper that: + +1. builds a `NotmuchFixture` (already exists, `tests/notmuchfixture.h`), +2. adds the messages that test needs and calls `index()`, +3. writes a `qtmaildir.conf` in the same `QTemporaryDir` with `notmuch_config` + pointing at the fixture's config, +4. loads it into a `Config` and returns both, so the caller constructs the + `MainWindow` and the fixture outlives it. + +**Lifetime is the trap here.** The fixture owns a `QTemporaryDir` which deletes +the tree in its destructor, and the worker holds the database open on another +thread. The fixture must outlive the `MainWindow`, so it cannot be a local in +the helper returned by value unless it is moved or heap-owned. Decide this when +writing it, and assert the database is readable after construction rather than +assuming. + +## Waiting for the worker + +The worker is on its own thread, so every assertion is asynchronous. +`QSignalSpy::wait()` against the specific signal, never a bare +`QTest::qWait(n)` with a guessed duration: a fixed sleep is a race that passes +on this machine and fails under load, and it also passes when the signal never +arrives at all, since nothing checks. + +Wait on `threadsReady` / `queryFinished` for a query, and on `threadLoaded` for +a selection. Assert the spy actually fired (`QVERIFY(spy.wait())`) before +asserting on what it delivered. + +## The item 66 reproduction + +Item 66: "selecting a thread root leaves the message pane blank until a reply +has been selected." The test: + +1. Fixture with one thread of at least two messages, the reply linked by a real + `In-Reply-To` header, since that is how `NotmuchFixture` builds a thread. +2. Run a query that returns it, wait for `queryFinished`. +3. Select the thread ROOT, the top-level row, without expanding it. +4. Wait for `threadLoaded`. +5. Assert the message pane is showing that thread. + +Assert on `MainWindow`'s own notion of what the pane holds, the accessor already +used elsewhere in `test_mainwindow` for "the pane is blanked", not on rendered +pixels. `CLAUDE.md` is explicit that rendering probes lie in repeatable ways and +that `viewport()->render()` returns blank images for several ordinary reasons; a +pixel assertion here would be measuring the probe, not the defect. + +**This test is expected to FAIL.** That is the deliverable. + +### If it passes + +A green test is a real finding, not a failure of the work. It means the defect +needs a condition the entry does not name, and the candidates in rough order of +likelihood are: it needs more than one message loaded before it manifests, it +needs the thread to be already expanded, it depends on the selection arriving +before a previous load completes, or it only appears against a large real +database and not a two-message one. + +In that case, record what was tried in the item 66 entry and stop. Do not widen +the test until it goes red: a test twisted until it fails proves nothing about +the defect the user actually sees. + +## Why the fix is excluded + +The user's decision, and the reason is the ordering. Item 66 has never been +isolated. Designing the fix in the same pass as the reproduction means designing +it while the diagnosis is still a hypothesis, which is where a wrong fix gets +locked in and then defended by the test written alongside it. A red test is a +complete, useful deliverable: it proves the defect is real, locates it, and +makes the fix verifiable when it comes. + +## Constraints + +- **Every existing `test_mainwindow` case must keep working unchanged.** The + helper is additive. +- **No test may read the real `/proc/locks`.** `init()` already points every + test at an empty lock table in its own `QTemporaryDir`, and no test restores + `"/proc/locks"` afterwards. That protection is suite-wide and this work must + not weaken it; `noTestCanSeeTheRealLockTable` fails if it is lost. +- **No personal data in fixtures.** Generated messages only, `example.org` + addresses, generic subjects. This was considered and declined explicitly on + 2026-08-04 and the repository is public. +- **A date fixture's weekday must match its date.** `Qt::RFC2822Date` validates + the two against each other, so `Thu, 14 Aug 2026` parses as INVALID because + that day is a Friday. Generate with `date -d <yyyy-mm-dd> +%A`, never from + memory. +- **A hung `test_mainwindow` leaves a stale binary** that a later `ctest` + re-runs, so a failure appears to persist after being fixed. Kill it and + rebuild before concluding anything about a hang. + +## Size + +**S–M.** The helper is small because the plumbing already exists. The +reproduction is the uncertain half: the first attempt may need several shapes +before it reproduces, and it may not reproduce at all, which is a legitimate +outcome rather than an overrun. |
