-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(shell): resolve '.' to process.cwd() in cwd() method #25688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
When calling $.cwd('.') or $.cwd(''), the defaultCwd variable was
undefined, causing the path to become 'undefined'. Now we fall back
to process.cwd() when defaultCwd is not set.
Fixes oven-sh#25579
Signed-off-by: lif <1835304752@qq.com>
WalkthroughThe changes modify how the current working directory is resolved in the shell implementation when Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (5)test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/regression/issue/**/*.test.ts📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
**/*.test.ts?(x)📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/regression/issue/*.test.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*.test.ts?(x)📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (21)📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-10-19T02:44:46.354ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-10-26T01:32:04.844ZApplied to files:
📚 Learning: 2025-12-16T00:21:32.179ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-07T17:28:51.204ZApplied to files:
📚 Learning: 2025-11-06T00:58:23.965ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 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.
📒 Files selected for processing (2)
src/js/builtins/shell.tstest/regression/issue/25579.test.ts
🧰 Additional context used
📓 Path-based instructions (7)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use.$call()and.$apply()instead of.call()and.apply()to prevent user tampering with function invocation
Use string literalrequire()statements only; dynamic requires are not permitted
Export modules usingexport default { ... }syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with$) such as$Array.from(),$isCallable(), and$newArrayWithSize()for performance-critical operations
Use private globals and methods with$prefix (e.g.,$Array,map.$set()) instead of public JavaScript globals
Use$debug()for debug logging and$assert()for assertions; both are stripped in release builds
Validate function arguments using validators frominternal/validatorsand throw$ERR_*error codes for invalid arguments
Useprocess.platformandprocess.archfor platform detection; these values are inlined and dead-code eliminated at build time
Files:
src/js/builtins/shell.ts
src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts
📄 CodeRabbit inference engine (src/js/CLAUDE.md)
Builtin functions must include
thisparameter typing in TypeScript to enable direct method binding in C++
Files:
src/js/builtins/shell.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.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
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/regression/issue/25579.test.ts
test/regression/issue/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Regression tests for specific issues go in
/test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory
Files:
test/regression/issue/25579.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/regression/issue/25579.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for specific GitHub issues in
test/regression/issue/${issueNumber}.test.tswith real issue numbers only
Files:
test/regression/issue/25579.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/regression/issue/25579.test.ts
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Export modules using `export default { ... }` syntax; modules are NOT ES modules
Applied to files:
src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `.$call()` and `.$apply()` instead of `.call()` and `.apply()` to prevent user tampering with function invocation
Applied to files:
src/js/builtins/shell.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use private globals and methods with `$` prefix (e.g., `$Array`, `map.$set()`) instead of public JavaScript globals
Applied to files:
src/js/builtins/shell.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/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory
Applied to files:
test/regression/issue/25579.test.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/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only
Applied to files:
test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/regression/issue/25579.test.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/regression/issue/25579.test.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} : 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/regression/issue/25579.test.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 `tempDir()` from harness to create temporary directories with files for multi-file tests instead of creating files manually
Applied to files:
test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Verify tests fail with `USE_SYSTEM_BUN=1 bun test <file>` and pass with `bun bd test <file>` - tests are invalid if they pass with USE_SYSTEM_BUN=1
Applied to files:
test/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/regression/issue/25579.test.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/regression/issue/25579.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Avoid shell commands like `find` or `grep` in tests - use Bun's Glob and built-in tools instead
Applied to files:
test/regression/issue/25579.test.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/regression/issue/25579.test.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/regression/issue/25579.test.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/regression/issue/25579.test.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: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)
Applied to files:
test/regression/issue/25579.test.ts
🔇 Additional comments (3)
test/regression/issue/25579.test.ts (1)
30-34: Good platform-aware testing for macOS symlink behavior.The test correctly handles the macOS-specific case where
/tmpis symlinked to/private/tmp. This prevents flaky test failures on macOS.src/js/builtins/shell.ts (2)
151-152: LGTM! Proper fallback to process.cwd() fixes undefined cwd issue.The addition of
?? process.cwd()correctly ensures that whendefaultCwdis undefined, the current working directory is used instead of leavingnewCwdas undefined. This properly resolves the issue where$.cwd('.'),$.cwd('./'), and$.cwd('')would result in undefined paths.
279-280: LGTM! Consistent fix applied in ShellPrototype.This change mirrors the fix in
ShellPromise.cwd()at line 152, ensuring consistent behavior across both code paths. The fallback toprocess.cwd()whendefaultCwdis undefined correctly handles current-directory indicators.
| describe("Shell cwd with dot", () => { | ||
| test("$.cwd('.') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd(".").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd('./') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd("./").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd('') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd("").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd() without argument should work", async () => { | ||
| const result = await Bun.$`pwd`.cwd().text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd('/tmp') should work with absolute path", async () => { | ||
| const result = await Bun.$`pwd`.cwd("/tmp").text(); | ||
| // On macOS, /tmp is symlinked to /private/tmp | ||
| expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider making tests concurrent for better performance.
These tests are independent and don't modify shared state, so they can safely run concurrently. Consider using describe.concurrent or individual test.concurrent calls.
🔎 Optional refactor using concurrent tests
-describe("Shell cwd with dot", () => {
+describe.concurrent("Shell cwd with dot", () => {
test("$.cwd('.') should work and use current directory", async () => {As per coding guidelines, prefer concurrent tests when multiple tests spawn processes (Bun.$ internally spawns pwd processes).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe("Shell cwd with dot", () => { | |
| test("$.cwd('.') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd(".").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('./') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd("./").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd("").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd() without argument should work", async () => { | |
| const result = await Bun.$`pwd`.cwd().text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('/tmp') should work with absolute path", async () => { | |
| const result = await Bun.$`pwd`.cwd("/tmp").text(); | |
| // On macOS, /tmp is symlinked to /private/tmp | |
| expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/); | |
| }); | |
| }); | |
| describe.concurrent("Shell cwd with dot", () => { | |
| test("$.cwd('.') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd(".").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('./') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd("./").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('') should work and use current directory", async () => { | |
| const result = await Bun.$`pwd`.cwd("").text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd() without argument should work", async () => { | |
| const result = await Bun.$`pwd`.cwd().text(); | |
| expect(result.trim()).toBe(process.cwd()); | |
| }); | |
| test("$.cwd('/tmp') should work with absolute path", async () => { | |
| const result = await Bun.$`pwd`.cwd("/tmp").text(); | |
| // On macOS, /tmp is symlinked to /private/tmp | |
| expect(result.trim()).toMatch(/^(\/tmp|\/private\/tmp)$/); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In test/regression/issue/25579.test.ts around lines 9-35, the tests are
independent and should run concurrently for performance; change the test suite
to run in parallel by replacing describe("Shell cwd with dot", ...) with
describe.concurrent("Shell cwd with dot", ...) or mark each test as
test.concurrent(...) so the Bun.$ spawned processes run concurrently without
shared-state issues.
| test("$.cwd('.') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd(".").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd('./') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd("./").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd('') should work and use current directory", async () => { | ||
| const result = await Bun.$`pwd`.cwd("").text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); | ||
|
|
||
| test("$.cwd() without argument should work", async () => { | ||
| const result = await Bun.$`pwd`.cwd().text(); | ||
| expect(result.trim()).toBe(process.cwd()); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider using parameterized tests for similar test cases.
The first four tests follow an identical pattern with different input values. Using describe.each() would reduce duplication and make the test suite more maintainable.
🔎 Optional refactor using describe.each
- test("$.cwd('.') should work and use current directory", async () => {
- const result = await Bun.$`pwd`.cwd(".").text();
- expect(result.trim()).toBe(process.cwd());
- });
-
- test("$.cwd('./') should work and use current directory", async () => {
- const result = await Bun.$`pwd`.cwd("./").text();
- expect(result.trim()).toBe(process.cwd());
- });
-
- test("$.cwd('') should work and use current directory", async () => {
- const result = await Bun.$`pwd`.cwd("").text();
- expect(result.trim()).toBe(process.cwd());
- });
-
- test("$.cwd() without argument should work", async () => {
- const result = await Bun.$`pwd`.cwd().text();
- expect(result.trim()).toBe(process.cwd());
- });
+ describe.each([
+ { input: ".", desc: "'.'" },
+ { input: "./", desc: "'./'" },
+ { input: "", desc: "''" },
+ { input: undefined, desc: "without argument" },
+ ])("$.cwd($desc) should work and use current directory", ({ input }) => {
+ test("", async () => {
+ const result = input === undefined
+ ? await Bun.$`pwd`.cwd().text()
+ : await Bun.$`pwd`.cwd(input).text();
+ expect(result.trim()).toBe(process.cwd());
+ });
+ });🤖 Prompt for AI Agents
In test/regression/issue/25579.test.ts around lines 10 to 28, the four nearly
identical tests that call Bun.$`pwd`.cwd(...) should be consolidated into a
single parameterized test to reduce duplication; replace the four separate test
blocks with a single describe.each or test.each that iterates over the input
values [".", "./", "", undefined] (use undefined to represent no-argument
.cwd()), run Bun.$`pwd`.cwd(value) (or .cwd() when value is undefined), get
.text(), and assert that result.trim() equals process.cwd(); preserve the
existing trimming and assertion logic and keep individual case labels for
clarity.
Summary
Test plan
Fixes #25579