From accdadc29ac2e6eb2a0c6ed9610d91bb6920b78e Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Mon, 12 Jan 2026 12:20:17 +0000 Subject: [PATCH] fix: address review comments - Use std.fs.path.sep instead of hardcoded '/' for cross-platform compatibility - Improve tests by reading stdout/stderr before awaiting exit - Add positive assertions to verify expanded paths are actually used Co-Authored-By: Claude Opus 4.5 --- src/resolver/resolve_path.zig | 2 +- test/regression/issue/25766.test.ts | 44 ++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig index d661b82c29..1c494143f7 100644 --- a/src/resolver/resolve_path.zig +++ b/src/resolver/resolve_path.zig @@ -2095,7 +2095,7 @@ pub fn expandTilde(allocator: std.mem.Allocator, path: []const u8) ?[]const u8 { const rest = path[2..]; // Skip ~/ const result = allocator.alloc(u8, home_dir.len + 1 + rest.len) catch return null; @memcpy(result[0..home_dir.len], home_dir); - result[home_dir.len] = '/'; + result[home_dir.len] = std.fs.path.sep; @memcpy(result[home_dir.len + 1 ..], rest); return result; } diff --git a/test/regression/issue/25766.test.ts b/test/regression/issue/25766.test.ts index 97e9a68607..534f671544 100644 --- a/test/regression/issue/25766.test.ts +++ b/test/regression/issue/25766.test.ts @@ -37,12 +37,21 @@ globalBinDir = "~/.bun/bin" stderr: "pipe", }); - await proc.exited; + // Read output before awaiting exited for better error messages + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); // The key assertion: a literal "~" directory should NOT be created in cwd // If the bug exists, it would create a directory literally named "~" const literalTildeDir = join(String(dir), "~"); expect(existsSync(literalTildeDir)).toBe(false); + + // Positive assertion: the expanded path under fake home should be used + const expandedBinDir = join(fakeHome, ".bun", "bin"); + expect(existsSync(expandedBinDir)).toBe(true); }); test("globalDir with tilde expands to home directory", async () => { @@ -71,11 +80,20 @@ globalDir = "~/.bun/install/global" stderr: "pipe", }); - await proc.exited; + // Read output before awaiting exited for better error messages + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); // No literal "~" directory should be created const literalTildeDir = join(String(dir), "~"); expect(existsSync(literalTildeDir)).toBe(false); + + // Positive assertion: the expanded path under fake home should be used + const expandedGlobalDir = join(fakeHome, ".bun", "install", "global"); + expect(existsSync(expandedGlobalDir)).toBe(true); }); test("cache.dir with tilde expands to home directory", async () => { @@ -104,11 +122,20 @@ dir = "~/.bun/install/cache" stderr: "pipe", }); - await proc.exited; + // Read output before awaiting exited for better error messages + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); // No literal "~" directory should be created const literalTildeDir = join(String(dir), "~"); expect(existsSync(literalTildeDir)).toBe(false); + + // Positive assertion: the expanded path under fake home should be used + const expandedCacheDir = join(fakeHome, ".bun", "install", "cache"); + expect(existsSync(expandedCacheDir)).toBe(true); }); test("cache shorthand with tilde expands to home directory", async () => { @@ -136,10 +163,19 @@ cache = "~/.bun/install/cache" stderr: "pipe", }); - await proc.exited; + // Read output before awaiting exited for better error messages + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); // No literal "~" directory should be created const literalTildeDir = join(String(dir), "~"); expect(existsSync(literalTildeDir)).toBe(false); + + // Positive assertion: the expanded path under fake home should be used + const expandedCacheDir = join(fakeHome, ".bun", "install", "cache"); + expect(existsSync(expandedCacheDir)).toBe(true); }); });