From eed74036e81a4708725cdbf84f3a751064f23a03 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 20 Mar 2023 13:45:35 -0700 Subject: [PATCH] cherry-pick(#21809): chore(ui): queue watch runs --- .../playwright-core/src/server/browserType.ts | 2 +- .../src/server/chromium/chromium.ts | 2 +- .../src/server/recorder/recorderApp.ts | 5 +- .../src/server/trace/viewer/traceViewer.ts | 5 +- packages/playwright-core/src/server/types.ts | 2 +- packages/playwright-test/src/runner/uiMode.ts | 20 ++--- packages/trace-viewer/src/ui/watchMode.tsx | 81 ++++++++++--------- tests/playwright-test/ui-mode-fixtures.ts | 22 +++++ .../ui-mode-test-watch.spec.ts | 43 ++++++++++ 9 files changed, 123 insertions(+), 59 deletions(-) diff --git a/packages/playwright-core/src/server/browserType.ts b/packages/playwright-core/src/server/browserType.ts index e3e871c884..1ae07171be 100644 --- a/packages/playwright-core/src/server/browserType.ts +++ b/packages/playwright-core/src/server/browserType.ts @@ -75,7 +75,7 @@ export abstract class BrowserType extends SdkObject { return browser; } - async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean, ignoreChromiumSwitches?: boolean }): Promise { + async launchPersistentContext(metadata: CallMetadata, userDataDir: string, options: channels.BrowserTypeLaunchPersistentContextOptions & { useWebSocket?: boolean }): Promise { options = this._validateLaunchOptions(options); const controller = new ProgressController(metadata, this); const persistent: channels.BrowserNewContextParams = options; diff --git a/packages/playwright-core/src/server/chromium/chromium.ts b/packages/playwright-core/src/server/chromium/chromium.ts index 8306044c93..af72f730fe 100644 --- a/packages/playwright-core/src/server/chromium/chromium.ts +++ b/packages/playwright-core/src/server/chromium/chromium.ts @@ -283,7 +283,7 @@ export class Chromium extends BrowserType { throw new Error('Playwright manages remote debugging connection itself.'); if (args.find(arg => !arg.startsWith('-'))) throw new Error('Arguments can not specify page to be opened'); - const chromeArguments = options.ignoreChromiumSwitches ? [] : [...chromiumSwitches]; + const chromeArguments = [...chromiumSwitches]; if (os.platform() === 'darwin') { // See https://github.com/microsoft/playwright/issues/7362 diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index 883506e5d2..503c91a1e4 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -128,11 +128,8 @@ export class RecorderApp extends EventEmitter implements IRecorderApp { channel: findChromiumChannel(sdkLanguage), args, noDefaultViewport: true, + ignoreDefaultArgs: ['--enable-automation'], colorScheme: 'no-override', - // Moving the mouse while starting Chromium on macOS kills the mouse. - // There is no exact switch that we can blame, but removing all reduces the - // probability of this happening by a couple of orders. - ignoreChromiumSwitches: true, headless: !!process.env.PWTEST_CLI_HEADLESS || (isUnderTest() && !headed), useWebSocket: !!process.env.PWTEST_RECORDER_PORT, handleSIGINT, diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index 2b9d40de6b..21d741d627 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -86,11 +86,8 @@ export async function showTraceViewer(traceUrls: string[], browserName: string, channel: findChromiumChannel(traceViewerPlaywright.options.sdkLanguage), args, noDefaultViewport: true, - // Moving the mouse while starting Chromium on macOS kills the mouse. - // There is no exact switch that we can blame, but removing all reduces the - // probability of this happening by a couple of orders. - ignoreChromiumSwitches: true, headless, + ignoreDefaultArgs: ['--enable-automation'], colorScheme: 'no-override', useWebSocket: isUnderTest(), }); diff --git a/packages/playwright-core/src/server/types.ts b/packages/playwright-core/src/server/types.ts index c1a23ba253..319b774839 100644 --- a/packages/playwright-core/src/server/types.ts +++ b/packages/playwright-core/src/server/types.ts @@ -149,7 +149,7 @@ export type NormalizedContinueOverrides = { export type EmulatedSize = { viewport: Size, screen: Size }; -export type LaunchOptions = channels.BrowserTypeLaunchOptions & { useWebSocket?: boolean, ignoreChromiumSwitches?: boolean }; +export type LaunchOptions = channels.BrowserTypeLaunchOptions & { useWebSocket?: boolean }; export type ProtocolLogger = (direction: 'send' | 'receive', message: object) => void; diff --git a/packages/playwright-test/src/runner/uiMode.ts b/packages/playwright-test/src/runner/uiMode.ts index 38f52d288d..52347846d0 100644 --- a/packages/playwright-test/src/runner/uiMode.ts +++ b/packages/playwright-test/src/runner/uiMode.ts @@ -96,14 +96,16 @@ class UIMode { async showUI() { this._page = await showTraceViewer([], 'chromium', { app: 'watch.html', headless: isUnderTest() && process.env.PWTEST_HEADED_FOR_TEST !== '1' }); - process.stdout.write = (chunk: string | Buffer) => { - this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stdout', chunk) }); - return true; - }; - process.stderr.write = (chunk: string | Buffer) => { - this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stderr', chunk) }); - return true; - }; + if (!process.env.PWTEST_DEBUG) { + process.stdout.write = (chunk: string | Buffer) => { + this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stdout', chunk) }); + return true; + }; + process.stderr.write = (chunk: string | Buffer) => { + this._dispatchEvent({ method: 'stdio', params: chunkToPayload('stderr', chunk) }); + return true; + }; + } const exitPromise = new ManualPromise(); this._page.on('close', () => exitPromise.resolve()); let queue = Promise.resolve(); @@ -193,7 +195,7 @@ class UIMode { dependenciesForTestFile(fileName).forEach(file => files.add(file)); } const watchedFiles = [...files].sort(); - if (this._testWatcher && JSON.stringify(this._testWatcher.watchedFiles.toString()) === JSON.stringify(watchedFiles)) + if (this._testWatcher && JSON.stringify(this._testWatcher.watchedFiles) === JSON.stringify(watchedFiles)) return; if (this._testWatcher) { diff --git a/packages/trace-viewer/src/ui/watchMode.tsx b/packages/trace-viewer/src/ui/watchMode.tsx index b3ffc9d10f..08176120ad 100644 --- a/packages/trace-viewer/src/ui/watchMode.tsx +++ b/packages/trace-viewer/src/ui/watchMode.tsx @@ -69,10 +69,11 @@ export const WatchModeView: React.FC<{}> = ({ const [testModel, setTestModel] = React.useState({ config: undefined, rootSuite: undefined }); const [progress, setProgress] = React.useState(); const [selectedItem, setSelectedItem] = React.useState<{ location?: Location, testCase?: TestCase }>({}); - const [visibleTestIds, setVisibleTestIds] = React.useState([]); + const [visibleTestIds, setVisibleTestIds] = React.useState>(new Set()); const [isLoading, setIsLoading] = React.useState(false); const [runningState, setRunningState] = React.useState<{ testIds: Set, itemSelectedByUser?: boolean } | undefined>(); const [watchAll, setWatchAll] = useSetting('watch-all', false); + const runTestPromiseChain = React.useRef(Promise.resolve()); const inputRef = React.useRef(null); @@ -110,22 +111,26 @@ export const WatchModeView: React.FC<{}> = ({ setProgress(undefined); }; - const runTests = (testIds: string[]) => { - // Clear test results. - { - const testIdSet = new Set(testIds); - for (const test of testModel.rootSuite?.allTests() || []) { - if (testIdSet.has(test.id)) - (test as TeleTestCase)._createTestResult('pending'); - } - setTestModel({ ...testModel }); - } + const runTests = React.useCallback((mode: 'queue-if-busy' | 'bounce-if-busy', testIds: Set) => { + if (mode === 'bounce-if-busy' && runningState) + return; - const time = ' [' + new Date().toLocaleTimeString() + ']'; - xtermDataSource.write('\x1B[2m—'.repeat(Math.max(0, xtermSize.cols - time.length)) + time + '\x1B[22m'); - setProgress({ total: testIds.length, passed: 0, failed: 0, skipped: 0 }); - setRunningState({ testIds: new Set(testIds) }); - sendMessage('run', { testIds }).then(() => { + runTestPromiseChain.current = runTestPromiseChain.current.then(async () => { + // Clear test results. + { + for (const test of testModel.rootSuite?.allTests() || []) { + if (testIds.has(test.id)) + (test as TeleTestCase)._createTestResult('pending'); + } + setTestModel({ ...testModel }); + } + + const time = ' [' + new Date().toLocaleTimeString() + ']'; + xtermDataSource.write('\x1B[2m—'.repeat(Math.max(0, xtermSize.cols - time.length)) + time + '\x1B[22m'); + setProgress({ total: testIds.size, passed: 0, failed: 0, skipped: 0 }); + setRunningState({ testIds }); + + await sendMessage('run', { testIds: [...testIds] }); // Clear pending tests in case of interrupt. for (const test of testModel.rootSuite?.allTests() || []) { if (test.results[0]?.duration === -1) @@ -134,7 +139,7 @@ export const WatchModeView: React.FC<{}> = ({ setTestModel({ ...testModel }); setRunningState(undefined); }); - }; + }, [runningState, testModel]); const isRunningTest = !!runningState; @@ -171,7 +176,7 @@ export const WatchModeView: React.FC<{}> = ({ projectFilters={projectFilters} setProjectFilters={setProjectFilters} testModel={testModel} - runTests={() => runTests(visibleTestIds)} /> + runTests={() => runTests('bounce-if-busy', visibleTestIds)} /> {!isRunningTest && !progress &&
Tests
} {!isRunningTest && progress &&
@@ -180,7 +185,7 @@ export const WatchModeView: React.FC<{}> = ({ {isRunningTest && progress &&
Running {progress.passed}/{runningState.testIds.size} passed ({(progress.passed / runningState.testIds.size) * 100 | 0}%)
} - runTests(visibleTestIds)} disabled={isRunningTest || isLoading}> + runTests('bounce-if-busy', visibleTestIds)} disabled={isRunningTest || isLoading}> sendMessageNoReply('stop')} disabled={!isRunningTest || isLoading}> , filterText: string, testModel: { rootSuite: Suite | undefined, config: FullConfig | undefined }, - runTests: (testIds: string[]) => void, + runTests: (mode: 'bounce-if-busy' | 'queue-if-busy', testIds: Set) => void, runningState?: { testIds: Set, itemSelectedByUser?: boolean }, watchAll?: boolean, isLoading?: boolean, - setVisibleTestIds: (testIds: string[]) => void, + setVisibleTestIds: (testIds: Set) => void, onItemSelected: (item: { testCase?: TestCase, location?: Location }) => void, }> = ({ statusFilters, projectFilters, filterText, testModel, runTests, runningState, watchAll, isLoading, onItemSelected, setVisibleTestIds }) => { const [treeState, setTreeState] = React.useState({ expandedItems: new Map() }); @@ -302,7 +307,7 @@ const TestList: React.FC<{ treeItemMap.set(treeItem.id, treeItem); }; visit(rootItem); - setVisibleTestIds([...visibleTestIds]); + setVisibleTestIds(visibleTestIds); return { rootItem, treeItemMap, fileNames }; }, [filterText, testModel, statusFilters, projectFilters, setVisibleTestIds]); @@ -349,7 +354,7 @@ const TestList: React.FC<{ const fileNames = new Set(); for (const itemId of watchedTreeIds.value) { const treeItem = treeItemMap.get(itemId)!; - const fileName = fileNameForTreeItem(treeItem); + const fileName = treeItem.location.file; if (fileName) fileNames.add(fileName); } @@ -359,29 +364,30 @@ const TestList: React.FC<{ const runTreeItem = (treeItem: TreeItem) => { setSelectedTreeItemId(treeItem.id); - runTests(collectTestIds(treeItem)); + runTests('bounce-if-busy', collectTestIds(treeItem)); }; - runWatchedTests = (fileNames: string[]) => { + runWatchedTests = (changedTestFiles: string[]) => { const testIds: string[] = []; - const set = new Set(fileNames); + const set = new Set(changedTestFiles); if (watchAll) { const visit = (treeItem: TreeItem) => { - const fileName = fileNameForTreeItem(treeItem); + const fileName = treeItem.location.file; if (fileName && set.has(fileName)) testIds.push(...collectTestIds(treeItem)); - treeItem.children.forEach(visit); + if (treeItem.kind === 'group' && treeItem.subKind === 'folder') + treeItem.children.forEach(visit); }; visit(rootItem); } else { for (const treeId of watchedTreeIds.value) { const treeItem = treeItemMap.get(treeId)!; - const fileName = fileNameForTreeItem(treeItem); + const fileName = treeItem.location.file; if (fileName && set.has(fileName)) testIds.push(...collectTestIds(treeItem)); } } - runTests(testIds); + runTests('queue-if-busy', new Set(testIds)); }; return { return undefined; }; -const fileNameForTreeItem = (treeItem?: TreeItem): string | undefined => { - return treeItem?.location.file; -}; - const locationToOpen = (treeItem?: TreeItem) => { if (!treeItem) return; return treeItem.location.file + ':' + treeItem.location.line; }; -const collectTestIds = (treeItem?: TreeItem): string[] => { +const collectTestIds = (treeItem?: TreeItem): Set => { + const testIds = new Set(); if (!treeItem) - return []; - const testIds: string[] = []; + return testIds; + const visit = (treeItem: TreeItem) => { if (treeItem.kind === 'case') - testIds.push(...treeItem.tests.map(t => t.id)); + treeItem.tests.map(t => t.id).forEach(id => testIds.add(id)); else if (treeItem.kind === 'test') - testIds.push(treeItem.id); + testIds.add(treeItem.id); else treeItem.children?.forEach(visit); }; diff --git a/tests/playwright-test/ui-mode-fixtures.ts b/tests/playwright-test/ui-mode-fixtures.ts index 442b890f99..792e0853b1 100644 --- a/tests/playwright-test/ui-mode-fixtures.ts +++ b/tests/playwright-test/ui-mode-fixtures.ts @@ -21,9 +21,16 @@ import type { TestChildProcess } from '../config/commonFixtures'; import { cleanEnv, cliEntrypoint, removeFolderAsync, test as base, writeFiles } from './playwright-test-fixtures'; import type { Files, RunOptions } from './playwright-test-fixtures'; import type { Browser, Page, TestInfo } from './stable-test-runner'; +import { createGuid } from '../../packages/playwright-core/src/utils/crypto'; + +type Latch = { + blockingCode: string; + open: () => void; +}; type Fixtures = { runUITest: (files: Files, env?: NodeJS.ProcessEnv, options?: RunOptions) => Promise; + createLatch: () => Latch; }; export function dumpTestTree(page: Page): () => Promise { @@ -99,6 +106,21 @@ export const test = base await testProcess?.close(); await removeFolderAsync(cacheDir); }, + createLatch: async ({}, use, testInfo) => { + await use(() => { + const latchFile = path.join(testInfo.project.outputDir, createGuid() + '.latch'); + return { + blockingCode: `await ((${waitForLatch})(${JSON.stringify(latchFile)}))`, + open: () => fs.writeFileSync(latchFile, 'ok'), + }; + }); + }, }); export { expect } from './stable-test-runner'; + +async function waitForLatch(latchFile: string) { + const fs = require('fs'); + while (!fs.existsSync(latchFile)) + await new Promise(f => setTimeout(f, 250)); +} diff --git a/tests/playwright-test/ui-mode-test-watch.spec.ts b/tests/playwright-test/ui-mode-test-watch.spec.ts index 934331b6ca..9013d76f21 100644 --- a/tests/playwright-test/ui-mode-test-watch.spec.ts +++ b/tests/playwright-test/ui-mode-test-watch.spec.ts @@ -219,3 +219,46 @@ test('should watch new file', async ({ runUITest, writeFiles }) => { ✅ test `); }); + +test('should queue watches', async ({ runUITest, writeFiles, createLatch }) => { + const latch = createLatch(); + const page = await runUITest({ + 'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + 'b.test.ts': `import { test } from '@playwright/test'; test('test', async () => { + ${latch.blockingCode} + });`, + 'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + 'd.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + }); + + await expect.poll(dumpTestTree(page), { timeout: 15000 }).toBe(` + ▼ ◯ a.test.ts + ◯ test + ▼ ◯ b.test.ts + ◯ test + ▼ ◯ c.test.ts + ◯ test + ▼ ◯ d.test.ts + ◯ test + `); + + await page.getByTitle('Watch all').click(); + await page.getByTitle('Run all').click(); + + await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)', { timeout: 15000 }); + + await writeFiles({ + 'a.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + 'b.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + 'c.test.ts': `import { test } from '@playwright/test'; test('test', () => {});`, + }); + + // Now watches should not kick in. + await new Promise(f => setTimeout(f, 1000)); + await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)', { timeout: 15000 }); + + // Allow test to finish and new watch to kick in. + latch.open(); + + await expect(page.getByTestId('status-line')).toHaveText('3/3 passed (100%)', { timeout: 15000 }); +});