Skip to content

test: add cache/dedupe + dns re-dispatch integration tests - #5535

Open
GiHoon1123 wants to merge 1 commit into
nodejs:mainfrom
GiHoon1123:fix-issue-5522-normalize-headers-interceptor-order
Open

test: add cache/dedupe + dns re-dispatch integration tests#5535
GiHoon1123 wants to merge 1 commit into
nodejs:mainfrom
GiHoon1123:fix-issue-5522-normalize-headers-interceptor-order

Conversation

@GiHoon1123

@GiHoon1123 GiHoon1123 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Relates to #5522.

#5536 contains the implementation fix and unit tests for flat header arrays. This PR keeps the integration coverage for the compose-chain paths that triggered the original bug.

The tests cover:

  • cache() + dns() after DNS re-dispatch
  • deduplicate() + dns(), which uses the same cache-key utilities

The earlier lib/util/cache.js changes were dropped after #5536 landed. This PR only changes test/interceptors/dns.js.

node --test test/interceptors/dns.js: 29 passing, 0 failing (3 IPv6-conditional skips).

Comment thread lib/util/cache.js Outdated
return true
}

return Array.isArray(val) && val.every(v => typeof v === 'string')

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.

use a proper for loop, .every() is slow.

@codecov-commenter

codecov-commenter commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.45%. Comparing base (4708df1) to head (00c975c).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5535   +/-   ##
=======================================
  Coverage   93.44%   93.45%           
=======================================
  Files         110      110           
  Lines       37418    37418           
=======================================
+ Hits        34967    34970    +3     
+ Misses       2451     2448    -3     

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

The underlying normalizeHeaders bug was fixed in nodejs#5536, which covers the
flat-header handling with unit tests. These integration tests exercise the
path the bug was actually reported through - a dns interceptor re-dispatch
landing in a composed cache() (and deduplicate(), which shares the same
cache-key utility) - so the compose-chain scenario stays covered end to end.
@GiHoon1123
GiHoon1123 force-pushed the fix-issue-5522-normalize-headers-interceptor-order branch from af670a2 to 00c975c Compare July 12, 2026 07:20
@GiHoon1123 GiHoon1123 changed the title fix: normalizeHeaders throws on flat header arrays from dns re-dispatch test: add cache/dedupe + dns re-dispatch integration tests Jul 12, 2026
@GiHoon1123

Copy link
Copy Markdown
Contributor Author

With #5536 now merged, I dropped the overlapping implementation changes from this PR and kept the integration
tests for the dns re-dispatch paths through cache() and deduplicate().

The tests are passing, and this PR is ready for merge.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants