test: the worker's dirty-marker test could not see the widget vanishing

Re-review finding, low severity and correct. The test asserted only
`!document.dirty`, and `DocumentCache::default()` is already clean — so it
could not tell "told it cleared" from "never told anything", and passed under
an injection that made `document_update` always return `None`. It is the test
that names the browser deployment, which is the one place nothing ever sends
ClientFrame::Save, so it is the only guard on that path.

It now records every marker the client is told and asserts `[true, false]` for
the edit. The bootstrap seeds its own clean marker first, so the sequence is
cleared after the handshake and the assertion is about the edit rather than
about the connect. Verified failing under both injections: dropping the
worker's re-projection gives `[true]`, and stopping the widget being projected
at all gives `[]`.

Declined, with the reason: a test for `GridLayout::record_cell_key` returning a
key for a `RecordCol::Category` column. The injection the reviewer describes
would make a coordinate column project `Derived` where a formula owns that
coordinate — and the reviewer's own harder case established that a formula
targeting a regular category SHOULD project `Derived` there, in both modes. So
the injected behaviour is not clearly a fault, the exhaustive match already
forces a decision on any new `RecordCol` variant, and a test would be pinning
an answer nobody has argued for. Recorded as a known gap rather than guarded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-20 11:59:55 -07:00
parent 24d9f61e35
commit 7d25762725
2 changed files with 26 additions and 1 deletions
+8
View File
@@ -2366,3 +2366,11 @@ Fault injected: dropping the re-projection after `persist()`. The client's
`DocumentCache` stays dirty. `MainThread` grew a `document` mirror and a
projection dispatch for it, rather than a bespoke reach into the worker — the
point is what a client folding the real stream ends up holding.
**A first version asserted only `!document.dirty`, and `DocumentCache::default()`
is already clean** — so it could not tell "told it cleared" from "never told
anything", and passed under an injection that stopped projecting the widget
entirely. Found in re-review. It now records **every** marker the client is
told and asserts `[true, false]` for the edit, with the bootstrap's own clean
marker cleared first so the sequence is about the edit rather than the
handshake. The stronger form fails under both injections.
@@ -47,6 +47,8 @@ struct MainThread {
worker: Worker,
grid: GridCache,
document: improvise_protocol::DocumentCache,
/// Every dirty marker this client has been told, in order.
dirty_marks: Vec<bool>,
}
impl MainThread {
@@ -63,6 +65,7 @@ impl MainThread {
worker: Worker::new(app),
grid: GridCache::default(),
document: improvise_protocol::DocumentCache::default(),
dirty_marks: Vec::new(),
}
}
@@ -114,7 +117,10 @@ impl MainThread {
fn apply(&mut self, projection: &Projection) {
match projection {
Projection::Grid(update) => self.grid.apply(update),
Projection::Document(update) => self.document.apply(update),
Projection::Document(update) => {
self.document.apply(update);
self.dirty_marks.push(update.dirty);
}
_ => {}
}
}
@@ -212,6 +218,7 @@ fn a_reload_over_the_same_origin_reopens_what_was_saved() {
worker: Worker::new(app),
grid: GridCache::default(),
document: improvise_protocol::DocumentCache::default(),
dirty_marks: Vec::new(),
};
reloaded.post(&ClientFrame::Viewport { rows: 10, cols: 4 });
assert_eq!(
@@ -363,6 +370,9 @@ fn the_automatic_persist_tells_the_client_the_document_is_clean() {
let mut main = MainThread::boot(storage, &path);
main.post(&ClientFrame::Viewport { rows: 10, cols: 4 });
// The bootstrap already seeded a clean marker; what this test is about is
// what the EDIT and its persist then say.
main.dirty_marks.clear();
let cell = main.cell(0, 0);
main.post(&main.caught_up(ModelCommand::SetCell {
cell,
@@ -373,6 +383,13 @@ fn the_automatic_persist_tells_the_client_the_document_is_clean() {
!main.worker.app().model_state.dirty,
"the worker must really have persisted, or this proves nothing"
);
// `DocumentCache::default()` is already clean, so the clean assertion alone
// cannot tell "told it cleared" from "never told anything".
assert_eq!(
main.dirty_marks,
vec![true, false],
"the client must see the marker raised and then cleared, in one batch"
);
assert!(
!main.document.dirty,
"the client must be told the document is clean — it is showing [+] otherwise"