Test failing on Node v20.x
If someone could help us resolve this, that would be great. (ref: https://github.com/pillarjs/router/actions/runs/15097652965/job/42434464916?pr=161)
Investigating — it seems like something changed in the latest Node 20 release which came out last week
nvm install 20.19.1
npm run test
[...]
678 passing (1s)
nvm install 20.19.2
npm run test
[...]
214 passing (374ms)
1 failing
1) Router
.route(path)
.query(...fn)
should respond to a QUERY request:
Error: expected 200 "OK", got 404 "Not Found"
at Context.<anonymous> (test/route.js:274:14)
This is interesting — I made a file test.js:
const http = require("http");
const server = http.createServer((req, res) => {
res.writeHead(200, { "Content-Type": "text/plain" });
res.end("Hello World\n");
});
server.listen(3000, async () => {
console.log("Server running at http://localhost:3000/");
const res = await fetch("http://localhost:3000/", {
method: "QUERY",
});
console.log(res.status);
});
I get different output on Node v20.19.1 vs v20.19.2:
nvm use 20.19.1
node test
Server running at http://localhost:3000/
400
nvm use 20.19.2
node test
Server running at http://localhost:3000/
200
More testing in test.js:
const { METHODS } = require("http");
console.log(METHODS);
Note that QUERY is added in v20.19.2:
nvm use 20.19.1
node test
[
'ACL', 'BIND', 'CHECKOUT',
'CONNECT', 'COPY', 'DELETE',
'GET', 'HEAD', 'LINK',
'LOCK', 'M-SEARCH', 'MERGE',
'MKACTIVITY', 'MKCALENDAR', 'MKCOL',
'MOVE', 'NOTIFY', 'OPTIONS',
'PATCH', 'POST', 'PROPFIND',
'PROPPATCH', 'PURGE', 'PUT',
'REBIND', 'REPORT', 'SEARCH',
'SOURCE', 'SUBSCRIBE', 'TRACE',
'UNBIND', 'UNLINK', 'UNLOCK',
'UNSUBSCRIBE'
]
nvm use 20.19.2
node test
[
'ACL', 'BIND', 'CHECKOUT',
'CONNECT', 'COPY', 'DELETE',
'GET', 'HEAD', 'LINK',
'LOCK', 'M-SEARCH', 'MERGE',
'MKACTIVITY', 'MKCALENDAR', 'MKCOL',
'MOVE', 'NOTIFY', 'OPTIONS',
'PATCH', 'POST', 'PROPFIND',
'PROPPATCH', 'PURGE', 'PUT',
'QUERY', 'REBIND', 'REPORT',
'SEARCH', 'SOURCE', 'SUBSCRIBE',
'TRACE', 'UNBIND', 'UNLINK',
'UNLOCK', 'UNSUBSCRIBE'
]
FWIW, we exclude the QUERY test in Node v21 (see https://github.com/pillarjs/router/pull/119#issuecomment-2323438421), so one remedy could be to do the same here and exclude the test on Node v20 as well. cc @wesleytodd in case you have more context on why exactly we're skipping on 21.
https://github.com/pillarjs/router/blob/e6d6b609fc355e558174ccd5b1db646f739fe88c/test/route.js#L257-L259
Note that the addition of QUERY to the http.METHODS constant in v20.19.2 effectively adds a test to the test suite, since tests are generated by iterating over the http.METHODS array.
That means that omitting the QUERY test would actually run the same tests as were run in v20.19.1. On v20.19.2, there are extra tests that weren't present before because there's the new element "QUERY" in the METHODS array. It seems like these extra tests are what are causing the breakage.
https://github.com/pillarjs/router/blob/e6d6b609fc355e558174ccd5b1db646f739fe88c/test/route.js#L252-L266
Seems to be an issue with supertest dependency.. Since we use it for making API calls, and it does not support the latest query method, the test fails with a 404 error. This bug is already reported on supertest repo:
Ref: https://github.com/ladjs/supertest/issues/860
This is due to the latest Node.js security patch, it's probably enough to just skip that test
https://github.com/expressjs/express/pull/6512
@bjohansebas cool, just opened a PR which skips those tests on Node <22
https://github.com/pillarjs/router/pull/169
Should we close this? The tests are passing