From a9fc4de37e4c4ef3c6a43a38271dc7c5e7ffc2a5 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 21 Mar 2024 14:28:07 -0700 Subject: [PATCH] chore: queue run and list commands from ui (#30033) --- .../src/isomorphic/testServerConnection.ts | 17 ++++++------ .../src/isomorphic/testServerInterface.ts | 2 +- packages/playwright/src/runner/testServer.ts | 26 +++++++++++-------- packages/trace-viewer/src/ui/uiModeView.tsx | 21 ++++++++++----- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/packages/playwright/src/isomorphic/testServerConnection.ts b/packages/playwright/src/isomorphic/testServerConnection.ts index 9c3f84e1ec..3a07fbcb31 100644 --- a/packages/playwright/src/isomorphic/testServerConnection.ts +++ b/packages/playwright/src/isomorphic/testServerConnection.ts @@ -15,7 +15,7 @@ */ import type { TestServerInterface, TestServerInterfaceEvents } from '@testIsomorphic/testServerInterface'; -import type { Location, TestError } from 'playwright/types/testReporter'; +import type * as reporterTypes from 'playwright/types/testReporter'; import * as events from './events'; export class TestServerConnection implements TestServerInterface, TestServerInterfaceEvents { @@ -110,7 +110,7 @@ export class TestServerConnection implements TestServerInterface, TestServerInte } async pingNoReply() { - await this._sendMessageNoReply('ping'); + this._sendMessageNoReply('ping'); } async watch(params: { fileNames: string[]; }): Promise { @@ -121,11 +121,11 @@ export class TestServerConnection implements TestServerInterface, TestServerInte this._sendMessageNoReply('watch', params); } - async open(params: { location: Location; }): Promise { + async open(params: { location: reporterTypes.Location; }): Promise { await this._sendMessage('open', params); } - openNoReply(params: { location: Location; }) { + openNoReply(params: { location: reporterTypes.Location; }) { this._sendMessageNoReply('open', params); } @@ -153,7 +153,7 @@ export class TestServerConnection implements TestServerInterface, TestServerInte return await this._sendMessage('runGlobalTeardown'); } - async listFiles(): Promise<{ projects: { name: string; testDir: string; use: { testIdAttribute?: string | undefined; }; files: string[]; }[]; cliEntryPoint?: string | undefined; error?: TestError | undefined; }> { + async listFiles(): Promise<{ projects: { name: string; testDir: string; use: { testIdAttribute?: string | undefined; }; files: string[]; }[]; cliEntryPoint?: string | undefined; error?: reporterTypes.TestError | undefined; }> { return await this._sendMessage('listFiles'); } @@ -161,11 +161,11 @@ export class TestServerConnection implements TestServerInterface, TestServerInte return await this._sendMessage('listTests', params); } - async runTests(params: { reporter?: string | undefined; locations?: string[] | undefined; grep?: string | undefined; testIds?: string[] | undefined; headed?: boolean | undefined; oneWorker?: boolean | undefined; trace?: 'off' | 'on' | undefined; projects?: string[] | undefined; reuseContext?: boolean | undefined; connectWsEndpoint?: string | undefined; }): Promise { - await this._sendMessage('runTests', params); + async runTests(params: { reporter?: string | undefined; locations?: string[] | undefined; grep?: string | undefined; testIds?: string[] | undefined; headed?: boolean | undefined; oneWorker?: boolean | undefined; trace?: 'off' | 'on' | undefined; projects?: string[] | undefined; reuseContext?: boolean | undefined; connectWsEndpoint?: string | undefined; }): Promise<{ status: reporterTypes.FullResult['status'] }> { + return await this._sendMessage('runTests', params); } - async findRelatedTestFiles(params: { files: string[]; }): Promise<{ testFiles: string[]; errors?: TestError[] | undefined; }> { + async findRelatedTestFiles(params: { files: string[]; }): Promise<{ testFiles: string[]; errors?: reporterTypes.TestError[] | undefined; }> { return await this._sendMessage('findRelatedTestFiles', params); } @@ -177,7 +177,6 @@ export class TestServerConnection implements TestServerInterface, TestServerInte this._sendMessageNoReply('stopTests'); } - async closeGracefully(): Promise { await this._sendMessage('closeGracefully'); } diff --git a/packages/playwright/src/isomorphic/testServerInterface.ts b/packages/playwright/src/isomorphic/testServerInterface.ts index c10a273ef6..77fdd6049c 100644 --- a/packages/playwright/src/isomorphic/testServerInterface.ts +++ b/packages/playwright/src/isomorphic/testServerInterface.ts @@ -66,7 +66,7 @@ export interface TestServerInterface { projects?: string[]; reuseContext?: boolean; connectWsEndpoint?: string; - }): Promise; + }): Promise<{ status: reporterTypes.FullResult['status'] }>; findRelatedTestFiles(params: { files: string[]; diff --git a/packages/playwright/src/runner/testServer.ts b/packages/playwright/src/runner/testServer.ts index 828c3fbdc0..b915834d24 100644 --- a/packages/playwright/src/runner/testServer.ts +++ b/packages/playwright/src/runner/testServer.ts @@ -19,7 +19,7 @@ import path from 'path'; import { registry, startTraceViewerServer } from 'playwright-core/lib/server'; import { ManualPromise, gracefullyProcessExitDoNotHang, isUnderTest } from 'playwright-core/lib/utils'; import type { Transport, HttpServer } from 'playwright-core/lib/utils'; -import type { FullResult, Location, TestError } from '../../types/testReporter'; +import type * as reporterTypes from '../../types/testReporter'; import { collectAffectedTestFiles, dependenciesForTestFile } from '../transform/compilationCache'; import type { FullConfigInternal } from '../common/config'; import { InternalReporter } from '../reporters/internalReporter'; @@ -93,10 +93,10 @@ class TestServerDispatcher implements TestServerInterface { private _config: FullConfigInternal; private _globalWatcher: Watcher; private _testWatcher: Watcher; - private _testRun: { run: Promise, stop: ManualPromise } | undefined; + private _testRun: { run: Promise, stop: ManualPromise } | undefined; readonly transport: Transport; private _queue = Promise.resolve(); - private _globalCleanup: (() => Promise) | undefined; + private _globalCleanup: (() => Promise) | undefined; readonly _dispatchEvent: TestServerInterfaceEventEmitters['dispatchEvent']; constructor(config: FullConfigInternal) { @@ -116,7 +116,7 @@ class TestServerDispatcher implements TestServerInterface { async ping() {} - async open(params: { location: Location }) { + async open(params: { location: reporterTypes.Location }) { if (isUnderTest()) return; // eslint-disable-next-line no-console @@ -138,7 +138,7 @@ class TestServerDispatcher implements TestServerInterface { await installBrowsers(); } - async runGlobalSetup(): Promise { + async runGlobalSetup(): Promise { await this.runGlobalTeardown(); const reporter = new InternalReporter(new ListReporter()); @@ -167,7 +167,7 @@ class TestServerDispatcher implements TestServerInterface { const runner = new Runner(this._config); return runner.listTestFiles(); } catch (e) { - const error: TestError = serializeError(e); + const error: reporterTypes.TestError = serializeError(e); error.location = prepareErrorStack(e.stack).location; return { projects: [], error }; } @@ -214,11 +214,15 @@ class TestServerDispatcher implements TestServerInterface { } async runTests(params: { reporter?: string; locations?: string[] | undefined; grep?: string | undefined; testIds?: string[] | undefined; headed?: boolean | undefined; oneWorker?: boolean | undefined; trace?: 'off' | 'on' | undefined; projects?: string[] | undefined; reuseContext?: boolean | undefined; connectWsEndpoint?: string | undefined; }) { - this._queue = this._queue.then(() => this._innerRunTests(params)).catch(printInternalError); + let status: reporterTypes.FullResult['status']; + this._queue = this._queue.then(async () => { + status = await this._innerRunTests(params).catch(printInternalError) || 'failed'; + }); await this._queue; + return { status: status! }; } - private async _innerRunTests(params: { reporter?: string; locations?: string[] | undefined; grep?: string | undefined; testIds?: string[] | undefined; headed?: boolean | undefined; oneWorker?: boolean | undefined; trace?: 'off' | 'on' | undefined; projects?: string[] | undefined; reuseContext?: boolean | undefined; connectWsEndpoint?: string | undefined; }) { + private async _innerRunTests(params: { reporter?: string; locations?: string[] | undefined; grep?: string | undefined; testIds?: string[] | undefined; headed?: boolean | undefined; oneWorker?: boolean | undefined; trace?: 'off' | 'on' | undefined; projects?: string[] | undefined; reuseContext?: boolean | undefined; connectWsEndpoint?: string | undefined; }): Promise { await this.stopTests(); const { testIds, projects, locations, grep } = params; @@ -244,7 +248,7 @@ class TestServerDispatcher implements TestServerInterface { return status; }); this._testRun = { run, stop }; - await run; + return await run; } async watch(params: { fileNames: string[]; }) { @@ -256,7 +260,7 @@ class TestServerDispatcher implements TestServerInterface { this._testWatcher.update([...files], [], true); } - findRelatedTestFiles(params: { files: string[]; }): Promise<{ testFiles: string[]; errors?: TestError[] | undefined; }> { + findRelatedTestFiles(params: { files: string[]; }): Promise<{ testFiles: string[]; errors?: reporterTypes.TestError[] | undefined; }> { const runner = new Runner(this._config); return runner.findRelatedTestFiles('out-of-process', params.files); } @@ -271,7 +275,7 @@ class TestServerDispatcher implements TestServerInterface { } } -export async function runTestServer(config: FullConfigInternal, options: { host?: string, port?: number }, openUI: (server: HttpServer, cancelPromise: ManualPromise) => Promise): Promise { +export async function runTestServer(config: FullConfigInternal, options: { host?: string, port?: number }, openUI: (server: HttpServer, cancelPromise: ManualPromise) => Promise): Promise { const testServer = new TestServer(config); const cancelPromise = new ManualPromise(); const sigintWatcher = new SigIntWatcher(); diff --git a/packages/trace-viewer/src/ui/uiModeView.tsx b/packages/trace-viewer/src/ui/uiModeView.tsx index 58e34a39e6..06bb83bc45 100644 --- a/packages/trace-viewer/src/ui/uiModeView.tsx +++ b/packages/trace-viewer/src/ui/uiModeView.tsx @@ -66,7 +66,7 @@ export const UIModeView: React.FC<{}> = ({ const [runningState, setRunningState] = React.useState<{ testIds: Set, itemSelectedByUser?: boolean } | undefined>(); const [watchAll, setWatchAll] = useSetting('watch-all', false); const [watchedTreeIds, setWatchedTreeIds] = React.useState<{ value: Set }>({ value: new Set() }); - const runTestPromiseChain = React.useRef(Promise.resolve()); + const commandQueue = React.useRef(Promise.resolve()); const runTestBacklog = React.useRef>(new Set()); const [collapseAllCount, setCollapseAllCount] = React.useState(0); const [isDisconnected, setIsDisconnected] = React.useState(false); @@ -114,7 +114,6 @@ export const UIModeView: React.FC<{}> = ({ }; }, [testServerConnection]); - // This is the main routine, every time connection updates it starts the // whole workflow. React.useEffect(() => { @@ -141,10 +140,18 @@ export const UIModeView: React.FC<{}> = ({ }); const updateList = async () => { - setIsLoading(true); - const result = await testServerConnection.listTests({}); - teleSuiteUpdater.processListReport(result.report); - setIsLoading(false); + commandQueue.current = commandQueue.current.then(async () => { + setIsLoading(true); + try { + const result = await testServerConnection.listTests({}); + teleSuiteUpdater.processListReport(result.report); + } catch (e) { + // eslint-disable-next-line no-console + console.log(e); + } finally { + setIsLoading(false); + } + }); }; setTestModel(undefined); @@ -221,7 +228,7 @@ export const UIModeView: React.FC<{}> = ({ return; runTestBacklog.current = new Set([...runTestBacklog.current, ...testIds]); - runTestPromiseChain.current = runTestPromiseChain.current.then(async () => { + commandQueue.current = commandQueue.current.then(async () => { const testIds = runTestBacklog.current; runTestBacklog.current = new Set(); if (!testIds.size)