[pull] main from microsoft:main

.github/workflows/tests_docker.yml

@@ -0,0 +1,131 @@
+name: tests docker
+
+on:
+  workflow_call:
+
+env:
+  FORCE_COLOR: 1
+
+jobs:
+  test_linux_docker:
+    name: "Docker ${{ matrix.docker_tag }} ${{ matrix.docker_arch }}"
+    environment: ${{ github.event_name == 'push' && 'allow-uploading-flakiness-results' || null }}
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+        - docker_tag: jammy
+          docker_arch: amd64
+          host_os: ubuntu-22.04
+        - docker_tag: jammy
+          docker_arch: arm64
+          host_os: ubuntu-22.04-arm
+        - docker_tag: noble
+          docker_arch: amd64
+          host_os: ubuntu-22.04
+        - docker_tag: noble
+          docker_arch: arm64
+          host_os: ubuntu-22.04-arm
+    runs-on: ${{ matrix.host_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:
+    - name: Create ~/.azure directory
+      run: mkdir -p ~/.azure
+    - uses: actions/checkout@v6
+    - uses: actions/setup-node@v6
+      with:
+        node-version: 22
+      env:
+        PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
+    - run: npm ci
+    - run: npm run build
+    # Used as a Pull-through cache for the Docker images. Only available on the
+    # main repo, where the secrets are present; forks fall back to Docker Hub.
+    - name: Azure Login
+      if: ${{ github.event_name == 'push' && github.repository == 'microsoft/playwright' }}
+      uses: azure/login@v3
+      with:
+        client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }}
+        tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }}
+        subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }}
+    - name: Login to ACR via OIDC
+      if: ${{ github.event_name == 'push' && github.repository == 'microsoft/playwright' }}
+      run: az acr login --name playwright
+    - name: Add Dockerfile cache
+      if: ${{ github.event_name == 'push' && github.repository == 'microsoft/playwright' }}
+      run: sed -i 's/FROM ubuntu/FROM playwright.azurecr.io\/cached\/ubuntu/' utils/docker/Dockerfile.${{matrix.docker_tag}}
+    - run: ./utils/docker/build.sh --${{ matrix.docker_arch }} ${{ matrix.docker_tag }} playwright:localbuild
+      shell: bash
+    - name: Launch container
+      run: |
+        docker run \
+          --rm \
+          --name docker-tests \
+          --platform linux/${{ matrix.docker_arch }} \
+          --user=pwuser \
+          --workdir /home/pwuser \
+          --env CI \
+          --env INSIDE_DOCKER=1 \
+          -v ~/.azure:/root/.azure \
+          -d \
+          -t \
+          playwright:localbuild /bin/bash
+    - name: Copy repository inside docker container
+      run: |
+        # Note: we cannot do explicit `git clone` inside container because it requires some smarts to decide if github.sha or
+        # github.ref should be used for checkout.
+        # The actions/checkout action already handled this complexity for us, so we'll just copy checkout.
+        docker cp . docker-tests:/home/pwuser/playwright
+        docker exec --user root docker-tests chown -R pwuser /home/pwuser/playwright
+
+        # GIT is now picky on directory ownership
+        # See https://github.blog/2022-04-12-git-security-vulnerability-announced/
+        docker exec \
+          --user root \
+          --workdir /home/pwuser/playwright docker-tests /bin/bash -c '
+            git config --global --add safe.directory /home/pwuser/playwright
+          '
+    - name: Run "npm ci" inside docker
+      run: docker exec --workdir /home/pwuser/playwright docker-tests npm ci
+
+    - name: Run "npm run build" inside docker
+      run: docker exec --workdir /home/pwuser/playwright docker-tests npm run build
+
+    - name: "Run @smoke tests inside docker"
+      run: docker exec --workdir /home/pwuser/playwright docker-tests xvfb-run --auto-servernum --server-args="-screen 0 1280x960x24" npm run test -- --grep "@smoke"
+
+    - name: Azure Login
+      if: ${{ !cancelled() && github.event_name == 'push' && github.repository == 'microsoft/playwright' }}
+      uses: azure/login@v3
+      with:
+        client-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_CLIENT_ID }}
+        tenant-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_TENANT_ID }}
+        subscription-id: ${{ secrets.AZURE_FLAKINESS_DASHBOARD_SUBSCRIPTION_ID }}
+    - name: Upload Flakiness dashboard
+      if: ${{ !cancelled() && github.event_name == 'push' && github.repository == 'microsoft/playwright' }}
+      run: |
+        # Flakiness dashboard has to be uploaded from-inside docker container so that it can collect meta information, e.g. arch
+        # See https://github.com/microsoft/playwright/blob/13b1e52d95df416fdfa7846c8a840aad8df263af/utils/upload_flakiness_dashboard.sh#L71-L86
+        docker exec \
+          --env GITHUB_REPOSITORY \
+          --env GITHUB_REF \
+          --env GITHUB_RUN_ID \
+          --user root \
+          --workdir /home/pwuser/playwright docker-tests /bin/bash -c '
+            set -e
+            # Azure-CLI is needed for upload_flakiness_dashboard.sh script.
+            curl -sL https://aka.ms/InstallAzureCLIDeb | bash
+            ./utils/upload_flakiness_dashboard.sh "./test-results/report.json"
+          '
+    - name: Copy blob report from container
+      if: ${{ !cancelled() && (github.event_name == 'pull_request' || failure()) }}
+      run: docker cp "docker-tests:/home/pwuser/playwright/blob-report" "./blob-report"
+
+    - name: Upload blob report
+      if: ${{ !cancelled() && (github.event_name == 'pull_request' || failure()) }}
+      uses: ./.github/actions/upload-blob-report
+      with:
+        report_dir: blob-report
+        job_name: docker-${{ matrix.docker_tag }}-${{ matrix.docker_arch }}

.github/workflows/tests_docker_changes.yml

@@ -0,0 +1,24 @@
+name: tests docker (changes)
+
+on:
+  push:
+    branches:
+      - main
+    paths:
+      - 'utils/docker/**'
+      - '.github/workflows/tests_docker*.yml'
+  pull_request:
+    branches:
+      - main
+      - release-*
+    paths:
+      - 'utils/docker/**'
+      - '.github/workflows/tests_docker*.yml'
+
+jobs:
+  test_linux_docker:
+    uses: ./.github/workflows/tests_docker.yml
+    permissions:
+      id-token: write
+      contents: read
+    secrets: inherit

.github/workflows/tests_docker_release.yml

@@ -0,0 +1,14 @@
+name: tests docker (release)
+
+on:
+  push:
+    branches:
+      - release-*
+
+jobs:
+  test_linux_docker:
+    uses: ./.github/workflows/tests_docker.yml
+    permissions:
+      id-token: write
+      contents: read
+    secrets: inherit

docs/src/test-api/class-testoptions.md

@@ -634,8 +634,8 @@ export default defineConfig({
 
 ## property: TestOptions.video
 * since: v1.10
-- type: <[Object]|[VideoMode]<"off"|"on"|"retain-on-failure"|"on-first-retry">>
-  - `mode` <[VideoMode]<"off"|"on"|"retain-on-failure"|"on-first-retry">> Video recording mode.
+- type: <[Object]|[VideoMode]<"off"|"on"|"retain-on-failure"|"on-first-retry"|"on-all-retries"|"retain-on-first-failure"|"retain-on-failure-and-retries"|"retain-all-failures">>
+  - `mode` <[VideoMode]<"off"|"on"|"retain-on-failure"|"on-first-retry"|"on-all-retries"|"retain-on-first-failure"|"retain-on-failure-and-retries"|"retain-all-failures">> Video recording mode.
   - `size` ?<[Object]> Size of the recorded video. Optional.
     - `width` <[int]>
     - `height` <[int]>
@@ -652,8 +652,12 @@ export default defineConfig({
 Whether to record video for each test. Defaults to `'off'`.
 * `'off'`: Do not record video.
 * `'on'`: Record video for each test.
-* `'retain-on-failure'`: Record video for each test, but remove all videos from successful test runs.
 * `'on-first-retry'`: Record video only when retrying a test for the first time.
+* `'on-all-retries'`: Record video only when retrying a test.
+* `'retain-on-failure'`: Record video for each test. When test run passes, remove the recorded video.
+* `'retain-on-first-failure'`: Record video for the first run of each test, but not for retries. When test run passes, remove the recorded video.
+* `'retain-on-failure-and-retries'`: Record video for each test run. Retains all videos when an attempt fails.
+* `'retain-all-failures'`: Record video for each test run. Retains the video only for attempts that failed, regardless of the final test outcome.
 
 To control video size, pass an object with `mode` and `size` properties. If video size is not specified, it will be equal to [`property: TestOptions.viewport`] scaled down to fit into 800x800. If `viewport` is not configured explicitly the video size defaults to 800x450. Actual picture of each page will be scaled down if necessary to fit the specified size.
 

packages/playwright-core/src/inprocess.ts

@@ -25,10 +25,10 @@ import type { Playwright as PlaywrightAPI } from './client/playwright';
 import type { Language } from '@isomorphic/locatorGenerators';
 
 export function createInProcessPlaywright(): PlaywrightAPI {
-  const playwright = createPlaywright({ sdkLanguage: (process.env.PW_LANG_NAME as Language | undefined) || 'javascript' });
+  const playwright = createPlaywright({ sdkLanguage: (process.env.PW_LANG_NAME as Language | undefined) || 'javascript', isClientCollocatedWithServer: true });
   const clientConnection = new Connection(nodePlatform(packageRoot));
   clientConnection.useRawBuffers();
-  const dispatcherConnection = new DispatcherConnection(true /* local */);
+  const dispatcherConnection = new DispatcherConnection(true /* in process */);
 
   // Dispatch synchronously at first.
   dispatcherConnection.onmessage = message => clientConnection.dispatch(message);

packages/playwright-core/src/server/artifact.ts

@@ -29,6 +29,7 @@ type CancelCallback = () => Promise<void>;
 export class Artifact extends SdkObject {
   private _localPath: string;
   private _unaccessibleErrorMessage: string | undefined;
+  private _missingFileErrorMessage: string | undefined;
   private _cancelCallback: CancelCallback | undefined;
   private _finishedPromise = new ManualPromise<void>();
   private _saveCallbacks: SaveCallback[] = [];
@@ -43,6 +44,14 @@ export class Artifact extends SdkObject {
     this._cancelCallback = cancelCallback;
   }
 
+  markMissingFileErrorMessage(errorMessage: string) {
+    this._missingFileErrorMessage = errorMessage;
+  }
+
+  missingFileErrorMessage(): string | undefined {
+    return this._missingFileErrorMessage;
+  }
+
   async localPathAfterFinished(progress: Progress): Promise<string> {
     return await progress.race(this._localPathAfterFinished());
   }

packages/playwright-core/src/server/browser.ts

@@ -80,7 +80,7 @@ export abstract class Browser extends SdkObject {
   private _startedClosing = false;
   private _contextForReuse: { context: BrowserContext, hash: string } | undefined;
   _closeReason: string | undefined;
-  _isCollocatedWithServer: boolean = true;
+  _isBrowserCollocatedWithServer: boolean = true;
   private _server: BrowserServer;
 
   constructor(parent: SdkObject, options: BrowserOptions) {

packages/playwright-core/src/server/chromium/chromium.ts

@@ -159,7 +159,7 @@ export class Chromium extends BrowserType {
       validateBrowserContextOptions(persistent, browserOptions);
       const browser = await progress.race(CRBrowser.connect(this.attribution.playwright, transport, browserOptions));
       if (!options.isLocal)
-        browser._isCollocatedWithServer = false;
+        browser._isBrowserCollocatedWithServer = false;
       browser.on(Browser.Events.Disconnected, doCleanup);
       return browser;
     } catch (error) {

packages/playwright-core/src/server/chromium/crBrowser.ts

@@ -63,7 +63,7 @@ export class CRBrowser extends Browser {
     const browser = new CRBrowser(parent, connection, options);
     browser._devtools = devtools;
     if (browser.isClank())
-      browser._isCollocatedWithServer = false;
+      browser._isBrowserCollocatedWithServer = false;
     const session = connection.rootSession;
     if ((options as any).__testHookOnConnectToBrowser)
       await (options as any).__testHookOnConnectToBrowser();

packages/playwright-core/src/server/dispatchers/artifactDispatcher.ts

@@ -46,11 +46,13 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactCh
   }
 
   async pathAfterFinished(params: channels.ArtifactPathAfterFinishedParams, progress: Progress): Promise<channels.ArtifactPathAfterFinishedResult> {
+    this._assertFileAccessible();
     const path = await this._object.localPathAfterFinished(progress);
     return { value: path };
   }
 
   async saveAs(params: channels.ArtifactSaveAsParams, progress: Progress): Promise<channels.ArtifactSaveAsResult> {
+    this._assertFileAccessible();
     return await progress.race(new Promise((resolve, reject) => {
       this._object.saveAs(progress, async (localPath, error) => {
         if (error) {
@@ -68,7 +70,14 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactCh
     }));
   }
 
+  private _assertFileAccessible(): void {
+    const errorMessage = this._object.missingFileErrorMessage();
+    if (errorMessage)
+      throw new Error(errorMessage);
+  }
+
   async saveAsStream(params: channels.ArtifactSaveAsStreamParams, progress: Progress): Promise<channels.ArtifactSaveAsStreamResult> {
+    this._assertFileAccessible();
     return await progress.race(new Promise((resolve, reject) => {
       this._object.saveAs(progress, async (localPath, error) => {
         if (error) {
@@ -94,6 +103,7 @@ export class ArtifactDispatcher extends Dispatcher<Artifact, channels.ArtifactCh
   }
 
   async stream(params: channels.ArtifactStreamParams, progress: Progress): Promise<channels.ArtifactStreamResult> {
+    this._assertFileAccessible();
     const fileName = await this._object.localPathAfterFinished(progress);
     const readable = fs.createReadStream(fileName, { highWaterMark: 1024 * 1024 });
     return { stream: new StreamDispatcher(this, readable) };

packages/playwright-core/src/server/dispatchers/dispatcher.ts

@@ -195,10 +195,10 @@ export class DispatcherConnection {
   readonly _dispatchersByBucket = new Map<string, Set<string>>();
   onmessage = (message: object) => {};
   private _waitOperations = new Map<string, CallMetadata>();
-  private _isLocal: boolean;
+  private _isInProcess: boolean;
 
-  constructor(isLocal?: boolean) {
-    this._isLocal = !!isLocal;
+  constructor(isInProcess?: boolean) {
+    this._isInProcess = !!isInProcess;
   }
 
   sendEvent(dispatcher: DispatcherScope, event: string, params: any) {
@@ -224,15 +224,15 @@ export class DispatcherConnection {
   private _validatorToWireContext(): ValidatorContext {
     return {
       tChannelImpl: this._tChannelImplToWire.bind(this),
-      binary: this._isLocal ? 'buffer' : 'toBase64',
+      binary: this._isInProcess ? 'buffer' : 'toBase64',
       isUnderTest,
     };
   }
 
   private _validatorFromWireContext(): ValidatorContext {
     return {
       tChannelImpl: this._tChannelImplFromWire.bind(this),
-      binary: this._isLocal ? 'buffer' : 'fromBase64',
+      binary: this._isInProcess ? 'buffer' : 'fromBase64',
       isUnderTest,
     };
   }

packages/playwright-core/src/server/download.ts

@@ -32,6 +32,11 @@ export class Download {
     if (!downloadPath)
       throw new Error(`Download filename '${downloadFilename}' escapes download directory`);
     this.artifact = new Artifact(page, downloadPath, unaccessibleErrorMessage, () => this.cancel());
+    if (!page.browserContext._browser._isBrowserCollocatedWithServer) {
+      this.artifact.markMissingFileErrorMessage(
+          `Downloaded file is not accessible from the Playwright server because the browser is running on a different host ` +
+          `(e.g. connected over CDP to a remote browser). Saving downloads requires the browser and the Playwright server to share a filesystem.`);
+    }
     this._page = page;
     this.url = url;
     this._uuid = uuid;

packages/playwright-core/src/server/fileUploadUtils.ts

@@ -36,6 +36,8 @@ async function filesExceedUploadLimit(files: string[]) {
 export async function prepareFilesForUpload(frame: Frame, params: Omit<channels.ElementHandleSetInputFilesParams, 'timeout'>): Promise<InputFilesItems> {
   const { payloads, streams, directoryStream } = params;
   let { localPaths, localDirectory } = params;
+  if (localPaths && !frame.attribution.playwright.options.isClientCollocatedWithServer)
+    throw new Error('localPaths are not allowed when the client is not local');
 
   if ([payloads, localPaths, localDirectory, streams, directoryStream].filter(Boolean).length !== 1)
     throw new Error('Exactly one of payloads, localPaths and streams must be provided');
@@ -57,7 +59,7 @@ export async function prepareFilesForUpload(frame: Frame, params: Omit<channels.
     lastModifiedMs?: number,
   }[] | undefined = payloads;
 
-  if (!frame._page.browserContext._browser._isCollocatedWithServer) {
+  if (!frame._page.browserContext._browser._isBrowserCollocatedWithServer) {
     // If the browser is on a different machine read files into buffers.
     if (localPaths) {
       if (await filesExceedUploadLimit(localPaths))

packages/playwright-core/src/server/playwright.ts

@@ -34,6 +34,7 @@ type PlaywrightOptions = {
   sdkLanguage: Language;
   isInternalPlaywright?: boolean;
   isServer?: boolean;
+  isClientCollocatedWithServer?: boolean;
 };
 
 export class Playwright extends SdkObject {