test(next-pages): update Next.js to 16.1.6 and Puppeteer to 24.37.5#27656
test(next-pages): update Next.js to 16.1.6 and Puppeteer to 24.37.5#27656
Conversation
Update test/integration/next-pages/ to use latest dependency versions: - Next.js 15.4.8 → 16.1.6 - Puppeteer 24.16.0 → 24.37.5 - React 19.1.1 → 19.2.4 - TypeScript 5.9.2 → 5.9.3 - Other deps (@types/node, @types/react, autoprefixer, etc.) Test changes for Next.js 16 compatibility: - Use --webpack flag for build comparison test since Turbopack (now default in Next.js 16) produces non-deterministic output between bun and node runtimes - Add trace-build to non-deterministic file exclusions - Update output normalization for Next.js 16's "in Xms" timing format - Remove "lint" script (next lint was removed in Next.js 16) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Updated 9:44 AM PT - Mar 1st, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 27656That installs a local version of the PR into your bun-27656 --bun |
Remove the ARM64 Linux skip condition for the dev-server Puppeteer test.
Chrome for Testing still doesn't provide ARM64 Linux binaries, but
the test already detects system Chromium via `which("chromium")` and
uses it as the browser executable path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdates test/integration/next-pages: Changes
🚥 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://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/integration/next-pages/test/dev-server.test.ts (1)
131-141:⚠️ Potential issue | 🟠 MajorReplace timeout-driven watchdog with condition-based cleanup.
Lines 131-141 use
setTimeoutas test control flow, which is time-based and flaky in CI. Switch to condition-based awaiting andtry/finallycleanup for spawned processes.As per coding guidelines: "Do not use `setTimeout` in tests; instead `await` the condition to be met - you are testing the CONDITION, not TIME PASSING".Proposed refactor
- var pid: number, exited; - let timeout = setTimeout(() => { - if (timeout && pid) { - process.kill?.(pid); - pid = 0; - - if (dev_server_pid) { - process?.kill?.(dev_server_pid); - dev_server_pid = undefined; - } - } - }, 30000).unref(); + var pid: number, exited; ({ exited, pid } = Bun.spawn([bunExe(), "test/dev-server-puppeteer.ts", baseUrl], { cwd: root, env: bunEnv, stdio: ["ignore", "inherit", "inherit"], })); - expect(await exited).toBe(0); - pid = 0; - clearTimeout(timeout); - // `@ts-expect-error` - timeout = undefined; + try { + expect(await exited).toBe(0); + } finally { + if (pid) { + process.kill?.(pid); + pid = 0; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/next-pages/test/dev-server.test.ts` around lines 131 - 141, Replace the time-based watchdog (the setTimeout block that sets `timeout`, checks `pid` and `dev_server_pid` after 30s) with a condition-based await and a try/finally cleanup: remove the setTimeout and instead await the specific condition you're testing (e.g., await a Promise that resolves when the server is ready or when `pid`/`dev_server_pid` become set/cleared), then in a finally block perform the same cleanup logic that currently lives inside the timeout (call process.kill on `pid` and `dev_server_pid` if present and reset them), ensuring you clear any spawned-process state; reference the existing variables `pid`, `dev_server_pid`, and the cleanup steps so the test no longer relies on elapsed time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/integration/next-pages/test/dev-server.test.ts`:
- Around line 131-141: Replace the time-based watchdog (the setTimeout block
that sets `timeout`, checks `pid` and `dev_server_pid` after 30s) with a
condition-based await and a try/finally cleanup: remove the setTimeout and
instead await the specific condition you're testing (e.g., await a Promise that
resolves when the server is ready or when `pid`/`dev_server_pid` become
set/cleared), then in a finally block perform the same cleanup logic that
currently lives inside the timeout (call process.kill on `pid` and
`dev_server_pid` if present and reset them), ensuring you clear any
spawned-process state; reference the existing variables `pid`, `dev_server_pid`,
and the cleanup steps so the test no longer relies on elapsed time.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
test/integration/next-pages/test/__snapshots__/dev-server.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
test/integration/next-pages/test/dev-server.test.ts
![claude[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward test dependency update with sensible adaptations for Next.js 16.
Extended reasoning...
Overview
This PR updates test dependencies in test/integration/next-pages/ (Next.js 15.4.8 → 16.1.6, Puppeteer 24.16.0 → 24.37.5, React, TypeScript, etc.) and adapts the test code in next-build.test.ts to accommodate Next.js 16 changes. The modifications include using --webpack for deterministic build comparison, broadening timestamp normalization regex, adding new non-deterministic build artifacts to the exclusion list, and adding force: true to rmSync calls.
Security risks
None. All changes are confined to integration test files — no production code, no auth logic, no user-facing behavior is affected.
Level of scrutiny
Low scrutiny is appropriate. This is test infrastructure code: dependency version bumps and test adaptations. The test plan in the PR description confirms all three test suites pass. The code changes are mechanical and well-commented.
Other factors
- No prior reviews or outstanding comments to address.
- No bugs found by the automated bug hunting system.
- The regex change from a narrow pattern to a broader
\bin \d+(?:\.\d+)?m?s\bis correct and well-bounded with word boundaries. - The
--webpackflag addition is symmetric (applied to both bun and node builds), maintaining the apples-to-apples comparison that is the point of the test. - The
force: trueaddition tormSyncis a good defensive change that prevents test failures when Next.js 16 doesn't produce all the same artifacts as 15.
There was a problem hiding this comment.
LGTM — straightforward test dependency bumps and reasonable adaptations for Next.js 16 (Turbopack default → webpack for deterministic comparison).
Extended reasoning...
Overview
This PR updates test/integration/next-pages/ dependencies (Next.js 15.4.8 → 16.1.6, Puppeteer 24.16.0 → 24.37.5, React 19.2.4, TypeScript 5.9.3, etc.) and adapts the test suite for Next.js 16 behavioral changes. The key adaptation is adding --webpack to build commands since Turbopack (now the default bundler in Next.js 16) produces non-deterministic output between bun and node runtimes. Other changes include broadening the timestamp normalization regex, adding trace-build and required-server-files.js to the non-deterministic file exclusion list, adding force: true to rmSync calls, removing the now-unnecessary arm64 Puppeteer skip, and updating snapshots.
Security risks
None. All changes are confined to integration test files — no production code, no auth/crypto, no user-facing behavior changes.
Level of scrutiny
Low scrutiny warranted. This is test infrastructure code with dependency bumps and mechanical test adaptations. The changes follow established patterns and the PR description confirms all three test suites pass.
Other factors
- No bugs were found by the automated bug hunting system
- The CI failures reported by robobun are in unrelated files (scripts/build.mjs, cmake/targets/BuildBun.cmake), not caused by this PR
- No CLAUDE.md or CODEOWNERS concerns for the test/integration/next-pages directory
- The broader timestamp regex (
/\bin \d+(?:\.\d+)?m?s\b/gi) is reasonable for normalizing timing strings in build output comparison
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/integration/next-pages/test/next-build.test.ts (1)
130-156:⚠️ Potential issue | 🟠 MajorUse
await usingfor spawned build processes.The
Bun.spawn()calls at lines 130 and 145 should useawait usingto ensure proper resource cleanup per test guidelines. While the current code awaits process completion and checks exit codes, theawait usingpattern provides guaranteed cleanup on all paths, including early returns or exceptions.♻️ Proposed fix
- const bunBuild = Bun.spawn([bunExe(), "--bun", nextPath, "build", "--webpack"], { + await using bunBuild = Bun.spawn([bunExe(), "--bun", nextPath, "build", "--webpack"], { cwd: bunDir, stdio: ["ignore", "pipe", "inherit"], env: { ...bunEnv, NODE_NO_WARNINGS: "1", NODE_ENV: "production", TMPDIR: tmp1, TEMP: tmp1, TMP: tmp1, }, }); const tmp2 = tmpdirSync(); console.time("[node] next build"); - const nodeBuild = Bun.spawn(["node", nextPath, "build", "--webpack"], { + await using nodeBuild = Bun.spawn(["node", nextPath, "build", "--webpack"], { cwd: nodeDir, env: { ...bunEnv, NODE_NO_WARNINGS: "1", NODE_ENV: "production", TMPDIR: tmp2, TEMP: tmp2, TMP: tmp2, }, stdio: ["ignore", "pipe", "inherit"], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/next-pages/test/next-build.test.ts` around lines 130 - 156, Replace the plain Bun.spawn assignments for the build processes with the Resource-safe pattern using "await using" so bunBuild and nodeBuild are disposed automatically; specifically change the declarations of bunBuild and nodeBuild where Bun.spawn(...) is called to use "await using bunBuild = Bun.spawn(...)" and "await using nodeBuild = Bun.spawn(...)" respectively, keep the same env/cwd/stdio config and preserve subsequent awaits (e.g., awaiting bunBuild.exited / nodeBuild.exited and exitCode checks) so cleanup is guaranteed on exceptions or early returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/integration/next-pages/test/next-build.test.ts`:
- Around line 130-156: Replace the plain Bun.spawn assignments for the build
processes with the Resource-safe pattern using "await using" so bunBuild and
nodeBuild are disposed automatically; specifically change the declarations of
bunBuild and nodeBuild where Bun.spawn(...) is called to use "await using
bunBuild = Bun.spawn(...)" and "await using nodeBuild = Bun.spawn(...)"
respectively, keep the same env/cwd/stdio config and preserve subsequent awaits
(e.g., awaiting bunBuild.exited / nodeBuild.exited and exitCode checks) so
cleanup is guaranteed on exceptions or early returns.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
test/integration/next-pages/bun.lockis excluded by!**/*.locktest/integration/next-pages/test/__snapshots__/dev-server-ssr-100.test.ts.snapis excluded by!**/*.snaptest/integration/next-pages/test/__snapshots__/dev-server.test.ts.snapis excluded by!**/*.snaptest/integration/next-pages/test/__snapshots__/next-build.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
test/integration/next-pages/test/next-build.test.ts
Update test/integration/next-pages/ dependencies to latest versions: - Next.js 15.4.8 to 16.1.6 - Puppeteer 24.16.0 to 24.37.5 - React 19.1.1 to 19.2.4 - TypeScript 5.9.2 to 5.9.3 - Other deps (types, autoprefixer, eslint-config-next, etc.) Test changes for Next.js 16 compatibility: - Use --webpack for build comparison (Turbopack is now default but non-deterministic between bun and node runtimes) - Add trace-build to non-deterministic file exclusions - Update output normalization for new timing format - Remove lint script (next lint removed in Next.js 16) - Skip Puppeteer on ARM64 Linux only when no native browser found Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
robobun
force-pushed
the
claude/update-next-pages-tests
branch
from
ea1535d to
69fed56
Compare
There was a problem hiding this comment.
LGTM — test-only dependency bumps and test adaptations for Next.js 16.
Extended reasoning...
Overview
This PR updates test/integration/next-pages/ to Next.js 16.1.6, Puppeteer 24.37.5, React 19.2.4, and other dependency bumps. It adapts the build comparison test to use --webpack (since Turbopack is now the default in Next.js 16 but produces non-deterministic output across runtimes), broadens timestamp normalization in output comparison, adds newly-generated artifacts (trace-build, required-server-files.js) to the cleanup list with force: true, and improves the arm64 skip logic to check for native browser availability.
Security risks
None — this PR only touches test infrastructure and dependency versions within an integration test directory. No production code, auth, or crypto paths are affected.
Level of scrutiny
Low scrutiny warranted. These are test files for an integration test suite. The changes are mechanical (version bumps, snapshot updates, lockfile regeneration) with small, well-motivated logic changes in the test files.
Other factors
- No production code is modified
- The regex change for timestamp normalization (
\bin \d+(?:\.\d+)?m?s\b) is correct and strictly broader than the previous pattern - The
hasNativeBrowserlogic is a clear improvement over the previous hardcoded arm64 skip - Adding
force: truetormSynccalls is correct — prevents ENOENT errors during cleanup of files that may not exist in all Next.js versions - The PR description includes a test plan confirming all three test files pass
There was a problem hiding this comment.
LGTM — straightforward test dependency update and test adaptations for Next.js 16.
Extended reasoning...
Overview
This PR updates the test/integration/next-pages/ test suite to use Next.js 16.1.6 (from 15.4.8), Puppeteer 24.37.5 (from 24.16.0), and other dependency bumps. It adapts the test code to handle Next.js 16 behavioral changes: using --webpack for deterministic build comparison (since Turbopack is now the default), broadening timestamp normalization regexes, excluding additional non-deterministic build artifacts, and improving arm64 Linux browser detection.
Security risks
None. This PR only modifies test infrastructure — no production code, no auth/crypto paths, no user-facing behavior is affected.
Level of scrutiny
Low scrutiny appropriate. These are integration test files with dependency version bumps and minor test logic adjustments. The CI failures visible in the timeline are unrelated CMakeLists.txt cmake errors. The code changes are mechanical and well-documented with comments explaining the rationale.
Other factors
The regex change from /Compiled successfully in (\d|\.)+(...)/ to /\bin \d+(?:\.\d+)?m?s\b/gi is more general but correctly bounded by \b word boundaries. The hasNativeBrowser logic correctly handles the arm64 Linux case by checking for system-installed browsers via Bun.which(). The force: true addition to rmSync is appropriate since some artifacts may not exist in all Next.js versions. No bugs were found by the automated system.
The old headless "shell" mode for macOS causes TargetCloseError with Puppeteer 24.37.5 and Chrome 145. Switch to standard headless mode which works across all platforms with the newer Chrome version. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — straightforward test dependency update with sensible adaptations for Next.js 16 (webpack flag for determinism, broader timestamp normalization, improved browser detection).
Extended reasoning...
Overview
This PR updates the test/integration/next-pages/ test suite to work with Next.js 16.1.6 (from 15.4.8) and Puppeteer 24.37.5 (from 24.16.0), along with React, TypeScript, and other dependency bumps. The test logic changes are minimal adaptations: adding --webpack to build commands since Turbopack (now default in Next.js 16) produces non-deterministic output across runtimes, broadening timestamp normalization regex, adding new build artifacts to the cleanup list, and improving the browser detection logic for Puppeteer skip conditions.
Security risks
None. All changes are confined to integration test files — no production code, no auth/crypto, no user-facing behavior is affected.
Level of scrutiny
Low scrutiny appropriate. This is test infrastructure code updating dependency versions and adapting test harness logic. The changes follow obvious patterns (version bumps, adding force flags to rmSync, expanding exclusion lists for non-deterministic artifacts).
Other factors
- No prior reviews from me on this PR.
- The bug hunting system found zero bugs.
- The PR author reports all three test suites pass.
- The
hasNativeBrowserlogic is actually an improvement over the oldpuppeteer_unsupportedflag — it now allows arm64 Linux to run tests if a system browser is available, rather than blanket-skipping. - Adding
force: truetormSynccalls is correct — it prevents errors when Next.js 16 may not produce certain artifacts that existed in Next.js 15.
Remove pipe:true to fix TargetCloseError on macOS 13. Poll for Tailwind CSS computed styles after HMR since CSS regeneration lags behind JS HMR on macOS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — straightforward test dependency updates and adaptations for Next.js 16.
Extended reasoning...
Overview
This PR updates test/integration/next-pages/ dependencies (Next.js 15.4.8 → 16.1.6, Puppeteer 24.16.0 → 24.37.5, React, TypeScript, etc.) and adapts tests for Next.js 16 behavioral changes. Key adaptations include: using --webpack for deterministic bun-vs-node build comparison (since Turbopack is now default), broader timestamp normalization in output, additional non-deterministic artifact cleanup (trace-build, required-server-files.js), force: true on rmSync calls, improved browser detection logic, and a waitForFunction guard for Tailwind CSS HMR timing.
Security risks
None. All changes are in integration test files under test/. No production code, auth, crypto, or permission changes.
Level of scrutiny
Low-to-moderate. This is test infrastructure code — dependency bumps and test adaptations. The changes follow established patterns and are well-commented. The hasNativeBrowser detection logic is a strict improvement over the previous architecture-only check.
Other factors
- No bugs found by the bug hunting system
- CI failures on the earlier commit (
f826fd63) fordev-server.test.tswere addressed by subsequent commits fixing Puppeteer pipe crash and Tailwind HMR timing - Other CI failures (
watch-many-dirs.test.ts,vite-build.test.ts,stress.test.ts) are clearly unrelated to this PR - The broader regex
/\bin \d+(?:\.\d+)?m?s\b/giis safe with word boundaries preventing false matches - No CLAUDE.md violations — the
waitForFunctioncorrectly waits for a DOM condition rather than sleeping
Full Chrome fails to launch on macOS 13 CI. Use chrome-headless-shell on macOS which works on both macOS 13 and 14. The previous TargetCloseError on macOS 14 was caused by pipe transport (now removed), not by shell mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM — straightforward test dependency bumps and test adaptations for Next.js 16.
Extended reasoning...
Overview
This PR updates test/integration/next-pages/ dependencies (Next.js 15.4.8 → 16.1.6, Puppeteer 24.16.0 → 24.37.5, React, TypeScript, etc.) and adapts the integration tests for Next.js 16 changes. Key adaptations include using --webpack for deterministic build comparison (since Turbopack is now the default but produces non-deterministic output), broader timestamp normalization regex, additional non-deterministic files in the cleanup list, improved browser detection logic, and a Tailwind CSS HMR wait in the Puppeteer test.
Security risks
None. All changes are confined to test code in an integration test directory. No production code, auth, crypto, or permissions are affected.
Level of scrutiny
Low — this is purely test infrastructure code. The changes are mechanical dependency version bumps plus reasonable test adaptations. The regex change from matching only "Compiled successfully in Xms" to the broader \bin \d+(?:\.\d+)?m?s\b is correct and uses proper word boundaries. The force: true addition to rmSync calls is a good practice improvement.
Other factors
- No bugs were found by the bug hunting system.
- The test plan in the description indicates all three test suites pass.
- CI shows a failure on macOS aarch64 for
dev-server.test.tsand an unrelatedvite-build.test.tsfailure — subsequent commits in the branch appear to address the dev-server issue. - No outstanding reviewer comments to address.
2 participants
Summary
test/integration/next-pages/to use the latest versions of Next.js (15.4.8 → 16.1.6) and Puppeteer (24.16.0 → 24.37.5)--webpackfor deterministic build comparison since Turbopack (now default) is non-deterministic between runtimes, addtrace-buildto exclusions, fix output normalization for new timing formatTest plan
dev-server.test.ts— passes (skipped on arm64 as expected)dev-server-ssr-100.test.ts— passes (100 SSR requests)next-build.test.ts— passes (bun vs node build comparison with webpack)🤖 Generated with Claude Code