From 13e7fb4ce1a3664aac49901d4e3c91aded5765d6 Mon Sep 17 00:00:00 2001 From: Jon Church Date: Wed, 5 Jun 2024 19:09:08 -0400 Subject: [PATCH 1/4] skip QUERY tests for Node 21 only, still not supported QUERY support has now landed in Node 22.2.0, but is still not supported in 21.7.3 QUERY showed up in http.METHODS in 21.7.2. Only Node versions after that will attempt to run tests for it, based on the way we dynamically test members of the http.METHODS array from Node --- test/support/utils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/support/utils.js b/test/support/utils.js index a4d9fb8b54..818656b3ee 100644 --- a/test/support/utils.js +++ b/test/support/utils.js @@ -77,12 +77,12 @@ function getMajorVersion(versionString) { } function shouldSkipQuery(versionString) { - // Temporarily skipping this test on 22 - // update this implementation to run on those release lines on supported versions once they exist - // upstream tracking https://github.com/nodejs/node/pull/51719 + // Skipping HTTP QUERY tests on 21, it is reported in http.METHODS on 21.7.2 but not supported + // 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 var majorsToSkip = { - "22": true + "21": true } return majorsToSkip[getMajorVersion(versionString)] } From 4474718ff3db651fcf00b5e5aeaa1758b76abc3d Mon Sep 17 00:00:00 2001 From: Jon Church Date: Wed, 5 Jun 2024 19:15:52 -0400 Subject: [PATCH 2/4] update CI to run on 21.7 and 22.2 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 02137e595e..920db416d6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,10 +134,10 @@ jobs: node-version: "20.11" - name: Node.js 21.x - node-version: "21.6" + node-version: "21.7" - name: Node.js 22.x - node-version: "22.0" + node-version: "22.2" steps: - uses: actions/checkout@v4 From 9d3bcfe7adbf6c07628da8472afa3c9b494a376a Mon Sep 17 00:00:00 2001 From: Jon Church Date: Fri, 7 Jun 2024 20:07:17 -0400 Subject: [PATCH 3/4] update query test to actually skip, simplify shouldSkipQuery --- test/app.router.js | 4 +++- test/res.send.js | 4 +++- test/support/utils.js | 7 ++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/test/app.router.js b/test/app.router.js index ae87092f00..e915b71501 100644 --- a/test/app.router.js +++ b/test/app.router.js @@ -39,9 +39,11 @@ describe('app.router', function(){ describe('methods', function(){ methods.concat('del').forEach(function(method){ if (method === 'connect') return; - if (method === 'query' && shouldSkipQuery(process.versions.node)) return it('should include ' + method.toUpperCase(), function(done){ + if (method === 'query' && shouldSkipQuery(process.versions.node)) { + this.skip("QUERY is not fully supported in this version of Node") + } var app = express(); app[method]('/foo', function(req, res){ diff --git a/test/res.send.js b/test/res.send.js index 1e1835f823..45c676a637 100644 --- a/test/res.send.js +++ b/test/res.send.js @@ -409,9 +409,11 @@ describe('res', function(){ methods.forEach(function (method) { if (method === 'connect') return; - if (method === 'query' && shouldSkipQuery(process.versions.node)) return it('should send ETag in response to ' + method.toUpperCase() + ' request', function (done) { + if (method === 'query' && shouldSkipQuery(process.versions.node)) { + this.skip("QUERY is not fully supported in this version of Node") + } var app = express(); app[method]('/', function (req, res) { diff --git a/test/support/utils.js b/test/support/utils.js index 818656b3ee..5ad4ca9841 100644 --- a/test/support/utils.js +++ b/test/support/utils.js @@ -77,13 +77,10 @@ function getMajorVersion(versionString) { } function shouldSkipQuery(versionString) { - // Skipping HTTP QUERY tests on 21, it is reported in http.METHODS on 21.7.2 but not supported + // Skipping HTTP QUERY tests on Node 21, it is reported in http.METHODS on 21.7.2 but not supported // 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 - var majorsToSkip = { - "21": true - } - return majorsToSkip[getMajorVersion(versionString)] + return Number(getMajorVersion(versionString)) === 21 } From fcf7ca62d260074b2c5e119ae79cda0a427f9c2f Mon Sep 17 00:00:00 2001 From: Jon Church Date: Fri, 7 Jun 2024 20:18:38 -0400 Subject: [PATCH 4/4] remove skip message, doesn't seem to work --- test/app.router.js | 2 +- test/res.send.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/app.router.js b/test/app.router.js index e915b71501..707333f043 100644 --- a/test/app.router.js +++ b/test/app.router.js @@ -42,7 +42,7 @@ describe('app.router', function(){ it('should include ' + method.toUpperCase(), function(done){ if (method === 'query' && shouldSkipQuery(process.versions.node)) { - this.skip("QUERY is not fully supported in this version of Node") + this.skip() } var app = express(); diff --git a/test/res.send.js b/test/res.send.js index 45c676a637..b4cf68a7df 100644 --- a/test/res.send.js +++ b/test/res.send.js @@ -412,7 +412,7 @@ describe('res', function(){ it('should send ETag in response to ' + method.toUpperCase() + ' request', function (done) { if (method === 'query' && shouldSkipQuery(process.versions.node)) { - this.skip("QUERY is not fully supported in this version of Node") + this.skip() } var app = express();