mirror of
https://github.com/oven-sh/bun
synced 2026-02-16 22:01:47 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user