chore: align internal test harness w/ @playwright/test (microsoft#9796)

Author: pavelfeldman

Diff for: packages/playwright-core/src/client/tracing.ts

@@ -99,10 +99,14 @@ export class Tracing implements api.Tracing {
 
     // Add sources.
     if (sources) {
-      for (const source of sources)
-        zipFile.addFile(source, 'resources/src@' + calculateSha1(source) + '.txt');
+      for (const source of sources) {
+        try {
+          if (fs.statSync(source).isFile())
+            zipFile.addFile(source, 'resources/src@' + calculateSha1(source) + '.txt');
+        } catch (e) {
+        }
+      }
     }
-
     await fs.promises.mkdir(path.dirname(filePath), { recursive: true });
     if (skipCompress) {
       // Local scenario, compress the entries.
@@ -120,7 +124,7 @@ export class Tracing implements api.Tracing {
     await artifact.saveAs(tmpPath);
     await artifact.delete();
 
-    yauzl.open(filePath!, (err, inZipFile) => {
+    yauzl.open(tmpPath!, (err, inZipFile) => {
       if (err) {
         promise.reject(err);
         return;
@@ -135,10 +139,11 @@ export class Tracing implements api.Tracing {
           }
           zipFile.addReadStream(readStream!, entry.fileName);
           if (--pendingEntries === 0) {
-            zipFile.end();
-            zipFile.outputStream.pipe(fs.createWriteStream(filePath)).on('close', () => {
-              fs.promises.unlink(tmpPath).then(() => {
-                promise.resolve();
+            zipFile.end(undefined, () => {
+              zipFile.outputStream.pipe(fs.createWriteStream(filePath)).on('close', () => {
+                fs.promises.unlink(tmpPath).then(() => {
+                  promise.resolve();
+                });
               });
             });
           }

Diff for: packages/playwright-core/src/utils/stackTrace.ts

@@ -35,6 +35,7 @@ const CLIENT_LIB = path.join(CORE_DIR, 'lib', 'client');
 const CLIENT_SRC = path.join(CORE_DIR, 'src', 'client');
 const TEST_DIR_SRC = path.resolve(CORE_DIR, '..', 'playwright-test');
 const TEST_DIR_LIB = path.resolve(CORE_DIR, '..', '@playwright', 'test');
+const WS_LIB = path.relative(process.cwd(), path.dirname(require.resolve('ws')));
 
 export type ParsedStackTrace = {
   allFrames: StackFrame[];
@@ -66,6 +67,11 @@ export function captureStackTrace(): ParsedStackTrace {
     // EventEmitter.emit has 'events.js' file.
     if (frame.file === 'events.js' && frame.function?.endsWith('.emit'))
       return null;
+    // Node 12
+    if (frame.file === '_stream_readable.js' || frame.file === '_stream_writable.js')
+      return null;
+    if (frame.file.startsWith(WS_LIB))
+      return null;
     const fileName = path.resolve(process.cwd(), frame.file);
     if (isTesting && fileName.includes(path.join('playwright', 'tests', 'config', 'coverage.js')))
       return null;

Diff for: packages/playwright-test/src/index.ts

@@ -31,6 +31,7 @@ type TestFixtures = PlaywrightTestArgs & PlaywrightTestOptions & {
 };
 type WorkerAndFileFixtures = PlaywrightWorkerArgs & PlaywrightWorkerOptions & {
   _browserType: BrowserType;
+  _browserOptions: LaunchOptions;
   _artifactsDir: () => string,
   _reuseBrowserContext: ReuseBrowserContextStorage,
 };
@@ -150,31 +151,9 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
       await removeFolders([dir]);
   }, { scope: 'worker' }],
 
-  _browserType: [async ({ playwright, browserName, headless, channel, launchOptions }, use) => {
-    if (!['chromium', 'firefox', 'webkit'].includes(browserName))
-      throw new Error(`Unexpected browserName "${browserName}", must be one of "chromium", "firefox" or "webkit"`);
-    const browserType = playwright[browserName];
-
-    const options: LaunchOptions = {
-      handleSIGINT: false,
-      timeout: 0,
-      ...launchOptions,
-    };
-    if (headless !== undefined)
-      options.headless = headless;
-    if (channel !== undefined)
-      options.channel = channel;
-
-    (browserType as any)._defaultLaunchOptions = options;
-    await use(browserType);
-    (browserType as any)._defaultLaunchOptions = undefined;
-  }, { scope: 'worker' }],
-
-  browser: [ async ({ _browserType }, use) => {
-    const browser = await _browserType.launch();
-    await use(browser);
-    await browser.close();
-  }, { scope: 'worker' } ],
+  _browserOptions: [browserOptionsWorkerFixture, { scope: 'worker' }],
+  _browserType: [browserTypeWorkerFixture, { scope: 'worker' }],
+  browser: [browserWorkerFixture, { scope: 'worker' } ],
 
   acceptDownloads: undefined,
   bypassCSP: undefined,
@@ -480,7 +459,54 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
 
 });
 
-export default test;
+export async function browserOptionsWorkerFixture(
+  {
+    headless,
+    channel,
+    launchOptions
+  }: {
+    headless: boolean | undefined,
+    channel: string | undefined,
+    launchOptions: LaunchOptions
+  }, use: (options: LaunchOptions) => Promise<void>) {
+  const options: LaunchOptions = {
+    handleSIGINT: false,
+    timeout: 0,
+    ...launchOptions,
+  };
+  if (headless !== undefined)
+    options.headless = headless;
+  if (channel !== undefined)
+    options.channel = channel;
+  await use(options);
+}
+
+export async function browserTypeWorkerFixture(
+  {
+    playwright,
+    browserName,
+    _browserOptions
+  }: {
+    playwright: any,
+    browserName: string,
+    _browserOptions: LaunchOptions
+  }, use: (browserType: BrowserType) => Promise<void>) {
+  if (!['chromium', 'firefox', 'webkit'].includes(browserName))
+    throw new Error(`Unexpected browserName "${browserName}", must be one of "chromium", "firefox" or "webkit"`);
+  const browserType = playwright[browserName];
+  (browserType as any)._defaultLaunchOptions = _browserOptions;
+  await use(browserType);
+  (browserType as any)._defaultLaunchOptions = undefined;
+}
+
+export async function browserWorkerFixture(
+  { _browserType }: { _browserType: BrowserType },
+  use: (browser: Browser) => Promise<void>) {
+  const browser = await _browserType.launch();
+  await use(browser);
+  await browser.close();
+}
+
 
 function formatPendingCalls(calls: ParsedStackTrace[]) {
   if (!calls.length)
@@ -517,3 +543,5 @@ type ParsedStackTrace = {
 };
 
 const kTracingStarted = Symbol('kTracingStarted');
+
+export default test;

Diff for: tests/browsercontext-proxy.spec.ts

@@ -16,7 +16,15 @@
 
 import { browserTest as it, expect } from './config/browserTest';
 
-it.use({ proxy: { server: 'per-context' } });
+it.use({
+  launchOptions: async ({ launchOptions }, use) => {
+    await use({
+      ...launchOptions,
+      proxy: { server: 'per-context' }
+    });
+  }
+});
+
 
 it.beforeEach(({ server }) => {
   server.setRoute('/target.html', async (req, res) => {

Diff for: tests/channels.spec.ts

@@ -20,7 +20,11 @@ import { playwrightTest as it, expect } from './config/browserTest';
 
 // Use something worker-scoped (e.g. launch args) to force a new worker for this file.
 // Otherwise, a browser launched for other tests in this worker will affect the expectations.
-it.use({ args: [] });
+it.use({
+  launchOptions: async ({ launchOptions }, use) => {
+    await use({ ...launchOptions, args: [] });
+  }
+});
 
 it('should scope context handles', async ({ browserType, browserOptions, server }) => {
   const browser = await browserType.launch(browserOptions);

Diff for: tests/chromium/js-coverage.spec.ts

@@ -16,7 +16,7 @@
 
 import { contextTest as it, expect } from '../config/browserTest';
 
-it.skip(({ trace }) => !!trace);
+it.skip(({ trace }) => trace === 'on');
 
 it('should work', async function({ page, server }) {
   await page.coverage.startJSCoverage();

Diff for: tests/chromium/oopif.spec.ts

@@ -16,7 +16,11 @@
 
 import { contextTest as it, expect } from '../config/browserTest';
 
-it.use({ args: ['--site-per-process'] });
+it.use({
+  launchOptions: async ({ launchOptions }, use) => {
+    await use({ ...launchOptions, args: ['--site-per-process'] });
+  }
+});
 
 it('should report oopif frames', async function({ page, browser, server }) {
   await page.goto(server.PREFIX + '/dynamic-oopif.html');

Diff for: tests/config/android.config.ts

@@ -18,12 +18,12 @@ import type { Config } from '@playwright/test';
 import * as path from 'path';
 import { test as pageTest } from '../page/pageTest';
 import { androidFixtures } from '../android/androidTest';
-import { PlaywrightOptions } from './browserTest';
+import { PlaywrightOptionsEx } from './browserTest';
 import { CommonOptions } from './baseTest';
 
 const outputDir = path.join(__dirname, '..', '..', 'test-results');
 const testDir = path.join(__dirname, '..');
-const config: Config<CommonOptions & PlaywrightOptions> = {
+const config: Config<CommonOptions & PlaywrightOptionsEx> = {
   testDir,
   outputDir,
   timeout: 120000,

Diff for: tests/config/baseTest.ts

@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-import { Fixtures, _baseTest } from '@playwright/test';
+import { Fixtures, VideoMode, _baseTest } from '@playwright/test';
 import * as path from 'path';
 import * as fs from 'fs';
 import { installCoverageHooks } from './coverage';
 import { start } from '../../packages/playwright-core/lib/outofprocess';
 import { GridClient } from 'playwright-core/lib/grid/gridClient';
-import type { LaunchOptions } from 'playwright-core';
+import type { LaunchOptions, ViewportSize } from 'playwright-core';
 import { commonFixtures, CommonFixtures, serverFixtures, ServerFixtures, ServerOptions } from './commonFixtures';
 
 export type BrowserName = 'chromium' | 'firefox' | 'webkit';
@@ -29,8 +29,8 @@ type BaseOptions = {
   mode: Mode;
   browserName: BrowserName;
   channel: LaunchOptions['channel'];
-  video: boolean | undefined;
-  trace: boolean | undefined;
+  video: VideoMode | { mode: VideoMode, size: ViewportSize };
+  trace: 'off' | 'on' | 'retain-on-failure' | 'on-first-retry' | /** deprecated */ 'retry-with-trace';
   headless: boolean | undefined;
 };
 type BaseFixtures = {

Diff for: tests/config/browserTest.ts

@@ -14,10 +14,10 @@
  * limitations under the License.
  */
 
-import type { Fixtures } from '@playwright/test';
+import type { Fixtures, PlaywrightTestOptions, PlaywrightWorkerOptions } from '@playwright/test';
 import type { Browser, BrowserContext, BrowserContextOptions, BrowserType, LaunchOptions, Page } from 'playwright-core';
 import { removeFolders } from 'playwright-core/lib/utils/utils';
-import { ReuseBrowserContextStorage } from '@playwright/test/src/index';
+import { browserOptionsWorkerFixture, browserTypeWorkerFixture, browserWorkerFixture, ReuseBrowserContextStorage } from '@playwright/test/src/index';
 import * as path from 'path';
 import * as fs from 'fs';
 import * as os from 'os';
@@ -26,21 +26,16 @@ import { baseTest, CommonWorkerFixtures } from './baseTest';
 import { CommonFixtures } from './commonFixtures';
 import type { ParsedStackTrace } from 'playwright-core/lib/utils/stackTrace';
 
-type PlaywrightWorkerOptions = {
-  executablePath: LaunchOptions['executablePath'];
-  proxy: LaunchOptions['proxy'];
-  args: LaunchOptions['args'];
-};
 export type PlaywrightWorkerFixtures = {
+  _browserType: BrowserType;
+  _browserOptions: LaunchOptions;
   browserType: BrowserType;
   browserOptions: LaunchOptions;
   browser: Browser;
   browserVersion: string;
-  _reuseBrowserContext: ReuseBrowserContextStorage,
-};
-type PlaywrightTestOptions = {
-  hasTouch: BrowserContextOptions['hasTouch'];
+  _reuseBrowserContext: ReuseBrowserContextStorage;
 };
+
 type PlaywrightTestFixtures = {
   createUserDataDir: () => Promise<string>;
   launchPersistent: (options?: Parameters<BrowserType['launchPersistentContext']>[1]) => Promise<{ context: BrowserContext, page: Page }>;
@@ -50,35 +45,18 @@ type PlaywrightTestFixtures = {
   context: BrowserContext;
   page: Page;
 };
-export type PlaywrightOptions = PlaywrightWorkerOptions & PlaywrightTestOptions;
+export type PlaywrightOptionsEx = PlaywrightWorkerOptions & PlaywrightTestOptions;
 
 export const playwrightFixtures: Fixtures<PlaywrightTestOptions & PlaywrightTestFixtures, PlaywrightWorkerOptions & PlaywrightWorkerFixtures, CommonFixtures, CommonWorkerFixtures> = {
-  executablePath: [ undefined, { scope: 'worker' } ],
-  proxy: [ undefined, { scope: 'worker' } ],
-  args: [ undefined, { scope: 'worker' } ],
   hasTouch: undefined,
 
-  browserType: [async ({ playwright, browserName }, run) => {
-    await run(playwright[browserName]);
-  }, { scope: 'worker' } ],
-
-  browserOptions: [async ({ headless, channel, executablePath, proxy, args }, run) => {
-    await run({
-      headless,
-      channel,
-      executablePath,
-      proxy,
-      args,
-      handleSIGINT: false,
-      devtools: process.env.DEVTOOLS === '1',
-    });
-  }, { scope: 'worker' } ],
+  _browserType: [browserTypeWorkerFixture, { scope: 'worker' } ],
+  _browserOptions: [browserOptionsWorkerFixture, { scope: 'worker' } ],
 
-  browser: [async ({ browserType, browserOptions }, run) => {
-    const browser = await browserType.launch(browserOptions);
-    await run(browser);
-    await browser.close();
-  }, { scope: 'worker' } ],
+  launchOptions: [ {}, { scope: 'worker' } ],
+  browserType: [async ({ _browserType }, use) => use(_browserType), { scope: 'worker' } ],
+  browserOptions: [async ({ _browserOptions }, use) => use(_browserOptions), { scope: 'worker' } ],
+  browser: [browserWorkerFixture, { scope: 'worker' } ],
 
   browserVersion: [async ({ browser }, run) => {
     await run(browser.version());
@@ -132,7 +110,7 @@ export const playwrightFixtures: Fixtures<PlaywrightTestOptions & PlaywrightTest
   contextOptions: async ({ video, hasTouch }, run, testInfo) => {
     const debugName = path.relative(testInfo.project.outputDir, testInfo.outputDir).replace(/[\/\\]/g, '-');
     const contextOptions = {
-      recordVideo: video ? { dir: testInfo.outputPath('') } : undefined,
+      recordVideo: video === 'on' ? { dir: testInfo.outputPath('') } : undefined,
       _debugName: debugName,
       hasTouch,
     } as BrowserContextOptions;
@@ -145,7 +123,7 @@ export const playwrightFixtures: Fixtures<PlaywrightTestOptions & PlaywrightTest
       const context = await browser.newContext({ ...contextOptions, ...options });
       contexts.set(context, { closed: false });
       context.on('close', () => contexts.get(context).closed = true);
-      if (trace)
+      if (trace === 'on')
         await context.tracing.start({ screenshots: true, snapshots: true, sources: true } as any);
       (context as any)._instrumentation.addListener({
         onApiCallBegin: (apiCall: string, stackTrace: ParsedStackTrace | null, userData: any) => {