Skip to content

Commit

Permalink
Test against Node 20, update tests for compatibility (#7636)
Browse files Browse the repository at this point in the history
Test against Node 20 and update tests as needed.

This unblocks our integrations from testing against Node 20 as well
since the testsuite is part of this changeset / has Node 20 test
failures.
trevor-scheer authored Jul 22, 2023
1 parent 9139a28 commit 42fc65c
Showing 6 changed files with 76 additions and 34 deletions.
6 changes: 6 additions & 0 deletions .changeset/four-mangos-hammer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@apollo/server-integration-testsuite': patch
'@apollo/server': patch
---

Update test suite for compatibility with Node v20
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -132,7 +132,6 @@ jobs:
command: "! git grep FIXM\x45"

workflows:
version: 2
Build:
jobs:
- NodeJS:
@@ -143,6 +142,7 @@ workflows:
- "14"
- "16"
- "18"
- "20"
- "Check for FIXM\x45"
- Prettier
- ESLint
2 changes: 1 addition & 1 deletion packages/integration-testsuite/src/httpServerTests.ts
Original file line number Diff line number Diff line change
@@ -382,7 +382,7 @@ export function defineIntegrationTestSuiteHttpServerTests(
return req.then((res) => {
expect(res.status).toEqual(400);
expect(
['Unexpected token f', 'Bad Request', 'Invalid JSON'].some(
['in JSON at position 1', 'Bad Request', 'Invalid JSON'].some(
(substring) => (res.error as HTTPError).text.includes(substring),
),
).toBe(true);
Original file line number Diff line number Diff line change
@@ -193,7 +193,7 @@ it('supporting doubly-encoded variables example from migration guide', async ()
query: 'query Hello($s: String!){hello(s: $s)}',
variables: '{malformed JSON}',
})
.expect(400, 'Unexpected token m in JSON at position 1');
.expect(400, /in JSON at position 1/);

await server.stop();
});
Original file line number Diff line number Diff line change
@@ -32,7 +32,6 @@ const a: any = require('awaiting');
const request: any = require('requisition');
import fs from 'fs';
import { Stopper } from '../../../plugin/drainHttpServer/stoppable';
import child from 'child_process';
import path from 'path';
import type { AddressInfo } from 'net';
import { describe, it, expect, afterEach, beforeEach } from '@jest/globals';
@@ -113,7 +112,7 @@ Object.keys(schemes).forEach((schemeName) => {
const err = await a.failure(
request(`${schemeName}://localhost:${p}`).agent(scheme.agent()),
);
expect(err.message).toMatch(/ECONNREFUSED/);
expect(err.code).toMatch(/ECONNREFUSED/);
expect(closed).toBe(1);
});

@@ -135,9 +134,54 @@ Object.keys(schemes).forEach((schemeName) => {
scheme.agent({ keepAlive: true }),
),
);
expect(err.message).toMatch(/ECONNREFUSED/);
expect(closed).toBe(0);
expect(err.code).toMatch(/ECONNREFUSED/);

// Node 19 (http) and 20.4+ (https) more aggressively close idle
// connections. `Stopper` is no longer needed for the purpose of closing
// idle connections in these versions. However, `Stopper` _is_ still
// useful for gracefully finishing in-flight requests within the timeout
// (and aborting requests beyond the timeout).
const isNode20 = !!process.version.match(/^v20\./);
expect(closed).toBe(isNode20 ? 1 : 0);
});

// This test specifically added for Node 20 fails for Node 14. Just going
// to skip it since we're dropping Node 14 soon anyway.
const node14 = !!process.version.match(/^v14\./);
(node14 ? it.skip : it)(
'with unfinished requests',
async () => {
const server = scheme.server(async (_req, res) => {
res.writeHead(200);
res.write('hi'); // note lack of end()!
});
// The server will prevent itself from closing while the connection
// remains open (default no timeout). This will close the connection
// after 100ms so the test can finish.
server.setTimeout(100);

server.listen(0);
const p = port(server);

const response = await request(
`${schemeName}://localhost:${p}`,
).agent(scheme.agent({ keepAlive: true }));
// ensure we got the headers, etc.
expect(response.status).toBe(200);

server.close();
await a.event(server, 'close');

try {
await response.text();
} catch (e: any) {
expect(e.code).toMatch(/ECONNRESET/);
}
// ensure the expectation in the catch block is reached (+ the one above)
expect.assertions(2);
},
35000,
);
});

describe('Stopper', function () {
@@ -159,7 +203,7 @@ Object.keys(schemes).forEach((schemeName) => {
const err = await a.failure(
request(`${schemeName}://localhost:${p}`).agent(scheme.agent()),
);
expect(err.message).toMatch(/ECONNREFUSED/);
expect(err.code).toMatch(/ECONNREFUSED/);

expect(closed).toBe(1);
expect(gracefully).toBe(true);
@@ -185,7 +229,7 @@ Object.keys(schemes).forEach((schemeName) => {
scheme.agent({ keepAlive: true }),
),
);
expect(err.message).toMatch(/ECONNREFUSED/);
expect(err.code).toMatch(/ECONNREFUSED/);

expect(closed).toBe(1);
expect(gracefully).toBe(true);
@@ -301,12 +345,21 @@ Object.keys(schemes).forEach((schemeName) => {

if (schemeName === 'http') {
it('with in-flights finishing before grace period ends', async () => {
const file = path.join(__dirname, 'stoppable', 'server.js');
const server = child.spawn('node', [file]);
const [data] = await a.event(server.stdout, 'data');
const port = +data.toString();
expect(typeof port).toBe('number');
const res = await request(`${schemeName}://localhost:${port}/`).agent(
let stopper: Stopper;
const killServerBarrier = resolvable();
const server = http.createServer(async (_, res) => {
res.writeHead(200);
res.write('hello');

await killServerBarrier;
res.end('world');
await stopper.stop();
});
stopper = new Stopper(server);
server.listen(0);
const p = port(server);

const res = await request(`${schemeName}://localhost:${p}/`).agent(
scheme.agent({ keepAlive: true }),
);
let gotBody = false;
@@ -320,13 +373,13 @@ Object.keys(schemes).forEach((schemeName) => {
expect(gotBody).toBe(false);

// Tell the server that its request should finish.
server.kill('SIGUSR1');
killServerBarrier.resolve();

const body = await bodyPromise;
expect(gotBody).toBe(true);
expect(body).toBe('helloworld');

// Wait for subprocess to go away.
// Wait for server to close.
await a.event(server, 'close');
});
}

This file was deleted.

0 comments on commit 42fc65c

Please sign in to comment.