From 958fdc58e86873bf2677b90ef4366891cbbce105 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 11 Apr 2024 21:22:49 -0700 Subject: [PATCH 1/5] Do not run tests outside test scope --- src/js/private.d.ts | 2 +- test/js/bun/shell/bunshell.test.ts | 3 +- test/js/bun/shell/commands/basename.test.ts | 3 +- test/js/bun/shell/commands/dirname.test.ts | 3 +- test/js/bun/shell/commands/exit.test.ts | 3 +- test/js/bun/shell/commands/false.test.ts | 3 +- test/js/bun/shell/commands/mv.test.ts | 3 +- test/js/bun/shell/commands/rm.test.ts | 3 +- test/js/bun/shell/commands/seq.test.ts | 3 +- test/js/bun/shell/commands/true.test.ts | 3 +- test/js/bun/shell/env.positionals.test.ts | 3 +- test/js/bun/shell/exec.test.ts | 3 +- test/js/bun/shell/leak.test.ts | 5 +- test/js/bun/shell/lex.test.ts | 3 +- test/js/bun/shell/parse.test.ts | 3 +- test/js/bun/shell/test_builder.ts | 493 ++++++++++---------- test/js/bun/shell/util.ts | 4 +- 17 files changed, 290 insertions(+), 253 deletions(-) diff --git a/src/js/private.d.ts b/src/js/private.d.ts index 9b4474c031b46..2b8d7d5bf6b70 100644 --- 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/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index b69fa71006b80..efb139af162ad 100644 --- 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()); diff --git a/test/js/bun/shell/commands/basename.test.ts b/test/js/bun/shell/commands/basename.test.ts index 93ebd3a59c125..44c77ba78597a 100644 --- 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 index a5f9f2b92a25f..f054d01ce9261 100644 --- 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 index 241a847a8d46e..5e757a9c80a4d 100644 --- 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 index f07de9d8fd375..785a495ee99c2 100644 --- 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 index ceea68aca2cb0..0a4fdec5201b5 100644 --- 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 index cff064032413c..51987e5984b25 100644 --- 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 => $`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 index 7052d11f88326..54504faa8bd91 100644 --- 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 index 490ab741f99b5..4dc491713e63c 100644 --- 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 index 802900be1dfac..407c54b1842cc 100644 --- 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 index 49ecbf5b10fa6..498add1fff59d 100644 --- 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 index fbfb4992a3720..056d8d1f089b4 100644 --- a/test/js/bun/shell/leak.test.ts +++ b/test/js/bun/shell/leak.test.ts @@ -4,7 +4,8 @@ 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); $.env(bunEnv); $.cwd(process.cwd()); @@ -83,6 +84,8 @@ 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; diff --git a/test/js/bun/shell/lex.test.ts b/test/js/bun/shell/lex.test.ts index 88b5cd883723c..b3bb1ba5c5722 100644 --- 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 index dbbb4dea21ce0..0f42686f68354 100644 --- 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", () => { diff --git a/test/js/bun/shell/test_builder.ts b/test/js/bun/shell/test_builder.ts index f9eaee87dce5f..ae905279e4c7d 100644 --- a/test/js/bun/shell/test_builder.ts +++ b/test/js/bun/shell/test_builder.ts @@ -1,4 +1,3 @@ -import { describe, test, afterAll, beforeAll, expect } from "bun:test"; import { ShellError, ShellOutput } from "bun"; import { ShellPromise } from "bun"; // import { tempDirWithFiles } from "harness"; @@ -6,34 +5,45 @@ import { join } from "node:path"; import * as os from "node:os"; import * as fs from "node:fs"; -export class TestBuilder { - private promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error }; - private _testName: string | undefined = undefined; +export function createTestBuilder(path: string) { + var { describe, test, afterAll, beforeAll, expect, beforeEach, afterEach } = Bun.jest(path); - private expected_stdout: string | ((stdout: string, tempdir: string) => void) = ""; - private expected_stderr: string | ((stderr: string, tempdir: string) => void) = ""; - private expected_exit_code: number = 0; - private expected_error: ShellError | string | boolean | undefined = undefined; - private file_equals: { [filename: string]: string } = {}; - private _doesNotExist: string[] = []; - private _timeout: number | undefined = undefined; + var insideTestScope = false; + beforeEach(() => { + insideTestScope = true; + }); + afterEach(() => { + insideTestScope = false; + }); - private tempdir: string | undefined = undefined; - private _env: { [key: string]: string } | undefined = undefined; + class TestBuilder { + private promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error }; + private _testName: string | undefined = undefined; - private __todo: boolean | string = false; + private expected_stdout: string | ((stdout: string, tempdir: string) => void) = ""; + private expected_stderr: string | ((stderr: string, tempdir: string) => void) = ""; + private expected_exit_code: number = 0; + private expected_error: ShellError | string | boolean | undefined = undefined; + private file_equals: { [filename: string]: string } = {}; + private _doesNotExist: string[] = []; + private _timeout: number | undefined = undefined; - static UNEXPECTED_SUBSHELL_ERROR_OPEN = - "Unexpected `(`, subshells are currently not supported right now. Escape the `(` or open a GitHub issue."; + private tempdir: string | undefined = undefined; + private _env: { [key: string]: string } | undefined = undefined; - static UNEXPECTED_SUBSHELL_ERROR_CLOSE = - "Unexpected `)`, subshells are currently not supported right now. Escape the `)` or open a GitHub issue."; + private __todo: boolean | string = false; - constructor(promise: TestBuilder["promise"]) { - this.promise = promise; - } + static UNEXPECTED_SUBSHELL_ERROR_OPEN = + "Unexpected `(`, subshells are currently not supported right now. Escape the `(` or open a GitHub issue."; + + static UNEXPECTED_SUBSHELL_ERROR_CLOSE = + "Unexpected `)`, subshells are currently not supported right now. Escape the `)` or open a GitHub issue."; - /** + constructor(promise: TestBuilder["promise"]) { + this.promise = promise; + } + + /** * Start the test builder with a command: * * @example @@ -43,259 +53,270 @@ export class TestBuilder { * TestBuilder.command`echo hi!`.stdout('hi!\n').runAsTest('echo works') * ``` */ - static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder { - try { - if (process.env.BUN_DEBUG_SHELL_LOG_CMD === "1") console.info("[ShellTestBuilder] Cmd", strings.join("")); - const promise = Bun.$(strings, ...expressions).nothrow(); - const This = new this({ type: "ok", val: promise }); - This._testName = strings.join(""); - return This; - } catch (err) { - return new this({ type: "err", val: err as Error }); + static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder { + try { + if (process.env.BUN_DEBUG_SHELL_LOG_CMD === "1") console.info("[ShellTestBuilder] Cmd", strings.join("")); + const promise = Bun.$(strings, ...expressions).nothrow(); + const This = new this({ type: "ok", val: promise }); + This._testName = strings.join(""); + return This; + } catch (err) { + return new this({ type: "err", val: err as Error }); + } } - } - - directory(path: string): this { - const tempdir = this.getTempDir(); - fs.mkdirSync(join(tempdir, path), { recursive: true }); - return this; - } - doesNotExist(path: string): this { - this._doesNotExist.push(path); - return this; - } + directory(path: string): this { + const tempdir = this.getTempDir(); + fs.mkdirSync(join(tempdir, path), { recursive: true }); + return this; + } - /** - * Create a file in a temp directory - * @param path Path to the new file, this will be inside the TestBuilder's temp directory - * @param contents Contents of the new file - * @returns - * - * @example - * ```ts - * TestBuilder.command`ls .` - * .file('hi.txt', 'hi!') - * .file('hello.txt', 'hello!') - * .runAsTest('List files') - * ``` - */ - file(path: string, contents: string): this { - const tempdir = this.getTempDir(); - fs.writeFileSync(join(tempdir, path), contents); - return this; - } + doesNotExist(path: string): this { + this._doesNotExist.push(path); + return this; + } - env(env: { [key: string]: string }): this { - this._env = env; - return this; - } + /** + * Create a file in a temp directory + * @param path Path to the new file, this will be inside the TestBuilder's temp directory + * @param contents Contents of the new file + * @returns + * + * @example + * ```ts + * TestBuilder.command`ls .` + * .file('hi.txt', 'hi!') + * .file('hello.txt', 'hello!') + * .runAsTest('List files') + * ``` + */ + file(path: string, contents: string): this { + const tempdir = this.getTempDir(); + fs.writeFileSync(join(tempdir, path), contents); + return this; + } - quiet(): this { - if (this.promise.type === "ok") { - this.promise.val.quiet(); + env(env: { [key: string]: string }): this { + this._env = env; + return this; } - return this; - } - testName(name: string): this { - this._testName = name; - return this; - } + quiet(): this { + if (this.promise.type === "ok") { + this.promise.val.quiet(); + } + return this; + } - /** - * Expect output from stdout - * - * @param expected - can either be a string or a function which itself calls `expect()` - */ - stdout(expected: string | ((stdout: string, tempDir: string) => void)): this { - this.expected_stdout = expected; - return this; - } + testName(name: string): this { + this._testName = name; + return this; + } - stderr(expected: string | ((stderr: string, tempDir: string) => void)): this { - this.expected_stderr = expected; - return this; - } + /** + * Expect output from stdout + * + * @param expected - can either be a string or a function which itself calls `expect()` + */ + stdout(expected: string | ((stdout: string, tempDir: string) => void)): this { + this.expected_stdout = expected; + return this; + } - /** - * Makes this test use a temp directory: - * - The shell's cwd will be set to the temp directory - * - All FS functions on the `TestBuilder` will use this temp directory. - * @returns - */ - ensureTempDir(): this { - this.getTempDir(); - return this; - } + stderr(expected: string | ((stderr: string, tempDir: string) => void)): this { + this.expected_stderr = expected; + return this; + } - error(expected?: ShellError | string | boolean): this { - if (expected === undefined || expected === true) { - this.expected_error = true; - } else if (expected === false) { - this.expected_error = false; - } else { - this.expected_error = expected; + /** + * Makes this test use a temp directory: + * - The shell's cwd will be set to the temp directory + * - All FS functions on the `TestBuilder` will use this temp directory. + * @returns + */ + ensureTempDir(): this { + this.getTempDir(); + return this; } - return this; - } - exitCode(expected: number): this { - this.expected_exit_code = expected; - return this; - } + error(expected?: ShellError | string | boolean): this { + if (expected === undefined || expected === true) { + this.expected_error = true; + } else if (expected === false) { + this.expected_error = false; + } else { + this.expected_error = expected; + } + return this; + } - fileEquals(filename: string, expected: string): this { - this.getTempDir(); - this.file_equals[filename] = expected; - return this; - } + exitCode(expected: number): this { + this.expected_exit_code = expected; + return this; + } - static tmpdir(): string { - const tmp = os.tmpdir(); - return fs.mkdtempSync(join(tmp, "test_builder")); - } + fileEquals(filename: string, expected: string): this { + this.getTempDir(); + this.file_equals[filename] = expected; + return this; + } - setTempdir(tempdir: string): this { - this.tempdir = tempdir; - if (this.promise.type === "ok") { - this.promise.val.cwd(this.tempdir!); + static tmpdir(): string { + const tmp = os.tmpdir(); + return fs.mkdtempSync(join(tmp, "test_builder")); } - return this; - } - getTempDir(): string { - if (this.tempdir === undefined) { - this.tempdir = TestBuilder.tmpdir(); + setTempdir(tempdir: string): this { + this.tempdir = tempdir; if (this.promise.type === "ok") { this.promise.val.cwd(this.tempdir!); } - return this.tempdir!; + return this; } - return this.tempdir; - } - timeout(ms: number): this { - this._timeout = ms; - return this; - } - - async run(): Promise { - if (this.promise.type === "err") { - const err = this.promise.val; - if (this.expected_error === undefined) throw err; - if (this.expected_error === true) return undefined; - if (this.expected_error === false) expect(err).toBeUndefined(); - if (typeof this.expected_error === "string") { - expect(err.message).toEqual(this.expected_error); - } else if (this.expected_error instanceof ShellError) { - expect(err).toBeInstanceOf(ShellError); - const e = err as ShellError; - expect(e.exitCode).toEqual(this.expected_error.exitCode); - expect(e.stdout.toString()).toEqual(this.expected_error.stdout.toString()); - expect(e.stderr.toString()).toEqual(this.expected_error.stderr.toString()); + getTempDir(): string { + if (this.tempdir === undefined) { + this.tempdir = TestBuilder.tmpdir(); + if (this.promise.type === "ok") { + this.promise.val.cwd(this.tempdir!); + } + return this.tempdir!; } - return undefined; + return this.tempdir; } - const output = await (this._env !== undefined ? this.promise.val.env(this._env) : this.promise.val); + timeout(ms: number): this { + this._timeout = ms; + return this; + } - const { stdout, stderr, exitCode } = output!; - const tempdir = this.tempdir || "NO_TEMP_DIR"; - if (this.expected_stdout !== undefined) { - if (typeof this.expected_stdout === "string") { - expect(stdout.toString()).toEqual(this.expected_stdout.replaceAll("$TEMP_DIR", tempdir)); - } else { - this.expected_stdout(stdout.toString(), tempdir); + async run(): Promise { + if (!insideTestScope) { + const err = new Error("TestBuilder.run() must be called inside a test scope"); + test("TestBuilder.run() must be called inside a test scope", () => { + throw err; + }); + return Promise.resolve(undefined); } - } - if (this.expected_stderr !== undefined) { - if (typeof this.expected_stderr === "string") { - expect(stderr.toString()).toEqual(this.expected_stderr.replaceAll("$TEMP_DIR", tempdir)); - } else { - this.expected_stderr(stderr.toString(), tempdir); + + if (this.promise.type === "err") { + const err = this.promise.val; + if (this.expected_error === undefined) throw err; + if (this.expected_error === true) return undefined; + if (this.expected_error === false) expect(err).toBeUndefined(); + if (typeof this.expected_error === "string") { + expect(err.message).toEqual(this.expected_error); + } else if (this.expected_error instanceof ShellError) { + expect(err).toBeInstanceOf(ShellError); + const e = err as ShellError; + expect(e.exitCode).toEqual(this.expected_error.exitCode); + expect(e.stdout.toString()).toEqual(this.expected_error.stdout.toString()); + expect(e.stderr.toString()).toEqual(this.expected_error.stderr.toString()); + } + return undefined; } - } - if (this.expected_exit_code !== undefined) expect(exitCode).toEqual(this.expected_exit_code); - for (const [filename, expected] of Object.entries(this.file_equals)) { - const actual = await Bun.file(join(this.tempdir!, filename)).text(); - expect(actual).toEqual(expected); - } + const output = await (this._env !== undefined ? this.promise.val.env(this._env) : this.promise.val); - for (const fsname of this._doesNotExist) { - expect(fs.existsSync(join(this.tempdir!, fsname))).toBeFalsy(); - } + const { stdout, stderr, exitCode } = output!; + const tempdir = this.tempdir || "NO_TEMP_DIR"; + if (this.expected_stdout !== undefined) { + if (typeof this.expected_stdout === "string") { + expect(stdout.toString()).toEqual(this.expected_stdout.replaceAll("$TEMP_DIR", tempdir)); + } else { + this.expected_stdout(stdout.toString(), tempdir); + } + } + if (this.expected_stderr !== undefined) { + if (typeof this.expected_stderr === "string") { + expect(stderr.toString()).toEqual(this.expected_stderr.replaceAll("$TEMP_DIR", tempdir)); + } else { + this.expected_stderr(stderr.toString(), tempdir); + } + } + if (this.expected_exit_code !== undefined) expect(exitCode).toEqual(this.expected_exit_code); - // return output; - } + for (const [filename, expected] of Object.entries(this.file_equals)) { + const actual = await Bun.file(join(this.tempdir!, filename)).text(); + expect(actual).toEqual(expected); + } - todo(reason?: string): this { - this.__todo = typeof reason === "string" ? reason : true; - return this; - } + for (const fsname of this._doesNotExist) { + expect(fs.existsSync(join(this.tempdir!, fsname))).toBeFalsy(); + } - runAsTest(name: string) { - // biome-ignore lint/complexity/noUselessThisAlias: - const tb = this; - if (this.__todo) { - test.todo(typeof this.__todo === "string" ? `${name} skipped: ${this.__todo}` : name, async () => { - await tb.run(); - }); - } else { - test( - name, - async () => { + // return output; + } + + todo(reason?: string): this { + this.__todo = typeof reason === "string" ? reason : true; + return this; + } + + runAsTest(name: string) { + // biome-ignore lint/complexity/noUselessThisAlias: + const tb = this; + if (this.__todo) { + test.todo(typeof this.__todo === "string" ? `${name} skipped: ${this.__todo}` : name, async () => { await tb.run(); - }, - this._timeout, - ); + }); + } else { + test( + name, + async () => { + await tb.run(); + }, + this._timeout, + ); + } } + + // async run(): Promise { + // async function doTest(tb: TestBuilder) { + // if (tb.promise.type === "err") { + // const err = tb.promise.val; + // if (tb.expected_error === undefined) throw err; + // if (tb.expected_error === true) return undefined; + // if (tb.expected_error === false) expect(err).toBeUndefined(); + // if (typeof tb.expected_error === "string") { + // expect(err.message).toEqual(tb.expected_error); + // } + // return undefined; + // } + + // const output = await tb.promise.val; + + // const { stdout, stderr, exitCode } = output!; + // if (tb.expected_stdout !== undefined) expect(stdout.toString()).toEqual(tb.expected_stdout); + // if (tb.expected_stderr !== undefined) expect(stderr.toString()).toEqual(tb.expected_stderr); + // if (tb.expected_exit_code !== undefined) expect(exitCode).toEqual(tb.expected_exit_code); + + // for (const [filename, expected] of Object.entries(tb.file_equals)) { + // const actual = await Bun.file(filename).text(); + // expect(actual).toEqual(expected); + // } + // return output; + // } + + // if (this._testName !== undefined) { + // test(this._testName, async () => { + // await doTest(this); + // }); + // } + // await doTest(this); + // } } + function generateRandomString(length: number): string { + const characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + let result = ""; + const charactersLength = characters.length; - // async run(): Promise { - // async function doTest(tb: TestBuilder) { - // if (tb.promise.type === "err") { - // const err = tb.promise.val; - // if (tb.expected_error === undefined) throw err; - // if (tb.expected_error === true) return undefined; - // if (tb.expected_error === false) expect(err).toBeUndefined(); - // if (typeof tb.expected_error === "string") { - // expect(err.message).toEqual(tb.expected_error); - // } - // return undefined; - // } - - // const output = await tb.promise.val; - - // const { stdout, stderr, exitCode } = output!; - // if (tb.expected_stdout !== undefined) expect(stdout.toString()).toEqual(tb.expected_stdout); - // if (tb.expected_stderr !== undefined) expect(stderr.toString()).toEqual(tb.expected_stderr); - // if (tb.expected_exit_code !== undefined) expect(exitCode).toEqual(tb.expected_exit_code); - - // for (const [filename, expected] of Object.entries(tb.file_equals)) { - // const actual = await Bun.file(filename).text(); - // expect(actual).toEqual(expected); - // } - // return output; - // } - - // if (this._testName !== undefined) { - // test(this._testName, async () => { - // await doTest(this); - // }); - // } - // await doTest(this); - // } -} -function generateRandomString(length: number): string { - const characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; - let result = ""; - const charactersLength = characters.length; + for (let i = 0; i < length; i++) { + result += characters.charAt(Math.floor(Math.random() * charactersLength)); + } - for (let i = 0; i < length; i++) { - result += characters.charAt(Math.floor(Math.random() * charactersLength)); + return result; } - return result; + return TestBuilder; } diff --git a/test/js/bun/shell/util.ts b/test/js/bun/shell/util.ts index cafe83ce04fb6..08a632d9b2890 100644 --- a/test/js/bun/shell/util.ts +++ b/test/js/bun/shell/util.ts @@ -4,9 +4,9 @@ import { ShellPromise } from "bun"; import { tempDirWithFiles } from "harness"; import { join } from "node:path"; import * as fs from "node:fs"; -import { TestBuilder } from "./test_builder"; +import { createTestBuilder } from "./test_builder"; -export { TestBuilder }; +export { createTestBuilder }; declare module "bun" { // Define the additional methods From d033c9fbe00e1500058f9c21746d21cf5a20c637 Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:49:58 +0200 Subject: [PATCH 2/5] Fix tests --- test/js/bun/shell/bunshell.test.ts | 12 ++++++++---- test/js/bun/shell/leak.test.ts | 6 ++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index efb139af162ad..3fbe3da4b2710 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -798,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") @@ -835,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/leak.test.ts b/test/js/bun/shell/leak.test.ts index 056d8d1f089b4..52248efd70204 100644 --- a/test/js/bun/shell/leak.test.ts +++ b/test/js/bun/shell/leak.test.ts @@ -92,7 +92,7 @@ describe("fd leak", () => { 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); @@ -114,7 +114,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); } From 2d7aec06aae035b81cd202664f77b4287b1d1890 Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Sun, 14 Apr 2024 20:19:32 +0200 Subject: [PATCH 3/5] Fix type errors and remove potentially precarious uses of unreachable --- src/shell/interpreter.zig | 15 ++++++++------ test/js/bun/shell/leak.test.ts | 3 ++- test/js/bun/shell/test_builder.ts | 34 +++++++++++++++---------------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 0bbc0416ece3b..bdb5e45573a6d 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -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."); } }; @@ -3237,7 +3237,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 }); @@ -4263,6 +4263,7 @@ pub const Interpreter = struct { (if (this.stdout) |*stdout| stdout.closed() else true) and (if (this.stderr) |*stderr| stderr.closed() else true); log("BufferedIOClosed(0x{x}) all_closed={any} stdin={any} stdout={any} stderr={any}", .{ @intFromPtr(this), ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true }); + std.debug.print("all_closed={any} stdin={any} stdout={any} stderr={any}\n", .{ ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true }); return ret; } @@ -4528,7 +4529,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 { @@ -7121,7 +7123,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, } } @@ -9686,7 +9689,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/leak.test.ts b/test/js/bun/shell/leak.test.ts index 52248efd70204..980510c86585a 100644 --- a/test/js/bun/shell/leak.test.ts +++ b/test/js/bun/shell/leak.test.ts @@ -6,6 +6,7 @@ import { tmpdir, devNull } from "os"; import { join } from "path"; import { createTestBuilder } from "./util"; const TestBuilder = createTestBuilder(import.meta.path); +type TestBuilder = InstanceType; $.env(bunEnv); $.cwd(process.cwd()); @@ -51,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"); diff --git a/test/js/bun/shell/test_builder.ts b/test/js/bun/shell/test_builder.ts index ae905279e4c7d..a43aa762a681d 100644 --- a/test/js/bun/shell/test_builder.ts +++ b/test/js/bun/shell/test_builder.ts @@ -17,29 +17,29 @@ export function createTestBuilder(path: string) { }); class TestBuilder { - private promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error }; - private _testName: string | undefined = undefined; + promise: { type: "ok"; val: ShellPromise } | { type: "err"; val: Error }; + _testName: string | undefined = undefined; - private expected_stdout: string | ((stdout: string, tempdir: string) => void) = ""; - private expected_stderr: string | ((stderr: string, tempdir: string) => void) = ""; - private expected_exit_code: number = 0; - private expected_error: ShellError | string | boolean | undefined = undefined; - private file_equals: { [filename: string]: string } = {}; - private _doesNotExist: string[] = []; - private _timeout: number | undefined = undefined; + expected_stdout: string | ((stdout: string, tempdir: string) => void) = ""; + expected_stderr: string | ((stderr: string, tempdir: string) => void) = ""; + expected_exit_code: number = 0; + expected_error: ShellError | string | boolean | undefined = undefined; + file_equals: { [filename: string]: string } = {}; + _doesNotExist: string[] = []; + _timeout: number | undefined = undefined; - private tempdir: string | undefined = undefined; - private _env: { [key: string]: string } | undefined = undefined; + tempdir: string | undefined = undefined; + _env: { [key: string]: string } | undefined = undefined; - private __todo: boolean | string = false; + __todo: boolean | string = false; - static UNEXPECTED_SUBSHELL_ERROR_OPEN = + UNEXPECTED_SUBSHELL_ERROR_OPEN = "Unexpected `(`, subshells are currently not supported right now. Escape the `(` or open a GitHub issue."; - static UNEXPECTED_SUBSHELL_ERROR_CLOSE = + UNEXPECTED_SUBSHELL_ERROR_CLOSE = "Unexpected `)`, subshells are currently not supported right now. Escape the `)` or open a GitHub issue."; - constructor(promise: TestBuilder["promise"]) { + public constructor(promise: TestBuilder["promise"]) { this.promise = promise; } @@ -53,7 +53,7 @@ export function createTestBuilder(path: string) { * TestBuilder.command`echo hi!`.stdout('hi!\n').runAsTest('echo works') * ``` */ - static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder { + public static command(strings: TemplateStringsArray, ...expressions: any[]): TestBuilder { try { if (process.env.BUN_DEBUG_SHELL_LOG_CMD === "1") console.info("[ShellTestBuilder] Cmd", strings.join("")); const promise = Bun.$(strings, ...expressions).nothrow(); @@ -65,7 +65,7 @@ export function createTestBuilder(path: string) { } } - directory(path: string): this { + public directory(path: string): this { const tempdir = this.getTempDir(); fs.mkdirSync(join(tempdir, path), { recursive: true }); return this; From c53b6a6055db0f91274c4354dc875208451bdbbf Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Sun, 14 Apr 2024 20:21:07 +0200 Subject: [PATCH 4/5] yoops --- src/shell/interpreter.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index bdb5e45573a6d..7f225719d2e36 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -4263,7 +4263,6 @@ pub const Interpreter = struct { (if (this.stdout) |*stdout| stdout.closed() else true) and (if (this.stderr) |*stderr| stderr.closed() else true); log("BufferedIOClosed(0x{x}) all_closed={any} stdin={any} stdout={any} stderr={any}", .{ @intFromPtr(this), ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true }); - std.debug.print("all_closed={any} stdin={any} stdout={any} stderr={any}\n", .{ ret, if (this.stdin) |stdin| stdin else true, if (this.stdout) |*stdout| stdout.closed() else true, if (this.stderr) |*stderr| stderr.closed() else true }); return ret; } @@ -4352,7 +4351,8 @@ pub const Interpreter = struct { io: IO, ) *Cmd { var cmd = interpreter.allocator.create(Cmd) catch |err| { - std.debug.print("Ruh roh: {any}\n", .{err}); + _ = err; // autofix + @panic("Ruh roh"); }; cmd.* = .{ From 35a1de0a0926d88243ce7b2773f7372d1a7b5a4a Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Mon, 15 Apr 2024 17:51:15 +0200 Subject: [PATCH 5/5] Remove all instances of "Ruh roh" --- src/shell/interpreter.zig | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 7f225719d2e36..2e2cb5f280e9b 100644 --- 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"); } @@ -2910,10 +2910,7 @@ pub const Interpreter = struct { parent: ParentPtr, io: IO, ) *Binary { - var binary = interpreter.allocator.create(Binary) catch |err| { - std.debug.print("Ruh roh: {any}\n", .{err}); - @panic("Ruh roh"); - }; + var binary = interpreter.allocator.create(Binary) catch bun.outOfMemory(); binary.node = node; binary.base = .{ .kind = .binary, .interpreter = interpreter, .shell = shell_state }; binary.parent = parent; @@ -4350,11 +4347,7 @@ pub const Interpreter = struct { parent: ParentPtr, io: IO, ) *Cmd { - var cmd = interpreter.allocator.create(Cmd) catch |err| { - _ = err; // autofix - - @panic("Ruh roh"); - }; + var cmd = interpreter.allocator.create(Cmd) catch bun.outOfMemory(); cmd.* = .{ .base = .{ .kind = .cmd, .interpreter = interpreter, .shell = shell_state }, .node = node,