Skip to content

Support test("name", {options}, () => {}) in addition to test("name", () => {}, {options}) #21531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions packages/bun-types/test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,29 @@ declare module "bun:test" {
each<T extends Readonly<[any, ...any[]]>>(
table: readonly T[],
): (label: DescribeLabel, fn: (...args: [...T]) => void | Promise<unknown>, options?: number | TestOptions) => void;
each<T extends Readonly<[any, ...any[]]>>(
table: readonly T[],
): (label: DescribeLabel, options: number | TestOptions, fn: (...args: [...T]) => void | Promise<unknown>) => void;
each<T extends any[]>(
table: readonly T[],
): (
label: DescribeLabel,
fn: (...args: Readonly<T>) => void | Promise<unknown>,
options?: number | TestOptions,
) => void;
each<T extends any[]>(
table: readonly T[],
): (
label: DescribeLabel,
options: number | TestOptions,
fn: (...args: Readonly<T>) => void | Promise<unknown>,
) => void;
each<T>(
table: T[],
): (label: DescribeLabel, fn: (...args: T[]) => void | Promise<unknown>, options?: number | TestOptions) => void;
each<T>(
table: T[],
): (label: DescribeLabel, options: number | TestOptions, fn: (...args: T[]) => void | Promise<unknown>) => void;
}
/**
* Describes a group of related tests.
Expand Down Expand Up @@ -379,6 +392,18 @@ declare module "bun:test" {
*/
options?: number | TestOptions,
): void;
(
label: string,
/**
* - If a `number`, sets the timeout for the test in milliseconds.
* - If an `object`, sets the options for the test.
* - `timeout` sets the timeout for the test in milliseconds.
* - `retry` sets the number of times to retry the test if it fails.
* - `repeats` sets the number of times to repeat the test, regardless of whether it passed or failed.
*/
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
): void;
/**
* Skips all other tests, except this test when run with the `--only` option.
*
Expand All @@ -391,6 +416,11 @@ declare module "bun:test" {
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
): void;
only(
label: string,
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
): void;
/**
* Skips this test.
*
Expand All @@ -403,6 +433,11 @@ declare module "bun:test" {
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
): void;
skip(
label: string,
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
): void;
/**
* Marks this test as to be written or to be fixed.
*
Expand All @@ -420,6 +455,11 @@ declare module "bun:test" {
fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
): void;
todo(
label: string,
options: number | TestOptions,
fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
): void;
/**
* Marks this test as failing.
*
Expand All @@ -440,6 +480,11 @@ declare module "bun:test" {
fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
): void;
failing(
label: string,
options: number | TestOptions,
fn?: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
): void;
/**
* Runs this test, if `condition` is true.
*
Expand All @@ -454,6 +499,13 @@ declare module "bun:test" {
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
) => void;
if(
condition: boolean,
): (
label: string,
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
) => void;
/**
* Skips this test, if `condition` is true.
*
Expand All @@ -466,6 +518,13 @@ declare module "bun:test" {
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
) => void;
skipIf(
condition: boolean,
): (
label: string,
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
) => void;
/**
* Marks this test as to be written or to be fixed, if `condition` is true.
*
Expand All @@ -478,6 +537,13 @@ declare module "bun:test" {
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
options?: number | TestOptions,
) => void;
todoIf(
condition: boolean,
): (
label: string,
options: number | TestOptions,
fn: (() => void | Promise<unknown>) | ((done: (err?: unknown) => void) => void),
) => void;
/**
* Returns a function that runs for each item in `table`.
*
Expand All @@ -486,12 +552,21 @@ declare module "bun:test" {
each<T extends Readonly<[any, ...any[]]>>(
table: readonly T[],
): (label: string, fn: (...args: [...T]) => void | Promise<unknown>, options?: number | TestOptions) => void;
each<T extends Readonly<[any, ...any[]]>>(
table: readonly T[],
): (label: string, options: number | TestOptions, fn: (...args: [...T]) => void | Promise<unknown>) => void;
each<T extends any[]>(
table: readonly T[],
): (label: string, fn: (...args: Readonly<T>) => void | Promise<unknown>, options?: number | TestOptions) => void;
each<T extends any[]>(
table: readonly T[],
): (label: string, options: number | TestOptions, fn: (...args: Readonly<T>) => void | Promise<unknown>) => void;
each<T>(
table: T[],
): (label: string, fn: (...args: T[]) => void | Promise<unknown>, options?: number | TestOptions) => void;
each<T>(
table: T[],
): (label: string, options: number | TestOptions, fn: (...args: T[]) => void | Promise<unknown>) => void;
}
/**
* Runs a test.
Expand Down
8 changes: 8 additions & 0 deletions simple.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Test just the original syntax first to make sure it works
import { test, expect } from "bun:test";

test("original syntax still works", () => {
expect(true).toBe(true);
}, { timeout: 1000 });

console.log("Original syntax test defined successfully");
19 changes: 10 additions & 9 deletions src/bun.js/test/jest.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1802,18 +1802,14 @@ inline fn createScope(
comptime tag: Tag,
) bun.JSError!JSValue {
const this = callframe.this();
const arguments = callframe.arguments_old(3);
const args = arguments.slice();

if (args.len == 0) {
if (callframe.argumentsCount() == 0) {
return globalThis.throwPretty("{s} expects a description or function", .{signature});
}

var description = args[0];
var function = if (args.len > 1) args[1] else .zero;
var options = if (args.len > 2) args[2] else .zero;
var description, var function, var options = callframe.argumentsAsArray(3);

if (args.len == 1 and description.isFunction()) {
if (description.isFunction()) {
function = description;
description = .zero;
} else {
Expand All @@ -1827,9 +1823,14 @@ inline fn createScope(
return globalThis.throwPretty("{s} expects first argument to be a named class, named function, number, or string", .{signature});
}

// Handle the case where options are the second parameter and function is the third
if (!function.isFunction()) {
std.mem.swap(JSValue, &function, &options);
}

if (!function.isFunction()) {
if (tag != .todo and tag != .skip) {
return globalThis.throwPretty("{s} expects second argument to be a function", .{signature});
return globalThis.throwPretty("{s} expects second argument to be a function or object, and third argument to be a function", .{signature});
}
}
}
Expand Down Expand Up @@ -1866,7 +1867,7 @@ inline fn createScope(

var timeout_ms: u32 = std.math.maxInt(u32);
if (options.isNumber()) {
timeout_ms = @as(u32, @intCast(@max(try args[2].coerce(i32, globalThis), 0)));
timeout_ms = @as(u32, @intCast(@max(try options.coerce(i32, globalThis), 0)));
} else if (options.isObject()) {
if (try options.get(globalThis, "timeout")) |timeout| {
if (!timeout.isNumber()) {
Expand Down
25 changes: 25 additions & 0 deletions test/integration/bun-types/fixture/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,31 @@ describe("bun:test", () => {
expect(undefined).toBeUndefined();
expect(undefined).not.toBeDefined();
});

// Test the new syntax - options as second parameter
test("new syntax - options as second parameter", { timeout: 1000 }, () => {
expect(true).toBe(true);
});

// Test with number options as second parameter
test("new syntax - number as second parameter", 500, () => {
expect(true).toBe(true);
});

// Test with only() using new syntax
test.only("only with new syntax", { timeout: 1000 }, () => {
expect(true).toBe(true);
});

// Test with skip() using new syntax
test.skip("skip with new syntax", { timeout: 1000 }, () => {
expect(true).toBe(true);
});

// Test with todo() using new syntax
test.todo("todo with new syntax", { timeout: 1000 }, () => {
expect(true).toBe(true);
});
});

// inference should work when data is passed directly in
Expand Down
2 changes: 1 addition & 1 deletion test/internal/ban-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
" == undefined": 0,
"!= alloc.ptr": 0,
"!= allocator.ptr": 0,
".arguments_old(": 280,
".arguments_old(": 279,
".stdDir()": 40,
".stdFile()": 18,
"// autofix": 168,
Expand Down
35 changes: 35 additions & 0 deletions test/js/bun/test/options-as-second-parameter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Test that both the original and new syntax work correctly
// This is a regression test to ensure the new parameter order doesn't break existing usage

describe("test() parameter order", () => {
// Original syntax: test(name, fn, options)
test(
"original syntax works",
() => {
expect(true).toBe(true);
},
{ timeout: 1000 },
);

test("original syntax with number timeout", () => {
expect(true).toBe(true);
}, 500);

// Vitest compatability syntax: test(name, options, fn)
test("vitest compatability syntax - options object as second parameter", { timeout: 1000 }, () => {
expect(true).toBe(true);
});

test("vitest compatability syntax - number as second parameter", 500, () => {
expect(true).toBe(true);
});

// Test other methods work with vitest compatability syntax
test.skip("skip with vitest compatability syntax", { timeout: 1000 }, () => {
expect(true).toBe(true);
});

test.todo("todo with vitest-compatible syntax", { timeout: 1000 });

test.skip("skip with options but no function", 300);
});
Loading