Skip to content

fix(assert): hint at reference-equality when assertEquals diff is empty (#6878)#7158

Open
LeSingh1 wants to merge 3 commits into
denoland:mainfrom
LeSingh1:fix/6878-asserteq-reference-hint
Open

fix(assert): hint at reference-equality when assertEquals diff is empty (#6878)#7158
LeSingh1 wants to merge 3 commits into
denoland:mainfrom
LeSingh1:fix/6878-asserteq-reference-hint

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

Fixes #6878.

When assertEquals decides two values are unequal but they stringify identically (typically because a property is a function compared by reference), the rendered diff is empty:

error: AssertionError: Values are not equal.


    [Diff] Actual / Expected


    {
      x: 1,
      y: [Function: y],
    }

That leaves users hunting for invisible differences. @lionel-rowe pointed at the underlying cause in the thread (functions, Promises, Requests, Blobs etc. fall back to reference equality) and suggested the diff should "explicitly state when there are invisible differences."

Fix

After building the diff, check whether it contains any added / removed entries. If not, append:

    Note: values stringify identically but are not structurally equal.
    Functions, Promises, Requests, Blobs, and other built-ins are compared by
    reference, so two distinct instances are never equal even when their
    representations match.

The hint is skipped on string-vs-string comparisons (which always produce a visible diff when they differ) and on real textual diffs (so existing failures aren't noisier).

Tests

Two cases in assert/equals_test.ts:

  • hints at reference-equality when objects with function props stringify identically (#6878) — reproduces the exact issue body's call and asserts the new hint text is present.
  • does not append reference-equality hint when there is a real textual diffassertEquals({ x: 1 }, { x: 2 }) must NOT include the hint.
$ deno test --allow-read --allow-write assert/
ok | 141 passed (25 steps) | 0 failed (1s)

$ deno lint assert/equals.ts assert/equals_test.ts
Checked 2 files

$ deno check assert/equals.ts assert/equals_test.ts

…6878)

`assertEquals` throws "Values are not equal" but renders an empty diff
when actual/expected stringify identically yet are deemed unequal —
the common case is a function property compared by reference. The
empty diff leaves users hunting for invisible differences.

Detect the case (diff result has zero `added`/`removed` entries) and
append a one-line hint explaining that functions, Promises, Requests,
Blobs, and other built-ins are compared by reference, so two distinct
instances are never equal even when their representations match. Skip
the hint on real textual diffs and on string-vs-string comparisons
(which always produce visible add/remove if they differ).

Two new tests cover the hint firing path and the negative case.
Full assert suite (141 tests) stays green.

Direction matches @lionel-rowe's suggestion in the issue thread.
@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.63%. Comparing base (1a13460) to head (0900058).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7158   +/-   ##
=======================================
  Coverage   94.63%   94.63%           
=======================================
  Files         629      630    +1     
  Lines       51894    51908   +14     
  Branches     9373     9377    +4     
=======================================
+ Hits        49108    49122   +14     
  Misses       2217     2217           
  Partials      569      569           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clean fix and, I think, the right approach to #6878. By keying off an empty diff (no added/removed lines) rather than walking the value tree, it avoids false positives — an object with an identical function reference on both sides but a different scalar field still produces a real diff line, so the hint correctly stays silent. It also adds no extra traversal and re-invokes no getters. And because @std/internal/format runs with depth/iterableLimit/strAbbreviateSize: Infinity, two values can't stringify identically merely from inspect truncation, so an empty diff really does imply a reference-compared distinction — the broader wording (Promises, Requests, Blobs, distinct Symbols, Weak* types) is accurate, not just plausible.

Only minor cleanups inline; nothing blocking.

Process note: this overlaps with your #7165, which tackles the same issue via a containsFunction tree-walk. The two are mutually exclusive. I'd land this one and close #7165 — it's simpler, avoids the false-positive and getter-re-invocation issues, and produces a more general/accurate hint. Worth cross-linking so they aren't reviewed independently.

Comment thread assert/equals.ts
// a hint pointing at the likely cause.
if (
!stringDiff &&
diffResult.every((r) => r.type !== "added" && r.type !== "removed")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DiffResult.type is "added" | "removed" | "common", so this reads more directly as diffResult.every((r) => r.type === "common").

Comment thread assert/equals.ts
Comment on lines +76 to +80
message = `${message}\n` +
" Note: values stringify identically but are not structurally equal. " +
"Functions, Promises, Requests, Blobs, and other built-ins are compared by " +
"reference, so two distinct instances are never equal even when their " +
"representations match.";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the Note: text hard-codes 4 leading spaces to align with buildMessage's [Diff] block — an implicit coupling to that formatter's indentation. Fine to leave, but a one-line comment noting why the 4 spaces are there would help the next person who touches it.

Comment thread assert/equals_test.ts
Comment on lines +219 to +235
let caught: AssertionError | undefined;
try {
assertEquals(
{ x: 1, y: () => 2 },
{ x: 1, y: () => 2 },
);
} catch (e) {
caught = e as AssertionError;
}
if (!caught) throw new Error("Expected assertEquals to throw");
assertStringIncludes(caught.message, "Values are not equal");
assertStringIncludes(
caught.message,
"stringify identically but are not structurally equal",
);
assertStringIncludes(caught.message, "compared by reference");
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: this file already imports assertThrows, which returns the thrown error — that's cleaner than the hand-rolled try/catch + if (!caught) throw:

const error = assertThrows(
  () => assertEquals({ x: 1, y: () => 2 }, { x: 1, y: () => 2 }),
  AssertionError,
);
assertStringIncludes(error.message, "Values are not equal");
assertStringIncludes(error.message, "stringify identically but are not structurally equal");
assertStringIncludes(error.message, "compared by reference");

Comment thread assert/equals_test.ts
Comment on lines +242 to +253
let caught: AssertionError | undefined;
try {
assertEquals({ x: 1 }, { x: 2 });
} catch (e) {
caught = e as AssertionError;
}
if (!caught) throw new Error("Expected assertEquals to throw");
assertEquals(
caught.message.includes("stringify identically"),
false,
"Hint should only fire when the diff is empty",
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same assertThrows-returns-the-error simplification applies here. For the negative check, assert(!error.message.includes("stringify identically")) (or assertNotMatch) is clearer than assertEquals(..., false, msg).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@std/assert/equals - assertEquals() fails with function properties without displaying why

2 participants