Skip to content

Conversation

@nektro
Copy link
Contributor

@nektro nektro commented Nov 29, 2025

No description provided.

@robobun
Copy link
Collaborator

robobun commented Nov 29, 2025

@nektro nektro marked this pull request as ready for review November 29, 2025 06:47
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2025

Walkthrough

The changes add Docker build tests for Alpine, Debian, Debian Slim, and Distroless base images, modify the distroless Dockerfile to expose /etc/alternatives/which via bind-mount during build, and adjust parallel test timeouts for Docker-related tests to 60,000 ms.

Changes

Cohort / File(s) Summary
Docker configuration
dockerhub/distroless/Dockerfile
Added bind-mount for /etc/alternatives/which from build stage in RUN step to enable symlink creation access
Test timeout configuration
scripts/runner.node.mjs
Adjusted getNodeParallelTestTimeout to use 60,000 ms timeout for test paths containing "-docker-"; reduced existing "test-dns" timeout from 90,000 ms to 60,000 ms
Docker build tests
test/js/bun/test/parallel/test-docker-build-*.ts
Added four new parallel test files (Alpine, Debian Slim, Debian, Distroless) that conditionally spawn Docker build processes with --progress=plain, --no-cache, and --rm flags when Docker is enabled

Possibly related PRs

Suggested reviewers

  • Jarred-Sumner
  • dylan-conway

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely missing, failing to meet the required template sections. Add a description following the template: explain what the PR does and how the changes were verified.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding regression tests for building docker containers.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
test/js/bun/test/parallel/test-docker-build-alpine.ts (1)

1-17: Alpine docker build test mirrors Debian Slim pattern

Logic and harness usage are consistent with the Debian Slim docker build test and look correct. The earlier note about possibly extracting a shared helper for these docker build tests applies here as well.

test/js/bun/test/parallel/test-docker-build-distroless.ts (1)

1-17: Distroless docker build regression test is consistent and targets the updated Dockerfile

This test correctly exercises dockerhub/distroless with the same docker build flags and assertions as the other images. It should catch regressions in the distroless Dockerfile, including the new bind-mount behavior for which.

test/js/bun/test/parallel/test-docker-build-debian.ts (1)

1-17: Debian docker build test completes the docker image coverage

The Debian docker build check matches the other image tests in behavior and harness usage, providing a simple exit-code regression guard for the dockerhub/debian context. Same optional helper/DRY suggestion as noted in the Debian Slim test applies here if you decide to consolidate later.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 56da7c4 and 12356e1.

📒 Files selected for processing (6)
  • dockerhub/distroless/Dockerfile (1 hunks)
  • scripts/runner.node.mjs (1 hunks)
  • test/js/bun/test/parallel/test-docker-build-alpine.ts (1 hunks)
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts (1 hunks)
  • test/js/bun/test/parallel/test-docker-build-debian.ts (1 hunks)
  • test/js/bun/test/parallel/test-docker-build-distroless.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • scripts/runner.node.mjs
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - your test is NOT VALID if it passes with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : For multi-file tests, prefer using `tempDir` from `harness` and `Bun.spawn` over other temporary directory creation methods

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-distroless.ts
  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`

Applied to files:

  • test/js/bun/test/parallel/test-docker-build-alpine.ts
  • test/js/bun/test/parallel/test-docker-build-debian.ts
  • test/js/bun/test/parallel/test-docker-build-debian-slim.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not set a timeout on tests. Bun already has timeouts

Applied to files:

  • scripts/runner.node.mjs
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time

Applied to files:

  • scripts/runner.node.mjs
📚 Learning: 2025-11-24T18:36:33.069Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.069Z
Learning: Applies to **/*.test.{ts,tsx} : Do not write flaky tests - do not use `setTimeout` in tests, instead `await` the condition to be met as you are testing the CONDITION not the TIME PASSING

Applied to files:

  • scripts/runner.node.mjs
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer concurrent tests over sequential tests using `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent

Applied to files:

  • scripts/runner.node.mjs
🧬 Code graph analysis (4)
test/js/bun/test/parallel/test-docker-build-distroless.ts (1)
test/harness.ts (2)
  • isDockerEnabled (865-888)
  • dockerExe (861-863)
test/js/bun/test/parallel/test-docker-build-alpine.ts (1)
test/harness.ts (2)
  • isDockerEnabled (865-888)
  • dockerExe (861-863)
test/js/bun/test/parallel/test-docker-build-debian.ts (1)
test/harness.ts (2)
  • isDockerEnabled (865-888)
  • dockerExe (861-863)
test/js/bun/test/parallel/test-docker-build-debian-slim.ts (1)
test/harness.ts (2)
  • isDockerEnabled (865-888)
  • dockerExe (861-863)
🔇 Additional comments (2)
dockerhub/distroless/Dockerfile (1)

73-77: Bind-mounting /etc/alternatives/which is a correct, minimal fix

Extending the BuildKit RUN mounts to include /etc/alternatives/which matches Debian’s which symlink layout and fixes the transient which invocation without impacting the final distroless image.

scripts/runner.node.mjs (1)

79-85: Timeout adjustments align with new docker build tests

Using a 60s timeout for both test-dns and *-docker-* node tests is consistent with their heavier I/O/network profile, while keeping the default at 20s for normal CI runs. The ordering and conditions look correct.

Copy link
Contributor

@RiskyMH RiskyMH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks legit, but it seems sus to use bun:test without a .test.ts and test() name callback lol (esp because it's in a way hard to actually know it was gonna run - the docker enabled check)

@nektro nektro merged commit fe0aba7 into main Dec 2, 2025
54 of 58 checks passed
@nektro nektro deleted the nektro-patch-59660 branch December 2, 2025 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants