diff --git a/.github/workflows/tests_components.yml b/.github/workflows/tests_components.yml deleted file mode 100644 index abc904d8919c6..0000000000000 --- a/.github/workflows/tests_components.yml +++ /dev/null @@ -1,44 +0,0 @@ -name: "components" - -on: - push: - branches: - - main - - release-* - pull_request: - paths-ignore: - - 'browser_patches/**' - - 'docs/**' - - 'packages/playwright/src/mcp/**' - - 'tests/mcp/**' - branches: - - main - - release-* - -env: - FORCE_COLOR: 1 - ELECTRON_SKIP_BINARY_DOWNLOAD: 1 - -jobs: - test_components: - name: ${{ matrix.os }} - Node.js ${{ matrix.node-version }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, macos-latest, windows-latest] - node-version: [20] - include: - - os: ubuntu-latest - node-version: 22 - - os: ubuntu-latest - node-version: 24 - runs-on: ${{ matrix.os }} - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-node@v6 - with: - node-version: ${{ matrix.node-version }} - - run: npm ci - - run: npm run build - - run: npx playwright install --with-deps - - run: npm run ct diff --git a/.github/workflows/tests_mcp.yml b/.github/workflows/tests_mcp.yml index b38e3ad86a3b2..3678bbc7266c1 100644 --- a/.github/workflows/tests_mcp.yml +++ b/.github/workflows/tests_mcp.yml @@ -34,7 +34,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-15, windows-latest] + os: [macos-15] runs-on: ${{ matrix.os }} permissions: id-token: write # This is required for OIDC login (azure/login) to succeed @@ -44,7 +44,7 @@ jobs: - uses: ./.github/actions/run-test with: node-version: "20" - command: npm run test-mcp + command: DEBUG=pw:browser npm run ctest-mcp -- --reporter=list bot-name: "mcp-${{ matrix.os }}" flakiness-client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }} flakiness-tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }} diff --git a/.github/workflows/tests_primary.yml b/.github/workflows/tests_primary.yml deleted file mode 100644 index 669c40841516a..0000000000000 --- a/.github/workflows/tests_primary.yml +++ /dev/null @@ -1,199 +0,0 @@ -name: "tests 1" - -on: - push: - branches: - - main - - release-* - pull_request: - paths-ignore: - - 'browser_patches/**' - - 'docs/**' - - 'packages/playwright/src/mcp/**' - - 'tests/mcp/**' - branches: - - main - - release-* - -concurrency: - # For pull requests, cancel all currently-running jobs for this workflow - # https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency - group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} - cancel-in-progress: true - -env: - # Force terminal colors. @see https://www.npmjs.com/package/colors - FORCE_COLOR: 1 - ELECTRON_SKIP_BINARY_DOWNLOAD: 1 - -jobs: - test_linux: - name: ${{ matrix.os }} (${{ matrix.browser }} - Node.js ${{ matrix.node-version }}) - environment: ${{ github.event_name == 'push' && 'allow-uploading-flakiness-results' || null }} - strategy: - fail-fast: false - matrix: - browser: [chromium, firefox, webkit] - os: [ubuntu-22.04] - node-version: [20] - include: - - os: ubuntu-22.04 - node-version: 22 - browser: chromium - - os: ubuntu-22.04 - node-version: 24 - browser: chromium - runs-on: ${{ matrix.os }} - permissions: - id-token: write # This is required for OIDC login (azure/login) to succeed - contents: read # This is required for actions/checkout to succeed - steps: - - uses: actions/checkout@v6 - - uses: ./.github/actions/run-test - with: - node-version: ${{ matrix.node-version }} - browsers-to-install: ${{ matrix.browser }} chromium - command: npm run test -- --project=${{ matrix.browser }}-* - bot-name: "${{ matrix.browser }}-${{ matrix.os }}-node${{ matrix.node-version }}" - flakiness-client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }} - flakiness-tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }} - flakiness-subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }} - - test_linux_chromium_tot: - name: ${{ matrix.os }} (chromium tip-of-tree) - environment: ${{ github.event_name == 'push' && 'allow-uploading-flakiness-results' || null }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-22.04] - runs-on: ${{ matrix.os }} - permissions: - id-token: write # This is required for OIDC login (azure/login) to succeed - contents: read # This is required for actions/checkout to succeed - steps: - - uses: actions/checkout@v6 - - uses: ./.github/actions/run-test - with: - browsers-to-install: chromium-tip-of-tree - command: npm run test -- --project=chromium-* - bot-name: "${{ matrix.os }}-chromium-tip-of-tree" - flakiness-client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }} - flakiness-tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }} - flakiness-subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }} - env: - PWTEST_CHANNEL: chromium-tip-of-tree - - test_test_runner: - name: Test Runner - environment: ${{ github.event_name == 'push' && 'allow-uploading-flakiness-results' || null }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest, macos-latest] - node-version: [20] - shardIndex: [1, 2] - shardTotal: [2] - include: - - os: ubuntu-latest - node-version: 22 - shardIndex: 1 - shardTotal: 2 - - os: ubuntu-latest - node-version: 22 - shardIndex: 2 - shardTotal: 2 - - os: ubuntu-latest - node-version: 24 - shardIndex: 1 - shardTotal: 2 - - os: ubuntu-latest - node-version: 24 - shardIndex: 2 - shardTotal: 2 - runs-on: ${{ matrix.os }} - permissions: - id-token: write # This is required for OIDC login (azure/login) to succeed - contents: read # This is required for actions/checkout to succeed - steps: - - uses: actions/checkout@v6 - - uses: ./.github/actions/run-test - with: - node-version: ${{matrix.node-version}} - command: npm run ttest -- --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --shard-weights=58:42 - bot-name: "${{ matrix.os }}-node${{ matrix.node-version }}-${{ matrix.shardIndex }}" - flakiness-client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }} - flakiness-tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }} - flakiness-subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }} - env: - PWTEST_CHANNEL: firefox-beta - - test_vscode_extension: - name: VSCode Extension - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - uses: actions/setup-node@v6 - with: - node-version: 20 - - run: npm ci - env: - DEBUG: pw:install - - run: npm run build - - run: npx playwright install chromium - - name: Checkout extension - run: git clone https://github.com/microsoft/playwright-vscode.git - - name: Print extension revision - run: git rev-parse HEAD - working-directory: ./playwright-vscode - - name: Remove @playwright/test from extension dependencies - run: node -e "const p = require('./package.json'); delete p.devDependencies['@playwright/test']; fs.writeFileSync('./package.json', JSON.stringify(p, null, 2));" - working-directory: ./playwright-vscode - - name: Build extension - run: npm ci && npm run build - working-directory: ./playwright-vscode - - name: Run extension tests - run: npm run test -- --workers=1 - working-directory: ./playwright-vscode - env: - PW_TAG: "@vscode-extension" - - name: Upload blob report - if: ${{ !cancelled() }} - uses: ./.github/actions/upload-blob-report - with: - report_dir: playwright-vscode/blob-report - job_name: vscode-extension - - test_package_installations: - name: "Installation Test ${{ matrix.os }}" - environment: ${{ github.event_name == 'push' && 'allow-uploading-flakiness-results' || null }} - strategy: - fail-fast: false - matrix: - os: - - ubuntu-latest - - macos-latest - - windows-latest - runs-on: ${{ matrix.os }} - timeout-minutes: 45 - permissions: - id-token: write # This is required for OIDC login (azure/login) to succeed - contents: read # This is required for actions/checkout to succeed - steps: - - uses: actions/checkout@v6 - - run: npm install -g yarn@1 - - run: npm install -g pnpm@8 - - name: Setup Ubuntu Binary Installation # TODO: Remove when https://github.com/electron/electron/issues/42510 is fixed - if: ${{ runner.os == 'Linux' }} - run: | - if grep -q "Ubuntu 24" /etc/os-release; then - sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 - fi - shell: bash - - uses: ./.github/actions/run-test - with: - command: npm run itest - bot-name: "package-installations-${{ matrix.os }}" - shell: ${{ matrix.os == 'windows-latest' && 'pwsh' || 'bash' }} - flakiness-client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }} - flakiness-tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }} - flakiness-subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }} diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 744f9493e7869..6caacdd55dbb5 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -533,6 +533,7 @@ export abstract class BrowserContext extends Sdk // Cleanup. const promises: Promise[] = []; for (const { context, artifact } of this._browser._idToVideo.values()) { + console.error('Waiting for video to finish before closing context', context === this, artifact.localPath()); // Wait for the videos to finish. if (context === this) promises.push(artifact.finishedPromise()); diff --git a/packages/playwright-core/src/server/videoRecorder.ts b/packages/playwright-core/src/server/videoRecorder.ts index 5a91f25fb0fd3..53d2ee4d7b848 100644 --- a/packages/playwright-core/src/server/videoRecorder.ts +++ b/packages/playwright-core/src/server/videoRecorder.ts @@ -112,6 +112,7 @@ export class VideoRecorder { } writeFrame(frame: Buffer, timestamp: number) { + console.error('Writing frame at timestamp', timestamp); this._launchPromise.then(error => { if (error) return; @@ -121,6 +122,7 @@ export class VideoRecorder { private _writeFrame(frame: Buffer, timestamp: number) { assert(this._process); + console.error('VideoRecorder: _writeFrame', this._isStopped, { timestamp }); if (this._isStopped) return; @@ -155,6 +157,7 @@ export class VideoRecorder { async stop() { // Only report the error on stop. This allows to make the constructor synchronous. const error = await this._launchPromise; + console.error('Stopping video recorder, error:', error); if (error) throw error; if (this._isStopped || !this._lastFrame) @@ -165,9 +168,12 @@ export class VideoRecorder { this._writeFrame(Buffer.from([]), this._lastFrame.timestamp + addTime); this._isStopped = true; try { + console.error('will wait _lastWritePromise'); await this._lastWritePromise; await this._gracefullyClose!(); + console.error('did stop video recorder'); } catch (e) { + console.error('Error while stopping ffmpeg', e); debugLogger.log('error', `ffmpeg failed to stop: ${String(e)}`); } } diff --git a/packages/playwright/src/mcp/browser/browserContextFactory.ts b/packages/playwright/src/mcp/browser/browserContextFactory.ts index db84d10316659..69a5a6e2b72e7 100644 --- a/packages/playwright/src/mcp/browser/browserContextFactory.ts +++ b/packages/playwright/src/mcp/browser/browserContextFactory.ts @@ -23,7 +23,7 @@ import * as playwright from 'playwright-core'; import { registryDirectory } from 'playwright-core/lib/server/registry/index'; import { startTraceViewerServer } from 'playwright-core/lib/server'; import { logUnhandledError, testDebug } from '../log'; -import { outputFile, tmpDir } from './config'; +import { outputFile } from './config'; import { firstRootPath } from '../sdk/server'; import type { FullConfig } from './config'; @@ -44,7 +44,7 @@ export function contextFactory(config: FullConfig): BrowserContextFactory { export type BrowserContextFactoryResult = { browserContext: playwright.BrowserContext; - close: (afterClose: () => Promise) => Promise; + close: () => Promise; }; export interface BrowserContextFactory { @@ -94,24 +94,25 @@ class BaseContextFactory implements BrowserContextFactory { async createContext(clientInfo: ClientInfo): Promise { testDebug(`create browser context (${this._logName})`); const browser = await this._obtainBrowser(clientInfo); - const browserContext = await this._doCreateContext(browser); + const browserContext = await this._doCreateContext(browser, clientInfo); await addInitScript(browserContext, this.config.browser.initScript); return { browserContext, - close: (afterClose: () => Promise) => this._closeBrowserContext(browserContext, browser, afterClose) + close: () => this._closeBrowserContext(browserContext, browser) }; } - protected async _doCreateContext(browser: playwright.Browser): Promise { + protected async _doCreateContext(browser: playwright.Browser, clientInfo: ClientInfo): Promise { throw new Error('Not implemented'); } - private async _closeBrowserContext(browserContext: playwright.BrowserContext, browser: playwright.Browser, afterClose: () => Promise) { + private async _closeBrowserContext(browserContext: playwright.BrowserContext, browser: playwright.Browser) { testDebug(`close browser context (${this._logName})`); if (browser.contexts().length === 1) this._browserPromise = undefined; - await browserContext.close().catch(logUnhandledError); - await afterClose(); + console.error('closing browser context'); + await browserContext.close().catch(e => console.error('Error closing browser context', e)); + console.error('browser context closed'); if (browser.contexts().length === 0) { testDebug(`close browser (${this._logName})`); await browser.close().catch(logUnhandledError); @@ -142,8 +143,8 @@ class IsolatedContextFactory extends BaseContextFactory { }); } - protected override async _doCreateContext(browser: playwright.Browser): Promise { - return browser.newContext(browserContextOptionsFromConfig(this.config)); + protected override async _doCreateContext(browser: playwright.Browser, clientInfo: ClientInfo): Promise { + return browser.newContext(await browserContextOptionsFromConfig(this.config, clientInfo)); } } @@ -206,7 +207,7 @@ class PersistentContextFactory implements BrowserContextFactory { const launchOptions: LaunchOptions & BrowserContextOptions = { tracesDir, ...this.config.browser.launchOptions, - ...browserContextOptionsFromConfig(this.config), + ...await browserContextOptionsFromConfig(this.config, clientInfo), handleSIGINT: false, handleSIGTERM: false, ignoreDefaultArgs: [ @@ -217,7 +218,7 @@ class PersistentContextFactory implements BrowserContextFactory { try { const browserContext = await browserType.launchPersistentContext(userDataDir, launchOptions); await addInitScript(browserContext, this.config.browser.initScript); - const close = (afterClose: () => Promise) => this._closeBrowserContext(browserContext, userDataDir, afterClose); + const close = () => this._closeBrowserContext(browserContext, userDataDir); return { browserContext, close }; } catch (error: any) { if (error.message.includes('Executable doesn\'t exist')) @@ -233,11 +234,14 @@ class PersistentContextFactory implements BrowserContextFactory { throw new Error(`Browser is already in use for ${userDataDir}, use --isolated to run multiple instances of the same browser`); } - private async _closeBrowserContext(browserContext: playwright.BrowserContext, userDataDir: string, afterClose: () => Promise) { + private async _closeBrowserContext(browserContext: playwright.BrowserContext, userDataDir: string) { testDebug('close browser context (persistent)'); testDebug('release user data dir', userDataDir); - await browserContext.close().catch(() => {}); - await afterClose(); + console.error('Closing browser context with user data dir:', userDataDir); + await browserContext.close().catch((e) => { + console.error('Error closing browser context', e); + }); + console.error('Browser context closed for user data dir:', userDataDir); this._userDataDirs.delete(userDataDir); if (process.env.PWMCP_PROFILES_DIR_FOR_TEST && userDataDir.startsWith(process.env.PWMCP_PROFILES_DIR_FOR_TEST)) await fs.promises.rm(userDataDir, { recursive: true }).catch(logUnhandledError); @@ -336,7 +340,7 @@ export class SharedContextFactory implements BrowserContextFactory { if (!contextPromise) return; const { close } = await contextPromise; - await close(async () => {}); + await close(); } } @@ -346,11 +350,12 @@ async function computeTracesDir(config: FullConfig, clientInfo: ClientInfo): Pro return await outputFile(config, clientInfo, `traces`, { origin: 'code', reason: 'Collecting trace' }); } -function browserContextOptionsFromConfig(config: FullConfig): playwright.BrowserContextOptions { +async function browserContextOptionsFromConfig(config: FullConfig, clientInfo: ClientInfo): Promise { const result = { ...config.browser.contextOptions }; if (config.saveVideo) { + const dir = await outputFile(config, clientInfo, `videos`, { origin: 'code', reason: 'Saving video' }); result.recordVideo = { - dir: tmpDir(), + dir, size: config.saveVideo, }; } diff --git a/packages/playwright/src/mcp/browser/config.ts b/packages/playwright/src/mcp/browser/config.ts index 970ecb19bbb8c..6a87338be994f 100644 --- a/packages/playwright/src/mcp/browser/config.ts +++ b/packages/playwright/src/mcp/browser/config.ts @@ -325,7 +325,7 @@ async function loadConfig(configFile: string | undefined): Promise { } } -export function tmpDir(): string { +function tmpDir(): string { return path.join(process.env.PW_TMPDIR_FOR_TEST ?? os.tmpdir(), 'playwright-mcp-output'); } diff --git a/packages/playwright/src/mcp/browser/context.ts b/packages/playwright/src/mcp/browser/context.ts index 5d823058e1742..ad6a2c6a000d3 100644 --- a/packages/playwright/src/mcp/browser/context.ts +++ b/packages/playwright/src/mcp/browser/context.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -import fs from 'fs'; - import { debug } from 'playwright-core/lib/utilsBundle'; import { escapeWithQuotes } from 'playwright-core/lib/utils'; import { selectors } from 'playwright-core'; @@ -25,7 +23,6 @@ import os from 'os'; import { logUnhandledError } from '../log'; import { Tab } from './tab'; import { outputFile } from './config'; -import { dateAsFileName } from './tools/utils'; import type * as playwright from '../../../types/test'; import type { FullConfig } from './config'; @@ -169,29 +166,9 @@ export class Context { await promise.then(async ({ browserContext, close }) => { if (this.config.saveTrace) await browserContext.tracing.stop(); - const videos = this.config.saveVideo ? browserContext.pages().map(page => page.video()).filter(video => !!video) : []; - await close(async () => { - for (const video of videos) { - const name = await this.outputFile(dateAsFileName('webm'), { origin: 'code', reason: 'Saving video' }); - const p = await video.path(); - // video.saveAs() does not work for persistent contexts. - if (fs.existsSync(p)) { - try { - await fs.promises.rename(p, name); - } catch (e) { - if (e.code !== 'EXDEV') - logUnhandledError(e); - // Retry operation (possibly cross-fs) with copy and unlink - try { - await fs.promises.copyFile(p, name); - await fs.promises.unlink(p); - } catch (e) { - logUnhandledError(e); - } - } - } - } - }); + console.error('will close'); + await close(); + console.error('did close'); }); } diff --git a/tests/mcp/video.spec.ts b/tests/mcp/video.spec.ts index b413d3c50266f..4e2eb119c619b 100644 --- a/tests/mcp/video.spec.ts +++ b/tests/mcp/video.spec.ts @@ -15,6 +15,7 @@ */ import fs from 'fs'; +import path from 'path'; import { test, expect } from './fixtures'; import type { Client } from '@modelcontextprotocol/sdk/client/index.js'; @@ -22,7 +23,7 @@ for (const mode of ['isolated', 'persistent']) { test(`should work with --save-video (${mode})`, async ({ startClient, server }, testInfo) => { const outputDir = testInfo.outputPath('output'); - const { client } = await startClient({ + const { client, stderr } = await startClient({ args: [ '--save-video=800x600', ...(mode === 'isolated' ? ['--isolated'] : []), @@ -31,14 +32,14 @@ for (const mode of ['isolated', 'persistent']) { }); await navigateToTestPage(client, server); - await expect(async () => { - await produceFrames(client); - await checkIntermediateVideoFileExists(); - }).toPass(); + await expect(async () => await produceFrames(client)).toPass(); await closeBrowser(client); - const [file] = await fs.promises.readdir(outputDir); - expect(file).toMatch(/page-.*\.webm/); + console.error('Client stderr:', stderr()); + + const videosDir = path.join(outputDir, 'videos'); + const [file] = await fs.promises.readdir(videosDir); + expect(file).toMatch(/.*\.webm/); }); test(`should work with { saveVideo } (${mode})`, async ({ startClient, server }, testInfo) => { @@ -53,14 +54,12 @@ for (const mode of ['isolated', 'persistent']) { }); await navigateToTestPage(client, server); - await expect(async () => { - await produceFrames(client); - await checkIntermediateVideoFileExists(); - }).toPass(); + await expect(async () => await produceFrames(client)).toPass(); await closeBrowser(client); - const [file] = await fs.promises.readdir(outputDir); - expect(file).toMatch(/page-.*\.webm/); + const videosDir = path.join(outputDir, 'videos'); + const [file] = await fs.promises.readdir(videosDir); + expect(file).toMatch(/.*\.webm/); }); test(`should work with recordVideo (${mode})`, async ({ startClient, server }, testInfo) => { @@ -83,10 +82,7 @@ for (const mode of ['isolated', 'persistent']) { }); await navigateToTestPage(client, server); - await expect(async () => { - await produceFrames(client); - await checkIntermediateVideoFileExists(videosDir); - }).toPass(); + await expect(async () => await produceFrames(client)).toPass(); await closeBrowser(client); }); } @@ -101,9 +97,11 @@ async function navigateToTestPage(client: Client, server: any) { } async function closeBrowser(client: Client) { - expect(await client.callTool({ + const response = await client.callTool({ name: 'browser_close', - })).toHaveResponse({ + }); + console.log('browser_close response', response); + expect(response).toHaveResponse({ code: expect.stringContaining(`page.close()`), }); } @@ -126,8 +124,3 @@ async function produceFrames(client: Client) { result: '"ok"', }); } - -async function checkIntermediateVideoFileExists(videosDir?: string) { - const files = await fs.promises.readdir(videosDir ?? test.info().outputPath('tmp', 'playwright-mcp-output')); - expect(files[0]).toMatch(/\.webm/); -}