Repository: vercel/next.js
Pull request: test: attempt to de-flake graceful shutdown test

the test has been flaking recently because (i guess) it takes a bit longer for the app to start rejecting new requests than the test was expecting. i wrapped the assertion in a `retry` so that the test is resillient to that. 

i've applied the same changes to a previously skipped test and un-skipped it -- i'm hoping it's gonna be okay now. also removed some unnecessary assertions about the app still being active at a given point, we don't really care about that.

Changed files:
filename: test/production/graceful-shutdown/index.test.ts
patch: @@ -8,6 +8,7 @@ import {
   launchApp,
   nextBuild,
   nextStart,
+  retry,
   waitFor,
 } from 'next-test-utils'
 import fs from 'fs-extra'
@@ -99,7 +100,7 @@ describe('Graceful Shutdown', () => {
       appPort = await findPort()
       app = await initNextServerScript(
         serverFile,
-        /- Local:/,
+        /✓ Ready in \d+m?s/,
         {
           ...process.env,
           NEXT_EXIT_TIMEOUT_MS: '10',
@@ -185,8 +186,7 @@ function runTests(dev = false) {
     })
 
     describe('should not accept new requests during shutdown cleanup', () => {
-      // TODO: investigate this is constantly failing
-      it.skip('when request is made before shutdown', async () => {
+      it('should finish pending requests but refuse new ones', async () => {
         const appKilledPromise = once(app, 'exit')
 
         const resPromise = fetchViaHTTP(appPort, '/api/long-running')
@@ -196,40 +196,34 @@ function runTests(dev = false) {
         process.kill(app.pid, 'SIGTERM')
         expect(app.exitCode).toBe(null)
 
-        // Long running response should still be running after a bit
-        await waitFor(LONG_RUNNING_MS / 2)
-        expect(app.exitCode).toBe(null)
-
-        // Second request should be rejected
-        await expect(
-          fetchViaHTTP(appPort, '/api/long-running')
-        ).rejects.toThrow()
+        // The app should start rejecting new connections soon
+        await waitForAppToStartRefusingConnections(
+          () => fetchViaHTTP(appPort, '/api/fast'),
+          1000
+        )
 
         // Original request responds as expected without being interrupted
         await expect(resPromise).resolves.toBeDefined()
         const res = await resPromise
         expect(res.status).toBe(200)
         expect(await res.json()).toStrictEqual({ hello: 'world' })
 
-        // App is still running briefly after response returns
-        expect(app.exitCode).toBe(null)
-
         // App finally shuts down
         expect(await appKilledPromise).toEqual([0, null])
         expect(app.exitCode).toBe(0)
       })
 
-      it('when there is no activity', async () => {
+      it('should stop accepting new requests when shutting down', async () => {
         const appKilledPromise = once(app, 'exit')
 
         process.kill(app.pid, 'SIGTERM')
         expect(app.exitCode).toBe(null)
 
-        // yield event loop to allow server to start the shutdown process
-        await waitFor(20)
-        await expect(
-          fetchViaHTTP(appPort, '/api/long-running')
-        ).rejects.toThrow()
+        // The app should start rejecting connections soon
+        await waitForAppToStartRefusingConnections(
+          () => fetchViaHTTP(appPort, '/api/fast'),
+          1000
+        )
 
         // App finally shuts down
         expect(await appKilledPromise).toEqual([0, null])
@@ -238,3 +232,22 @@ function runTests(dev = false) {
     })
   }
 }
+
+async function waitForAppToStartRefusingConnections(
+  sendRequest: () => Promise<import('node-fetch').Response>,
+  maxDuration: number
+) {
+  // shutdown is async and can take a moment, so this is retried
+  await retry(
+    async () => {
+      await expect(sendRequest).rejects.toEqual(
+        expect.objectContaining({
+          code: 'ECONNREFUSED',
+        })
+      )
+    },
+    maxDuration,
+    100,
+    'wait for app to start rejecting connections'
+  )
+}

filename: test/production/graceful-shutdown/src/pages/api/fast.ts
patch: @@ -0,0 +1,5 @@
+import { NextApiRequest, NextApiResponse } from 'next'
+
+export default async (_req: NextApiRequest, res: NextApiResponse) => {
+  res.json({ hello: 'world' })
+}