Conversation
|
Welcome to Node.js, and thank you for your first contribution! Before review, please take a moment to read:
Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal. |
19d8829 to
dd7dfd4
Compare
|
This PR is failing linting tests. See the comments in the tests for more detail. See also Pull requests > Step 6: Test with further details in the linked document section BUILDING > Running tests.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66501 +/- ##
==========================================
- Coverage 90.43% 90.42% -0.02%
==========================================
Files 790 790
Lines 275435 275491 +56
Branches 52811 52836 +25
==========================================
+ Hits 249100 249119 +19
- Misses 16730 16779 +49
+ Partials 9605 9593 -12
🚀 New features to boost your workflow:
|
BridgeAR
left a comment
There was a problem hiding this comment.
This can only surface the top level in case a simple object is compared. As soon as the object deviates at a nested level, it would not be reported.
So while this works for simple cases, I do not think this is a good way of handling this. Instead, we might want to surface more information using util.inspect(). That way it would work as expected.
dd7dfd4 to
41fb434
Compare
deepStrictEqual requires both values to share the same prototype, but
the generated diff may not make that failure cause obvious: when both
values are inspected identically (e.g. an instance of an anonymous
class compared to a plain object) the structural diff shows no
difference at all, and when a subclass is involved the mismatch is
only visible implicitly through the inspected class-name prefix.
Append an explicit "Object prototypes differ: X !== Y" line to the
generated message when the operator is deepStrictEqual, both values
are objects, their top-level prototypes differ, and at least one of
the two prototypes is not a default prototype (Object.prototype,
Array.prototype or null), because those cases are already clearly
visible in the inspect output. The diagnostic covers the top-level
values only; prototype differences of nested objects are not
reported.
The diagnostic is derived defensively: the prototype reads and the
prototype names (derived from a single read of the constructor's
name) are wrapped in a try/catch so that exotic objects (e.g. a Proxy
with a throwing `getPrototypeOf` trap or a stateful `name` getter)
cannot replace the assertion error with a different exception nor
leave the global `Error.stackTraceLimit` modified; the hint is simply
omitted in that case. When the comparison is made with
`skipPrototype: true` (via `new assert.Assert({
skipPrototype: true })`),
the hint is not added because the explicitly ignored difference is
not the cause of the failure.
Refs: nodejs#50397
Assisted-by: a closed-source coding agent
Signed-off-by: vaputa <2475834+vaputa@users.noreply.github.com>
41fb434 to
8178893
Compare
Thanks for the feedback. I agree that the top-level-only diagnostic leaves nested prototype mismatches unresolved. |
Checklist
assert: …, includesAssisted-bytrailer per the AI guidelines)test/parallel/test-assert-prototype-mismatch.js+ 4 message snapshot updates)doc/api/assert.md)make -j4 testfull run — targeted assert family (14 parallel tests + 4 message snapshots + new regression = 18/18) passes locally; relying on CI for the full suiteDescription of change
assert.deepStrictEqual()hides prototype mismatches whenever both sides inspect identically. The reported scenario:only hints at the prototype via the class-name prefix, and the worse hidden case loses the failure reason entirely:
This adds an explicit diagnostic line to the deep-diff message when the top-level prototypes differ:
Values have same structure but are not reference-equal + Object prototypes differ: (anonymous) !== ObjectDesign notes, following the reporting/diagnostics direction discussed in #50397 (previously explored in #61716 and #62944) rather than reconstructing prototypes via
util.inspect:ProxygetPrototypeOftrap still producesERR_ASSERTIONwithstackTraceLimituntouchedctor.nameis read once, inside the guardskipPrototype: true(no hint when prototype comparison is opted out)Fixes #50397
AI use disclosure
This change was developed with the help of an AI coding agent (ZCode, powered by GLM — anonymized in the commit trailer per the AI guidelines' naming recommendation). Personal verification performed by the contributor: the diff was reviewed line by line, the targeted assert test family (18/18 including the new regression tests) was run locally on a from-source build, an independent second AI review pass (Codex) was run to audit the patch before submission, and its findings (exception-guard for Proxy traps, single
nameread,skipPrototypehandling) were applied and re-verified. The contributor takes full responsibility for this change and will engage with review feedback personally.