From 61421a8c0c2abf011868d90df93813992e3c7563 Mon Sep 17 00:00:00 2001 From: Jon Church Date: Sat, 8 Jun 2024 23:25:42 -0400 Subject: [PATCH] skip QUERY tests for Node 21 only, still not supported (#5695) * 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 * update CI to run on 21.7 and 22.2 --- .github/workflows/ci.yml | 4 ++-- test/app.router.js | 4 +++- test/res.send.js | 4 +++- test/support/utils.js | 11 ++++------- 4 files changed, 12 insertions(+), 11 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 diff --git a/test/app.router.js b/test/app.router.js index ae87092f00..707333f043 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() + } var app = express(); app[method]('/foo', function(req, res){ diff --git a/test/res.send.js b/test/res.send.js index 1e1835f823..b4cf68a7df 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() + } var app = express(); app[method]('/', function (req, res) { diff --git a/test/support/utils.js b/test/support/utils.js index a4d9fb8b54..5ad4ca9841 100644 --- a/test/support/utils.js +++ b/test/support/utils.js @@ -77,13 +77,10 @@ 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 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 = { - "22": true - } - return majorsToSkip[getMajorVersion(versionString)] + return Number(getMajorVersion(versionString)) === 21 }