Skip to content

Commit 61421a8

Browse files
authored
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
1 parent f42b160 commit 61421a8

File tree

4 files changed

+12
-11
lines changed

4 files changed

+12
-11
lines changed

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ jobs:
134134
node-version: "20.11"
135135

136136
- name: Node.js 21.x
137-
node-version: "21.6"
137+
node-version: "21.7"
138138

139139
- name: Node.js 22.x
140-
node-version: "22.0"
140+
node-version: "22.2"
141141

142142
steps:
143143
- uses: actions/checkout@v4

test/app.router.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ describe('app.router', function(){
3939
describe('methods', function(){
4040
methods.concat('del').forEach(function(method){
4141
if (method === 'connect') return;
42-
if (method === 'query' && shouldSkipQuery(process.versions.node)) return
4342

4443
it('should include ' + method.toUpperCase(), function(done){
44+
if (method === 'query' && shouldSkipQuery(process.versions.node)) {
45+
this.skip()
46+
}
4547
var app = express();
4648

4749
app[method]('/foo', function(req, res){

test/res.send.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,11 @@ describe('res', function(){
409409

410410
methods.forEach(function (method) {
411411
if (method === 'connect') return;
412-
if (method === 'query' && shouldSkipQuery(process.versions.node)) return
413412

414413
it('should send ETag in response to ' + method.toUpperCase() + ' request', function (done) {
414+
if (method === 'query' && shouldSkipQuery(process.versions.node)) {
415+
this.skip()
416+
}
415417
var app = express();
416418

417419
app[method]('/', function (req, res) {

test/support/utils.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,10 @@ function getMajorVersion(versionString) {
7777
}
7878

7979
function shouldSkipQuery(versionString) {
80-
// Temporarily skipping this test on 22
81-
// update this implementation to run on those release lines on supported versions once they exist
82-
// upstream tracking https://github.com/nodejs/node/pull/51719
80+
// Skipping HTTP QUERY tests on Node 21, it is reported in http.METHODS on 21.7.2 but not supported
81+
// update this implementation to run on supported versions of 21 once they exist
82+
// upstream tracking https://github.com/nodejs/node/issues/51562
8383
// express tracking issue: https://github.com/expressjs/express/issues/5615
84-
var majorsToSkip = {
85-
"22": true
86-
}
87-
return majorsToSkip[getMajorVersion(versionString)]
84+
return Number(getMajorVersion(versionString)) === 21
8885
}
8986

0 commit comments

Comments
 (0)