test(mail): stateful IMAP mock harness + fix a flaky webhook capture test #384

2026-08-10T19:19:35Z

KofTwentyTwo commented

2026-08-10 19:19:35 +00:00

(Migrated from github.com)

Summary of Changes

Test-infrastructure only. No production code path changes.

1. A stateful, multi-connection IMAP server for offline tests (crates/nuncio-mail/src/test_server.rs, behind a test-support feature).

wiremock is HTTP-only, so JMAP and the DAV protocols have a mock and IMAP does not. What stood in for one was a scripted responder rewritten inline in each test — one connection, no mailbox state. That shape cannot express the property the sync work needs to assert: two clients observing each other's writes through shared server state. Nor can it reproduce the response forms that decide whether a mutation actually applied, so a client reporting success for a no-op cannot currently be caught by any test.

The server binds a real loopback listener, shares mailbox state across every accepted connection, and speaks IMAP4rev1 plus the parts of RFC 7162 (CONDSTORE/QRESYNC), RFC 4315 (UIDPLUS), and RFC 6851 (MOVE) the engine exercises. ServerProfile selects the advertised capabilities — Qresync (Fastmail/Cyrus, Dovecot), Condstore (Gmail, which has no QRESYNC), Basic (Exchange, which has neither) — because the sync ladder branches on this and a suite that only ever runs against a QRESYNC server proves nothing about the path most real accounts take.

Seven tests, each pinning a behaviour that matters:

Test	Pins
shared state across connections	B sees A's move
move of an already-moved UID	returns `OK` with no COPYUID (RFC 3501 §6.4.8)
`UNCHANGEDSINCE` → `[MODIFIED n]`	in the tagged response
QRESYNC `SELECT` → `VANISHED (EARLIER)`	removals are reported
QRESYNC refused without `ENABLE`; refused on CONDSTORE-only	RFC 7162 §3.2.3
`VANISHED` vs `EXPUNGE` on delete	RFC 6851 §4.4

2. A fix for a pre-existing flaky test (crates/nuncio-filter/src/webhook.rs).

test_blocked_webhook_logs_warn_with_host_and_reason failed ~3 runs in 10 on dev, measured independently of this branch. Callsite Interest is cached process-globally, and a raw Subscriber installed thread-locally does not participate in how tracing_subscriber's registry handles that cache — so whichever sibling test reached the dispatch callsites first decided whether these two could ever see them.

Two narrower fixes were tried and measured worse, and are recorded in the commit message so nobody repeats them: rebuild_interest_cache() after set_default (5 failures in 6 — the rebuild is itself global, so concurrent capture tests clobber each other), and Interest::sometimes() from register_callsite (9 in 10 — callsite registration consults the global dispatcher and never asks a thread-local subscriber).

The fix adopts the registry-backed capture helper nuncio-cal and nuncio-contacts already use. Assertions now match on level and named fields rather than substrings of a formatted line, so "the host was logged" can no longer pass because the host appeared in some unrelated field.

Measured: 0 failures in 12 stress runs, against 3 in 10 before.

Type of Change

Bug fix (non-breaking change fixing an issue)
New feature (non-breaking change adding functionality)
Breaking change (fix or feature causing existing functionality to change)
Documentation update

Test infrastructure plus one test-only bug fix.

Related Issues

No issue filed yet. The harness is the prerequisite for the sync-model work (server-side deletion reconciliation, verified mutations) and the flake fix is adjacent to #251 (assertion-free test theater), though distinct from it — these tests asserted correctly and simply never saw the data.

Quality Verification Checklist

I have verified code formatting (cargo fmt --all -- --check)
I have verified clippy lints (cargo clippy --all-targets --workspace -- -D warnings), plus -p nuncio-mail --features test-support so the gated module is linted too
I have executed the full test suite (cargo test --workspace) and all tests pass (603 passed, 0 failed)
Business logic is implemented in headless engine crates, keeping UI presentation shells skinny
No secrets, keys, or credentials are added to tracked files

Notes for review

The harness is pub only under test-support; default builds never compile it. Because the feature makes it non-cfg(test) code, the workspace unwrap_used/expect_used/panic denials apply to it, and it is written to satisfy them.
Deliberately not a conformant IMAP server. It implements the commands the engine issues, in the response shapes the RFCs require, and answers BAD to everything else.
No .proto changes, so the descriptor golden is untouched and this does not consume the one-proto-story-in-flight slot.

## Summary of Changes Test-infrastructure only. No production code path changes. **1. A stateful, multi-connection IMAP server for offline tests** (`crates/nuncio-mail/src/test_server.rs`, behind a `test-support` feature). `wiremock` is HTTP-only, so JMAP and the DAV protocols have a mock and IMAP does not. What stood in for one was a scripted responder rewritten inline in each test — one connection, no mailbox state. That shape cannot express the property the sync work needs to assert: **two clients observing each other's writes through shared server state.** Nor can it reproduce the response forms that decide whether a mutation actually applied, so a client reporting success for a no-op cannot currently be caught by any test. The server binds a real loopback listener, shares mailbox state across every accepted connection, and speaks IMAP4rev1 plus the parts of RFC 7162 (CONDSTORE/QRESYNC), RFC 4315 (UIDPLUS), and RFC 6851 (MOVE) the engine exercises. `ServerProfile` selects the advertised capabilities — `Qresync` (Fastmail/Cyrus, Dovecot), `Condstore` (Gmail, which has no QRESYNC), `Basic` (Exchange, which has neither) — because the sync ladder branches on this and a suite that only ever runs against a QRESYNC server proves nothing about the path most real accounts take. Seven tests, each pinning a behaviour that matters: | Test | Pins | | --- | --- | | shared state across connections | B sees A's move | | move of an already-moved UID | returns `OK` with **no COPYUID** (RFC 3501 §6.4.8) | | `UNCHANGEDSINCE` → `[MODIFIED n]` | in the **tagged** response | | QRESYNC `SELECT` → `VANISHED (EARLIER)` | removals are reported | | QRESYNC refused without `ENABLE`; refused on CONDSTORE-only | RFC 7162 §3.2.3 | | `VANISHED` vs `EXPUNGE` on delete | RFC 6851 §4.4 | **2. A fix for a pre-existing flaky test** (`crates/nuncio-filter/src/webhook.rs`). `test_blocked_webhook_logs_warn_with_host_and_reason` failed **~3 runs in 10 on `dev`**, measured independently of this branch. Callsite `Interest` is cached process-globally, and a raw `Subscriber` installed thread-locally does not participate in how `tracing_subscriber`'s registry handles that cache — so whichever sibling test reached the dispatch callsites first decided whether these two could ever see them. Two narrower fixes were tried and **measured worse**, and are recorded in the commit message so nobody repeats them: `rebuild_interest_cache()` after `set_default` (5 failures in 6 — the rebuild is itself global, so concurrent capture tests clobber each other), and `Interest::sometimes()` from `register_callsite` (9 in 10 — callsite registration consults the global dispatcher and never asks a thread-local subscriber). The fix adopts the registry-backed capture helper `nuncio-cal` and `nuncio-contacts` already use. Assertions now match on level and named fields rather than substrings of a formatted line, so "the host was logged" can no longer pass because the host appeared in some unrelated field. **Measured: 0 failures in 12 stress runs, against 3 in 10 before.** ## Type of Change - [x] Bug fix (non-breaking change fixing an issue) - [ ] New feature (non-breaking change adding functionality) - [ ] Breaking change (fix or feature causing existing functionality to change) - [ ] Documentation update Test infrastructure plus one test-only bug fix. ## Related Issues No issue filed yet. The harness is the prerequisite for the sync-model work (server-side deletion reconciliation, verified mutations) and the flake fix is adjacent to #251 (assertion-free test theater), though distinct from it — these tests asserted correctly and simply never saw the data. ## Quality Verification Checklist - [x] I have verified code formatting (`cargo fmt --all -- --check`) - [x] I have verified clippy lints (`cargo clippy --all-targets --workspace -- -D warnings`), plus `-p nuncio-mail --features test-support` so the gated module is linted too - [x] I have executed the full test suite (`cargo test --workspace`) and all tests pass (**603 passed, 0 failed**) - [x] Business logic is implemented in headless engine crates, keeping UI presentation shells skinny - [x] No secrets, keys, or credentials are added to tracked files ## Notes for review - The harness is `pub` only under `test-support`; default builds never compile it. Because the feature makes it non-`cfg(test)` code, the workspace `unwrap_used`/`expect_used`/`panic` denials apply to it, and it is written to satisfy them. - Deliberately **not** a conformant IMAP server. It implements the commands the engine issues, in the response shapes the RFCs require, and answers `BAD` to everything else. - No `.proto` changes, so the descriptor golden is untouched and this does not consume the one-proto-story-in-flight slot.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#384