Skip to content

test(tui): make frame snapshots independent of the crate version - #525

Merged
emal-avala merged 1 commit into
mainfrom
fix/snapshot-version-independent
Jul 27, 2026
Merged

test(tui): make frame snapshots independent of the crate version#525
emal-avala merged 1 commit into
mainfrom
fix/snapshot-version-independent

Conversation

@emal-avala

Copy link
Copy Markdown
Member

Summary

  • main is currently red and so is every open PR rebased onto it: the four frame-snapshot tests fail because the goldens embed agent-code 0.27.0 in the header and the v0.28.0 release bumped the crate version.
  • assert_snapshot now masks this crate's version with a fixed x.y.z placeholder before comparing (and before writing, so regeneration stays stable). The replacement is padded to the same byte length, so column alignment — and therefore the style rows — stay comparable across versions of differing width.
  • Goldens regenerated: the diff is exactly the version line in each of the four files, nothing else.
  • Without this, every release would break the snapshot suite the same way and require a regeneration commit.

Test plan

  • cargo test --all-targets — snapshot suite 9/9 green; full suite green apart from the 3 known environmental bwrap_* failures
  • Verified against a future bump: set the crate version to 0.29.0 locally and re-ran — snapshots still pass
  • cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check
  • Confirmed the failure reproduces on pristine origin/main before the fix (4 failures), and passes after

The header renders `agent-code <version>`, so the goldens embedded
0.27.0 and the v0.28.0 release bump turned all four frame snapshots
red — on main, and on every branch that rebased onto it.

assert_snapshot now masks this crate's version with a fixed
placeholder before comparing (and before writing, so regeneration
stays stable). The replacement keeps the same byte length, so column
alignment and the style rows stay comparable across versions of
differing width. Goldens regenerated: the diff is the version line
in each of the four files and nothing else.

Verified by bumping the crate version locally to 0.29.0 — snapshots
still pass, so the next release cannot break them the same way.
@emal-avala
emal-avala merged commit f40c735 into main Jul 27, 2026
13 checks passed
@emal-avala
emal-avala deleted the fix/snapshot-version-independent branch July 27, 2026 06:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50187b3f1c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +97 to +101
match version.len().cmp(&PLACEHOLDER.len()) {
// Keep the byte length identical so nothing shifts columns.
std::cmp::Ordering::Greater => {
replacement.push_str(&" ".repeat(version.len() - PLACEHOLDER.len()))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize the placeholder to a fixed width

When a release version has a different byte length, such as 0.100.0, this branch pads x.y.z to that new length rather than the width stored in the goldens. The following header fields and their style cells are therefore still rendered at version-dependent columns, so all four snapshots will fail again despite the masking. Use a fixed test version before rendering, or normalize the corresponding glyph and style columns to a constant width.

Useful? React with 👍 / 👎.

emal-avala added a commit that referenced this pull request Jul 27, 2026
Conflicts were the four frame goldens. main (#525) made them independent
of the crate version by masking it to x.y.z, which is the better fix than
the version bump this branch had pinned in; took main's side and dropped
that commit's effect.

Regenerating the goldens on this branch reproduces main's files
byte-for-byte: none of the snapshot fixtures issues a TodoWrite, so the
checklist never renders in these frames and the feature leaves them
unchanged.
emal-avala added a commit that referenced this pull request Jul 27, 2026
My previous commit reverted #525. Regenerating the goldens against a
working tree that still held the pre-merge snapshot.rs, then staging
everything, deleted mask_version and re-pinned all four headers to a
concrete 0.28.0 — reintroducing precisely the breakage #525 landed to
end, where a release bump turns every frame snapshot red. Review
flagged it as a P1 and was right.

snapshot.rs is taken verbatim from main; this branch never had a reason
to touch that file. The goldens are regenerated on top of it, so they
now carry both the x.y.z placeholder and this branch's palette routing.

Verified the masking rather than assuming it: with crates/cli set to
0.29.0 the snapshot suite still passes, and every header reads
agent-code x.y.z.

Every style row that differs from main is accounted for by the colour
routing, and no glyph moved in any of the four frames:

  - DarkGray becomes Rgb(92, 99, 112), one-dark's muted;
  - Gray becomes Rgb(124, 131, 144), one-dark's inactive;
  - White becomes Rgb(171, 178, 191), the text slot;
  - Green becomes Rgb(152, 195, 121), the success slot;
  - Black on the warning fill becomes Rgb(40, 44, 52), which is what
    on_fill picks for one-dark.

transcript_basic also loses a legend entry, and its style row shifts
from i to e: bold Green and bold success are now the same style, so the
two entries dedupe into one. Same pixels, one fewer legend line.
transcript_light resolves against solarized-light instead — muted
#93a1a1 and inactive #819295 — which is the point of routing through
the palette rather than naming colours.

Taking main's copy of any file this PR has no intent to change is the
rule I should have applied when resolving the merge in the first place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant