Skip to content

Commit

Permalink
fix(reuse): disable trace/video when reusing the context (#19764)
Browse files Browse the repository at this point in the history
Previously, we disabled reuse when trace/video was on. Component testing
keeps this behavior.

References #19059.
  • Loading branch information
dgozman authored Dec 29, 2022
1 parent 137070d commit 5cdf118
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 22 deletions.
22 changes: 12 additions & 10 deletions packages/playwright-test/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type { Fixtures, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWor
import { store as _baseStore } from './store';
import type { TestInfoImpl } from './testInfo';
import { rootTestType, _setProjectSetup } from './testType';
import { type ContextReuseMode } from './types';
export { expect } from './expect';
export { addRunnerPlugin as _addRunnerPlugin } from './plugins';
export const _baseTest: TestType<{}, {}> = rootTestType.test;
Expand All @@ -43,7 +44,7 @@ if ((process as any)['__pw_initiator__']) {

type TestFixtures = PlaywrightTestArgs & PlaywrightTestOptions & {
_combinedContextOptions: BrowserContextOptions,
_contextReuseEnabled: boolean,
_contextReuseMode: ContextReuseMode,
_reuseContext: boolean,
_setupContextOptionsAndArtifacts: void;
_contextFactory: (options?: BrowserContextOptions) => Promise<BrowserContext>;
Expand Down Expand Up @@ -238,7 +239,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({

_snapshotSuffix: [process.platform, { scope: 'worker' }],

_setupContextOptionsAndArtifacts: [async ({ playwright, _snapshotSuffix, _combinedContextOptions, _browserOptions, _artifactsDir, trace, screenshot, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => {
_setupContextOptionsAndArtifacts: [async ({ playwright, _snapshotSuffix, _combinedContextOptions, _reuseContext, _artifactsDir, trace, screenshot, actionTimeout, navigationTimeout, testIdAttribute }, use, testInfo) => {
if (testIdAttribute)
playwrightLibrary.selectors.setTestIdAttribute(testIdAttribute);
testInfo.snapshotSuffix = _snapshotSuffix;
Expand All @@ -250,7 +251,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const traceMode = normalizeTraceMode(trace);
const defaultTraceOptions = { screenshots: true, snapshots: true, sources: true };
const traceOptions = typeof trace === 'string' ? defaultTraceOptions : { ...defaultTraceOptions, ...trace, mode: undefined };
const captureTrace = shouldCaptureTrace(traceMode, testInfo);
const captureTrace = shouldCaptureTrace(traceMode, testInfo) && !_reuseContext;
const temporaryTraceFiles: string[] = [];
const temporaryScreenshots: string[] = [];
const testInfoImpl = testInfo as TestInfoImpl;
Expand Down Expand Up @@ -462,9 +463,9 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}));
}, { auto: 'all-hooks-included', _title: 'playwright configuration' } as any],

_contextFactory: [async ({ browser, video, _artifactsDir }, use, testInfo) => {
_contextFactory: [async ({ browser, video, _artifactsDir, _reuseContext }, use, testInfo) => {
const videoMode = normalizeVideoMode(video);
const captureVideo = shouldCaptureVideo(videoMode, testInfo);
const captureVideo = shouldCaptureVideo(videoMode, testInfo) && !_reuseContext;
const contexts = new Map<BrowserContext, { pages: Page[] }>();

await use(async options => {
Expand Down Expand Up @@ -518,10 +519,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
testInfo.errors.push({ message: prependToError });
}, { scope: 'test', _title: 'context' } as any],

_contextReuseEnabled: !!process.env.PW_TEST_REUSE_CONTEXT,
_contextReuseMode: process.env.PW_TEST_REUSE_CONTEXT === 'when-possible' ? 'when-possible' : (process.env.PW_TEST_REUSE_CONTEXT ? 'force' : 'none'),

_reuseContext: async ({ video, trace, _contextReuseEnabled }, use, testInfo) => {
const reuse = _contextReuseEnabled && !shouldCaptureVideo(normalizeVideoMode(video), testInfo) && !shouldCaptureTrace(normalizeTraceMode(trace), testInfo);
_reuseContext: async ({ video, trace, _contextReuseMode }, use, testInfo) => {
const reuse = _contextReuseMode === 'force' ||
(_contextReuseMode === 'when-possible' && !shouldCaptureVideo(normalizeVideoMode(video), testInfo) && !shouldCaptureTrace(normalizeTraceMode(trace), testInfo));
await use(reuse);
},

Expand Down Expand Up @@ -602,7 +604,7 @@ export function normalizeVideoMode(video: VideoMode | 'retry-with-video' | { mod
return videoMode;
}

export function shouldCaptureVideo(videoMode: VideoMode, testInfo: TestInfo) {
function shouldCaptureVideo(videoMode: VideoMode, testInfo: TestInfo) {
return (videoMode === 'on' || videoMode === 'retain-on-failure' || (videoMode === 'on-first-retry' && testInfo.retry === 1));
}

Expand All @@ -615,7 +617,7 @@ export function normalizeTraceMode(trace: TraceMode | 'retry-with-trace' | { mod
return traceMode;
}

export function shouldCaptureTrace(traceMode: TraceMode, testInfo: TestInfo) {
function shouldCaptureTrace(traceMode: TraceMode, testInfo: TestInfo) {
return traceMode === 'on' || traceMode === 'retain-on-failure' || (traceMode === 'on-first-retry' && testInfo.retry === 1);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/playwright-test/src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import type { Fixtures, Locator, Page, BrowserContextOptions, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, BrowserContext } from './types';
import type { Fixtures, Locator, Page, BrowserContextOptions, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, BrowserContext, ContextReuseMode } from './types';
import type { Component, JsxComponent, MountOptions } from '../types/component';

let boundCallbacksForMount: Function[] = [];
Expand All @@ -29,9 +29,9 @@ export const fixtures: Fixtures<
mount: (component: any, options: any) => Promise<MountResult>;
},
PlaywrightWorkerArgs & PlaywrightWorkerOptions & { _ctWorker: { context: BrowserContext | undefined, hash: string } },
{ _contextFactory: (options?: BrowserContextOptions) => Promise<BrowserContext>, _contextReuseEnabled: boolean }> = {
{ _contextFactory: (options?: BrowserContextOptions) => Promise<BrowserContext>, _contextReuseMode: ContextReuseMode }> = {

_contextReuseEnabled: true,
_contextReuseMode: 'when-possible',

serviceWorkers: 'block',

Expand Down
2 changes: 2 additions & 0 deletions packages/playwright-test/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,5 @@ export interface FullProjectInternal extends FullProjectPublic {
export interface ReporterInternal extends Reporter {
_onExit?(): void | Promise<void>;
}

export type ContextReuseMode = 'none' | 'force' | 'when-possible';
50 changes: 41 additions & 9 deletions tests/playwright-test/playwright.reuse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { test, expect } from './playwright-test-fixtures';
import fs from 'fs';

test('should reuse context', async ({ runInlineTest }) => {
const result = await runInlineTest({
Expand Down Expand Up @@ -56,7 +57,7 @@ test('should reuse context', async ({ runInlineTest }) => {
expect(result.passed).toBe(5);
});

test('should not reuse context with video', async ({ runInlineTest }) => {
test('should not reuse context with video if mode=when-possible', async ({ runInlineTest }, testInfo) => {
const result = await runInlineTest({
'playwright.config.ts': `
export default {
Expand All @@ -65,23 +66,54 @@ test('should not reuse context with video', async ({ runInlineTest }) => {
`,
'src/reuse.test.ts': `
const { test } = pwt;
let lastContext;
let lastContextGuid;
test('one', async ({ context }) => {
lastContext = context;
lastContextGuid = context._guid;
});
test('two', async ({ context }) => {
expect(context).not.toBe(lastContext);
expect(context._guid).not.toBe(lastContextGuid);
});
`,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: 'when-possible' });

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-one', 'video.webm'))).toBeFalsy();
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-two', 'video.webm'))).toBeFalsy();
});

test('should reuse context and disable video if mode=force', async ({ runInlineTest }, testInfo) => {
const result = await runInlineTest({
'playwright.config.ts': `
export default {
use: { video: 'on' },
};
`,
'reuse.test.ts': `
const { test } = pwt;
let lastContextGuid;
test('one', async ({ context, page }) => {
lastContextGuid = context._guid;
await page.waitForTimeout(2000);
});
test('two', async ({ context, page }) => {
expect(context._guid).toBe(lastContextGuid);
await page.waitForTimeout(2000);
});
`,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: '1' });

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-one', 'video.webm'))).toBeFalsy();
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-two', 'video.webm'))).toBeFalsy();
});

test('should not reuse context with trace', async ({ runInlineTest }) => {
test('should not reuse context with trace if mode=when-possible', async ({ runInlineTest }) => {
const result = await runInlineTest({
'playwright.config.ts': `
export default {
Expand All @@ -90,17 +122,17 @@ test('should not reuse context with trace', async ({ runInlineTest }) => {
`,
'src/reuse.test.ts': `
const { test } = pwt;
let lastContext;
let lastContextGuid;
test('one', async ({ context }) => {
lastContext = context;
lastContextGuid = context._guid;
});
test('two', async ({ context }) => {
expect(context).not.toBe(lastContext);
expect(context._guid).not.toBe(lastContextGuid);
});
`,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: '1' });
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: 'when-possible' });

expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2);
Expand Down

0 comments on commit 5cdf118

Please sign in to comment.