diff --git a/.changeset/wait-for-page-cleanup.md b/.changeset/wait-for-page-cleanup.md new file mode 100644 index 0000000000..60687c2e0e --- /dev/null +++ b/.changeset/wait-for-page-cleanup.md @@ -0,0 +1,5 @@ +--- +'@web/test-runner-core': patch +--- + +Wait for in-flight page cleanup and the browser shutdown it starts before completing test runner shutdown. diff --git a/packages/test-runner-core/src/runner/TestScheduler.ts b/packages/test-runner-core/src/runner/TestScheduler.ts index e13d44b174..258a816fb2 100644 --- a/packages/test-runner-core/src/runner/TestScheduler.ts +++ b/packages/test-runner-core/src/runner/TestScheduler.ts @@ -14,6 +14,7 @@ export class TestScheduler { private browsers: BrowserLauncher[]; private finishedBrowsers = new Set(); private stopPromises = new Set>(); + private sessionStopPromises = new Set>(); private browserStartTimeoutMsg: string; constructor( @@ -69,8 +70,9 @@ export class TestScheduler { this.runNextScheduled(); } - stop(): Promise { + async stop(): Promise { this.timeoutHandler.clearAllTimeouts(); + await Promise.all(Array.from(this.sessionStopPromises)); return Promise.all(Array.from(this.stopPromises)); } @@ -163,6 +165,16 @@ export class TestScheduler { } async stopSession(session: TestSession, errors: TestResultError[] = []) { + const stopPromise = this.stopSessionInternal(session, errors); + this.sessionStopPromises.add(stopPromise); + try { + await stopPromise; + } finally { + this.sessionStopPromises.delete(stopPromise); + } + } + + private async stopSessionInternal(session: TestSession, errors: TestResultError[]) { if (this.timeoutHandler.isStale(session)) { return; } diff --git a/packages/test-runner-core/test/src/runner/TestRunner.test.ts b/packages/test-runner-core/test/src/runner/TestRunner.test.ts index 29aa9b5633..88a1b3e205 100644 --- a/packages/test-runner-core/test/src/runner/TestRunner.test.ts +++ b/packages/test-runner-core/test/src/runner/TestRunner.test.ts @@ -1,14 +1,19 @@ import assert from 'node:assert/strict'; import path from 'node:path'; import { describe, it, mock } from 'node:test'; +import { setImmediate } from 'node:timers/promises'; import portfinder from 'portfinder'; -import type { BrowserLauncher } from '../../../dist/browser-launcher/BrowserLauncher.js'; +import type { + BrowserLauncher, + SessionResult, +} from '../../../dist/browser-launcher/BrowserLauncher.js'; import type { TestRunnerCoreConfig } from '../../../dist/config/TestRunnerCoreConfig.js'; import type { TestRunnerGroupConfig } from '../../../dist/index.js'; import type { Logger } from '../../../dist/logger/Logger.js'; import { TestRunner } from '../../../dist/runner/TestRunner.js'; +import { TestRunnerServer } from '../../../dist/server/TestRunnerServer.js'; import { SESSION_STATUS } from '../../../dist/test-session/TestSessionStatus.js'; function createBrowserStub(): BrowserLauncher { @@ -134,6 +139,125 @@ describe('TestRunner', function () { assert.equal(passed, false, 'test runner quits with false'); }); + for (const cleanupPending of [false, true]) { + it( + cleanupPending + ? 'waits for in-flight page cleanup during shutdown' + : 'closes the browser after completed page cleanup during shutdown', + async () => { + const { runner, browser } = await createTestRunner({ watch: true }); + const [session] = Array.from(runner.sessions.all()); + const pageStopped = Promise.withResolvers(); + const testCoverage = {}; + const stopped = mock.fn(); + let finishing: Promise | undefined; + let stopping: Promise | undefined; + runner.on('stopped', stopped); + browser.stopSession.mock.mockImplementation(() => pageStopped.promise); + + try { + await runner.start(); + await setImmediate(); + assert.equal(browser.startSession.mock.callCount(), 1); + finishing = new Promise(resolve => { + const onStatus = (updatedSession: typeof session) => { + if ( + updatedSession.id === session.id && + updatedSession.status === SESSION_STATUS.FINISHED + ) { + runner.sessions.off('session-status-updated', onStatus); + resolve(); + } + }; + runner.sessions.on('session-status-updated', onStatus); + }); + runner.sessions.updateStatus( + { ...runner.sessions.get(session.id)!, passed: true }, + SESSION_STATUS.TEST_FINISHED, + ); + assert.equal(browser.stopSession.mock.callCount(), 1, 'page cleanup has started'); + + if (!cleanupPending) { + pageStopped.resolve({ testCoverage }); + await finishing; + } + stopping = runner.stop(); + await setImmediate(); + if (cleanupPending) { + assert.equal( + browser.stop.mock.callCount(), + 0, + 'browser shutdown must wait for in-flight page cleanup', + ); + assert.equal( + stopped.mock.callCount(), + 0, + 'shutdown must not finish before page cleanup', + ); + } + } finally { + pageStopped.resolve({ testCoverage }); + await finishing; + await stopping; + await runner.stop(); + } + + assert.equal(browser.stop.mock.callCount(), 1, 'the browser is closed after cleanup'); + assert.equal(stopped.mock.callCount(), 1, 'shutdown completes exactly once'); + assert.equal(runner.sessions.get(session.id)!.status, SESSION_STATUS.FINISHED); + assert.equal(runner.sessions.get(session.id)!.testCoverage, testCoverage); + }, + ); + } + + it('waits for browser cleanup created by page cleanup during shutdown', async t => { + const { runner, browser } = await createTestRunner(); + const stopServer = t.mock.method(TestRunnerServer.prototype, 'stop'); + const pageStopped = Promise.withResolvers(); + const browserStopping = Promise.withResolvers(); + const browserStopped = Promise.withResolvers(); + const stopped = mock.fn(); + let stopping: Promise | undefined; + runner.on('stopped', stopped); + browser.stopSession.mock.mockImplementation(() => pageStopped.promise); + browser.stop.mock.mockImplementation(() => { + browserStopping.resolve(); + return browserStopped.promise; + }); + + try { + await runner.start(); + await setImmediate(); + const [session] = Array.from(runner.sessions.all()); + runner.sessions.updateStatus({ ...session, passed: true }, SESSION_STATUS.TEST_FINISHED); + assert.equal(browser.stopSession.mock.callCount(), 1, 'page cleanup has started'); + stopping = runner.stop(); + pageStopped.resolve({}); + await browserStopping.promise; + await setImmediate(); + + assert.equal( + stopServer.mock.callCount(), + 0, + 'server shutdown must wait for browser cleanup created by page cleanup', + ); + assert.equal( + stopped.mock.callCount(), + 0, + 'shutdown waits for the newly created browser cleanup', + ); + } finally { + pageStopped.resolve({}); + browserStopped.resolve(); + await stopping; + await runner.stop(); + } + + assert.equal(browser.stop.mock.callCount(), 1); + assert.equal(stopServer.mock.callCount(), 1); + assert.equal(stopped.mock.callCount(), 1); + }); + describe('groups', () => { it('can create a group in addition to the default group', async () => { const { runner } = await createTestRunner(undefined, [