mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
test(terminal): close the review gaps in the cluster and snapshot contracts
Six review findings, all coverage on code that was already correct, plus one
enforcement change.
Clusters. The join guard has two halves and only one was exercised: every
fixture built a plain cluster after a marked one, none the reverse. Drop
`joinable` and a release build silently emits `Span { text: "xye\u{301}",
cluster_count: 3 }` -- four chars counted as three, so the consumer misplaces
everything after it. Also fixture the `u16::MAX` join refusal, which is live
rather than defensive: `Size.columns` is an unclamped `usize` with no caller
bounding it, so a 70000-column row reaches it and must split rather than wrap.
Promote the encoder's `debug_assert` to `assert!`. This is a wire invariant,
and a check that vanishes in release vanishes exactly where the corruption
ships: with the defect present, the promoted build fails at the producer
rather than leaving an undecodable span for the renderer to misdraw. Measured
at +5.9us on a 65us encode for 80x24 -- 0.04% of a 60Hz frame.
Snapshot. The four fixtures asserted the damage plane and nothing else, so a
capture that dropped the screen's last row, lied about the cursor, or stamped
a stale grid identity passed all of them. `visible_text` trims and filters
empty lines, which is what let the missing row disappear -- a helper written
for legibility concealing the defect it should have exposed. Assert the row
set rather than the text, the cursor against a real position and DECTCEM, the
whole `Viewport` rather than `columns` alone, and that a full-grid copy is
billed to the renderer's meter and not the reader's.
Findings from Sami's P2a and P2b reviews; enforcement call from Eva; encode
cost measured by Sami.
Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
co-authored by
tlongwell-block
parent
3a5c7d576e
commit
e887bdb2c4
@@ -381,7 +381,14 @@ fn spans(cells: &[Cell]) -> Vec<Span> {
|
||||
}
|
||||
open = joinable;
|
||||
}
|
||||
debug_assert!(
|
||||
// Enforced in release, not just in debug. This is a *wire* invariant: a
|
||||
// span that violates it is undecodable by the rule in [`Span`], and the
|
||||
// consumer's failure is silent misplacement of every cluster after it.
|
||||
// A `debug_assert` here would vanish in exactly the build where that
|
||||
// corruption ships. The cost is one pass over text already in cache --
|
||||
// the same order as building the spans -- and it buys a loud, local
|
||||
// failure instead of a renderer quietly drawing the wrong columns.
|
||||
assert!(
|
||||
spans.iter().all(Span::counts_are_consistent),
|
||||
"cluster_count must be 1 or the span's char count"
|
||||
);
|
||||
|
||||
@@ -199,6 +199,84 @@ fn cluster_count_distinguishes_a_marked_cluster_from_a_plain_run() {
|
||||
assert!(marked.counts_are_consistent() && plain.counts_are_consistent());
|
||||
}
|
||||
|
||||
/// The join guard has two halves: the previous cell must not have carried
|
||||
/// marks (`open`), and the current cell must not carry them (`joinable`).
|
||||
/// Every fixture above exercises only the first half -- a plain cluster
|
||||
/// following a marked one. This one exercises the second: a *marked* cluster
|
||||
/// arriving after a plain run, which is the only path on which the run in
|
||||
/// progress is handed text holding more `char`s than the one cluster its
|
||||
/// count is about to be incremented by.
|
||||
///
|
||||
/// Sami found the hole. With `joinable` dropped from the guard, a release
|
||||
/// build silently emits `Span { column: 0, text: "xyé", cluster_count: 3 }`:
|
||||
/// four chars counted as three, so the consumer's rule splits per char and
|
||||
/// places the combining mark on top of `z`.
|
||||
#[test]
|
||||
fn a_marked_cluster_after_a_plain_run_starts_its_own_span() {
|
||||
let (spans, _actions) = render("xye\u{0301}z");
|
||||
assert_eq!(
|
||||
placements(&spans),
|
||||
vec![
|
||||
(0, "x".into()),
|
||||
(1, "y".into()),
|
||||
(2, "e\u{0301}".into()),
|
||||
(3, "z".into()),
|
||||
],
|
||||
"a marked cluster must not be absorbed into the run in front of it"
|
||||
);
|
||||
}
|
||||
|
||||
/// `cluster_count` is a `u16` and `Size.columns` is an unclamped `usize`
|
||||
/// (`lib.rs:50`) that no production caller bounds yet, so a row of uniform
|
||||
/// cells wider than `u16::MAX` reaches the join guard's overflow refusal.
|
||||
/// The guard is live code, not paranoia, and this fixture is what says so.
|
||||
///
|
||||
/// Refusing to join produces a shape the consumer already handles -- the run
|
||||
/// ends and a new span starts at the next column -- whereas wrapping produces
|
||||
/// an undecodable span, the same failure as the marked-after-plain case above.
|
||||
#[test]
|
||||
fn a_run_longer_than_u16_max_splits_rather_than_wrapping() {
|
||||
let columns = 70_000;
|
||||
let size = Size {
|
||||
columns,
|
||||
screen_lines: 1,
|
||||
scrollback: 0,
|
||||
};
|
||||
let (term, _actions) = Terminal::new(size, Fences::ALL);
|
||||
let shared = SharedTerminal::new(term);
|
||||
// One character is enough: the rest of the row is blank cells of the same
|
||||
// style, so the whole row is a single candidate run.
|
||||
shared.feed(b"a");
|
||||
let mut encoder = Encoder::new();
|
||||
let frame = shared.render(&mut encoder);
|
||||
|
||||
let spans = &frame
|
||||
.rows
|
||||
.iter()
|
||||
.find(|row| row.line == 0)
|
||||
.expect("the fed row must be present")
|
||||
.spans;
|
||||
|
||||
assert!(
|
||||
spans.iter().all(|span| span.counts_are_consistent()),
|
||||
"an oversized run must not wrap its count: {spans:?}"
|
||||
);
|
||||
let counts: Vec<u16> = spans.iter().map(|span| span.cluster_count).collect();
|
||||
let columns_at: Vec<usize> = spans.iter().map(|span| span.column).collect();
|
||||
assert_eq!(
|
||||
counts,
|
||||
vec![u16::MAX, (columns - u16::MAX as usize) as u16],
|
||||
"the run must end at the last representable count"
|
||||
);
|
||||
assert_eq!(
|
||||
columns_at,
|
||||
vec![0, u16::MAX as usize],
|
||||
"the second span starts where the first left off"
|
||||
);
|
||||
let chars: usize = spans.iter().map(|span| span.text.chars().count()).sum();
|
||||
assert_eq!(chars, columns, "no cell may be dropped by the split");
|
||||
}
|
||||
|
||||
/// Wrapping marks the last cell of the row with `WRAPLINE` (upstream
|
||||
/// `term/mod.rs:968`). That bit records where the text happened to wrap, not
|
||||
/// how the text looks, so it must not reach the style key: if it did, the last
|
||||
|
||||
@@ -143,6 +143,7 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() {
|
||||
let mut encoder = Encoder::new();
|
||||
let before = shared.snapshot(&mut encoder);
|
||||
assert_eq!(before.viewport.columns, 20);
|
||||
let first_generation = before.viewport.generation;
|
||||
|
||||
let resized = shared.resize(Size {
|
||||
columns: 10,
|
||||
@@ -150,6 +151,10 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() {
|
||||
scrollback: 100,
|
||||
});
|
||||
assert_eq!(resized.columns, 10);
|
||||
assert!(
|
||||
resized.generation > first_generation,
|
||||
"an applied resize advances the generation"
|
||||
);
|
||||
|
||||
// Same encoder, new geometry: every row must be re-sent, not suppressed
|
||||
// as unchanged against hashes taken at the old width.
|
||||
@@ -158,6 +163,17 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() {
|
||||
after.viewport.columns, 10,
|
||||
"the capture-time grid is stamped"
|
||||
);
|
||||
// Columns alone does not identify a grid. `Viewport`'s own doc says the
|
||||
// three fields travel together *because* a consumer comparing two of the
|
||||
// three can be wrong -- and this fixture used to compare one. A resize
|
||||
// that changed only `screen_lines`, or 20 -> 10 -> 20, leaves columns
|
||||
// matching while the generation has moved. `resize.rs` asserts this on
|
||||
// `render()` frames five times and never once on a snapshot, which is
|
||||
// what Sami's T3 mutant walked through; Mari's reattach reads this stamp.
|
||||
assert_eq!(
|
||||
after.viewport, resized,
|
||||
"a snapshot stamps the identity of the grid it actually captured"
|
||||
);
|
||||
assert!(after.full, "a snapshot is a repaint");
|
||||
assert!(
|
||||
!after.rows.is_empty(),
|
||||
@@ -165,6 +181,90 @@ fn a_snapshot_realigns_a_reused_encoders_dedup_state() {
|
||||
);
|
||||
}
|
||||
|
||||
/// A snapshot carries *every* row of the viewport, including the last one,
|
||||
/// and stamps the cursor plane truthfully.
|
||||
///
|
||||
/// Both properties are asserted here rather than in the fixtures above
|
||||
/// because of what those fixtures' helper hides: `visible_text` trims and
|
||||
/// drops empty lines, so a capture that skipped the bottom row of the screen
|
||||
/// reads identically to one that didn't whenever the content sits in the top
|
||||
/// rows -- which it does in every other fixture in this file. Sami's T2
|
||||
/// mutant (`0..screen_lines - 1`) survived all four for exactly that reason.
|
||||
/// So this fixture puts content on the last row and asserts the row *set*,
|
||||
/// not the text.
|
||||
///
|
||||
/// The cursor half is the same shape of gap: nothing checked that a snapshot's
|
||||
/// cursor was the terminal's cursor rather than a plausible default.
|
||||
#[test]
|
||||
fn a_snapshot_carries_every_row_and_the_true_cursor() {
|
||||
let (shared, _actions) = terminal(20, 4);
|
||||
// Write the bottom row of the screen, then park the cursor at line 4,
|
||||
// column 6 (1-based) -- row 3, column 5 to us.
|
||||
shared.feed(b"\x1b[4;1Hbottom\x1b[4;6H");
|
||||
|
||||
let mut attaching = Encoder::new();
|
||||
let frame = shared.snapshot(&mut attaching);
|
||||
|
||||
let lines: Vec<usize> = frame.rows.iter().map(|row| row.line).collect();
|
||||
assert_eq!(
|
||||
lines,
|
||||
vec![0, 1, 2, 3],
|
||||
"a snapshot must carry the whole viewport, last row included"
|
||||
);
|
||||
assert!(
|
||||
visible_text(&frame).contains(&"bottom".to_string()),
|
||||
"content on the last row must reach an attaching subscriber, got {:?}",
|
||||
visible_text(&frame)
|
||||
);
|
||||
|
||||
assert_eq!(frame.cursor.line, 3, "the snapshot's cursor line is real");
|
||||
assert_eq!(
|
||||
frame.cursor.column, 5,
|
||||
"the snapshot's cursor column is real"
|
||||
);
|
||||
assert!(frame.cursor.visible, "the cursor is shown by default");
|
||||
|
||||
// ...and a hidden cursor is reported hidden, so `visible` tracks the mode
|
||||
// rather than being a constant that happens to match the default.
|
||||
shared.feed(b"\x1b[?25l");
|
||||
let mut second = Encoder::new();
|
||||
assert!(
|
||||
!shared.snapshot(&mut second).cursor.visible,
|
||||
"DECTCEM off must reach the attaching subscriber"
|
||||
);
|
||||
}
|
||||
|
||||
/// Taking a snapshot is billed to the renderer plane.
|
||||
///
|
||||
/// The two planes are metered separately because pooling them lets the
|
||||
/// reader's millions of fast acquires dilute the renderer's tail into a false
|
||||
/// pass (`shared.rs` module docs). A full-grid copy is the single most
|
||||
/// expensive thing that takes this lock, so misfiling it under the reader
|
||||
/// would corrupt the very instrument the renderer's budget is judged by --
|
||||
/// and no fixture noticed until Sami's T4.
|
||||
#[test]
|
||||
fn a_snapshot_is_billed_to_the_renderer_plane() {
|
||||
let (shared, _actions) = terminal(20, 4);
|
||||
shared.feed(b"content");
|
||||
|
||||
shared.reader_acquire().reset();
|
||||
shared.renderer_acquire().reset();
|
||||
|
||||
let mut attaching = Encoder::new();
|
||||
let _ = shared.snapshot(&mut attaching);
|
||||
|
||||
assert_eq!(
|
||||
shared.renderer_acquire().snapshot().acquisitions,
|
||||
1,
|
||||
"the snapshot's lock acquisition belongs to the renderer plane"
|
||||
);
|
||||
assert_eq!(
|
||||
shared.reader_acquire().snapshot().acquisitions,
|
||||
0,
|
||||
"a full-grid copy must not be charged to the reader plane"
|
||||
);
|
||||
}
|
||||
|
||||
/// Two consecutive snapshots with no output between them still both carry the
|
||||
/// screen. A snapshot is not a one-shot: reattach may happen repeatedly, and
|
||||
/// nothing about the first may disarm the second.
|
||||
|
||||
Reference in New Issue
Block a user