Skip to content
Open
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
2 changes: 1 addition & 1 deletion docs/test/snapshots.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Inline snapshots don't record which runner wrote them, so Bun accepts a match in

With `--update-snapshots`, Bun removes entries that no test uses anymore from a file in Jest's or Vitest's format. It keeps entries for tests that were skipped, filtered out or failed. A file in Bun's format keeps only the entries of tests that ran.

Earlier versions of Bun printed `[native code]` for an accessor instead of its value, included non-enumerable own properties, and printed DOM nodes as plain objects. Snapshots written that way still pass. Bun leaves them alone until `--update-snapshots` rewrites them.
Earlier versions of Bun printed `[native code]` for an accessor instead of its value, included non-enumerable own properties, printed DOM nodes as plain objects, and left out a property named `constructor`. Snapshots written that way still pass. Bun leaves them alone until `--update-snapshots` rewrites them. For that reason, a snapshot that has no `constructor` line also passes when the object has a property of that name.

## Best Practices

Expand Down
3 changes: 0 additions & 3 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3038,9 +3038,6 @@ pub mod formatter {
is_symbol: bool,
is_private_symbol: bool,
) -> Option<TagResult> {
if key.eq_ascii(b"constructor") {
return None;
}
if ctx.formatter.failed {
return None;
}
Expand Down
28 changes: 19 additions & 9 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5989,6 +5989,7 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
if (fast) {
bool anyHits = false;
JSC::JSObject* objectToUse = prototypeObject.getObject();
const bool isPrototypeLayer = objectToUse != object;

// The iter callback can run user code (a nested value's inspect.custom, Proxy
// traps) that adds properties to this object, rehashing the PropertyTable
Expand All @@ -6005,9 +6006,10 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
if ((attributes & (PropertyAttribute::Function)) == 0 && (attributes & (PropertyAttribute::Builtin)) != 0)
return;

if (prop == propertyNames->constructor
// Only a prototype's `constructor` is left out. An own one prints like any other key, also when it holds the class.
if ((isPrototypeLayer && (prop == propertyNames->constructor || prop == propertyNames->__esModule))
|| prop == propertyNames->underscoreProto
|| prop == propertyNames->toStringTagSymbol || (objectToUse != object && prop == propertyNames->__esModule))
|| prop == propertyNames->toStringTagSymbol)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
return;

if (builtinNames.bunNativePtrPrivateName() == prop)
Expand Down Expand Up @@ -6125,8 +6127,8 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
if (property.isNull()) [[unlikely]]
continue;

// ignore constructor
if (property == propertyNames->constructor || builtinNames.bunNativePtrPrivateName() == property)
// As in the quick walk: only a prototype's `constructor` is left out.
if ((property == propertyNames->constructor && iterating != object) || builtinNames.bunNativePtrPrivateName() == property)
continue;

if constexpr (nonIndexedOnly) {
Expand Down Expand Up @@ -6277,7 +6279,7 @@ extern "C" [[ZIG_EXPORT(nothrow)]] bool JSC__isBigIntInInt64Range(JSC::EncodedJS
return high == JSBigInt::ComparisonResult::LessThan || high == JSBigInt::ComparisonResult::Equal;
}

static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, void* arg2, void (*iter)(JSC::JSGlobalObject* arg0, void* ctx, EncodedSlice* arg2, JSC::EncodedJSValue JSValue3, bool isSymbol, bool isPrivateSymbol), bool callGetters, bool withNonEnumerable = false)
static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, void* arg2, void (*iter)(JSC::JSGlobalObject* arg0, void* ctx, EncodedSlice* arg2, JSC::EncodedJSValue JSValue3, bool isSymbol, bool isPrivateSymbol), bool callGetters, bool withNonEnumerable = false, bool* hasKeyOlderBunLeftOut = nullptr)
{
JSC::JSValue value = JSC::JSValue::decode(JSValue0);
JSC::JSObject* object = value.getObject();
Expand Down Expand Up @@ -6307,10 +6309,17 @@ static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlob
if (property.isNull()) [[unlikely]]
continue;

// ignore constructor
if (property == vm.propertyNames->constructor || clientData->builtinNames().bunNativePtrPrivateName() == property)
if (clientData->builtinNames().bunNativePtrPrivateName() == property)
continue;

// Older Bun left out every key spelled `constructor`, a symbol with that description too.
if (property == vm.propertyNames->constructor || (property.isSymbol() && WTF::equal(property.impl(), "constructor"_s))) [[unlikely]] {
if (withNonEnumerable)
continue;
if (hasKeyOlderBunLeftOut)
*hasKeyOlderBunLeftOut = true;
Comment thread
robobun marked this conversation as resolved.
Resolved
}

JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
bool hasProperty = object->getPropertySlot(globalObject, property, slot);
(void)scope.tryClearException();
Expand Down Expand Up @@ -6364,8 +6373,9 @@ extern "C" bool JSC__JSValue__forEachPropertyOrderedCallingGetters(JSC::EncodedJ
{
JSC::JSObject* object = JSC::JSValue::decode(JSValue0).getObject();
bool hasOnlyEnumerableValues = object && object->structure()->canPerformFastPropertyEnumeration() && !object->structure()->hasNonEnumerableProperties();
forEachPropertyOrderedImpl(JSValue0, globalObject, arg2, iter, true);
return hasOnlyEnumerableValues;
bool hasKeyOlderBunLeftOut = false;
forEachPropertyOrderedImpl(JSValue0, globalObject, arg2, iter, true, false, &hasKeyOlderBunLeftOut);
return hasOnlyEnumerableValues && !hasKeyOlderBunLeftOut;
Comment thread
robobun marked this conversation as resolved.
Resolved
}

extern "C" void JSC__JSValue__forEachPropertyOrderedWithNonEnumerable(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, void* arg2, void (*iter)(JSC::JSGlobalObject* arg0, void* ctx, EncodedSlice* arg2, JSC::EncodedJSValue JSValue3, bool isSymbol, bool isPrivateSymbol))
Expand Down
4 changes: 0 additions & 4 deletions src/runtime/test_runner/pretty_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,10 +947,6 @@ impl<'a, 'f, W: bun_io::Write, const ENABLE_ANSI_COLORS: bool>
return;
}

if key.eq_ascii(b"constructor") {
return;
}

// SAFETY: ctx_ptr was passed as `&mut Self as *mut c_void` by the caller of for_each.
let Some(ctx) = (unsafe { ctx_ptr.cast::<Self>().as_mut() }) else { return };
if ctx.formatter.failed {
Expand Down
21 changes: 21 additions & 0 deletions test/js/bun/test/jest-each.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,27 @@ describe.concurrent("titles", () => {
});
});

it("%p, %o, %O and %s print a row's own constructor key, and not the one of its class", async () => {
expect(
await titlesOf(
[
[
"%p|%o|%O|%s",
`[{ constructor: "K", a: 1 }, { constructor: "L", a: 1 }, new Foo()].map(value => [value, value, value, value])`,
],
],
`class Foo { m() {} }`,
),
).toEqual({
titles: [
'{ constructor: "K", a: 1 }|{ constructor: "K", a: 1 }|{ constructor: "K", a: 1 }|{ constructor: "K", a: 1 }',
'{ constructor: "L", a: 1 }|{ constructor: "L", a: 1 }|{ constructor: "L", a: 1 }|{ constructor: "L", a: 1 }',
"Foo { m: [Function: m] }|Foo { m: [Function: m] }|Foo { m: [Function: m] }|Foo { m: [Function: m] }",
],
exitCode: 0,
});
});

it("%#, %$ and %% take no value, and anything else after a % is kept", async () => {
expect(
await titlesOf([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,21 @@ exports[`it will create a snapshot file if it doesn't exist 7`] = `
"j": Any<RegExp>,
}
`;

exports[`an own enumerable 'constructor' property is part of the snapshot 1`] = `
{
"list": [
{
"constructor": "K",
},
],
"object": {
"a": 1,
"constructor": "K",
},
"parsed": {
"constructor": "_class",
},
"prototype": Foo {},
}
`;
220 changes: 220 additions & 0 deletions test/js/bun/test/snapshot-tests/bun-snapshots.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { describe, expect, it, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import { readFileSync } from "node:fs";
import { join } from "node:path";

test("it will create a snapshot file if it doesn't exist", () => {
expect({ a: { b: { c: false } }, c: 2, jkfje: 99238 }).toMatchSnapshot({ a: { b: { c: expect.any(Boolean) } } });
Expand All @@ -14,6 +17,223 @@ test("it will create a snapshot file if it doesn't exist", () => {
});
});

// These are in the snapshot file because an inline snapshot also passes with what Jest or an older Bun wrote.
test("an own enumerable 'constructor' property is part of the snapshot", () => {
class Foo {}
expect({
object: { constructor: "K", a: 1 },
parsed: JSON.parse('{"constructor":"_class"}'),
list: [{ constructor: "K" }],
// The non-enumerable constructor that a class puts on its prototype is still left out.
prototype: Foo.prototype,
}).toMatchSnapshot();
});

// Bun 1.4.3 and 1.4.4 wrote `olderBun`. Until Bun printed an object's own `constructor` key, it left out every key of that name.
describe.concurrent("a snapshot that an older Bun wrote for objects with a 'constructor' key", () => {
const values = `
class Foo { m() {} }
function Big(value) { this.c = [value]; this.constructor = Big; }
function form(constructor) {
const form = new FormData();
form.set("constructor", constructor);
form.set("a", "b");
return form;
}
const values = (constructor = process.env.CONSTRUCTOR_VALUE ?? "K") => ({
own: { constructor, a: 1 },
getter: { get constructor() { return constructor; }, a: 1 },
symbol: { [Symbol("constructor")]: constructor, a: 1 },
hidden: Object.defineProperty({ constructor, a: 1 }, "hidden", { value: 2 }),
prototype: Foo.prototype,
list: [{ constructor }],
class: new Big(7),
form: form(constructor),
});
`;
const olderBun = `{
"class": Big {
"c": [
7,
],
},
"form": FormData (entries) FormData FormData {
"a": "b",
},
"getter": {
"a": 1,
},
"hidden": {
"a": 1,
"hidden": 2,
},
"list": [
{},
],
"own": {
"a": 1,
},
"prototype": Foo {
"m": [Function: m],
},
"symbol": {
"a": 1,
},
}`;
const now = `{
"class": Big {
"c": [
7,
],
"constructor": [Function: Big],
},
"form": FormData (entries) FormData FormData {
"a": "b",
"constructor": "K",
},
"getter": {
"a": 1,
"constructor": "K",
},
"hidden": {
"a": 1,
"constructor": "K",
},
"list": [
{
"constructor": "K",
},
],
"own": {
"a": 1,
"constructor": "K",
},
"prototype": Foo {},
"symbol": {
"a": 1,
[Symbol(constructor)]: "K",
},
}`;
const header = "// Bun Snapshot v1, https://bun.sh/docs/test/snapshots\n";
const entry = (name: string, text: string) => `\nexports[\`${name} 1\`] = \`\n${text}\n\`;\n`;
const inAFile = `${values}\ntest("in a file", () => { expect(values()).toMatchSnapshot(); });`;
const read = (dir: string, ...path: string[]) => readFileSync(join(dir, ...path), "utf8");
const saved = (dir: string) => read(dir, "__snapshots__", "a.test.js.snap");

async function run(cwd: string, env: Record<string, string> = {}, ...args: string[]) {
await using proc = Bun.spawn({
cmd: [bunExe(), "test", ...args],
env: { ...bunEnv, CI: "true", ...env },
cwd,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const count = (what: string) => Number(stderr.match(new RegExp(`^ *(\\d+) ${what}$`, "m"))?.[1] ?? -1);
return { pass: count("pass"), fail: count("fail"), exitCode, stderr };
}

test.each(["bun:test", "vitest"])("passes and is left alone, in a test of %j", async module => {
const testFile = `
import { test, expect } from ${JSON.stringify(module)};
${values}
// An older Bun printed nothing else of this value in another way.
test("the key alone", () => {
expect({ constructor: "K", a: 1 }).toMatchSnapshot();
expect({ constructor: "K", a: 1 }).toMatchInlineSnapshot(\`\n{\n "a": 1,\n}\n\`);
});
test("in a file", () => { expect(values()).toMatchSnapshot(); });
test("inline", () => { expect(values()).toMatchInlineSnapshot(\`\n${olderBun}\n\`); });
test("with property matchers", () => {
expect({ constructor: "K", id: 5 }).toMatchSnapshot({ id: expect.any(Number) });
});
test("but for one line", () => {
expect(() => expect({ ...values(), more: 1 }).toMatchInlineSnapshot(\`\n${olderBun}\n\`)).toThrow('+ "more": 1,');
});
`;
const snapshots =
header +
entry("the key alone", `{\n "a": 1,\n}`) +
entry("in a file", olderBun) +
entry("with property matchers", `{\n "id": Any<Number>,\n}`);
using dir = tempDir("snapshot-constructor-key", {
"a.test.js": testFile,
"__snapshots__/a.test.js.snap": snapshots,
});
const { stderr, ...result } = await run(String(dir));
expect({ ...result, stderr: result.exitCode ? stderr : "" }).toEqual({ pass: 5, fail: 0, exitCode: 0, stderr: "" });
expect({ testFile: read(String(dir), "a.test.js"), snapshots: saved(String(dir)) }).toEqual({
testFile,
snapshots,
});
});

// The older print leaves the key out, so it cannot tell a value that has the key from one that never had it.
test("without the key also passes for a value that has gained it", async () => {
using dir = tempDir("snapshot-constructor-key-gained", {
"a.test.js": `
const value = () => (process.env.CONSTRUCTOR_VALUE ? { a: 1, constructor: process.env.CONSTRUCTOR_VALUE } : { a: 1 });
test("in a file", () => { expect(value()).toMatchSnapshot(); });
test("inline", () => { expect(value()).toMatchInlineSnapshot(\`\n{\n "a": 1,\n}\n\`); });
`,
"__snapshots__/a.test.js.snap": header + entry("in a file", `{\n "a": 1,\n}`),
});
const [without, gained] = await Promise.all([run(String(dir)), run(String(dir), { CONSTRUCTOR_VALUE: "K" })]);
expect({
without: { pass: without.pass, fail: without.fail, exitCode: without.exitCode },
gained: { pass: gained.pass, fail: gained.fail, exitCode: gained.exitCode },
}).toEqual({
without: { pass: 2, fail: 0, exitCode: 0 },
gained: { pass: 2, fail: 0, exitCode: 0 },
});
});

test("--update-snapshots adds the key, once", async () => {
// `inline` and `matched` are the text between the backticks of the two inline snapshots.
const testFile = (inline: string, matched: string) =>
[
values,
`test("in a file", () => { expect(values()).toMatchSnapshot(); });`,
`test("inline", () => { expect(values()).toMatchInlineSnapshot(\`${inline}\`); });`,
`test("inline with a property matcher", () => {`,
` expect({ constructor: "K", id: 5 }).toMatchInlineSnapshot({ id: expect.any(Number) }, \`${matched}\`);`,
`});`,
``,
].join("\n");
using dir = tempDir("snapshot-constructor-key-update", {
"a.test.js": testFile(`\n${olderBun}\n`, `\n{\n "id": Any<Number>,\n}\n`),
"__snapshots__/a.test.js.snap": header + entry("in a file", olderBun),
});
const updated = {
testFile: testFile(`\n${now}\n`, `\n{\n "constructor": "K",\n "id": Any<Number>,\n}\n`),
snapshots: header + entry("in a file", now),
};
const contents = () => ({ testFile: read(String(dir), "a.test.js"), snapshots: saved(String(dir)) });

const first = await run(String(dir), {}, "--update-snapshots");
expect(contents()).toEqual(updated);
const second = await run(String(dir), {}, "--update-snapshots");
expect(contents()).toEqual(updated);
expect([first.exitCode, second.exitCode]).toEqual([0, 0]);
});

test("with the key, fails for another value of it", async () => {
using dir = tempDir("snapshot-constructor-key-value", {
"a.test.js": inAFile,
"__snapshots__/a.test.js.snap": header + entry("in a file", now),
});
const [same, other] = await Promise.all([run(String(dir)), run(String(dir), { CONSTRUCTOR_VALUE: "L" })]);
expect(other.stderr).toContain('- "constructor": "K",\n+ "constructor": "L",');
expect({
same: { pass: same.pass, fail: same.fail, exitCode: same.exitCode },
other: { pass: other.pass, fail: other.fail, exitCode: other.exitCode },
}).toEqual({
same: { pass: 1, fail: 0, exitCode: 0 },
other: { pass: 0, fail: 1, exitCode: 1 },
});
});
});

test("ArrayBuffer values are serialized like typed arrays", () => {
expect(new Uint8Array([1, 2, 3]).buffer).toMatchInlineSnapshot(`
ArrayBuffer [
Expand Down
Loading
Loading