shell: Fix some cd returning the wrong exit code (#11092)

oven-sh · May 18, 2024 · 38122a9 · 38122a9
1 parent 16920a5
commit 38122a9
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 9 deletions.
diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig
@@ -7020,7 +7020,7 @@ pub const Interpreter = struct {
             pub fn start(this: *Cd) Maybe(void) {
                 const args = this.bltn.argsSlice();
                 if (args.len > 1) {
-                    this.writeStderrNonBlocking("too many arguments", .{});
+                    this.writeStderrNonBlocking("too many arguments\n", .{});
                     // yield execution
                     return Maybe(void).success;
                 }
@@ -7060,28 +7060,28 @@ pub const Interpreter = struct {
                 switch (errno) {
                     @as(usize, @intFromEnum(bun.C.E.NOTDIR)) => {
                         if (!this.bltn.stderr.needsIO()) {
-                            const buf = this.bltn.fmtErrorArena(.cd, "not a directory: {s}", .{new_cwd_});
+                            const buf = this.bltn.fmtErrorArena(.cd, "not a directory: {s}\n", .{new_cwd_});
                             _ = this.bltn.writeNoIO(.stderr, buf);
                             this.state = .done;
                             this.bltn.done(1);
                             // yield execution
                             return Maybe(void).success;
                         }
 
-                        this.writeStderrNonBlocking("not a directory: {s}", .{new_cwd_});
+                        this.writeStderrNonBlocking("not a directory: {s}\n", .{new_cwd_});
                         return Maybe(void).success;
                     },
                     @as(usize, @intFromEnum(bun.C.E.NOENT)) => {
                         if (!this.bltn.stderr.needsIO()) {
-                            const buf = this.bltn.fmtErrorArena(.cd, "not a directory: {s}", .{new_cwd_});
+                            const buf = this.bltn.fmtErrorArena(.cd, "not a directory: {s}\n", .{new_cwd_});
                             _ = this.bltn.writeNoIO(.stderr, buf);
                             this.state = .done;
                             this.bltn.done(1);
                             // yield execution
                             return Maybe(void).success;
                         }
 
-                        this.writeStderrNonBlocking("not a directory: {s}", .{new_cwd_});
+                        this.writeStderrNonBlocking("not a directory: {s}\n", .{new_cwd_});
                         return Maybe(void).success;
                     },
                     else => return Maybe(void).success,
@@ -7100,7 +7100,7 @@ pub const Interpreter = struct {
                 }
 
                 this.state = .done;
-                this.bltn.done(0);
+                this.bltn.done(1);
             }
 
             pub fn deinit(this: *Cd) void {

diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts
@@ -37,6 +37,40 @@ afterAll(async () => {
 const BUN = bunExe();
 
 describe("bunshell", () => {
+  describe("exit codes", async () => {
+    const failing_cmds = [
+      process.platform === "win32" ? "cat ldkfjsldf" : null,
+      "touch -alskdjfakjfhasjfh",
+      "mkdir",
+      "export",
+      "cd lskfjlsdkjf",
+      process.platform !== "win32" ? "echo hi > /dev/full" : null,
+      "pwd sldfkj sfks jdflks flksd f",
+      "which",
+      "rm lskdjfskldjfksdjflkjsldfj",
+      "mv lskdjflskdjf lskdjflskjdlf",
+      "ls lksdjflksdjf",
+      "exit sldkfj sdjf ls f",
+      // "true",
+      // "false",
+      // "yes",
+      // "seq",
+      "dirname",
+      "basename",
+      "cp ksdjflksjdfks lkjsdflksjdfl",
+    ];
+
+    failing_cmds.forEach(cmdstr =>
+      !!cmdstr
+        ? TestBuilder.command`${{ raw: cmdstr }}`
+            .exitCode(c => c !== 0)
+            .stdout(() => {})
+            .stderr(() => {})
+            .runAsTest(cmdstr)
+        : "",
+    );
+  });
+
   describe("concurrency", () => {
     test("writing to stdout", async () => {
       await Promise.all([

diff --git a/test/js/bun/shell/test_builder.ts b/test/js/bun/shell/test_builder.ts
@@ -22,7 +22,7 @@ export function createTestBuilder(path: string) {
 
     expected_stdout: string | ((stdout: string, tempdir: string) => void) = "";
     expected_stderr: string | ((stderr: string, tempdir: string) => void) | { contains: string } = "";
-    expected_exit_code: number = 0;
+    expected_exit_code: number | ((code: number) => void) = 0;
     expected_error: ShellError | string | boolean | undefined = undefined;
     file_equals: { [filename: string]: string | (() => string | Promise<string>) } = {};
     _doesNotExist: string[] = [];
@@ -175,7 +175,7 @@ export function createTestBuilder(path: string) {
       return this;
     }
 
-    exitCode(expected: number): this {
+    exitCode(expected: number | ((code: number) => void)): this {
       this.expected_exit_code = expected;
       return this;
     }
@@ -232,7 +232,9 @@ export function createTestBuilder(path: string) {
           expect(stderr.toString()).toContain(this.expected_stderr.contains);
         }
       }
-      if (this.expected_exit_code !== undefined) expect(exitCode).toEqual(this.expected_exit_code);
+      if (typeof this.expected_exit_code === "number") {
+        expect(exitCode).toEqual(this.expected_exit_code);
+      } else if (typeof this.expected_exit_code === "function") this.expected_exit_code(exitCode);
 
       for (const [filename, expected_raw] of Object.entries(this.file_equals)) {
         const expected = typeof expected_raw === "string" ? expected_raw : await expected_raw();