test(magecommand): synthetic di verify fixtures for the omitted-artifact shapes - #114
Merged
Merged
Conversation
…-ASCII js phrases
Both found by running `static deploy` against real shops and diffing the result
against `setup:static-content:deploy`.
**`getExcludes` plugin ordering.** `min_resolver_excludes` assumed every plugin
on `Minification::getExcludes` was an `around` that appends after `$proceed`
returns, and reversed the appends unconditionally. `after` plugins are the
common shape and apply in ASCENDING execution order, so on a store with two of
them the excludes came out swapped. That is not cosmetic:
`requirejs-min-resolver.js` embeds the list, and every JS bundle embeds that
file, so one wrong order changed the resolver plus a bundle per locale. Now
bucketed by interception kind — the around chain unwinds innermost-first, then
every `after` applies in order — decided from the plugin's own source.
**Non-ASCII phrases were dropped from `js-translation.json`.**
`glue_concatenations` walked the content as bytes and did `out.push(c as char)`,
so each byte of a multi-byte sequence became its own code point:
`You\u{2019}re` came out as `You\u{e2}\u{80}\u{99}re`. The mangled phrase matches
no dictionary key, so it is silently omitted from the deployed translation file.
Two admin strings vanished on the store that surfaced it, and structurally any
source phrase containing a curly quote, arrow or accent — in any locale. Bytes
are now collected and decoded once; only whole ASCII runs are ever removed, so
the result stays valid UTF-8.
Verified against a real `setup:static-content:deploy` on seven shops. On the
backend-only one the two fixes take it from 9079/9102 identical to 9100/9103
with nothing missing or extra; `requirejs-min-resolver.js` and every en_US
bundle became byte-identical.
Known remaining divergences, all characterised and none of them these two: css
`url()` is not relocated through the theme fallback (images resolve to a
different area/theme); css minification is unimplemented, so a store with
`dev/css/minify_files=1` gets `X.css` where the deploy writes `X.min.css`; the
admin `styles.css` emits the same rule set in a different order; and our
minifier is documented as semantically — not byte — equivalent to cssmin, which
on one file means we preserve an 8-digit hex colour that Magento's own minifier
corrupts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…kind The rest of the static suite is gated on an external corpus and skips without it, which is why nothing caught the `after`/`around` mix-up before a real store did. This fixture is self-contained so it runs everywhere. Three cases over a synthetic root: two `after` plugins must append in execution order (the regression), two `around` plugins must append innermost-first as the chain unwinds, and a mix must put the around append before the after one. The `around` cases are the over-correction guard — "never reverse" is just the original bug pointing the other way, and it fails them. Confirmed to fail with the fix reverted: the `after` case comes out swapped, exactly as the deployed `requirejs-min-resolver.js` did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Every
di verifyregression that reached a client pipeline had one shape: an archive from a Magento that compiled every module on disk, checked against magecommand, which compiles only the enabled ones. The extra artifacts are expected, and the classifier's job is to say so.Three times it said so for only part of a component:
code/prefix — 254 files explained in one layout, 0 in the other/Interceptor.phprecognizedNone of these needed a real store to catch. They need an archive holding artifacts the output lacks, in those shapes — so the fixture compiles a synthetic root for real, snapshots that output as the archive, and adds what the older compiler would also have written. No PHP, no corpus.
Cases
The last two matter as much as the first two: a rule that explains everything is worthless.
Each positive case fails without its fix
I verified this rather than assuming it — a fixture that passes either way is worse than none:
code/prefix strip → the layout case fails;Why this suite specifically
di_compile_*fixtures already existed; there was nodi_verify_*fixture at all, and every recurring bug lived in the verify classifier. That was the gap.🤖 Generated with Claude Code