test: stop jsx-production.test.ts from timing out on debug builds - #38102
test: stop jsx-production.test.ts from timing out on debug builds#38102robobun wants to merge 1 commit into
Conversation
…stub runtime Each of the 32 cases spawned a bun process that imported react-dom/server.browser, rendered, ran Bun.build() on itself with react-dom bundled in, and imported the bundle. On a debug+ASAN build that is 2-5s per spawn, so with describe.concurrent a large share of the cases hit the default 5s timeout (19 of 32 on one run here), and the file took 15-23s on CI lanes. The fixture now lives in a tempDir with a stub `react` whose jsx-runtime and jsx-dev-runtime return their own name. app.tsx imports child.tsx, prints which runtime both files were compiled against, then Bun.build()s itself with process.env.NODE_ENV defined to CHILD_NODE_ENV and executes the bundle, which prints the same line for the bundled pass. The matrix and the assertions from before are kept (a define of "production" has to bundle against jsx and "development" against jsxDEV, and the bundle has to execute); NODE_ENV=production at runtime is additionally pinned to jsx. The whole file runs in under a second on a release build and about 8s on a debug build, with no case above 1.2s.
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Status: reproduced, fixed, CI green.
|
There was a problem hiding this comment.
LGTM — test-only refactor that swaps the react-dom fixture for a stub runtime to fix debug-build timeouts, with coverage preserved.
What was reviewed:
- Assertion parity with the old fixture: the
child_node_env→jsx/jsxDEVpins and bundle-import-must-run check are kept; the dropped react-dom render produced identical output under both runtimes so wasn't load-bearing. - Deleted fixtures (
jsx-production.tsx,jsx-production-entry.ts,jsx-dev/) have no remaining references; theTSCONFIG_JSXenv var the old test set was never read anywhere. - Harness conventions:
tempDir,{...bunEnv, ...}, concurrent pipe drain viaPromise.all, stderr asserted before exit code,toBeOneOfis an existing matcher. - Stub
node_modules/react/jsx-{,dev-}runtime.jswithout a package.json matches the pattern inbundler_jsx.test.tsandbundler_edgecase.test.ts.
Extended reasoning...
Overview
This PR rewrites test/bundler/transpiler/jsx-production.test.ts to stop it timing out on debug builds. The old test spawned 32 concurrent subprocesses that each imported react-dom/server.browser, rendered, ran Bun.build() bundling react-dom, and imported the result — three passes over react-dom-sized source per case with the transpiler cache disabled. The new version writes a tempDir fixture with stub react/jsx-runtime and react/jsx-dev-runtime modules whose functions return their own name, so each case observes directly which runtime module the transpiler and bundler picked. The three fixture files and the jsx-dev/ directory are deleted; only the test file remains.
Security risks
None. Test-only change, no source code touched, no network access, hermetic tempDir fixture.
Level of scrutiny
Medium — the concern for a test rewrite is whether it silently weakens coverage. I compared the old and new assertions:
- Old:
code.includes("jsx_dev_runtime.jsxDEV")whenCHILD_NODE_ENV === "development"andcode.includes("jsx_runtime.jsx")when=== "production", plus the bundle must import without throwing, plus react-dom must render<div>Hello World</div>twice. - New:
bundlePassis pinned to["jsx"]/["jsxDEV"]for those same twochild_node_envvalues, the bundle is still imported (and now its stdout is asserted), and the react-dom render is dropped. The render produced identical HTML under either runtime, so it distinguished nothing — the PR body calls this out and it checks out.
The new test adds two assertions the old one lacked: NODE_ENV=production in the parent env pins the bun run pass to jsx, and both the entry point and an imported .tsx file must be compiled against the same runtime (exercising the separate entry-point vs. resolved-import paths in bundle_v2.rs). It also asserts stderr === "" where the old test used stderr: "inherit". Net coverage is equal or slightly stronger.
Other factors
- Grepped for references to the deleted fixture files and the
TSCONFIG_JSXenv var — none remain, so no dangling consumers. toBeOneOfis an established matcher (used in 8 other test files).- The stub-runtime-in-
node_modules/react/pattern without a package.json is already used intest/bundler/bundler_jsx.test.tsandbundler_edgecase.test.ts. - Harness conventions are followed:
tempDirwithusing,{...bunEnv, KEY: undefined}spread,Promise.all([stdout.text(), stderr.text(), exited])to avoid pipe deadlock, stdout/stderr asserted before exit code. - The PR body documents both a
bun bd testrun (32 pass, 6.6-9s) and a mutation check (removing the define fails exactly the 8 expected cases), satisfying the "prove the test fails for the right reason" bar. - No prior human reviews or outstanding comments on the PR.
|
Updated 6:45 AM PT - Aug 13th, 2026
✅ @robobun, your commit 398fe5bb56ceec1b424c47999f9b86e035406ce5 passed in 🧪 To try this PR locally: bunx bun-pr 38102That installs a local version of the PR into your bun-38102 --bun |
Problem
bun bd test test/bundler/transpiler/jsx-production.test.tsfails on a debug build withthis test timed out after 5000ms: 19 of the 32 cases on one run here, 4 on another, all 32 in the container that reported it. The file takes 37-43s.describe.concurrentblock runs the 4x4x2 matrix together, cases cross the 5s default timeout more or less at random.test/bundler/transpiler/jsx-production.tsx(or the byte-identicaljsx-dev/jsx-dev.tsx), which importsreact-dom/server.browser, renders, runsBun.build()on itself with react-dom bundled in, and imports the bundle. With the runtime transpiler cache disabled (bunEnvsetsBUN_RUNTIME_TRANSPILER_CACHE_PATH=0) that is three passes over react-dom-sized source per case: the runtime transpile, the bundler's parse, and the runtime transpile of the resulting bundle.test/expected-durations.jsonlists the file at 15.7s (default lane), 19.7s (asan) and 22.8s (windows). cli: stop --jsx-* flags from switching the automatic JSX runtime to production #36209 and Bind the JSX runtime and bun:wrap helpers with require() in CommonJS-wrapped modules #38012 both ran into these timeouts on debug builds and worked around them (a separate test file, and a note in the PR body, respectively).Fix
tempDirwith a stubreactwhosejsx-runtimeandjsx-dev-runtimereturn their own name.app.tsximportschild.tsx, prints which runtime the two files were compiled against, then runsBun.build()on itself withprocess.env.NODE_ENVdefined toCHILD_NODE_ENVand executes the bundle, which prints the same line for the bundled pass.jsx-production.tsx,jsx-production-entry.tsandjsx-dev/are deleted.test/bundler/transpiler/tsconfig.json(added alongside them) is left alone: it applies to every other file in the directory, so removing it is a separate question.NODE_ENV, adefineofprocess.env.NODE_ENV, and tsconfigjsx. The stub observes exactly that (which module's function actually ran, in both files), where the old fixture observed it indirectly: acode.includes("jsx_runtime.jsx")check on the bundle text, plus a react-dom render that produces the same HTML under either runtime.bundler_jsx.test.tsandjsx-tsconfig-react-jsx.test.tsalready use stub runtimes for the same reason."production"has to bundle againstjsxand"development"againstjsxDEVwhatever the parent process and tsconfig say, and the bundle has to run. Two additions:NODE_ENV=productionin the environment is pinned tojsxfor thebun runpass, and the imported file is now JSX too, so the bundler's separate entry-point and import paths forjsx.development(src/bundler/bundle_v2.rs) are both exercised. The combinations that fall back to tsconfig / the parent environment are deliberately left unpinned, as before; several open PRs are adjusting those semantics and this file only needs the fallback to produce a runtime that exists and to be applied consistently to both files.bun bd test test/bundler/transpiler/jsx-production.test.ts: 32 pass, 6.6-9s for the file, slowest case 1.2s.USE_SYSTEM_BUN=1 bun teston the file: 32 pass in 0.67s. The old file on the same debug build: 19 of 32 timed out, 37.5s.process.env.NODE_ENVdefine from the fixture fails exactly the 8 cases whose define disagrees with the fallback, and passes the other 24, so the pins are live.Background
"jsx": "react-jsx"or"react-jsxdev", JSX compiles to calls into a runtime module that Bun auto-imports:jsx/jsxsfrom<importSource>/jsx-runtime(production) orjsxDEVfrom<importSource>/jsx-dev-runtime(development). The import source defaults toreact, which is why a stub package namedreactin the fixture'snode_modulesis all the resolver needs.configure_definesinsrc/bundler/transpiler.rsreads aprocess.env.NODE_ENVdefine or the environment'sNODE_ENVand setsproduction/force_node_env; otherwise the tsconfigjsxvalue decides.src/bundler/bundle_v2.rsapplies that to each parse task, in one place for entry points and in another for resolved imports, which is why the fixture now has JSX in both an entry point and an imported file.BUN_ENVtakes precedence overNODE_ENVin Bun's env loader, so the test clears it; the matrix otherwise inheritsbunEnv.