Repository navigation
[compiler-conformance] add implementation-neutral golden fixtures - #2011
Open
imdreamrunner wants to merge 7 commits into
Open
imdreamrunner wants to merge 7 commits into
imdreamrunner wants to merge 7 commits into
Conversation
Extracted from #1623 (2f2281b) without the Rust/SWC plugin, so the behavioral contract can land independently of the Rust implementation. packages/compiler-conformance holds eight fixtures as plain data: a manifest saying how to compile the entry file, and a recorded result pinning the generated JavaScript, StyleX metadata, generated CSS and the diagnostics. Nothing in it is Babel-specific, so a compiler written in any language can read the fixtures and check itself against the same contract. The Babel plugin is the first consumer, through an adapter that keeps parser options, console capture and rule processing out of the shared package. Generated JavaScript is compared as a normalized syntax tree, so how an implementation prints a program is not part of the contract. Metadata, CSS and diagnostics are compared exactly, after absolute paths are tokenized and implementation-specific diagnostic decoration is removed. define-vars-theme exercises a successful defineVars/createTheme transform. In #1623 it expected a module-resolution error, which pinned a misconfiguration rather than behavior; failed transforms are still covered by validation-error.
…omparing Babel's code frame syntax-highlights whenever it believes the environment supports color, and `@babel/code-frame` treats a set `CI` variable as exactly that. So on GitHub Actions the validation-error fixture's message arrived wrapped in SGR escapes, the code-frame matcher no longer recognized the excerpt lines it was meant to drop, and the whole frame was compared against a recorded message that has none. The same test passes on a developer's machine, where nothing colorizes. Color is presentation, never contract, so normalizeDiagnostic now removes escape sequences before it does anything else -- which also keeps them from hiding an absolute path from the path tokenizer.
Avoid semantic false positives around __proto__, malformed adapter results, diagnostic prefixes, code-frame detection, and path normalization. Keep fixture defaults isolated and parse identical JavaScript before declaring it equivalent.
…vers `rewrite-theme-extension` imports `./tokens.stylex.js` by relative path, so it only pins the last step of `rewriteAliases`: the resolved `.stylex.js` specifier is re-emitted without its `.js` extension. It never goes through an `aliases` entry, which is the option's main job. An alias cannot be expressed here honestly. The documented `aliases` option maps to absolute paths, which fixtures avoid because output must not depend on the checkout location. A relative alias target such as `"@tokens/*": ["./*"]` does work in Babel, but only because `moduleResolve` happens to resolve it against the importing file; that is undocumented, and a second implementation resolving it against the project root would be just as reasonable. Pinning it would turn an accident into contract. So the fixture is renamed to `strip-stylex-import-extension`, and its description now says what it covers and what it leaves out. Its `unstable_moduleResolution: commonJS` option is dropped: the rewrite code never reads it, and the recorded output is unchanged without it (a full `STYLEX_UPDATE_GOLDEN=1` run rewrites no expected.json). Deleting the extension strip from the plugin still fails the fixture.
Two ways a fixture can go wrong without any test noticing: - `loadFixture` ignores manifest fields it does not know. A misspelled `pluginOption` compiles the fixture with default options, and re-recording writes that default output as the expectation, which the golden test then happily matches. The same happens with `"syntax": "jsx"` written as a string: it becomes the parser plugins `j`, `s`, `x`, and the recorded golden turns into a JSX parse error. Both were reproduced: in each case the re-recorded golden test passed 8/8. - `readExpected` normalizes the stored file before comparing it. A hand edit that normalization undoes (unsorted keys, trailing CSS whitespace, an absolute path or code frame in a diagnostic) keeps the golden test green while expected.json drifts from what `writeExpected` records, and from what an implementation reading the file directly compares against. Each fixture now gets two compiler-independent checks: its manifest uses only fields from the README schema, has a non-empty description, gives `syntax` as an array of strings, and names an entry that exists; and its expected.json, serialized, equals its own normalization. The old "ships an entry file and a recorded result" test is folded in: the entry check moves over and the expected.json read replaces the existence check. The README now says a description is required, unknown fields are rejected, and goldens are stored normalized, so a port only normalizes its own output. Validation lives in the test rather than in `loadFixture` because the fixtures are this repository's data: they are checked once here, and every consumer reads already-validated files.
Each harness test was checked against the mutation it is meant to catch.
Fixed:
- "still reports a computed key" compared `{a: 1}` with `{[a]: 1}`, which
differ in their `computed` flag no matter what. Deleting the
`computed === false` guard from key canonicalization left it green. The
pair that guard protects is `{[a]: 1}` versus `{["a"]: 1}`: without it both
keys reduce to the name "a" and compare equal although one names whatever
`a` holds. The test now uses that pair and fails under the mutation.
Removed, because a kept test fails under every mutation they catch:
- "[1, 2] is not a valid value." is covered by the `[E100]` case: collapsing
any bracketed prefix fails both, and collapsing any space-free one fails
only `[E100]`.
- "|start| is reserved." is covered by the "| string" alternatives case:
making the code-frame line number optional fails only the alternatives
case, and treating any pipe-led line as a frame fails both.
- "a missing status" is covered by "an unknown status": rejecting only a
missing status, or only a non-string one, fails just the unknown case.
- "fails loudly on unparseable output" (one side valid): swallowing the
parse error fails the kept identical-sources test too, and in the golden
test differing sources fall back to a text comparison that fails anyway.
The identical case is the one that matters, since invalid output that was
recorded would otherwise match itself forever.
Removed, because no reachable regression depends on them:
- "reports a difference when only one side produced output": one side has
no JavaScript only when the statuses differ, and the exact comparison
fails on status first. Making `isJsEquivalent` return true for a missing
side and flipping a golden to an error still fails the golden test.
- `jsPart(failed)` being `null`: returning `undefined` instead changes
nothing, since `isJsEquivalent(undefined, undefined)` is true as well.
Kept, now with a comment naming what they guard: `exactPart` drops `js`
and `jsPart` returns the code. With `jsPart` returning null, or `exactPart`
keeping `js`, Babel's golden test still passes 8/8; only these assertions
fail.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
meta-cla
Bot
added
the
CLA Signed
2 tasks done
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
workflow: benchmarks/perfComparison of performance test results, measured in operations per second. Larger is better.
|
workflow: benchmarks/sizeComparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.
|
…regexes CodeQL (js/polynomial-redos) flagged two end-anchored patterns in normalize.js: `/[\\/]+$/` when stripping a path root's trailing separators, and `/\s+$/` when stripping trailing whitespace from each CSS line. The regex engine retries such a pattern from every character of a run, so a long run of separators or spaces that is not at the end of the string takes quadratic time. A 100,000-character run followed by one other character took about 5 seconds for each pattern. The separator strip is now a loop from the end, and the CSS line strip uses `String.prototype.trimEnd`, which removes exactly the whitespace and line terminators that `\s` matches. Both are linear. Over 200,000 random strings mixing separators, ASCII and Unicode whitespace, BOM and other characters, the old and new forms returned identical results, so normalization output is unchanged.
This branch has not been deployed
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.
What changed / motivation ?
Extracted the implementation-neutral golden test suite from #1623 (commit
2f2281bcc9adae76929c2c7a32a5e03e2aa45640) so the behavioral contract can landindependently of the Rust implementation.
Nothing here adds or depends on the Rust/SWC plugin. No changes to
packages/@stylexjs/swc-plugin, no Rust configuration or dependencies, no RustCI jobs, and none of the unrelated tests from that PR.
The shared suite
packages/compiler-conformanceis a new private workspace package holding eightgolden fixtures as plain data. Each fixture is a directory with:
manifest.json— how to compile the entry (entry,syntax,pluginOptions,processOptions)expected.json— the recorded resultThe recorded result pins all four observable outputs: generated JavaScript,
StyleX metadata, generated CSS, and diagnostics. Nothing in the package is
Babel-specific, so a compiler written in any language can read the fixtures and
check itself against the same contract.
README.mddocuments the schema andwhat an adapter has to provide.
Babel as the first consumer
packages/@stylexjs/babel-plugin/test-utils/babel-conformance-adapter.jscompiles one fixture and returns the result in the shared shape. Parser
configuration, console capture and
processStylexRulesall stay here.packages/@stylexjs/babel-plugin/__tests__/golden-fixtures-test.jsruns everyfixture through that adapter.
Re-record with
STYLEX_UPDATE_GOLDEN=1 npx jest golden-fixturesfrom thebabel-plugin package.
Coverage
create-basicstylex.createdefine-vars-themedefineVars+createTheme, incl. aprefers-color-schemeoverridekeyframes-and-whenkeyframesandwhen.ancestoroptions-warningprops-and-mergepropswith merged styles, in JSXstrip-stylex-import-extensionrewriteAliasesdropping.jsfrom a relative.stylex.jsimportruntime-injectionruntimeInjectionvalidation-errorBetween them: generated JavaScript, metadata, CSS, warnings, errors, and both
successful and failed transforms.
How results are compared
Generated JavaScript is compared semantically, as a normalized syntax tree
(
@babel/parser; positions, comments and raw literal text stripped, keyssorted). Whitespace, quote style, semicolons, comments, how a non-computed
property key is spelled, and property shorthand (except the semantic
__proto__case) are all ignored, so no implementation has to match another'sprinter. When the programs genuinely
differ, the test falls back to asserting on the sources so the failure shows a
readable diff.
Metadata, CSS, diagnostics and status are compared exactly, after
deterministic normalization: line endings normalized, trailing whitespace
stripped from each CSS line, absolute paths replaced with
<FIXTURE_ROOT>/<REPO_ROOT>, and — in diagnostics — the location prefix and numberedsource excerpt dropped and the adapter's declared leading implementation tag
collapsed to
[stylex].Object keys are sorted; array order is preserved, because rule order is part of
the contract.
These rules are themselves tested, without involving any compiler, in
packages/compiler-conformance/__tests__/conformance-harness-test.js. The samefile also checks every fixture: its manifest may set only the fields in the
README schema (so a misspelled
pluginOptionfails instead of silently compilingwith defaults), and its
expected.jsonmust already be stored normalized (so ahand edit that normalization would undo can't drift while the golden test stays
green). Each harness test was checked by breaking the code it guards and
confirming the test fails.
define-vars-themeIn #1623 this fixture recorded a module-resolution error, which pinned a
misconfiguration rather than any behavior. It now exercises a successful
defineVars/createThemetransform: the entry is a.stylex.jsfile and thefixture sets
unstable_moduleResolution: {"type": "commonJS"}. Failed-transformcoverage is still provided by
validation-error.The canonical name
commonJSresolution derives for it —compiler-conformance:fixtures/define-vars-theme/input.stylex.js— is hashedinto the generated variables. It is stable across checkouts (verified by copying
the package elsewhere and recompiling), but changes if the fixture moves or the
package is renamed. The README says so.
Linked PR/Issues
Extracted from #1623.
Supersedes #1829: same commits, moved from a fork to a branch in this repository so the benchmark jobs can post their results.
Additional Context
On the recorded output. The
expected.jsonfiles are regenerated fromcurrent
mainrather than copied from #1623. The ones in that PR look stale —their recorded
jsis missing the copyright header that their owninput.jsfiles carry. Regenerating avoids importing a stale baseline; it changes nothing
about the comparison, since the AST normalizer ignores comments.
Validation, run locally on Node 24:
yarn build— passyarn test:packages— pass, including the 8 golden fixtures and the 49harness tests
yarn flow check— passyarn prettier:report— passyarn lint:report— passyarn.lockis unchanged:@babel/parser@^7.29.0resolves to an entry thelockfile already has.
Pre-flight checklist
Contribution Guidelines