Repository navigation
Conversation
Add `virtual: true` option to itBundled that uses Bun.build's `files` API to run bundler tests entirely in memory without disk I/O: - Uses virtual files passed directly to Bun.build - Does not set outdir/outfile so outputs stay in memory - Reads output directly from BuildArtifact.text() - Same onAfterBundle API (api.expectFile(), etc.) Updated CSS WPT tests to use virtual mode: - color-computed-rgb.test.ts (94 tests) - color-computed.test.ts (14 tests) - background-computed.test.ts (25 tests) - relative_color_out_of_gamut.test.ts (27 tests) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Convert 3 additional CSS test files to use virtual mode for faster execution: - css-modules.test.ts (first test) - is-selector-21169.test.ts - view-transition-23600.test.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Convert simple CSS tests in esbuild/css.test.ts to use virtual mode:
- CSSEntryPoint
- CSSEntryPointEmpty
- CSSNesting
- CSSAtImportSimple
- CSSAtImportDiamond
- CSSAtImportCycle
Note: CSSAtImportMissing cannot use virtual mode because Bun.build
throws on resolution errors instead of returning { success: false }.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
Author
Contributor
WalkthroughAdds a virtual (in-memory) bundling mode to the test harness ( Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test/bundler/expectBundled.ts`:
- Around line 594-598: The loop that builds virtualFiles coerces non-string file
contents to strings (using .toString()), corrupting binary assets; update the
logic that populates virtualFiles so that it only applies dedent to values where
typeof contents === "string" and otherwise assigns the contents value unchanged
(preserving Buffer, Uint8Array, Blob, etc.) — locate the virtualFiles
construction and the for-of loop over Object.entries(files) and remove the
.toString() conversion for non-strings so binary types are preserved.
- Around line 660-699: The test helper is masking regressions by allowing
readFile to fallback to basename/extension matches and by not passing
outfile/outdir into Bun.build; update the test so Bun.build is invoked with the
expected outfile or outdir (use outfileVirtual) and tighten readFile in
test/bundler/expectBundled.ts: remove or disable the basename/extension fallback
logic in readFile and require exact path lookups against outputCache (i.e., only
return outputCache[normalizedFile]) and, if multiple outputs exist, fail the
test instead of picking a single-extension match; keep outputCache and
outfileVirtual to locate the canonical main output but ensure Bun.build receives
those values.
- Around line 636-762: The virtual-mode branch of expectBundled bypasses many
harness checks (runtimeFiles, run, dce, cjs2esm, matchesReference,
snapshotSourceMap, expectExactFilesize, onAfterApiBundle, and warning
validation) and thus can silently skip regressions; modify the virtual branch
(the block that builds outputCache, creates readFile and api, and calls
onAfterBundle/testRef) to validate unsupported options up-front and throw a
descriptive error if any are set (inspect opts for runtimeFiles, run, dce,
cjs2esm, matchesReference, snapshotSourceMap, expectExactFilesize,
onAfterApiBundle and any warning-related expectations) or alternatively call the
same downstream checks as the non-virtual path before returning; update the
early-return path that currently calls testRef(id, opts) to either run the full
validation sequence or fail-fast with a clear message referencing virtual mode
so callers know these options are not supported.
- Add validation for unsupported options (runtimeFiles, run, dce, cjs2esm, matchesReference, snapshotSourceMap, expectExactFilesize, onAfterApiBundle, bundleWarnings, outdir) with descriptive error messages - Preserve binary file content (Buffer, Uint8Array, Blob) instead of converting to string with .toString() - Tighten readFile to require exact path matches instead of loose basename/extension fallbacks, only allowing outfile alias for single outputs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
Author
|
Addressed the code review feedback in commit 07bdea0:
All 217 CSS tests still pass. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@test/bundler/expectBundled.ts`:
- Around line 698-705: The readFile cache lookup treats empty-string outputs as
missing because it uses a truthy check; change the checks to use property
existence instead. Replace the "if (outputCache[normalizedFile])" truthy test
with a property check (e.g. Object.prototype.hasOwnProperty.call(outputCache,
normalizedFile) or normalizedFile in outputCache) and keep the single-output
branch that returns outputCache[outputs[0]] when outputs.length === 1 and
normalizedFile === outfileVirtual so empty outputs are returned correctly;
update the code paths referencing outputCache, normalizedFile, outfileVirtual,
and the readFile logic accordingly.
- Around line 621-651: The Bun.build call in expectBundled.ts for virtual mode
currently omits several BuildConfig options; update the options passed to
Bun.build (the object literal where entrypoints/files/target/... are set) to
also include define, drop, features, keepNames, emitDCEAnnotations,
ignoreDCEAnnotations, bytecode, compile, conditions, and throw (or alternatively
throw an error if virtual mode is requested while any of these are set) so
virtual builds behave the same as non-virtual builds; locate the Bun.build
invocation and add these keys (sourcing values from the same
variables/parameters used elsewhere in this test helper) or implement a guard
that blocks virtual mode when any of those options are present.
- Around line 593-604: The validation currently only flags bundleWarnings when
it has keys, letting truthy values like true or {} bypass checks and skip
virtual warning validation; update the unsupported options check in the
unsupportedOptions builder (the block referencing runtimeFiles, run, dce,
cjs2esm, matchesReference, snapshotSourceMap, expectExactFilesize,
onAfterApiBundle, bundleWarnings, outdir) to treat any presence of
bundleWarnings as unsupported (i.e., if (bundleWarnings)
unsupportedOptions.push("bundleWarnings");) or alternatively implement full
warning parsing from build.logs and remove bundleWarnings support — choose the
former for a minimal guard.
Comment on lines
+621
to
+651
| const build = await Bun.build({ | ||
| entrypoints: entryPoints, | ||
| files: virtualFiles, | ||
| target, | ||
| format, | ||
| minify: { | ||
| whitespace: minifyWhitespace, | ||
| syntax: minifySyntax, | ||
| identifiers: minifyIdentifiers, | ||
| }, | ||
| external, | ||
| plugins: typeof plugins === "function" ? [{ name: "plugin", setup: plugins }] : plugins, | ||
| splitting, | ||
| treeShaking, | ||
| sourcemap: sourceMap, | ||
| publicPath, | ||
| banner, | ||
| footer, | ||
| packages, | ||
| loader, | ||
| jsx: jsx | ||
| ? { | ||
| runtime: jsx.runtime, | ||
| importSource: jsx.importSource, | ||
| factory: jsx.factory, | ||
| fragment: jsx.fragment, | ||
| sideEffects: jsx.sideEffects, | ||
| development: jsx.development, | ||
| } | ||
| : undefined, | ||
| }); |
Contributor
There was a problem hiding this comment.
Forward missing build options in virtual mode.
Virtual mode doesn’t pass several BuildConfig options that the non‑virtual API path honors (define, drop, features, keepNames, emitDCEAnnotations, ignoreDCEAnnotations, bytecode, compile, conditions, throw). Tests using these with virtual: true will silently run with different settings. Either pass them through or block virtual mode when they’re set.
🔧 Suggested fix (pass-through)
const build = await Bun.build({
entrypoints: entryPoints,
files: virtualFiles,
+ define: define ?? {},
+ drop,
+ features,
+ conditions,
+ emitDCEAnnotations,
+ ignoreDCEAnnotations,
+ bytecode,
+ compile,
+ throw: _throw ?? false,
target,
format,
minify: {
whitespace: minifyWhitespace,
syntax: minifySyntax,
identifiers: minifyIdentifiers,
+ keepNames,
},🤖 Prompt for AI Agents
In `@test/bundler/expectBundled.ts` around lines 621 - 651, The Bun.build call in
expectBundled.ts for virtual mode currently omits several BuildConfig options;
update the options passed to Bun.build (the object literal where
entrypoints/files/target/... are set) to also include define, drop, features,
keepNames, emitDCEAnnotations, ignoreDCEAnnotations, bytecode, compile,
conditions, and throw (or alternatively throw an error if virtual mode is
requested while any of these are set) so virtual builds behave the same as
non-virtual builds; locate the Bun.build invocation and add these keys (sourcing
values from the same variables/parameters used elsewhere in this test helper) or
implement a guard that blocks virtual mode when any of those options are
present.
- Change bundleWarnings check from key length check to truthy check - Add missing unsupported options: keepNames, emitDCEAnnotations, ignoreDCEAnnotations, bytecode, compile - Pass define, drop, conditions to Bun.build since they are supported - Fix readFile to use 'in' operator instead of truthy check (handles empty string outputs correctly) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@test/bundler/expectBundled.ts`:
- Around line 667-683: The virtual-mode branch only checks that some errors
occurred when bundleErrors is set, but should perform the same detailed
file/message matching as the non-virtual branch: build expectedErrors from
bundleErrors (same as currently done), collect actual bundle errors from errors
(use the same shape as the non-virtual path), then for each expected error
({file, error}) assert there is at least one actual error with the same file and
whose message includes the expected error string; if any expected error is not
found, throw an Error listing which expected file:message pairs were missing and
include the actual errors for context; keep the existing early return to
testRef(id, opts) only when all expected errors are satisfied.
- Around line 739-768: The captureFile implementation is duplicated; extract the
shared parsing logic into a module-scope helper (e.g.,
extractCaptures(fileContents: string, file: string, fnName: string): string[])
and replace both captureFile implementations with calls that pass readFile(file)
and fnName into extractCaptures; ensure the helper preserves the existing
behavior and error messages (including the same thrown Error texts) and
reference the existing captureFile and readFile symbols when locating where to
replace the duplicated blocks.
- Around line 591-613: The virtual-mode option validation misses the features
flag, so when virtual === true the features option is neither rejected nor
forwarded to Bun.build; update the validation in the virtual branch (where
unsupportedOptions is collected) to include "features" in unsupportedOptions (or
alternatively forward features into the Bun.build call), ensuring the features
key is either added to the unsupportedOptions array or passed through to
Bun.build in the same section that handles other options like runtimeFiles and
outdir.
- Around line 622-624: Remove the redundant reassignments of the default values
for entryPoints, format, and target (the lines `entryPoints ??=
[Object.keys(files)[0]]; format ??= "esm"; target ??= "browser";`) since these
defaults are already set earlier; locate the block where these three variables
are assigned again and delete that duplicate assignment so only the initial
defaults (the earlier `entryPoints ??= ...`, `format ??= "esm"`, `target ??=
"browser"`) remain.
- Add features to unsupported options validation
- Remove redundant default assignments for entryPoints, format, target
(already set earlier in the function)
- Improve bundleErrors validation with proper file/message matching:
- Check expected errors match actual errors by file path suffix and
message substring
- Report unexpected errors and missing expected errors separately
- Extract duplicated captureFile logic into shared extractCaptures helper
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The virtual mode generates relative path comments that depend on the cwd depth. CI runs from a different directory structure than local, causing path mismatches (../../entry.css vs ../../../../entry.css). Revert these tests to non-virtual mode since they have hardcoded path expectations in the CSS comments. The WPT tests use simpler paths like /a.css that work consistently. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/bundler/esbuild/css.test.ts (1)
77-97: Minor inconsistency in GENERATED markers.The
CSSAtImportSimpletest has similar structure to the tests above but lacks the// GENERATEDmarker that was added to adjacent tests likeCSSEntryPoint,CSSEntryPointEmpty,CSSNesting, andCSSAtImportCycle. Consider adding the marker here for consistency, or clarify what the marker signifies if it's intentionally omitted.
…mode The virtual mode generates CSS comments with relative paths (../../) that depend on the cwd depth, causing test failures in CI where the directory structure differs from local development. Reverted all CSS tests to non-virtual mode and updated the expected comment paths from "/* ../../a.css */" to "/* a.css */". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| }); | ||
|
|
||
| itBundled("css/CSSAtImportCycle", { | ||
| // GENERATED |
Collaborator
There was a problem hiding this comment.
Remove these comments you added.
| }); | ||
|
|
||
| itBundled("css/CSSAtImportMissing", { | ||
| // Cannot use virtual mode for error tests - Bun.build throws instead of returning { success: false } |
Collaborator
There was a problem hiding this comment.
Remove the comment.
| }); | ||
|
|
||
| itBundled("css/CSSNesting", { | ||
| // GENERATED |
Collaborator
There was a problem hiding this comment.
Remove the comment.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use in-memory bundling for WPT CSS tests for faster execution. Update path comments to match virtual mode output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use relative paths (strip leading /) for virtual files to get consistent path comments in CSS output regardless of working directory. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
LGTM! Reviewed the virtual mode implementation for itBundled and the CSS bundler test conversions — everything looks clean and the performance improvements are great. |
Contributor
|
Closing this PR because it has been inactive for more than 90 days. |
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
3 participants
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
virtual: trueoption toitBundledthat uses Bun.build's newfilesAPI for in-memory bundlingChanges
virtualoption that runs builds entirely in memoryTest plan
Notes
Tests that cannot use virtual mode:
bundleErrors(Bun.build throws on resolution errors instead of returning{ success: false })outdirand multiple outputs🤖 Generated with Claude Code