test(node-fetch): re-enable the set-cookie header combining test

[pacocartones]

This relates to...

Nothing open. It removes an it.skip in test/node-fetch/main.js that has outlived its cause by about four years.

Rationale

should allow get all responses of a header was skipped in 7e085e0 (#1216, 2022-02-09) with a bare // TODO: fix test.. The reason it was failing is visible in the diff of that commit: it asserts that a response carrying two Set-Cookie headers exposes them combined,

const url = `${base}cookie`     // server sends res.setHeader('Set-Cookie', ['a=1', 'b=1'])
return fetch(url).then(res => {
  const expected = 'a=1, b=1'
  assert.strictEqual(res.headers.get('set-cookie'), expected)
  assert.strictEqual(res.headers.get('Set-Cookie'), expected)
})

and at that point filterResponse() dropped every forbidden response-header name — set-cookie included — from basic responses, so get('set-cookie') was null.

That filtering was removed three months later in bf6d5a1 (#1469, "feat: remove headers filtering", fixes #1262 / #1463). The test has been able to pass ever since; only the skip stayed behind.

Checked on both sides of that commit, with a small script driving a server that sends Set-Cookie: ['a=1', 'b=1']:

=== 0e64274 (bf6d5a1^) ===
get(set-cookie) = null
get(Set-Cookie) = null
=== bf6d5a1 ===
get(set-cookie) = "a=1, b=1"
get(Set-Cookie) = "a=1, b=1"

The assertion is still meaningful. A test that passes is not automatically a test that still checks something, so I broke the behaviour on purpose to confirm it is not vacuous. Making HeadersList.append in lib/web/fetch/headers.js keep only the first set-cookie instead of combining, with the test file untouched:

✖ should allow get all responses of a header (19.492838ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

  'a=1' !== 'a=1, b=1'

    actual: 'a=1',
    expected: 'a=1, b=1',

So it exercises the real header-combining path, and it fails when that path regresses.

Order-independence: it passes on its own (--test-name-pattern, 3/3) and inside the whole file (3/3).

On the other five skips in this file: they are left alone deliberately. All five use the chai-era expect(...).to.timeout global under /* global expect */; there is no such global under node:test, so unskipping one just gives ReferenceError: expect is not defined at main.js:1468. They need a rewrite against the current runner, not an unskip, and that does not belong in this PR.

Changes

test/node-fetch/main.js: it.skip → it for should allow get all responses of a header, and the stale // TODO: fix test. above it removed. No source changes.

Before (node --test test/node-fetch/main.js):

ℹ tests 130
ℹ pass 124
ℹ fail 0
ℹ skipped 6

After:

ℹ tests 130
ℹ pass 125
ℹ fail 0
ℹ skipped 5

npm run test:node-fetch on the branch: tests 189 / pass 177 / fail 0 / skipped 12, exit code 0. npm run lint passes.

Features

N/A

Bug Fixes

N/A

Breaking Changes and Deprecations

N/A

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• [S] Benchmarked (optional)
• [S] Documented
• Review ready
• In review
• Merge ready

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (ede2a74) to head (98b7f74).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5730      +/-   ##
==========================================
+ Coverage   93.46%   93.47%   +0.01%     
==========================================
  Files         110      110              
  Lines       38939    38939              
==========================================
+ Hits        36394    36398       +4     
+ Misses       2545     2541       -4     

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