Review and complete the examples/test chart fixtures - #189
Open
nulltask wants to merge 3 commits into
Open
Conversation
`benchmark-command-mix-100.bms` parsed to zero events. It opened `#SETRANDOM 2` and `#RANDOM 2` back to back, and since both push a control-flow scope while only one `#ENDRANDOM` pops one, the capture stack never drained and all 100 measures of object data were swallowed into the trailing branch. Resolving the control flow recovered 3 events out of ~600. The same mistake repeated for `#SETSWITCH 3` / `#SWITCH 3` and the second `#SETRANDOM 3` / `#RANDOM 3`, so drop the three redundant scope openers and keep the deterministic `#SET*` form. `sample.roundtrip.bms` was BMSON JSON under a `.bms` name, byte-identical to `sample.roundtrip.bmson`. Delete it. `sample.json`, `sample.edit.json` and `sjis-encoding-test.json` still carried a pre-`bms`/`bmson`-extension IR shape that no current code path can produce, and `sample.roundtrip.bmson` put `resolution` at the document root instead of inside `info` and omitted `lines`. Regenerate all four through the editor API. `lane-order-test.bms` and `sjis-encoding-test.bms` declared `#WAV01 test.wav`, which does not exist in this directory, so neither chart was playable. Point both at `sample.wav`.
…ixtures 19 of the 36 files in `examples/test` were read by no test at all, which is how a benchmark chart that parses to zero events and a stale round-trip pair went unnoticed. Add `packages/parser/src/fixtures.test.ts` as the corpus-level guard: it scans the directory, parses every chart off disk, and requires each one to yield playable events after control-flow resolution, so a new fixture is picked up automatically and a broken one fails a test. New fixtures, one per feature cluster that had none: - `extended-directives.bms` — `#EXWAV` / `#EXBMP` / `#ARGB` / `#BGA` / `#SWBGA` / `#WAVCMD` / `#EXRANK` / `#CHANGEOPTION` with payloads the `@be-music/chart` value parsers accept. `four-measure-command-combo-test.bms` proves the parser routes these headers, but its payloads are placeholders (`#EXWAV01 ex_sample.wav`, `#BGA01 01`, `#WAVCMD legacy`) that every value parser rejects, so nothing covered a realistic payload off a real file. - `scroll-speed-gimmick.bms` — one measure per timing / drawing gimmick, including the `SP` channel and the `#SCROLL 0` / `#SCROLL < 0` cases the player has an explicit compatibility policy for. - `bus-volume-channels.bms` — the `97` / `98` dynamic volume buses. - `base62-ids.bms` — `#BASE 62` case-distinct IDs in indexed headers, channel streams, `#LNOBJ`, and inside a `#RANDOM` / `#IF` block. - `invisible-notes.bms` — `3x` / `4x` invisible keysound channels, and the guarantee that 2P-side invisible channels do not turn a single-player chart into a DP chart. - `lntype2-long-note.bms` — `#LNTYPE 2` continuous runs, including two runs split by a gap and a run crossing a measure boundary. - `nonpositive-bpm.bms` — channel `08` pointed at zero, negative and undefined `#BPMxx` slots. - `5key-sp.bms`, `10key-dp.bms`, `7key-longnote.bml` — the lane-mode branches that had no fixture. Together with the existing charts this covers every `ChartPlayVariant` the resolver can return, and gives the `.bml` extension allow-lists a real file to open. - `bmson-key-channels-mines.bmson` — beatoraja `key_channels` mines, `info.ln_type` and `notes[].t`. Move the inline chart text in three tests onto these fixtures (`#SPEED` / `SP`, `#BASE 62`, bmson `key_channels`). Inline inputs stay inline where a file would hide the assertion: malformed-input probes, charset byte sequences, and table-driven micro-cases such as the 20 `resolveChartPlayVariant` channel lists. Wire up the fixtures that existed but were orphaned: the `sample.*` quintet now backs a live import → edit → export round trip in the editor, and the retrigger and ogg charts are rendered through the audio renderer instead of being described by hand-built IR.
A directory of 40-plus fixtures with no index is what let the broken and stale ones sit there unnoticed. Document what every chart covers, which tests read the round-trip sets, and the conventions a new fixture should follow.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
View logs |
be-music-player-demo | 48e9b49 | Aug 23 2026, 04:37 PM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reviewed every fixture in
examples/test, fixed the broken ones, added charts for the feature clusters that had none, and made the unit tests read charts from this directory instead of building them inline.19 of the 36 files there were read by no test, script or doc. That is what let real breakage sit unnoticed.
Fixtures that were broken
benchmark-command-mix-100.bmsparsed to zero events. It opened#SETRANDOM 2and#RANDOM 2back to back. Both push a control-flow scope but only one#ENDRANDOMpops one, so the capture stack never drained and all 100 measures of object data were swallowed into the trailing branch — resolving the control flow recovered 3 events out of ~600. The same mistake repeated twice more. Fixed by dropping the redundant scope openers.sample.roundtrip.bmswas BMSON JSON under a.bmsname, byte-identical tosample.roundtrip.bmson. Deleted.sample.json,sample.edit.json,sjis-encoding-test.jsonwere stale — they carried a pre-bms/bmson-extension IR shape no current code path can produce.sample.roundtrip.bmsonputresolutionat the document root instead of insideinfoand omittedlines. All four regenerated through the editor API.lane-order-test.bmsandsjis-encoding-test.bmsdeclared#WAV01 test.wav, which does not exist here, so neither chart was playable. Repointed atsample.wav.Also worth knowing:
four-measure-command-combo-test.bmsdeclares#EXWAV01 ex_sample.wav,#EXBMP01 ex_image.bmp,#BGA01 01,#SWBGA01 02and#WAVCMD legacy— every one of those is rejected by its own value parser in@be-music/chart. That chart's job is proving the parser routes the headers, and it does that well, so it is left alone; the newextended-directives.bmscovers valid payloads instead.New fixtures
One per feature cluster that had none:
extended-directives.bms#EXWAV/#EXBMP/#ARGB/#BGA/#SWBGA/#WAVCMD/#EXRANK/#CHANGEOPTIONwith payloads the value parsers acceptscroll-speed-gimmick.bmsSPchannel and the#SCROLL 0/#SCROLL < 0cases the player has a documented compatibility policy forbus-volume-channels.bms97/98dynamic volume busesbase62-ids.bms#BASE 62case-distinct IDs in indexed headers, channel streams,#LNOBJ, and inside a#RANDOM/#IFblockinvisible-notes.bms3x/4xinvisible keysound channels, and that 2P-side invisible channels do not make a single-player chart a DP chartlntype2-long-note.bms#LNTYPE 2continuous runs, including two runs split by a gap and one crossing a measure boundarynonpositive-bpm.bms08pointed at zero, negative and undefined#BPMxxslots5key-sp.bms,10key-dp.bms,7key-longnote.bmlChartPlayVariant, and gives the.bmlallow-lists a real file to openbmson-key-channels-mines.bmsonkey_channelsmines,info.ln_type,notes[].tTests
packages/parser/src/fixtures.test.tsis the new corpus-level guard: it scans the directory, parses every chart off disk, and requires each one to yield playable events after control-flow resolution. A new fixture is picked up automatically; a broken one fails a test. This is what caught both the benchmark chart and the misnamed round-trip file.Migrated to fixtures:
#SPEED/SP,#BASE 62, and bmsonkey_channels.Deliberately left inline: malformed-input probes, charset byte sequences, and table-driven micro-cases such as the 20
resolveChartPlayVariantchannel lists — a file per case would hide the assertion. The audio renderer's retrigger tests also keep their hand-built IR, because they are calibrated numeric probes rather than realistic charts.Orphaned-but-good fixtures are now wired up: the
sample.*quintet backs a live import → edit → export round trip in the editor, and the retrigger and ogg charts are rendered through the audio renderer.examples/test/README.md/README.ja.mdindex what every chart covers and the conventions a new one should follow.Notes
ogg-test.wav(247 KB) is the uncompressed source forogg-test.oggand is read by no test. Left in place as source material and documented as such rather than deleted.Test plan
pnpm test— 1971 passed (was 1940)oxlintandoxfmt --checkclean on every changed filetsc -p tsconfig.typecheck.jsonclean inparser,editor,audio-renderer,player-tui🤖 Generated with Claude Code