Repository navigation
Conversation
The diff matchers (toEqual, toStrictEqual, toMatchObject, toHaveProperty, toHaveBeenCalledWith and friends) print both sides through JestPrettyFormat, which walked every own property of a value and ignored the nodejs.util.inspect.custom / Bun.inspect.custom hook that toBe and console.log already honour. A value with a large reachable graph (a DOM node) produced a multi-megabyte failure message. JestPrettyFormat now probes the hook in its classifier, behind a FormatOptions flag that the diff path sets and the snapshot path does not, so .snap output does not change. The console formatter's custom inspect call is shared through a new bun_jsc wrapper. When both sides print alike, the diff reports 'serializes to the same string'.
Collaborator
Author
Collaborator
Author
|
Closing after a self-review. The formatter rewrite in #44268 (dylan-conway) deletes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #44880
Problem
toEqual,toStrictEqual,toMatchObject,toHaveProperty,toHaveBeenCalledWith(and theLast/Nthvariants) ignoreSymbol.for("nodejs.util.inspect.custom")andBun.inspect.customwhen they print the failure diff. They walk every own property of the value. The issue's repro gives a 1.3 MB message fortoEqualwhiletoBeprints<Node>in 65 chars. A DOM node that reacheswindowcan exhaust memory.Tag::getinsrc/runtime/test_runner/pretty_format.rs. It classifies by JSType only and never probes the hook. The console formatter (src/jsc/ConsoleObject.rs:2135) thattoBeuses does probe it.Fix
Formatter::tag_ofwrapsTag::getand, whenFormatOptions.inspect_customis set, probes the hook for Object, Array, Map, Set, Error, Function and Class tags. A callable hook gives the newTag::CustomFormatted.print_custom_formattedcalls the hook with(depth, options, inspect), writes a string result verbatim, formats any other result, and falls back to the default printer when the hook returnsthis.diff_format.rs) setsinspect_custom: true. The snapshot path (mod.rs) sets it tofalse, so.snapoutput does not change and matches Jest.bun_jsc::console_object::call_custom_inspect. When both sides print alike,DiffFormatternow reportsReceived: serializes to the same stringastoBeand Jest do.test/js/bun/test/expect-inspect-custom.test.ts(20 tests, 16 fail on bun 1.4.3). Alsotest/js/bun/test/snapshot-tests,printing,expect.test.js,expect/,mock-fn.test.js,spyMatchers.test.ts,expect-extend*,test/cli/test/bun-test.test.ts,test/js/bun/util/inspect.test.js. No.snapfile changed.Background
JestPrettyFormatis a port of Jest'spretty-format. It prints both sides of a matcher diff and every snapshot.Tagis its value classification, computed once per value before the printer dispatches on it.ConsoleObject.rsis a separate printer used byconsole.log,Bun.inspectand the non-diff matchers (toBe,toBeNull). It gained theinspect.customprobe in 2024. The snapshot formatter never followed.Tag::Objectprint arm only (about 60 lines). It runs afterTag::getclassified the value by JSType, so a hookedclass A extends Arrayorextends Errorwould still be walked whiletoBeprints<A>. The classifier-level probe covers those for the same per-value cost.callToJSONport. The cap lets the walk happen and throws the text away.toJSONchanges snapshot-adjacent output. Both left out.Downsides
inspect.custom, so a happy-dom element with no hook still prints its full graph. Users can define the hook onNode.prototypein a preload.toBetoday. Passing matchers never call it (tested).toMatchSnapshot): one bool load and one tag compare per value, no allocation, no host call. Interleaved A/B of 2000toMatchSnapshotcalls on a 50-key nested object, debug build: main 60.5 s, PR 60.6 s, spread 2 s. Diff path on a hook-less 1000-key object (one extrafast_getper object): main 31.1 s, PR 31.5 s, spread 1.3 s. Debug binary text +25,688 bytes (size). No release toolchain,bloaty,valgrindorperfin this environment.Notes
Repro message lengths (
repro.test.tsfrom the issue), bun 1.4.3 vs this PR:Which values are probed: the probe runs after the JSType match and only for tags whose printer walks the value (Object, Array, Map, Set, Error, Function, Class), never for ProxyObject or a cell
can_get()rejects. So TypedArray and Buffer keep the per-byte{"data": [...]}diff (Buffer.prototype has a hook), DOM wrappers stay onTag::Private, Proxy and Date stay on the JSON arm.Depth argument:
indentalready counts nesting levels, so the hook receives(8 - indent, {depth: 8}), the same pair the console formatter passes from itsmax_depthof 8. A nested hook sees a smaller depth than the root (tested).Stash:
tag_ofstores(this, hook)on the formatter and the matchingformatcall consumes it. If a peek site tagged a different value in between,print_custom_formattedlooks the hook up again instead of calling a stale one.Formattergrows by onebool(fits in padding) and oneOption<(JSValue, JSValue)>(24 bytes).TagResultstays 2 bytes.Pre-existing failures in this environment, reproduced on a main build too:
snapshot-tests/snapshots/snapshot.test.ts"error snapshots" (expects ANSI colors, passes withFORCE_COLOR=1), thenested too deeplycases inbun-test.test.tsandpretty-format-overflow.test.ts(5 s timeouts under the debug ASAN build).Left as is:
expect.addSnapshotSerializerstill throws "Not implemented". No depth or size cap on the diff printer.toJSONis still only called forFormData.