Do not run shell tests outside test scope (#10199)

* Do not run tests outside test scope * Fix tests * Fix type errors and remove potentially precarious uses of unreachable * yoops * Remove all instances of "Ruh roh" --------- Co-authored-by: Zack Radisic <[email protected]>
oven-sh · Apr 16, 2024 · 291a39b · 291a39b
1 parent fbe2fe0
commit 291a39b
Show file tree

Hide file tree

Showing 18 changed files with 315 additions and 269 deletions.
diff --git a/src/js/private.d.ts b/src/js/private.d.ts
@@ -105,7 +105,7 @@ declare module "bun" {
   var TOML: {
     parse(contents: string): any;
   };
-  function jest(): typeof import("bun:test");
+  function jest(path: string): typeof import("bun:test");
   var main: string;
   var tty: Array<{ hasColors: boolean }>;
   var FFI: any;

diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig
@@ -58,7 +58,7 @@ const stderr_no = 2;
 
 pub fn OOM(e: anyerror) noreturn {
     if (comptime bun.Environment.allow_assert) {
-        if (e != error.OutOfMemory) @panic("Ruh roh");
+        if (e != error.OutOfMemory) bun.outOfMemory();
     }
     @panic("Out of memory");
 }
@@ -2123,7 +2123,7 @@ pub const Interpreter = struct {
                 return;
             }
 
-            unreachable;
+            @panic("Invalid child to Expansion, this indicates a bug in Bun. Please file a report on Github.");
         }
 
         fn onGlobWalkDone(this: *Expansion, task: *ShellGlobTask) void {
@@ -2742,7 +2742,7 @@ pub const Interpreter = struct {
                 return;
             }
 
-            unreachable;
+            @panic("Invalid child to Assigns expression, this indicates a bug in Bun. Please file a report on Github.");
         }
     };
 
@@ -2910,7 +2910,7 @@ pub const Interpreter = struct {
             parent: ParentPtr,
             io: IO,
         ) *Binary {
-            var binary = interpreter.allocator.create(Binary) catch |err| std.debug.panic("Ruh roh: {any}\n", .{err});
+            var binary = interpreter.allocator.create(Binary) catch bun.outOfMemory();
             binary.node = node;
             binary.base = .{ .kind = .binary, .interpreter = interpreter, .shell = shell_state };
             binary.parent = parent;
@@ -3234,7 +3234,7 @@ pub const Interpreter = struct {
                         if (ptr == @as(usize, @intCast(child.ptr.repr._ptr))) break :brk i;
                     }
                 }
-                unreachable;
+                @panic("Invalid pipeline state");
             };
 
             log("pipeline child done {x} ({d}) i={d}", .{ @intFromPtr(this), exit_code, idx });
@@ -4347,7 +4347,7 @@ pub const Interpreter = struct {
             parent: ParentPtr,
             io: IO,
         ) *Cmd {
-            var cmd = interpreter.allocator.create(Cmd) catch |err| std.debug.panic("Ruh roh: {any}\n", .{err});
+            var cmd = interpreter.allocator.create(Cmd) catch bun.outOfMemory();
             cmd.* = .{
                 .base = .{ .kind = .cmd, .interpreter = interpreter, .shell = shell_state },
                 .node = node,
@@ -4522,7 +4522,8 @@ pub const Interpreter = struct {
                 this.next();
                 return;
             }
-            unreachable;
+
+            @panic("Expected Cmd child to be Assigns or Expansion. This indicates a bug in Bun. Please file a GitHub issue. ");
         }
 
         fn initSubproc(this: *Cmd) void {
@@ -7115,7 +7116,8 @@ pub const Interpreter = struct {
                 while (!(this.state == .err or this.state == .done)) {
                     switch (this.state) {
                         .waiting_io => return,
-                        .idle, .done, .err => unreachable,
+                        .idle => @panic("Unexpected \"idle\" state in Pwd. This indicates a bug in Bun. Please file a GitHub issue."),
+                        .done, .err => unreachable,
                     }
                 }
 
@@ -9680,7 +9682,7 @@ pub const Interpreter = struct {
 
             pub fn next(this: *Exit) void {
                 switch (this.state) {
-                    .idle => unreachable,
+                    .idle => @panic("Unexpected \"idle\" state in Exit. This indicates a bug in Bun. Please file a GitHub issue."),
                     .waiting_io => {
                         return;
                     },

diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts
@@ -10,7 +10,8 @@ import { mkdir, mkdtemp, realpath, rm, stat } from "fs/promises";
 import { bunEnv, bunExe, runWithErrorPromise, tempDirWithFiles } from "harness";
 import { tmpdir } from "os";
 import { join, sep } from "path";
-import { TestBuilder, sortedShellOutput } from "./util";
+import { createTestBuilder, sortedShellOutput } from "./util";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 $.env(bunEnv);
 $.cwd(process.cwd());
@@ -797,9 +798,10 @@ describe("deno_task", () => {
 
     TestBuilder.command`echo 1 | echo 2 && echo 3`.stdout("2\n3\n").runAsTest("pipe in conditional");
 
-    await TestBuilder.command`echo $(sleep 0.1 && echo 2 & echo 1) | BUN_DEBUG_QUIET_LOGS=1 BUN_TEST_VAR=1 ${BUN} -e 'await process.stdin.pipe(process.stdout)'`
+    TestBuilder.command`echo $(sleep 0.1 && echo 2 & echo 1) | BUN_DEBUG_QUIET_LOGS=1 BUN_TEST_VAR=1 ${BUN} -e 'await process.stdin.pipe(process.stdout)'`
       .stdout("1 2\n")
-      .run();
+      .todo("& not supported")
+      .runAsTest("complicated pipeline");
 
     TestBuilder.command`echo 2 | echo 1 | BUN_TEST_VAR=1 ${BUN} -e 'process.stdin.pipe(process.stdout)'`
       .stdout("1\n")
@@ -834,9 +836,12 @@ describe("deno_task", () => {
   });
 
   describe("redirects", async function igodf() {
-    await TestBuilder.command`echo 5 6 7 > test.txt`.fileEquals("test.txt", "5 6 7\n").run();
+    TestBuilder.command`echo 5 6 7 > test.txt`.fileEquals("test.txt", "5 6 7\n").runAsTest("basic redirect");
 
-    await TestBuilder.command`echo 1 2 3 && echo 1 > test.txt`.stdout("1 2 3\n").fileEquals("test.txt", "1\n").run();
+    TestBuilder.command`echo 1 2 3 && echo 1 > test.txt`
+      .stdout("1 2 3\n")
+      .fileEquals("test.txt", "1\n")
+      .runAsTest("basic redirect with &&");
 
     // subdir
     TestBuilder.command`mkdir subdir && cd subdir && echo 1 2 3 > test.txt`

diff --git a/test/js/bun/shell/commands/basename.test.ts b/test/js/bun/shell/commands/basename.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("basename", async () => {
   TestBuilder.command`basename`.exitCode(1).stdout("").stderr("usage: basename string\n").runAsTest("shows usage");

diff --git a/test/js/bun/shell/commands/dirname.test.ts b/test/js/bun/shell/commands/dirname.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("dirname", async () => {
   TestBuilder.command`dirname`.exitCode(1).stdout("").stderr("usage: dirname string\n").runAsTest("shows usage");

diff --git a/test/js/bun/shell/commands/exit.test.ts b/test/js/bun/shell/commands/exit.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 import { sortedShellOutput } from "../util";
 import { join } from "path";
 

diff --git a/test/js/bun/shell/commands/false.test.ts b/test/js/bun/shell/commands/false.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("false", async () => {
   TestBuilder.command`false`.exitCode(1).runAsTest("works");

diff --git a/test/js/bun/shell/commands/mv.test.ts b/test/js/bun/shell/commands/mv.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 import { sortedShellOutput } from "../util";
 import { join } from "path";
 

diff --git a/test/js/bun/shell/commands/rm.test.ts b/test/js/bun/shell/commands/rm.test.ts
@@ -10,7 +10,8 @@ import { $ } from "bun";
 import path from "path";
 import { mkdirSync, writeFileSync } from "node:fs";
 import { ShellOutput } from "bun";
-import { TestBuilder, sortedShellOutput } from "../util";
+import { createTestBuilder, sortedShellOutput } from "../util";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 const fileExists = async (path: string): Promise<boolean> =>
   $`ls -d ${path}`.then(o => o.stdout.toString() === `${path}\n`);

diff --git a/test/js/bun/shell/commands/seq.test.ts b/test/js/bun/shell/commands/seq.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("seq", async () => {
   TestBuilder.command`seq`

diff --git a/test/js/bun/shell/commands/true.test.ts b/test/js/bun/shell/commands/true.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "../test_builder";
+import { createTestBuilder } from "../test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("true", async () => {
   TestBuilder.command`true`.exitCode(0).runAsTest("works");

diff --git a/test/js/bun/shell/env.positionals.test.ts b/test/js/bun/shell/env.positionals.test.ts
@@ -1,6 +1,7 @@
 import { $, spawn } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "./test_builder";
+import { createTestBuilder } from "./test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 import { bunEnv, bunExe } from "harness";
 import * as path from "node:path";
 

diff --git a/test/js/bun/shell/exec.test.ts b/test/js/bun/shell/exec.test.ts
@@ -1,6 +1,7 @@
 import { $ } from "bun";
 import { describe, test, expect } from "bun:test";
-import { TestBuilder } from "./test_builder";
+import { createTestBuilder } from "./test_builder";
+const TestBuilder = createTestBuilder(import.meta.path);
 import { bunEnv } from "harness";
 
 const BUN = process.argv0;

diff --git a/test/js/bun/shell/leak.test.ts b/test/js/bun/shell/leak.test.ts
@@ -4,7 +4,9 @@ import { bunEnv } from "harness";
 import { appendFileSync, closeSync, openSync, writeFileSync } from "node:fs";
 import { tmpdir, devNull } from "os";
 import { join } from "path";
-import { TestBuilder } from "./util";
+import { createTestBuilder } from "./util";
+const TestBuilder = createTestBuilder(import.meta.path);
+type TestBuilder = InstanceType<typeof TestBuilder>;
 
 $.env(bunEnv);
 $.cwd(process.cwd());
@@ -50,7 +52,7 @@ const TESTS: [name: string, builder: () => TestBuilder, runs?: number][] = [
 ];
 
 describe("fd leak", () => {
-  function fdLeakTest(name: string, builder: () => TestBuilder, runs: number = 500, threshold: number = 5) {
+  function fdLeakTest(name: string, builder: () => TestBuilder, runs: number = 1000, threshold: number = 5) {
     test(`fdleak_${name}`, async () => {
       Bun.gc(true);
       const baseline = openSync(devNull, "r");
@@ -83,13 +85,15 @@ describe("fd leak", () => {
       writeFileSync(tempfile, testcode);
 
       const impl = /* ts */ `
+              const TestBuilder = createTestBuilder(import.meta.path);
+
               const threshold = ${threshold}
               let prev: number | undefined = undefined;
               let prevprev: number | undefined = undefined;
               for (let i = 0; i < ${runs}; i++) {
                 Bun.gc(true);
                 await (async function() {
-                  await ${builder.toString().slice("() =>".length)}.quiet().run()
+                  await ${builder.toString().slice("() =>".length)}.quiet().runAsTest('iter:', i)
                 })()
                 Bun.gc(true);
                 Bun.gc(true);
@@ -111,7 +115,9 @@ describe("fd leak", () => {
         env: bunEnv,
       });
       // console.log('STDOUT:', stdout.toString(), '\n\nSTDERR:', stderr.toString());
-      console.log("\n\nSTDERR:", stderr.toString());
+      if (exitCode != 0) {
+        console.log("\n\nSTDERR:", stderr.toString());
+      }
       expect(exitCode).toBe(0);
     }, 100_000);
   }

diff --git a/test/js/bun/shell/lex.test.ts b/test/js/bun/shell/lex.test.ts
@@ -1,7 +1,8 @@
 import { $ } from "bun";
-import { TestBuilder, redirect } from "./util";
+import { createTestBuilder, redirect } from "./util";
 import { shellInternals } from "bun:internal-for-testing";
 const { lex } = shellInternals;
+const TestBuilder = createTestBuilder(import.meta.path);
 
 const BUN = process.argv0;
 

diff --git a/test/js/bun/shell/parse.test.ts b/test/js/bun/shell/parse.test.ts
@@ -1,6 +1,7 @@
-import { TestBuilder, redirect } from "./util";
+import { createTestBuilder, redirect } from "./util";
 import { shellInternals } from "bun:internal-for-testing";
 const { parse } = shellInternals;
+const TestBuilder = createTestBuilder(import.meta.path);
 
 describe("parse shell", () => {
   test("basic", () => {