Skip to content

Commit

Permalink
skip QUERY tests for Node 21 only, still not supported (#5695)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jonchurch authored Jun 9, 2024
1 parent f42b160 commit 61421a8
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/app.router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Expand Down
4 changes: 3 additions & 1 deletion test/res.send.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 4 additions & 7 deletions test/support/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 61421a8

Please sign in to comment.