fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(tests): cap the integration tests' comments

Browse Source 
grid_frame_agreement.rs is the strongest test in the last wave and its
comments were the record of six injected faults; all of it is now in
context/test-evidence/tui.md, keyed by test name.

THE_MEMBERSHIP_CORRECT_SPANS and its assert_ne! are UNTOUCHED. Its comment
now says what it is — a deliberately wrong constant and a live tripwire —
and points at the derivation rather than carrying it.

shipped_documents_load.rs keeps its two load-bearing claims in the file
(directory walk, full-name match) because both sit directly over the code
that would otherwise be "simplified" back into the bug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-20 09:51:38 -07:00
parent 13bba7f400
commit f7d377b010
3 changed files with 26 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/grid_frame_agreement.rs
+21 -77
View File
@@ -1,24 +1,5 @@
//! The two front ends draw the same grid frame (improvise-anr9/s1xw).
//!
//! The divergence this exists to rule out was real and shipped:
//! `GridCache::row_labels` was a `Vec<String>` built by
//! `GridLayout::row_label`, which **slash-joins a multi-level tuple** — the
//! exact rendering `src/ui/grid.rs::two_row_categories_produce_cross_product_labels`
//! asserts must not appear. So the protocol carried a form of the labels the
//! TUI itself considered wrong, and the browser drew `Food/Alice` where the
//! TUI drew two header columns, with no groups and no collapse indicators.
//!
//! The shape of the check matters. Each front end is driven the way it is
//! really driven — the widget into a ratatui buffer, the client through a
//! `Snapshot` of the same cache's `seed()` — and the two are compared on
//! what they *draw*, not on what they were handed. Comparing the caches
//! would be vacuous: it is one cache.
//!
//! Injected-fault record: re-introducing the slash-join on either side —
//! `labels: vec![rows.item(r).join("/")]` in the client's `render_model`, or
//! `rows.item(ri).join("/")` in the widget's row header — makes
//! `the_two_front_ends_draw_the_same_row_header` fail, naming the row and
//! the two spellings. Watched both fail before keeping it.
//! The two front ends draw the same grid frame (improvise-anr9/s1xw). The
//! shipped divergence and the injected faults: `context/test-evidence/tui.md`.
use improvise::model::cell::{CellKey, CellValue};
use improvise::ui::app::{App, AppMode, ViewCursor};
@@ -39,17 +20,8 @@ fn coord(pairs: &[(&str, &str)]) -> CellKey {
)
}
/// Both axes multi-level and grouped — the cross product the slash-join used
/// to flatten, on rows *and* columns, so the frame carries header levels,
/// repeat suppression, group spans and a collapse state in one fixture.
///
/// Two deliberate asymmetries. `Carol` sits outside the `Family` group and
/// *before* it, so collapsing `Family` still leaves a row: an all-collapsed
/// axis draws no group headers at all, in both front ends and before this
/// change (see the hand-back), and a fixture that hit it would be testing
/// that bug. And `Month` has one grouped quarter and one ungrouped month, so
/// the column runs are not uniform — a `col_groups` that emitted one span
/// per column, or one span for the whole axis, would pass a uniform fixture.
/// Both axes multi-level and grouped. Its two deliberate asymmetries — Carol
/// outside Family, Sep ungrouped — are in `context/test-evidence/tui.md`.
fn two_level_rows() -> Workbook {
let mut m = Workbook::new("Test");
m.add_category("Type").unwrap();
@@ -105,12 +77,8 @@ fn whole_grid(wb: &mut Workbook) -> (App, GridCache) {
(app, cache)
}
/// The row-header text the TUI *paints*, one entry per drawn row, each split
/// into its level sub-columns and trimmed.
///
/// It slices by `AxisFrame::header_widths` because that is the same rule the
/// widget lays the sub-columns out with (improvise-1125) — reading the
/// widths a second way here would be the very duplication under test.
/// The row-header text the TUI *paints*, sliced by `AxisFrame::header_widths`
/// — the widget's own rule, never a second reading (improvise-1125).
fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
let lines = tui_lines(wb, cache);
let sep = header_rule(&lines);
@@ -136,12 +104,8 @@ fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
.collect()
}
/// One painted header line, cut into its columns and trimmed.
///
/// It cuts by the cache's own `col_widths` past the row header, because that
/// is the geometry the widget lays the columns out with — measuring a second
/// way here would be the duplication under test. Labels are right-aligned
/// into `width - 1`, so a trim recovers them.
/// One painted header line, cut by the cache's own `col_widths` for the same
/// reason. Labels are right-aligned into `width - 1`, so a trim recovers them.
fn tui_header_cells(line: &str, cache: &GridCache) -> Vec<String> {
let chars: Vec<char> = line.chars().collect();
let mut at: usize = cache
@@ -242,13 +206,12 @@ fn the_two_front_ends_draw_the_same_row_header() {
);
for (i, (painted, decided)) in tui.iter().zip(browser.iter()).enumerate() {
// A group header is one label across the whole row header; a data
// row is one label per level. The TUI's slicing puts a group label
// in the first sub-column, so compare the joined non-empty text.
// A group label lands in the first sub-column, a data row's tuple one
// per level, so the group arm compares the joined non-empty text.
let painted_text = painted.join("").trim().to_string();
match decided {
// The full painted string, glyph and name: a branch that checked
// only the glyph would pass on any group name whatsoever.
// Glyph AND name: checking only the glyph would pass on any group
// name whatsoever.
RenderRow::Group { label, collapsed } => assert_eq!(
painted_text,
format!("{} {label}", indicator(*collapsed)),
@@ -272,10 +235,7 @@ fn indicator(collapsed: bool) -> &'static str {
}
}
/// The column side of the same agreement, and it is a separate test because
/// `the_two_front_ends_draw_the_same_row_header` drives **one arm**: a
/// `col_levels` that stopped suppressing repeats, or a `col_groups` that
/// emitted one span per column, would sit green behind it
/// The column side, separate because the row test drives ONE arm
/// (`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`).
#[test]
fn the_two_front_ends_draw_the_same_column_header() {
@@ -294,10 +254,8 @@ fn the_two_front_ends_draw_the_same_column_header() {
"precondition: Q1 groups two of the three months"
);
// Each header line, cut into its columns and compared *exactly* to what
// the browser decided. A `contains` would be one-directional: it cannot
// see a label the browser wrongly blanked (the filter would skip it) and
// it cannot see one painted under the wrong column.
// Compared EXACTLY: a `contains` is one-directional and can see neither a
// wrongly-blanked label nor one painted under the wrong column.
for (level, labels) in model.col_levels.iter().enumerate() {
assert_eq!(
&tui_header_cells(&lines[rule - levels + level], &cache),
@@ -332,19 +290,8 @@ fn the_two_front_ends_draw_the_same_column_header() {
See `THE_MEMBERSHIP_CORRECT_SPANS` for what improvise-gd83 makes \
this line say."
);
// Spelled out rather than guessed, because a wrong future expectation is
// worse than none: whoever picks up improvise-gd83 will read this and
// believe it. The frame here is NOT one Q1 run — `_Measure` is the outer
// level, so the column axis is:
//
// col_cats = ["_Measure", "Month"]
// Group(Q1) [Revenue,Jan] [Revenue,Feb] [Revenue,Sep]
// Group(Q1) [Cost,Jan] [Cost,Feb] [Cost,Sep]
//
// Two Q1 headers, one per measure, and the ungrouped `Sep` sits *inside*
// each run rather than after both. So membership-correct spans are four
// runs, not two — and they do not coalesce across the second header,
// because `Sep` breaks the run between them.
// A DELIBERATELY WRONG constant and a live tripwire — do not remove it.
// Derived, not guessed: see `context/test-evidence/tui.md`.
const THE_MEMBERSHIP_CORRECT_SPANS: [(Option<&str>, usize); 4] =
[(Some("Q1"), 2), (None, 1), (Some("Q1"), 2), (None, 1)];
assert_ne!(
@@ -362,9 +309,8 @@ fn the_two_front_ends_draw_the_same_column_header() {
);
}
/// The joined form appears in neither, which is the bug's own words. It is a
/// separate assertion because the agreement above would also hold if *both*
/// ends started slash-joining.
/// The bug's own words, separate because the agreement above would also hold
/// if *both* ends started slash-joining.
#[test]
fn neither_front_end_produces_a_slash_joined_label() {
let mut wb = two_level_rows();
@@ -388,10 +334,8 @@ fn neither_front_end_produces_a_slash_joined_label() {
}
}
/// The group's collapse state reaches the browser as a *fact*, not as a
/// glyph. The TUI spells it ▼/▶ (presentation, gotcha 26); the client gets
/// the boolean and spells it however the DOM wants. Both states are checked,
/// because a `collapsed` hard-wired to `false` would pass the first half.
/// The collapse state crosses as a boolean, not a glyph (gotcha 26). BOTH
/// states, because a hard-wired `false` would pass the first half.
#[test]
fn the_collapse_state_crosses_as_a_fact_not_a_glyph() {
for collapse in [false, true] {
tests/shipped_documents_load.rs
+3 -20
View File
@@ -1,21 +1,5 @@
//! Every `.improv` document this repository ships must load (improvise-odbc).
//!
//! The finding behind this test: `bank-info.improv` sat in the repo root for
//! five months unable to parse — hand-edited through the v2025-04-09 format
//! change rather than round-tripped, so its item lines carried bare names the
//! grammar cannot spell (`- JORDAN MARTINEZ`) and its data lines named a
//! category `Measure` the file declares as `_Measure`. Nothing loaded it, so
//! nothing said so.
//!
//! The grammar itself is covered far better than a fixture can manage, by the
//! generator-driven proptests in `improvise-io`. What *those* cannot see is a
//! checked-in document drifting out of the format, because they generate their
//! inputs. This test is aimed at exactly that gap, and it is deliberately a
//! directory walk rather than a list of names: a list is a second place to
//! remember, and the file that gets forgotten is the one nobody added to it.
//!
//! The scope is the two directories `.gitignore` un-ignores — `examples/` and
//! `web/`. Everything else matching `*.improv` is somebody's working file.
//! Why a DIRECTORY WALK and not a list: `context/test-evidence/tui.md`.
use std::path::{Path, PathBuf};
@@ -25,9 +9,8 @@ fn shipped_documents() -> Vec<PathBuf> {
.iter()
.flat_map(|dir| std::fs::read_dir(root.join(dir)).expect("a shipped-document directory"))
.map(|entry| entry.expect("a readable directory entry").path())
// `.improv.gz` is the same document gzipped, and `extension()` calls
// it `gz` — so matching on the extension alone would skip it and put
// back the very hole this test exists to close.
// `extension()` of `x.improv.gz` is `gz`, so matching on the extension
// alone would skip it and put back the hole this test closes.
.filter(|p| {
p.to_str()
.is_some_and(|s| s.ends_with(".improv") || s.ends_with(".improv.gz"))
tests/web_demo_model.rs
+2 -17
View File
@@ -1,20 +1,5 @@
//! The demo document the standalone page opens must be one a visitor can
//! *type into* (improvise-bck).
//!
//! The bug this guards against is not hypothetical: `examples/demo.improv`
//! leaves `Customer` and `Date` on the None axis, so every cell in it is an
//! aggregate over records the grid does not name, and an aggregate is not
//! settable — correct behaviour, and a demo where the first thing a visitor
//! clicks does nothing.
//!
//! The question is asked exactly the way the browser asks it. ADR 002 says a
//! cell is settable iff its `CellId` is `Record`, `project::cell_id` is the
//! one place that decides, and `render.rs::editable` in the wasm client is
//! that variant and nothing else (gotcha 42). So this test projects the
//! demo's whole grid through `grid_of` — the same function the server calls
//! per frame — and asserts on the surrogate, not on the axes. Asserting the
//! axes instead would re-derive the rule and could agree with itself while
//! disagreeing with the client.
//! The demo document must be one a visitor can *type into* (improvise-bck),
//! asked as the browser asks it. `context/test-evidence/tui.md`.
use improvise::protocol::CellId;
use improvise_command::ui::app::App;