Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
11 changes: 10 additions & 1 deletion src/runtime/test_runner/expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,14 +836,17 @@ impl Expect {
let prev_unhandled_pending_rejection_to_capture = vm.unhandled_pending_rejection_to_capture;
vm.unhandled_pending_rejection_to_capture = Some(&raw mut return_value);
vm.on_unhandled_rejection = VirtualMachine::on_quiet_unhandled_rejection_handler_capture_value;
return_value_from_function = match value.call(global_this, JSValue::UNDEFINED, &[]) {
let call_result = value.call(global_this, JSValue::UNDEFINED, &[]);
let threw_sync = call_result.is_err();
return_value_from_function = match call_result {
Ok(v) => v,
Err(err) => global_this.take_exception(err),
};
vm.unhandled_pending_rejection_to_capture = prev_unhandled_pending_rejection_to_capture;

vm.global().handle_rejected_promises();

let captured_rejection = !return_value.is_empty();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if return_value.is_empty() {
return_value = return_value_from_function;
}
Expand Down Expand Up @@ -871,6 +874,12 @@ impl Expect {

scope.apply(vm);

if !threw_sync && !captured_rejection {
// The function returned normally with a non-Promise value. A returned
// Error instance is not a throw (matches Jest and Vitest).
return Ok((None, return_value_from_function));
}

Ok((
return_value.to_error().or_else(|| return_value_from_function.to_error()),
return_value_from_function,
Expand Down
18 changes: 11 additions & 7 deletions test/js/bun/ffi/ffi-viewSource-non-object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@

describe.skipIf(isFFIUnavailable)("FFI viewSource", () => {
test("rejects non-object symbol descriptor values", () => {
// These should throw a TypeError because each symbol descriptor
// must be an object like { args: [...], returns: "void" }.
// Previously, non-object values like numbers or strings would
// cause a debug assertion failure (crash) in generateSymbolForFunction.
expect(() => Bun.FFI.viewSource({ myFunc: 42 })).toThrow("Expected an object");
expect(() => Bun.FFI.viewSource({ myFunc: "not_an_object" })).toThrow("Expected an object");
expect(() => Bun.FFI.viewSource({ myFunc: true })).toThrow("Expected an object");
// Each symbol descriptor must be an object like { args: [...], returns: "void" }.
// Previously, non-object values like numbers or strings would cause a debug
// assertion failure (crash) in generateSymbolForFunction.
// viewSource currently returns the TypeError rather than throwing it; this
// test used to rely on toThrow() accepting a returned Error, which it no
// longer does.

Check warning on line 13 in test/js/bun/ffi/ffi-viewSource-non-object.test.ts

View check run for this annotation

Claude / Claude Code Review

Test comment exceeds 3-line limit and contains PR-history context

This comment is now 6 lines, which exceeds CLAUDE.md rule 13 ("Keep code comments to 3 lines max"), and the last three lines are PR-history rationale ("this test used to rely on toThrow() accepting a returned Error, which it no longer does") — the Landing PRs section says "no bug history — that belongs in the PR description", and it's already covered there. Consider trimming to something durable like: ```ts // viewSource returns (rather than throws) a TypeError when a symbol descriptor is not an
Comment thread
robobun marked this conversation as resolved.
Outdated
for (const value of [42, "not_an_object", true]) {
const result = Bun.FFI.viewSource({ myFunc: value });
expect(result).toBeInstanceOf(TypeError);
expect((result as TypeError).message).toContain("Expected an object");
}
});
});
63 changes: 63 additions & 0 deletions test/js/bun/test/expect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,69 @@ describe("expect()", () => {
}
});

test("toThrow does not treat a returned Error as thrown", () => {
// An Error that is *returned* (not thrown) must not satisfy toThrow().
const returnsError = () => new TypeError("boom");
const assertFails = (/** @type {() => void} */ fn) =>
expect(fn).toThrow(/did not throw|didn't throw|to throw an error/);

assertFails(() => expect(returnsError).toThrow());
assertFails(() => expect(returnsError).toThrow("boom"));
assertFails(() => expect(returnsError).toThrow(/boom/));
assertFails(() => expect(returnsError).toThrow(TypeError));
assertFails(() => expect(returnsError).toThrow(new TypeError("boom")));
assertFails(() => expect(returnsError).toThrow(expect.objectContaining({ name: "TypeError" })));
assertFails(() => expect(returnsError).toThrow(expect.any(TypeError)));
assertFails(() => expect(returnsError).toThrowErrorMatchingSnapshot());
if (!isVitest) {
assertFails(() => expect(returnsError).toThrowErrorMatchingInlineSnapshot(`"boom"`));
}

expect(returnsError).not.toThrow();
expect(returnsError).not.toThrow("boom");
expect(returnsError).not.toThrow(/boom/);
expect(returnsError).not.toThrow(TypeError);
expect(returnsError).not.toThrow(new TypeError("boom"));

// Error subclass and plain object with a message property are still just return values
class MyError extends Error {}
assertFails(() => expect(() => new MyError("sub")).toThrow());
expect(() => new MyError("sub")).not.toThrow();
assertFails(() => expect(() => ({ message: "not an error" })).toThrow());
expect(() => ({ message: "not an error" })).not.toThrow();

// Actually throwing still works for every expected-value shape
const throwsError = () => {
throw new TypeError("boom");
};
expect(throwsError).toThrow();
expect(throwsError).toThrow("boom");
expect(throwsError).toThrow(/boom/);
expect(throwsError).toThrow(TypeError);
expect(throwsError).toThrow(new TypeError("boom"));
expect(throwsError).toThrow(expect.objectContaining({ name: "TypeError" }));
expect(throwsError).toThrow(expect.any(TypeError));
expect(() => expect(throwsError).not.toThrow()).toThrow();

// Throwing non-Error values still counts as throwing
for (const v of [42, "str", null, undefined, { a: 1 }, [1, 2]]) {
expect(() => {
throw v;
}).toThrow();
}

if (isBun) {
// Bun-only extension: a Promise-returning function is awaited.
// Returning an Error is still not a throw under .resolves.
expect(() => Promise.reject(new Error("rej"))).toThrow("rej");
expect(async () => {
throw new Error("async");
}).toThrow("async");
expect(() => Promise.resolve(new Error("ok"))).not.toThrow();
expect(async () => new TypeError("boom")).not.toThrow();
}
});

test("deepEquals derived strings and strings", () => {
let a = new String("hello");
let b = "hello";
Expand Down
Loading