yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

make ui-visual-update ignores UI_ARGS and re-records every baseline #204

New Issue
Closed
opened 2026-08-23 07:41:46 +00:00 by logan · 2 comments
logan commented 2026-08-23 07:41:46 +00:00
Collaborator
Copy Link
Copy Source

Report

make ui-visual-update UI_ARGS='test/components/chrome.test.ts'
re-records every screenshot baseline in the repo, not the one file
named. The filter is silently ignored, and so is a second one:

$ make ui-visual-update UI_ARGS="test/components/chrome.test.ts"
 Test Files  99 passed (99)
      Tests  1092 passed (1092)

$ make ui-visual-update UI_ARGS="test/components/chrome.test.ts test/components/now-playing.test.ts"
 Test Files  1 passed (1)          # now-playing only; chrome never ran
      Tests  22 passed (22)

Cause

The target is npx vitest run --update $(UI_ARGS), and vitest's
--update takes the following positional as its value rather than
being parsed as a boolean. So the first path is eaten by the flag and
whatever is left becomes the filter — one path means no filter at all
and the whole suite is re-recorded; two means only the second file runs.

Measured directly, from frontend/:

$ npx vitest run --update test/components/track-info.test.ts
 Test Files  99 passed (99)
$ npx vitest run --update=true test/components/track-info.test.ts
 Test Files  1 passed (1)

Why it matters

This is exactly the hazard #196 was filed about, in the tool meant to
resolve it. #196's rule is "refresh the reference your change moved,
having read the image, and never one you did not cause" — and the
documented way to refresh one blesses all ten in silence. A baseline
that is stale for an unrelated reason is re-recorded as correct, and
the diff shows a changed PNG that nobody asked about.

It bit this session: the intent was to re-record two references and
the run re-recorded the set. Nothing was lost, because the other eight
happened to be byte-identical and git status said so — which is the
only reason it was noticed at all.

Direction

--update=true in the ui-visual-update recipe. make ui-visual
itself is unaffected (its $(UI_ARGS) follows run, with no flag to
swallow it), and make ui-test's filter works, which is why the trap
is specific to this one target.

Worth checking the same shape on any other target that interpolates
UI_ARGS after a flag before closing it.

**Report** `make ui-visual-update UI_ARGS='test/components/chrome.test.ts'` re-records **every** screenshot baseline in the repo, not the one file named. The filter is silently ignored, and so is a second one: ``` $ make ui-visual-update UI_ARGS="test/components/chrome.test.ts" Test Files 99 passed (99) Tests 1092 passed (1092) $ make ui-visual-update UI_ARGS="test/components/chrome.test.ts test/components/now-playing.test.ts" Test Files 1 passed (1) # now-playing only; chrome never ran Tests 22 passed (22) ``` **Cause** The target is `npx vitest run --update $(UI_ARGS)`, and vitest's `--update` takes the following positional as its *value* rather than being parsed as a boolean. So the first path is eaten by the flag and whatever is left becomes the filter — one path means no filter at all and the whole suite is re-recorded; two means only the second file runs. Measured directly, from `frontend/`: ``` $ npx vitest run --update test/components/track-info.test.ts Test Files 99 passed (99) $ npx vitest run --update=true test/components/track-info.test.ts Test Files 1 passed (1) ``` **Why it matters** This is exactly the hazard #196 was filed about, in the tool meant to resolve it. #196's rule is "refresh the reference your change moved, having read the image, and never one you did not cause" — and the documented way to refresh one blesses all ten in silence. A baseline that is stale for an unrelated reason is re-recorded as correct, and the diff shows a changed PNG that nobody asked about. It bit this session: the intent was to re-record two references and the run re-recorded the set. Nothing was lost, because the other eight happened to be byte-identical and `git status` said so — which is the only reason it was noticed at all. **Direction** `--update=true` in the `ui-visual-update` recipe. `make ui-visual` itself is unaffected (its `$(UI_ARGS)` follows `run`, with no flag to swallow it), and `make ui-test`'s filter works, which is why the trap is specific to this one target. Worth checking the same shape on any other target that interpolates `UI_ARGS` after a flag before closing it.
logan added the Kind/Bug
Reviewed
Confirmed
1
Priority
Medium
3
 labels 2026-08-23 07:41:46 +00:00
logan self-assigned this 2026-08-23 08:31:23 +00:00
logan added the
Status
In Progress
 label 2026-08-23 08:31:23 +00:00
logan commented 2026-08-23 08:31:23 +00:00
Author
Collaborator
Copy Link
Copy Source

Picking this up. Branch fix/204-ui-visual-update-filter.

Taking the Direction as written: --update=true in ui-visual-update,
so the flag stops swallowing the first positional and UI_ARGS is a
filter again. Will also sweep the other targets that interpolate args
after a flag, which is the issue's closing note.

Picking this up. Branch `fix/204-ui-visual-update-filter`. Taking the Direction as written: `--update=true` in `ui-visual-update`, so the flag stops swallowing the first positional and `UI_ARGS` is a filter again. Will also sweep the other targets that interpolate args after a flag, which is the issue's closing note.
logan commented 2026-08-23 08:49:34 +00:00
Author
Collaborator
Copy Link
Copy Source

PR #206 — #206

--update=true in the ui-visual-update recipe, which is the
Direction as written. The issue's closing sweep is done too: every
other place a variable follows a flag (--seed, --fresh, --label,
--range, E2E_ARGS, and UI_ARGS's two other call sites) is either
a value flag that always carries a value or a hand-rolled case
parser, so the trap is specific to this one recipe.

Measured before and after with vitest list --filesOnly, which honours
the same filter: bare --update <path> selects 99 files,
--update=true <path> selects 1; --update a b selects only b
while --update=true a b selects both. make ui-visual-update UI_ARGS='test/components/transport.test.ts' then ran 1 file / 40 tests
with a clean git status afterwards, and the flag was checked to still
update: with a deliberately wrong image in seek-bar's baseline slot
that command passes and rewrites it where make ui-visual fails. The
committed baseline was restored byte-for-byte.

CI green (check and e2e). Not merged.

One note for whoever merges this and #196: that branch's new
references/ui-tier.md carries an "Until #204 lands …" bullet
describing the workaround, and whichever lands second should trim it to
the rule.

PR #206 — https://git.ljones.me/yonlu/yellowjacket/pulls/206 `--update=true` in the `ui-visual-update` recipe, which is the Direction as written. The issue's closing sweep is done too: every other place a variable follows a flag (`--seed`, `--fresh`, `--label`, `--range`, `E2E_ARGS`, and `UI_ARGS`'s two other call sites) is either a value flag that always carries a value or a hand-rolled `case` parser, so the trap is specific to this one recipe. Measured before and after with `vitest list --filesOnly`, which honours the same filter: bare `--update <path>` selects **99 files**, `--update=true <path>` selects **1**; `--update a b` selects only `b` while `--update=true a b` selects both. `make ui-visual-update UI_ARGS='test/components/transport.test.ts'` then ran 1 file / 40 tests with a clean `git status` afterwards, and the flag was checked to still *update*: with a deliberately wrong image in `seek-bar`'s baseline slot that command passes and rewrites it where `make ui-visual` fails. The committed baseline was restored byte-for-byte. CI green (`check` and `e2e`). Not merged. One note for whoever merges this and #196: that branch's new `references/ui-tier.md` carries an "Until #204 lands …" bullet describing the workaround, and whichever lands second should trim it to the rule.
logan closed this issue 2026-08-25 16:37:53 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-25 16:38:07 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Kind/Bug
Priority
Medium
3
Reviewed
Confirmed
1
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees logan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#204