test(magecommand): pin the min-exclude append order per interception kind - #115
Merged
Conversation
…kind The `after`/`around` mix-up fixed in #111 shipped without a test: the rest of the static suite is gated on an external corpus (`MAGECOMMAND_CORPUS`) and skips silently when it is absent, so CI never exercised that path and a real store found it instead. This fixture is self-contained so it runs everywhere. Modules append their JS minification excludes from a plugin on `Minification::getExcludes`, and those appends land in `requirejs-min-resolver.js` — a file every JS bundle then embeds — so their order is deployed bytes. Three cases over a synthetic root: two `after` plugins append in execution order (the regression), two `around` plugins append innermost-first as the chain unwinds, and a mix puts 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 against main: passes with the fix, and with the unconditional reverse restored the `after` case comes out swapped, exactly as the deployed resolver 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
1 participant
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.
The
after/aroundmix-up fixed in #111 shipped without a test — my fixture commit didn't make it into the squash merge. This adds it against currentmain.Why it was missed in the first place
The rest of the static suite is gated on an external corpus (
MAGECOMMAND_CORPUS) and skips silently when it is absent, so CI never exercised that path and a real store found the bug instead. This fixture is self-contained so it runs everywhere.What it pins
Modules append their JS minification excludes from a plugin on
Minification::getExcludes, and those appends land inrequirejs-min-resolver.js— a file every JS bundle embeds — so their order is deployed bytes.afterplugins append in execution order — the regression;aroundplugins append innermost-first as the chain unwinds;The
aroundcases are the over-correction guard: "never reverse" is just the original bug pointing the other way, and it fails them.Verified both directions
Passes against
mainas it stands. With the unconditional reverse restored, theaftercase comes out swapped — exactly as the deployed resolver did on the store that surfaced this.Worth deciding separately
The corpus gate is still a hole:
static_deploy,static_fulltree,static_jsbundle,static_less_fileandstatic_requirejsall no-op withoutMAGECOMMAND_CORPUS, and nothing says so in CI. Making the skip loud, or pointing CI at a corpus, would close it properly.🤖 Generated with Claude Code