From 228ae3a06fd9221d3044ce690d562a29aa81ace5 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Wed, 5 Feb 2025 13:32:47 -0500 Subject: [PATCH] fix bug in assert.equals --- src/bun.js/bindings/bindings.cpp | 30 +++++++++++++----------- src/js/node/assert.ts | 5 ++-- test/js/bun/test/jest.d.ts | 4 ++++ test/js/node/assert/assert.spec.ts | 37 ++++++++++++++++++++++++++++++ test/js/node/assert/assert.test.ts | 11 --------- 5 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 test/js/node/assert/assert.spec.ts delete mode 100644 test/js/node/assert/assert.test.ts diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 14967128df7fd8..2ce7068dce9988 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -1545,6 +1545,19 @@ bool Bun__deepMatch( return true; } +// anonymous namespace to avoid name collision +namespace { + template + inline bool deepEqualsWrapperImpl(JSC__JSValue a, JSC__JSValue b, JSC__JSGlobalObject* global) + { + auto& vm = global->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + Vector, 16> stack; + MarkedArgumentBuffer args; + return Bun__deepEquals(global, JSC::JSValue::decode(a), JSC::JSValue::decode(b), args, stack, &scope, true); + } +} + extern "C" { bool WebCore__FetchHeaders__isEmpty(WebCore__FetchHeaders* arg0) @@ -2452,33 +2465,24 @@ bool JSC__JSValue__isSameValue(JSC__JSValue JSValue0, JSC__JSValue JSValue1, return JSC::sameValue(globalObject, left, right); } -#define IMPL_DEEP_EQUALS_WRAPPER(strict, enableAsymmetricMatchers, globalObject, a, b) \ - ASSERT_NO_PENDING_EXCEPTION(globalObject); \ - JSValue v1 = JSValue::decode(a); \ - JSValue v2 = JSValue::decode(b); \ - ThrowScope scope = DECLARE_THROW_SCOPE(globalObject->vm()); \ - Vector, 16> stack; \ - MarkedArgumentBuffer args; \ - return Bun__deepEquals(globalObject, v1, v2, args, stack, &scope, true) - bool JSC__JSValue__deepEquals(JSC__JSValue JSValue0, JSC__JSValue JSValue1, JSC__JSGlobalObject* globalObject) { - IMPL_DEEP_EQUALS_WRAPPER(false, false, globalObject, JSValue0, JSValue1); + return deepEqualsWrapperImpl(JSValue0, JSValue1, globalObject); } bool JSC__JSValue__jestDeepEquals(JSC__JSValue JSValue0, JSC__JSValue JSValue1, JSC__JSGlobalObject* globalObject) { - IMPL_DEEP_EQUALS_WRAPPER(false, true, globalObject, JSValue0, JSValue1); + return deepEqualsWrapperImpl(JSValue0, JSValue1, globalObject); } bool JSC__JSValue__strictDeepEquals(JSC__JSValue JSValue0, JSC__JSValue JSValue1, JSC__JSGlobalObject* globalObject) { - IMPL_DEEP_EQUALS_WRAPPER(true, false, globalObject, JSValue0, JSValue1); + return deepEqualsWrapperImpl(JSValue0, JSValue1, globalObject); } bool JSC__JSValue__jestStrictDeepEquals(JSC__JSValue JSValue0, JSC__JSValue JSValue1, JSC__JSGlobalObject* globalObject) { - IMPL_DEEP_EQUALS_WRAPPER(true, true, globalObject, JSValue0, JSValue1); + return deepEqualsWrapperImpl(JSValue0, JSValue1, globalObject); } #undef IMPL_DEEP_EQUALS_WRAPPER diff --git a/src/js/node/assert.ts b/src/js/node/assert.ts index b95c454b32cc28..b32ee2ba246a7a 100644 --- a/src/js/node/assert.ts +++ b/src/js/node/assert.ts @@ -189,9 +189,8 @@ assert.equal = function equal(actual: unknown, expected: unknown, message?: stri if (arguments.length < 2) { throw $ERR_MISSING_ARGS("actual", "expected"); } - // eslint-disable-next-line eqeqeq - // if (actual != expected && (!NumberIsNaN(actual) || !NumberIsNaN(expected))) { - if (actual != expected && !(isNaN(actual) && isNaN(expected))) { + + if (actual != expected && (!NumberIsNaN(actual) || !NumberIsNaN(expected))) { innerFail({ actual, expected, diff --git a/test/js/bun/test/jest.d.ts b/test/js/bun/test/jest.d.ts index fccb87b409e6ea..2d21e87e051282 100644 --- a/test/js/bun/test/jest.d.ts +++ b/test/js/bun/test/jest.d.ts @@ -3,3 +3,7 @@ declare var describe: typeof import("bun:test").describe; declare var test: typeof import("bun:test").test; declare var expect: typeof import("bun:test").expect; declare var it: typeof import("bun:test").it; +declare var beforeEach: typeof import("bun:test").beforeEach; +declare var afterEach: typeof import("bun:test").afterEach; +declare var beforeAll: typeof import("bun:test").beforeAll; +declare var afterAll: typeof import("bun:test").afterAll; diff --git a/test/js/node/assert/assert.spec.ts b/test/js/node/assert/assert.spec.ts new file mode 100644 index 00000000000000..d4dc871b590098 --- /dev/null +++ b/test/js/node/assert/assert.spec.ts @@ -0,0 +1,37 @@ +import { describe, it, expect } from "bun:test"; +import assert from "assert"; + +describe("assert(expr)", () => { + // https://github.com/oven-sh/bun/issues/941 + it.each([true, 1, "foo"])(`assert(%p) does not throw`, expr => { + expect(() => assert(expr)).not.toThrow(); + }); + + it.each([false, 0, "", null, undefined])(`assert(%p) throws`, expr => { + expect(() => assert(expr)).toThrow(assert.AssertionError); + }); + + it("is an alias for assert.ok", () => { + expect(assert).toBe(assert.ok); + }); +}); + +describe("assert.equal(actual, expected)", () => { + it.each([ + ["foo", "foo"], + [1, 1], + [1, true], + [0, ""], + [0, false], + ])(`%p == %p`, (actual, expected) => { + expect(() => assert.equal(actual, expected)).not.toThrow(); + }); + it.each([ + // + ["foo", "bar"], + [1, 0], + [true, false], + ])("%p != %p", (actual, expected) => { + expect(() => assert.equal(actual, expected)).toThrow(assert.AssertionError); + }); +}); diff --git a/test/js/node/assert/assert.test.ts b/test/js/node/assert/assert.test.ts deleted file mode 100644 index 2ba97531ab0c8e..00000000000000 --- a/test/js/node/assert/assert.test.ts +++ /dev/null @@ -1,11 +0,0 @@ -import assert from "assert"; -import { expect, test } from "bun:test"; - -// https://github.com/oven-sh/bun/issues/941 -test("assert as a function does not throw", () => assert(true)); -test("assert as a function does throw", () => { - try { - assert(false); - expect.unreachable(); - } catch (e) {} -});