Skip to content

assert: avoid expensive diff for large values#62798

Open
semimikoh wants to merge 1 commit intonodejs:mainfrom
semimikoh:fix-assert-large-diff
Open

assert: avoid expensive diff for large values#62798
semimikoh wants to merge 1 commit intonodejs:mainfrom
semimikoh:fix-assert-large-diff

Conversation

@semimikoh
Copy link
Copy Markdown
Contributor

@semimikoh semimikoh commented Apr 18, 2026

This limits the input size used for simple assertion diffs.

When an assertion fails for large inspected values, such as
assert.strictEqual(Buffer.alloc(n), [buffer]), AssertionError currently
runs the full line-based Myers diff over the inspected output. For large
Buffers this can make the failure path much slower than linear even though the
values are trivially different.

This change keeps the existing rich diff behavior for smaller values, but skips
the Myers diff for very large inspected outputs in the default simple diff
mode. In that case, the error message includes a truncated representation of
both sides and marks skipped lines instead.

diff: 'full' remains available for callers that explicitly request the full
diff.

Refs: #62792

Test

  • Added coverage in test/parallel/test-assert-deep.js
  • Not run locally: this machine has Apple clang 16.0.0 / Command Line Tools
    16.0, while this tree requires a newer macOS toolchain. Local build fails in
    V8 because std::atomic_ref is unavailable.

@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run. labels Apr 18, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.68%. Comparing base (d080801) to head (f6f68cb).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/assert/assertion_error.js 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62798      +/-   ##
==========================================
- Coverage   91.55%   89.68%   -1.87%     
==========================================
  Files         355      706     +351     
  Lines      149381   218286   +68905     
  Branches    23364    41790   +18426     
==========================================
+ Hits       136765   195779   +59014     
- Misses      12354    14408    +2054     
- Partials      262     8099    +7837     
Files with missing lines Coverage Δ
lib/internal/assert/assertion_error.js 95.89% <93.75%> (+3.47%) ⬆️

... and 476 files with indirect coverage changes

🚀 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.

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

Labels

assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants