diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 10:33:42 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 10:33:42 +0200 |
| commit | 8c78dd139a77e896c72b7fc8b799af3c0df34344 (patch) | |
| tree | a27955739d4af23c1bc4caece94fdccc5d2c02ae | |
| parent | 05a7038468fc880c3b1dfb7a1bb37b17e2e0ec07 (diff) | |
| download | qtmaildir-8c78dd139a77e896c72b7fc8b799af3c0df34344.tar.gz qtmaildir-8c78dd139a77e896c72b7fc8b799af3c0df34344.zip | |
docs: record the item 176 regression test flake as item 183
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 64 |
1 files changed, 64 insertions, 0 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 54e4b43..8edc6bd 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 @@ -256,6 +256,7 @@ taking that too literally. | 180 | The repaint rules are discovered one hole at a time | maintenance | S-L | open, 2026-08-29, from the notes, and a QUESTION rather than a defect. Items 105, 107, 109, 110 and 170 are each one hole in the same surface, all found by hand. Three mechanisms (optimistic repaint, `syncViewMembership()`, revert) agree by documentation rather than by code. Cheapest answer is one invariant test, not a rewrite; the user decides which, and that decides the size | | 181 | The thread dashboard does not follow a write to the conversation it shows | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`, from the notes. The dashboard draws a `ThreadDigest` built by the worker from the INDEX, which arrived only on selection, so a tag write moved the model and the card and left the pane reporting the count the conversation had when it was opened. Reachable from the dashboard's OWN Mark all read button. Re-requested from `onTagsApplied()`, where the write is confirmed: queued beside the write it races it and answers from the state before it, which is how the first fix passed review and failed the test. Section in the closed file | | 182 | An edit made during a sync is announced twice and never says it is waiting | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`, found by hand. The hold branches set a deliberately NON-transient label; all three callers overwrote it a line later with the bare action, so the user was told the write had landed and then told again when it really did. `announceAction()` adds the wait to the action rather than replacing it, since that announcement is what stands in for the confirmation dialog this project rules out. Section in the closed file | +| 183 | `undoingAMarkReadRestoresOnlyWhatWasUnread` fails about 1 run in 9 under the full suite | testing | ? | open, 2026-08-29, measured. Item 176's regression test, which guards the undo that rewrote 44 messages of real mail. Nine runs on master: 4 standalone, 3 under `ctest -R mainwindow`, 3 under the FULL parallel suite, and the single failure was in the last group. Not a regression, the base commit behaves the same. Probably the same root cause as item 136 and worth solving with it | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1383,3 +1384,66 @@ not refresh" from a hand-test finding into a suite failure. - This needs the user to say whether they want the test or the refactor. The note asks a question rather than reporting a fault, and the answer changes the size from S to L. + +## 183. `undoingAMarkReadRestoresOnlyWhatWasUnread` fails about 1 run in 9 under the full suite + +**Observed, 2026-08-29**, while verifying the `thread-row-identity` merge. A +full `ctest` run reported TWO failures in `test_mainwindow` where every earlier +run had reported one: the known `undoMovesTheMessageBack` (item 136) and +`undoingAMarkReadRestoresOnlyWhatWasUnread`, which had never failed before. + +**Measured rather than theorised**, because the first reading was wrong: it +looked like a regression from the three commits made that day, and it is not. + +| condition | runs | this test failed | +|---|---|---| +| `d7c4d03`, before those commits, standalone | 1 | 0 | +| master, standalone binary | 4 | 0 | +| master, `ctest -R mainwindow` | 3 | 0 | +| master, FULL parallel `ctest` | 3 | **1** | + +The base commit shows the same single known failure and the same test count +relationship (301 passed there, 304 on master, which is exactly the three tests +those commits added). So this is a pre-existing flake that happens to be rare +enough not to have been seen before. + +Note `ctest -R mainwindow` is NOT a reproduction of the condition: it runs one +suite, so there is no contention and it proves nothing about load. Only the +full run reproduces it, which is why the counts above separate the two. + +**Why this one matters more than an ordinary flake.** It is item 176's +regression test. That defect inverted a thread-scoped undo over every message +in the conversation, marked 43 of 44 messages unread on the user's real mail, +and `maildir.synchronize_flags` would have carried it to the server. A guard +that fails under load is a guard that cannot be fully trusted, and this one +stands in front of the worst bug this project has shipped. + +**Cause, read from the test rather than measured.** It is `WorkerBackedWindow` +over a real notmuch database and it waits three times with +`QTRY_VERIFY_WITH_TIMEOUT(..., 15000)`, each polling `notmuchCount()`, which +opens the database on every poll. The full suite runs 43 suites in parallel and +several of them are notmuch-backed, so the polls contend for the index and the +15s budget can expire before the write lands. + +That is the same shape item 136 records for itself: three 15s timeouts giving +45s against a whole-suite run, and a failure that means the work never happened +rather than that a race was lost. **They are probably one defect**, which would +make 136 more valuable to solve rather than less. + +**Approach.** Not decided, and this needs measuring before it needs code. The +question is whether the timeout is too short for a loaded machine or whether +the write genuinely never happens under contention, and those want opposite +fixes. Instrument what the count actually is when the wait expires: item 136's +own row records that the assertion which fails names the real question, and the +same discipline applies here. + +**Constraints.** + +- Do NOT lengthen the timeout to make it green. A test that passes because it + waited longer hides whichever of the two causes is real, and this test's job + is to fail when the undo is wrong. +- Solve it with item 136 or immediately after it. Two flakes in one binary with + the same shape are one investigation, and fixing one without the other leaves + the suite with a failure that masks the next real one. +- The suite baseline is currently ONE known failure. Anything that makes it two + intermittently costs the property that a red suite means something. |
