test_runner: sort Set/Map entries when rendering the toEqual diff
Closed
RO
2d ago+312+312+012345678901234567890123456789-16-16-01234567890123456789
What does this PR do?
Fixes #13015.
Repro
import { test, expect } from "bun:test";
test("set diff", () => {
expect(new Set(["asdf", "xx"])).toEqual(new Set(["xx", "asdf", "sdf"]));
});
Before:
Set {
- "xx",
"asdf",
- "sdf",
+ "xx",
}
- Expected - 2
+ Received + 1
"xx" is in both sets, yet it shows as both - and +.
After:
Set {
"asdf",
- "sdf",
"xx",
}
- Expected - 1
+ Received + 0
Cause
DiffFormatter serializes both received and expected with JestPrettyFormat (which iterates Sets/Maps in insertion order) and then runs a line-based text diff on the two strings. Two Sets with the same members inserted in a different order serialize to different text, so the line diff reports spurious -/+ pairs for members shared by both sides.
Fix
Add FormatOptions::ordered_collections / Formatter::ordered_collections. When set, the Tag::Set / Tag::Map branches of print_as buffer each entry into its own Vec<u8>, sort the buffers, and emit them in sorted order. Two collections with the same members now serialize to identical text regardless of insertion order, so the line diff shows only true membership differences.
Only DiffFormatter enables the flag. Bun.inspect, console.log, and snapshot serialization keep insertion order. WeakSet/WeakMap are excluded from the sorted path.
Verification
bun bd test test/regression/issue/13015/13015.test.tspasses (fails on the released binary)bun bd test test/js/bun/test/expect.test.jspasses (415 tests)bun bd test test/js/bun/test/spyMatchers.test.tspasses (147 tests)Bun.inspect(new Set(["c","a","b"]))andtoMatchInlineSnapshotstill print insertion order
[review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
ASAN without fix: 6 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/regression/issue/13015/13015.test.ts"
bun test v1.4.0 (d385ee762)
test/regression/issue/13015/13015.test.ts:
46 | test("Set: shared element is not shown as both removed and added", () => {
47 | const { body, footer } = diffBlock(stderr, "Set diff shows only membership differences");
48 | // "xx" and "asdf" are in both sets; they must not appear on a +/- line.
49 | for (const line of body) {
50 | if (line.startsWith("+ ") || line.startsWith("- ")) {
51 | expect(line).not.toContain('"xx"');
^
error: expect(received).not.toContain(expected)
Expected to not contain: "\"xx\""
Received: "- \"xx\","
at <anonymous> (/workspace/bun/test/regression/issue/13015/13015.test.ts:51:26)
(fail) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: shared element is not shown as both removed and added [46.57ms]
60 | test("Set: numeric members", () => {
61 | const { body, footer } = diffBlock(stderr, "Set diff with numbers");
62 | // 1 and 2 are in both sets
... (truncated)
release without fix: all passed
bun test v1.4.0-canary.1 (962e14c94)
test/regression/issue/13015/13015.test.ts:
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: shared element is not shown as both removed and added [0.76ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: numeric members [0.37ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Map: shared entry is not shown as both removed and added [0.17ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set nested in object [0.20ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: shared Promise is order-independent even after a long sibling [0.27ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > sibling after a Set does not depend on Set insertion order [0.28ms]
6 pass
0 fail
24 expect() calls
Ran 6 tests across 1 file. [418.00ms]
__F:0:S:0
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/regression/issue/13015/13015.test.ts"
bun test v1.4.0 (d385ee762)
test/regression/issue/13015/13015.test.ts:
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: shared element is not shown as both removed and added [44.26ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: numeric members [31.02ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Map: shared entry is not shown as both removed and added [22.39ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set nested in object [29.53ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > Set: shared Promise is order-independent even after a long sibling [28.04ms]
(pass) issue #13015: toEqual diff for Set/Map is order-insensitive > sibling after a Set does not depend on Set insertion order [28.67ms]
6 pass
0 fail
24 expect() calls
Ran 6 tests across 1 file. [8.06s]
__F:0:S:0
release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 2131ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/51] gen cpp.rs (cppbind)
[2/51] cxx obj/src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp.o
[3/51] cxx obj/src/jsc/bindings/webcore/streams/BunStreamSource.cpp.o
[4/51] cxx obj/src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp.o
[5/51] cxx obj/src/jsc/bindings/webcore/streams/CrossRealmTransform.cpp.o
[6/51] cxx obj/unified/UnifiedSource-src_jsc_bindings_node-0.cpp.o
[7/51] cxx obj/src/jsc/bindings/BunObject.cpp.o
[8/51] cxx obj/src/jsc/bindings/napi.cpp.o
[9/51] cxx obj/src/jsc/bindings/BunProcess.cpp.o
[10/51] cxx obj/unified/UnifiedSource-src_jsc_modules-0.cpp.o
[11/51] cxx obj/unified/UnifiedSource-src_jsc_bindings-2.cpp.o
[12/51] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o
[13/51] cxx obj/src/jsc/bindings/bindings.cpp.o
[14/51] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-3.cpp.o
[15/51] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-4.cpp.o
[16/51] cxx obj/unified/UnifiedSource-src_jsc_bindings-3.cpp.o
[17/51] cxx obj/unified/UnifiedSource-src_jsc_bind
... (truncated)
diff hotspot
src/runtime/test_runner/diff_format.rs | 1 +
src/runtime/test_runner/mod.rs | 1 +
src/runtime/test_runner/pretty_format.rs | 155 ++++++++++++++++++++++++---
test/regression/issue/13015/13015.fixture.ts | 40 +++++++
test/regression/issue/13015/13015.test.ts | 131 ++++++++++++++++++++++
5 files changed, 312 insertions(+), 16 deletions(-)
gate history · 4 passed · 0 rejected · iteration 0
evidence per changed file
file reads edits tests
src/runtime/test_runner/diff_format.rs 1 1 0
src/runtime/test_runner/mod.rs 1 1 0
src/runtime/test_runner/pretty_format.rs 17 19 0
test/regression/issue/13015/13015.fixture.ts 2 3 0
test/regression/issue/13015/13015.test.ts 3 5 0