test(core): cover the partly-pruned group, and pin the collapsed frame exactly
Self-review under the code-review skill, attacking my own three prune tests by trying to build a wrong prune_axis_items that passes all of them. The gap it found: all three fixtures put a group in an ALL-or-NOTHING state — fully collapsed, fully pruned, or intact — so between them they said nothing about the mixed case, which is the one a user meets first. A group where some items prune and some survive now has its own test. a_collapsed_groups_header_survives_pruning now asserts the whole row_items sequence instead of any(|e| is the Q1 header). An any() cannot see a header emitted in the wrong position or an extra entry beside it. The attack itself found no defect, and what it established is worth more than if it had. Rewriting run_had_item to mean "some item was REJECTED" rather than "there was an item" is a no-op with the whole workspace green: the header is pushed eagerly by the FIRST SURVIVING item, so pending_header is already None before any rejection can be recorded, and the flag is only ever read in runs where nothing survived — where the two readings coincide. The eager push is what makes the collapsed/pruned distinction robust rather than delicate. Also recorded, because it bounds what the new assertion may claim: two consecutive GroupEnds are not reachable, since expand_category never emits them, so "no stray GroupEnd" is a property of the writer and not something any fixture can falsify. The commit that introduced the run_open tracking claimed it as a virtue; it is one, but it is argued and not tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -616,6 +616,8 @@ Fault injected by deleting either emission: the corresponding test fails with
|
||||
|
||||
### a_header_whose_items_all_prune_away_is_dropped
|
||||
|
||||
### a_partly_pruned_group_keeps_the_header_its_survivors_sit_under
|
||||
|
||||
### an_ungrouped_item_after_a_pruned_group_belongs_to_no_group
|
||||
|
||||
improvise-lhi4 and improvise-u6gc: two defects in one `match` block, and they
|
||||
@@ -648,10 +650,33 @@ header is closed even though the header between them was pruned. That keeps
|
||||
gotcha 1's shape — still a predicate over the data index, no `&View`
|
||||
parameter, no flag vector.
|
||||
|
||||
The three tests are separate because each is satisfied by a fix that misses
|
||||
another. `a_header_whose_items_all_prune_away_is_dropped` **passes before the
|
||||
change** and is the discriminator: it is what stops "keep every header" from
|
||||
counting as a fix for lhi4, and it is the acceptance criterion lhi4 names.
|
||||
The tests are separate because each is satisfied by a fix that misses another.
|
||||
`a_header_whose_items_all_prune_away_is_dropped` **passes before the change**
|
||||
and is the discriminator: it is what stops "keep every header" from counting as
|
||||
a fix for lhi4, and it is the acceptance criterion lhi4 names.
|
||||
|
||||
`a_partly_pruned_group_keeps_the_header_its_survivors_sit_under` was added by
|
||||
self-review and covers the **commonest** case, which the other three do not
|
||||
reach: a group where some items prune and some do not. All three of the others
|
||||
put a group in an all-or-nothing state, so between them they said nothing about
|
||||
the mixed one — and it is the case a user meets first.
|
||||
|
||||
Reviewing it turned up a fact worth keeping about the shape of the code rather
|
||||
than a defect. Rewriting `run_had_item` to mean "some item was **rejected**"
|
||||
instead of "there was an item" is a **no-op**, and the whole workspace stays
|
||||
green: the header is pushed eagerly by the *first surviving item*, so
|
||||
`pending_header` is already `None` by the time any rejection can be recorded,
|
||||
and the flag is therefore only ever read in runs where **nothing** survived —
|
||||
where "no item at all" and "no item accepted" coincide. The eager push is what
|
||||
makes the collapsed/pruned distinction robust rather than delicate, and a
|
||||
reviewer trying to break it by attacking that flag is attacking the wrong half.
|
||||
|
||||
`a_collapsed_groups_header_survives_pruning` asserts the **whole** `row_items`
|
||||
sequence rather than `any(|e| is Q1 header)`. An `any()` cannot see a header
|
||||
emitted in the wrong position or an extra entry beside it; the sequence can.
|
||||
Note what it still cannot claim: two consecutive `GroupEnd`s are not reachable,
|
||||
because `expand_category` never emits them, so "no stray `GroupEnd`" is a
|
||||
structural property of the writer and not something any fixture falsifies.
|
||||
|
||||
Faults injected, each against the fixed code, and each fails a **different**
|
||||
pair — which is the evidence that no one of the three tests stands in for the
|
||||
|
||||
@@ -1382,9 +1382,18 @@ mod tests {
|
||||
let mut wb = grouped_month_book(&[("Jan", Some("Q1")), ("Sep", None)], &["Jan", "Sep"]);
|
||||
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
|
||||
let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
|
||||
assert_eq!(layout.row_count(), 1, "only Sep is a data row");
|
||||
assert!(
|
||||
has_header(&layout.row_items, "Q1"),
|
||||
// The whole sequence, not `any(is Q1)`: that cannot see a header
|
||||
// pushed in the wrong place, nor a stray `GroupEnd`.
|
||||
assert_eq!(
|
||||
layout.row_items,
|
||||
vec![
|
||||
AxisEntry::GroupHeader {
|
||||
cat_name: "Month".into(),
|
||||
group_name: "Q1".into()
|
||||
},
|
||||
AxisEntry::GroupEnd,
|
||||
AxisEntry::DataItem(vec!["Sep".into()]),
|
||||
],
|
||||
"Q1 is collapsed, not empty — its header is the only way back"
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -1408,6 +1417,24 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The commonest case and the one the other three cannot see: SOME of a
|
||||
/// group's items prune. context/test-evidence/core.md.
|
||||
#[test]
|
||||
fn a_partly_pruned_group_keeps_the_header_its_survivors_sit_under() {
|
||||
let wb = grouped_month_book(
|
||||
&[("Jan", Some("Q1")), ("Feb", Some("Q1")), ("Sep", None)],
|
||||
&["Jan", "Sep"],
|
||||
);
|
||||
let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
|
||||
assert_eq!(layout.row_count(), 2, "Feb pruned; Jan and Sep remain");
|
||||
assert_eq!(
|
||||
layout.row_group_for(0),
|
||||
Some(("Month".into(), "Q1".into())),
|
||||
"Jan survived, so Q1 still has a member and must keep its header"
|
||||
);
|
||||
assert_eq!(layout.row_group_for(1), None, "Sep is in no group");
|
||||
}
|
||||
|
||||
/// improvise-u6gc: pruning the intervening header re-opens the earlier
|
||||
/// run, so the `GroupEnd` that closed it must survive. Regresses gd83.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user