test: the two front ends draw the same grid frame
The check gotcha 44's divergence never had. Each front end is driven the way
it is really driven — the widget into a ratatui buffer, the browser client
through a `Snapshot` of that same cache's `seed()` — and they are compared on
what they *draw*, not on what they were handed; comparing the caches would be
vacuous, since it is one cache.
Injected both faults and watched them fail before keeping it:
`labels: vec![rows.item(r).join("/")]` in the client's `render_model`, and the
same join in the widget's row header. Each names the row and both spellings.
`improvise-wasm-client` becomes a dev-dependency of the root crate for this.
It builds natively — that is how its own tests already run — and being a dev
dependency the terminal binary does not grow a wasm client.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -38,6 +38,13 @@ clap = { version = "4.6.7", features = ["derive"] }
|
||||
enum_dispatch = "0.3.13"
|
||||
|
||||
[dev-dependencies]
|
||||
# Only `tests/grid_frame_agreement.rs` needs it: that test drives the TUI
|
||||
# widget and the browser client over ONE `GridCache` and asserts they draw
|
||||
# the same grid frame (improvise-anr9). A dev-dependency, so the terminal
|
||||
# binary does not grow a wasm client; `improvise-wasm-client` builds
|
||||
# natively, which is how its own tests already run.
|
||||
improvise-wasm-client = { path = "crates/improvise-wasm-client" }
|
||||
improvise-protocol = { path = "crates/improvise-protocol" }
|
||||
pest = "2.9.1"
|
||||
pest_derive = "2.9.1"
|
||||
pest_meta = "2.9.1"
|
||||
|
||||
@@ -243,10 +243,15 @@ handed a job the store should have done.
|
||||
today.** Two front ends deriving the same fact will diverge, and the divergence
|
||||
is invisible until someone compares them side by side. `improvise-edp` is the
|
||||
worked example — the category tree was flattened twice, two different ways, and
|
||||
the fix put `viewmodel::flatten_cat_tree` where both ends call it. Gotcha 44 is
|
||||
the same defect still open: the TUI derives the grid's *frame* from
|
||||
`GridLayout`, the browser cannot, and so the browser renders `Food/Alice` where
|
||||
the TUI renders two header columns.
|
||||
the fix put `viewmodel::flatten_cat_tree` where both ends call it. Gotcha 44
|
||||
was the same defect and is now closed the same way (improvise-anr9): the grid's
|
||||
*frame* — header levels, label tuples, group runs and their collapse state —
|
||||
is `improvise_protocol::AxisFrame`, and both `src/ui/grid.rs` and
|
||||
`improvise-wasm-client/src/render.rs` ask it rather than deriving one. Note
|
||||
what the second cure cost that the first did not: a `Vec<String>` of labels
|
||||
had to change **type**, and a shared derivation could not be added beside the
|
||||
old one — `row_levels` next to `row_labels` would have been two answers to
|
||||
"what are the labels", which is the disease and not the cure.
|
||||
|
||||
**Sharing the table beats testing two copies of it.** Where a front end must
|
||||
turn input into actions, share the mapping rather than mirroring it: two
|
||||
|
||||
@@ -0,0 +1,266 @@
|
||||
//! 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.
|
||||
|
||||
use improvise::model::cell::{CellKey, CellValue};
|
||||
use improvise::ui::app::{App, AppMode, ViewCursor};
|
||||
use improvise::ui::grid::GridWidget;
|
||||
use improvise::view::{Axis, GridLayout};
|
||||
use improvise::workbook::Workbook;
|
||||
use improvise_command::ui::project;
|
||||
use improvise_protocol::{GridCache, Snapshot, ViewState};
|
||||
use improvise_wasm_client::render::RenderRow;
|
||||
use ratatui::{buffer::Buffer, layout::Rect, widgets::Widget};
|
||||
|
||||
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
||||
CellKey::new(
|
||||
pairs
|
||||
.iter()
|
||||
.map(|(c, i)| (c.to_string(), i.to_string()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Two categories on Row — the cross product the slash-join used to flatten
|
||||
/// — with two of the three `Recipient`s in a group, so the frame carries
|
||||
/// header levels, repeat suppression *and* a group with a collapse state in
|
||||
/// one fixture. `Carol` sits outside the group and *before* it, so that
|
||||
/// 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.
|
||||
fn two_level_rows() -> Workbook {
|
||||
let mut m = Workbook::new("Test");
|
||||
m.add_category("Type").unwrap();
|
||||
m.add_category("Month").unwrap();
|
||||
m.add_category("Recipient").unwrap();
|
||||
m.add_item("Type", "Food").unwrap();
|
||||
m.add_item("Type", "Clothing").unwrap();
|
||||
m.add_item("Month", "Jan").unwrap();
|
||||
m.add_item("Recipient", "Carol").unwrap();
|
||||
m.add_item_in_group("Recipient", "Alice", "Family").unwrap();
|
||||
m.add_item_in_group("Recipient", "Bob", "Family").unwrap();
|
||||
m.active_view_mut().set_axis("Recipient", Axis::Row);
|
||||
for t in ["Food", "Clothing"] {
|
||||
for r in ["Carol", "Alice", "Bob"] {
|
||||
m.model.set_cell(
|
||||
coord(&[("Type", t), ("Month", "Jan"), ("Recipient", r)]),
|
||||
CellValue::Number(1.0),
|
||||
);
|
||||
}
|
||||
}
|
||||
m
|
||||
}
|
||||
|
||||
/// The one cache both ends are driven from — built exactly as the TUI builds
|
||||
/// it, over the whole grid at offsets zero (gotcha 44).
|
||||
fn whole_grid(wb: &mut Workbook) -> (App, GridCache) {
|
||||
let none_cats = wb.active_view().none_cats();
|
||||
wb.model.recompute_formulas(&none_cats);
|
||||
let app = App::new(wb.clone(), None);
|
||||
let cache = project::grid_of(
|
||||
&app,
|
||||
project::Viewport {
|
||||
row_offset: 0,
|
||||
col_offset: 0,
|
||||
rows: app.layout.row_count(),
|
||||
cols: app.layout.col_count(),
|
||||
},
|
||||
);
|
||||
(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.
|
||||
fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
|
||||
let area = Rect::new(0, 0, 60, 24);
|
||||
let mut buf = Buffer::empty(area);
|
||||
let layout = GridLayout::new(&wb.model, wb.active_view());
|
||||
let view_name = wb.active_view.clone();
|
||||
let buffers = std::collections::HashMap::new();
|
||||
GridWidget::new(
|
||||
cache,
|
||||
ViewCursor::default(),
|
||||
&view_name,
|
||||
&layout,
|
||||
&AppMode::Normal,
|
||||
"",
|
||||
&buffers,
|
||||
None,
|
||||
)
|
||||
.render(area, &mut buf);
|
||||
|
||||
let width = area.width as usize;
|
||||
let lines: Vec<String> = buf
|
||||
.content()
|
||||
.chunks(width)
|
||||
// Strip the block's left and right border columns.
|
||||
.map(|row| {
|
||||
row[1..width - 1]
|
||||
.iter()
|
||||
.map(|c| c.symbol())
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect();
|
||||
|
||||
// The body starts after the one line that is nothing but the header rule.
|
||||
let sep = lines
|
||||
.iter()
|
||||
.position(|l| !l.trim().is_empty() && l.chars().all(|c| c == '─'))
|
||||
.expect("the header separator");
|
||||
|
||||
let widths = cache.row_frame.header_widths();
|
||||
lines[sep + 1..]
|
||||
.iter()
|
||||
// The body ends at the blank space below the grid, or at the rule
|
||||
// above the totals strip — which is not a row of the frame.
|
||||
.take_while(|l| !l.trim().is_empty() && !l.trim_end().chars().all(|c| c == '─'))
|
||||
.map(|line| {
|
||||
let chars: Vec<char> = line.chars().collect();
|
||||
let mut at = 0usize;
|
||||
widths
|
||||
.iter()
|
||||
.map(|w| {
|
||||
let end = (at + *w as usize).min(chars.len());
|
||||
let cell: String = chars[at.min(chars.len())..end].iter().collect();
|
||||
at = end;
|
||||
cell.trim_end().to_string()
|
||||
})
|
||||
.collect()
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// What the browser client decides to draw, from a `Snapshot` of the same
|
||||
/// cache — the real bootstrap path, not a hand-built client state.
|
||||
fn browser_rows(cache: &GridCache) -> Vec<RenderRow> {
|
||||
let mut client = improvise_wasm_client::init();
|
||||
client.bootstrap(Snapshot {
|
||||
view: ViewState::default(),
|
||||
seed: cache
|
||||
.seed()
|
||||
.into_iter()
|
||||
.map(improvise_protocol::Projection::Grid)
|
||||
.collect(),
|
||||
});
|
||||
client.model().rows
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_two_front_ends_draw_the_same_row_header() {
|
||||
let mut wb = two_level_rows();
|
||||
let (_app, cache) = whole_grid(&mut wb);
|
||||
|
||||
let tui = tui_row_headers(&wb, &cache);
|
||||
let browser = browser_rows(&cache);
|
||||
|
||||
assert_eq!(
|
||||
tui.len(),
|
||||
browser.len(),
|
||||
"the two ends drew a different number of rows\nTUI: {tui:?}\nbrowser: {browser:?}"
|
||||
);
|
||||
assert!(
|
||||
tui.len() > 4,
|
||||
"precondition: the fixture must have group headers AND several data \
|
||||
rows, or nothing here is being compared: {tui:?}"
|
||||
);
|
||||
|
||||
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.
|
||||
let painted_text = painted.join("").trim().to_string();
|
||||
match decided {
|
||||
RenderRow::Group { label, .. } => assert!(
|
||||
painted_text.starts_with('▼') || painted_text.starts_with('▶'),
|
||||
"row {i}: the browser calls it the group {label:?}, the TUI \
|
||||
painted {painted_text:?}"
|
||||
),
|
||||
RenderRow::Data { labels, .. } => assert_eq!(
|
||||
painted, labels,
|
||||
"row {i}: the TUI painted {painted:?} and the browser decided \
|
||||
{labels:?} — one of them re-derived the frame"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// 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.
|
||||
#[test]
|
||||
fn neither_front_end_produces_a_slash_joined_label() {
|
||||
let mut wb = two_level_rows();
|
||||
let (_app, cache) = whole_grid(&mut wb);
|
||||
|
||||
for row in browser_rows(&cache) {
|
||||
for label in row.labels() {
|
||||
assert!(
|
||||
!label.contains('/'),
|
||||
"the browser drew the joined label {label:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
for painted in tui_row_headers(&wb, &cache) {
|
||||
for label in painted {
|
||||
assert!(
|
||||
!label.contains('/'),
|
||||
"the TUI painted the joined label {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.
|
||||
#[test]
|
||||
fn the_collapse_state_crosses_as_a_fact_not_a_glyph() {
|
||||
for collapse in [false, true] {
|
||||
let mut wb = two_level_rows();
|
||||
if collapse {
|
||||
wb.active_view_mut()
|
||||
.toggle_group_collapse("Recipient", "Family");
|
||||
}
|
||||
let (_app, cache) = whole_grid(&mut wb);
|
||||
|
||||
let groups: Vec<(String, bool)> = browser_rows(&cache)
|
||||
.into_iter()
|
||||
.filter_map(|r| match r {
|
||||
RenderRow::Group { label, collapsed } => Some((label, collapsed)),
|
||||
RenderRow::Data { .. } => None,
|
||||
})
|
||||
.collect();
|
||||
assert!(
|
||||
!groups.is_empty(),
|
||||
"precondition (collapse={collapse}): a collapsed group still owes \
|
||||
the user a header to expand again"
|
||||
);
|
||||
for (label, collapsed) in groups {
|
||||
assert_eq!(label, "Family");
|
||||
assert_eq!(collapsed, collapse, "the frame states the view's answer");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user