From:
kapouer@melix.org
Le jeu. 5 juin 2025 à 02:28, Santiago Vila <
sanvila@debian.org> a écrit :
Thanks a lot for the investigation!
So, it seems the logical thing to do here is to disable those failing
tests,
as they were not really intended to be run with nodejs 20.
Is there any team preference about how to do that?
I can think of at least three methods.
Method 1: Skip also when running nodejs 20
--- a/test/body-parser.js
+++ b/test/body-parser.js
@@ -82,7 +82,7 @@ describe('bodyParser()', function () {
// update this implementation to run on supported versions of 21
once they exist
// upstream tracking https://github.com/nodejs/node/issues/51562
// express tracking issue: https://github.com/expressjs/express/issues/5615
- return getMajorVersion(versionString) === '21'
+ return getMajorVersion(versionString) === '20' || getMajorVersion(versionString) === '21'
}
methods.slice().sort().forEach(function (method) {
Method 2: Make the function to return true unconditionally, since this is targeted
for trixie which will have nodejs 20.
--- a/test/body-parser.js
+++ b/test/body-parser.js
@@ -82,7 +82,7 @@ describe('bodyParser()', function () {
// update this implementation to run on supported versions of 21
once they exist
// upstream tracking https://github.com/nodejs/node/issues/51562
// express tracking issue: https://github.com/expressjs/express/issues/5615
- return getMajorVersion(versionString) === '21'
+ return true
}
methods.slice().sort().forEach(function (method) {
Method 3: Assume that the function would return true in the place where
the return value is used:
--- a/test/body-parser.js
+++ b/test/body-parser.js
@@ -89,7 +89,7 @@ describe('bodyParser()', function () {
if (method === 'connect') return
it('should support ' + method.toUpperCase() + ' requests',
function (done) {
- if (method === 'query' && shouldSkipQuery(process.versions.node))
{
+ if (method === 'query') {
this.skip()
}
request(this.server)[method]('/')
I could make a team upload if some authorized voice tells me which
solution is best/preferred.
(My personal preference would be method 2).
Thanks.
The best fix would be in the embedded copy of llhttp in nodejs 20.19.2 -
remove the HTTP QUERY support that was
added in
https://github.com/nodejs/node/commit/eb25047b1b08180ed68dcefe2299682fbc7966ab
This would avoid potentially hidden bugs in the nodejs-dependents packages. Shall we reassign this bug to nodejs ?
<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le jeu. 5 juin 2025 à 02:28, Santiago Vila <<a href="mailto:
sanvila@debian.org" target="_blank">
sanvila@debian.org</a>> a é
crit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks a lot for the investigation!<br>
So, it seems the logical thing to do here is to disable those failing tests,<br>
as they were not really intended to be run with nodejs 20.<br>
Is there any team preference about how to do that?<br>
I can think of at least three methods.<br>
Method 1: Skip also when running nodejs 20<br>
--- a/test/body-parser.js<br>
+++ b/test/body-parser.js<br>
@@ -82,7 +82,7 @@ describe('bodyParser()', function () {<br>
// update this implementation to run on supported versions of 21 once they exist<br>
// upstream tracking <a href="
https://github.com/nodejs/node/issues/51562" rel="noreferrer" target="_blank">
https://github.com/nodejs/node/issues/51562</a><br>
// express tracking issue: <a href="
https://github.com/expressjs/express/issues/5615" rel="noreferrer" target="_blank">
https://github.com/expressjs/express/issues/5615</a><br>
- return getMajorVersion(versionString) === '21'<br>
+ return getMajorVersion(versionString) === '20' || getMajorVersion(versionString) === '21'<br>
}<br>
methods.slice().sort().forEach(function (method) {<br>
Method 2: Make the function to return true unconditionally, since this is targeted<br>
for trixie which will have nodejs 20.<br>
--- a/test/