Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(reuse): make reuse work with tracing (#19733)

Browse source 
Fixes #19059.
This commit is contained in:
Dmitry Gozman 2023-01-05 14:50:47 -08:00 committed by GitHub
parent 10ccfa9517
commit 31a63b5c2a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
packages/playwright-core/src/client/browser.ts
View file

@ -64,7 +64,9 @@ export class Browser extends ChannelOwner<channels.BrowserChannel> implements ap
async _newContextForReuse(options: BrowserContextOptions = {}): Promise<BrowserContext> { async _newContextForReuse(options: BrowserContextOptions = {}): Promise<BrowserContext> {
for (const context of this._contexts) { for (const context of this._contexts) {
await this._browserType._onWillCloseContext?.(context); await this._wrapApiCall(async () => {
await this._browserType._onWillCloseContext?.(context);
}, true);
for (const page of context.pages()) for (const page of context.pages())
page._onClose(); page._onClose();
context._onClose(); context._onClose();

54
packages/playwright-test/src/index.ts
View file

@ -251,10 +251,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const traceMode = normalizeTraceMode(trace); const traceMode = normalizeTraceMode(trace);
const defaultTraceOptions = { screenshots: true, snapshots: true, sources: true }; const defaultTraceOptions = { screenshots: true, snapshots: true, sources: true };
const traceOptions = typeof trace === 'string' ? defaultTraceOptions : { ...defaultTraceOptions, ...trace, mode: undefined }; const traceOptions = typeof trace === 'string' ? defaultTraceOptions : { ...defaultTraceOptions, ...trace, mode: undefined };
const captureTrace = shouldCaptureTrace(traceMode, testInfo) && !_reuseContext; const captureTrace = shouldCaptureTrace(traceMode, testInfo);
const temporaryTraceFiles: string[] = []; const temporaryTraceFiles: string[] = [];
const temporaryScreenshots: string[] = []; const temporaryScreenshots: string[] = [];
const testInfoImpl = testInfo as TestInfoImpl; const testInfoImpl = testInfo as TestInfoImpl;
const reusedContexts = new Set<BrowserContext>();
const createInstrumentationListener = (context?: BrowserContext) => { const createInstrumentationListener = (context?: BrowserContext) => {
return { return {
@ -282,7 +283,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}; };
}; };
const startTracing = async (tracing: Tracing) => { const startTraceChunkOnContextCreation = async (tracing: Tracing) => {
if (captureTrace) { if (captureTrace) {
const title = [path.relative(testInfo.project.testDir, testInfo.file) + ':' + testInfo.line, ...testInfo.titlePath.slice(1)].join(' › '); const title = [path.relative(testInfo.project.testDir, testInfo.file) + ':' + testInfo.line, ...testInfo.titlePath.slice(1)].join(' › ');
if (!(tracing as any)[kTracingStarted]) { if (!(tracing as any)[kTracingStarted]) {
@ -302,20 +303,19 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const onDidCreateBrowserContext = async (context: BrowserContext) => { const onDidCreateBrowserContext = async (context: BrowserContext) => {
context.setDefaultTimeout(actionTimeout || 0); context.setDefaultTimeout(actionTimeout || 0);
context.setDefaultNavigationTimeout(navigationTimeout || actionTimeout || 0); context.setDefaultNavigationTimeout(navigationTimeout || actionTimeout || 0);
await startTracing(context.tracing); await startTraceChunkOnContextCreation(context.tracing);
const listener = createInstrumentationListener(context); const listener = createInstrumentationListener(context);
(context as any)._instrumentation.addListener(listener); (context as any)._instrumentation.addListener(listener);
(context.request as any)._instrumentation.addListener(listener); (context.request as any)._instrumentation.addListener(listener);
}; };
const onDidCreateRequestContext = async (context: APIRequestContext) => { const onDidCreateRequestContext = async (context: APIRequestContext) => {
const tracing = (context as any)._tracing as Tracing; const tracing = (context as any)._tracing as Tracing;
await startTracing(tracing); await startTraceChunkOnContextCreation(tracing);
(context as any)._instrumentation.addListener(createInstrumentationListener()); (context as any)._instrumentation.addListener(createInstrumentationListener());
}; };
const startedCollectingArtifacts = Symbol('startedCollectingArtifacts'); const startedCollectingArtifacts = Symbol('startedCollectingArtifacts');
const stopTraceChunkOnContextClosure = async (tracing: Tracing) => {
const stopTracing = async (tracing: Tracing) => {
(tracing as any)[startedCollectingArtifacts] = true; (tracing as any)[startedCollectingArtifacts] = true;
if (captureTrace) { if (captureTrace) {
// Export trace for now. We'll know whether we have to preserve it // Export trace for now. We'll know whether we have to preserve it
@ -346,7 +346,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}; };
const onWillCloseContext = async (context: BrowserContext) => { const onWillCloseContext = async (context: BrowserContext) => {
await stopTracing(context.tracing); // When reusing context, we get all previous contexts closed at the start of next test.
// Do not record empty traces and useless screenshots for them.
if (reusedContexts.has(context))
return;
await stopTraceChunkOnContextClosure(context.tracing);
if (screenshotMode === 'on' || screenshotMode === 'only-on-failure') { if (screenshotMode === 'on' || screenshotMode === 'only-on-failure') {
// Capture screenshot for now. We'll know whether we have to preserve them // Capture screenshot for now. We'll know whether we have to preserve them
// after the test finishes. // after the test finishes.
@ -356,7 +360,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
const onWillCloseRequestContext = async (context: APIRequestContext) => { const onWillCloseRequestContext = async (context: APIRequestContext) => {
const tracing = (context as any)._tracing as Tracing; const tracing = (context as any)._tracing as Tracing;
await stopTracing(tracing); await stopTraceChunkOnContextClosure(tracing);
}; };
// 1. Setup instrumentation and process existing contexts. // 1. Setup instrumentation and process existing contexts.
@ -365,7 +369,10 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
(browserType as any)._onWillCloseContext = onWillCloseContext; (browserType as any)._onWillCloseContext = onWillCloseContext;
(browserType as any)._defaultContextOptions = _combinedContextOptions; (browserType as any)._defaultContextOptions = _combinedContextOptions;
const existingContexts = Array.from((browserType as any)._contexts) as BrowserContext[]; const existingContexts = Array.from((browserType as any)._contexts) as BrowserContext[];
await Promise.all(existingContexts.map(onDidCreateBrowserContext)); if (_reuseContext)
existingContexts.forEach(c => reusedContexts.add(c));
else
await Promise.all(existingContexts.map(onDidCreateBrowserContext));
} }
{ {
(playwright.request as any)._onDidCreateContext = onDidCreateRequestContext; (playwright.request as any)._onDidCreateContext = onDidCreateRequestContext;
@ -416,23 +423,21 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
(playwright.request as any)._onWillCloseContext = undefined; (playwright.request as any)._onWillCloseContext = undefined;
testInfoImpl._onTestFailureImmediateCallbacks.delete(screenshotOnTestFailure); testInfoImpl._onTestFailureImmediateCallbacks.delete(screenshotOnTestFailure);
const stopTraceChunk = async (tracing: Tracing): Promise<boolean> => { const stopTraceChunkOnTestFinish = async (tracing: Tracing) => {
// When we timeout during context.close(), we might end up with context still alive // When we timeout during context.close(), we might end up with context still alive
// but artifacts being already collected. In this case, do not collect artifacts // but artifacts being already collected. In this case, do not collect artifacts
// for the second time. // for the second time.
if ((tracing as any)[startedCollectingArtifacts]) if ((tracing as any)[startedCollectingArtifacts])
return false; return;
if (preserveTrace) if (preserveTrace)
await tracing.stopChunk({ path: addTraceAttachment() }); await tracing.stopChunk({ path: addTraceAttachment() });
else if (captureTrace) else if (captureTrace)
await tracing.stopChunk(); await tracing.stopChunk();
return true;
}; };
// 5. Collect artifacts from any non-closed contexts. // 5. Collect artifacts from any non-closed contexts.
await Promise.all(leftoverContexts.map(async context => { await Promise.all(leftoverContexts.map(async context => {
if (!await stopTraceChunk(context.tracing)) await stopTraceChunkOnTestFinish(context.tracing);
return;
if (captureScreenshots) { if (captureScreenshots) {
await Promise.all(context.pages().map(async page => { await Promise.all(context.pages().map(async page => {
if ((page as any)[screenshottedSymbol]) if ((page as any)[screenshottedSymbol])
@ -444,7 +449,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
} }
}).concat(leftoverApiRequests.map(async context => { }).concat(leftoverApiRequests.map(async context => {
const tracing = (context as any)._tracing as Tracing; const tracing = (context as any)._tracing as Tracing;
await stopTraceChunk(tracing); await stopTraceChunkOnTestFinish(tracing);
}))); })));
// 6. Either remove or attach temporary traces and screenshots for contexts closed // 6. Either remove or attach temporary traces and screenshots for contexts closed
@ -464,12 +469,13 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}, { auto: 'all-hooks-included', _title: 'playwright configuration' } as any], }, { auto: 'all-hooks-included', _title: 'playwright configuration' } as any],
_contextFactory: [async ({ browser, video, _artifactsDir, _reuseContext }, use, testInfo) => { _contextFactory: [async ({ browser, video, _artifactsDir, _reuseContext }, use, testInfo) => {
const testInfoImpl = testInfo as TestInfoImpl;
const videoMode = normalizeVideoMode(video); const videoMode = normalizeVideoMode(video);
const captureVideo = shouldCaptureVideo(videoMode, testInfo) && !_reuseContext; const captureVideo = shouldCaptureVideo(videoMode, testInfo) && !_reuseContext;
const contexts = new Map<BrowserContext, { pages: Page[] }>(); const contexts = new Map<BrowserContext, { pages: Page[] }>();
await use(async options => { await use(async options => {
const hook = hookType(testInfo); const hook = hookType(testInfoImpl);
if (hook) { if (hook) {
throw new Error([ throw new Error([
`"context" and "page" fixtures are not supported in "${hook}" since they are created on a per-test basis.`, `"context" and "page" fixtures are not supported in "${hook}" since they are created on a per-test basis.`,
@ -490,7 +496,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
return context; return context;
}); });
const prependToError = (testInfo as any)._didTimeout ? const prependToError = testInfoImpl._didTimeout ?
formatPendingCalls((browser as any)._connection.pendingProtocolCalls()) : ''; formatPendingCalls((browser as any)._connection.pendingProtocolCalls()) : '';
let counter = 0; let counter = 0;
@ -521,9 +527,8 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
_contextReuseMode: process.env.PW_TEST_REUSE_CONTEXT === 'when-possible' ? 'when-possible' : (process.env.PW_TEST_REUSE_CONTEXT ? 'force' : 'none'), _contextReuseMode: process.env.PW_TEST_REUSE_CONTEXT === 'when-possible' ? 'when-possible' : (process.env.PW_TEST_REUSE_CONTEXT ? 'force' : 'none'),
_reuseContext: async ({ video, trace, _contextReuseMode }, use, testInfo) => { _reuseContext: async ({ video, _contextReuseMode }, use, testInfo) => {
const reuse = _contextReuseMode === 'force' || const reuse = _contextReuseMode === 'force' || (_contextReuseMode === 'when-possible' && !shouldCaptureVideo(normalizeVideoMode(video), testInfo));
(_contextReuseMode === 'when-possible' && !shouldCaptureVideo(normalizeVideoMode(video), testInfo) && !shouldCaptureTrace(normalizeTraceMode(trace), testInfo));
await use(reuse); await use(reuse);
}, },
@ -575,11 +580,10 @@ function formatStackFrame(frame: StackFrame) {
return `${file}:${frame.line || 1}:${frame.column || 1}`; return `${file}:${frame.line || 1}:${frame.column || 1}`;
} }
function hookType(testInfo: TestInfo): 'beforeAll' | 'afterAll' | undefined { function hookType(testInfo: TestInfoImpl): 'beforeAll' | 'afterAll' | undefined {
if ((testInfo as any)._timeoutManager._runnable?.type === 'beforeAll') const type = testInfo._timeoutManager.currentRunnableType();
return 'beforeAll'; if (type === 'beforeAll' || type === 'afterAll')
if ((testInfo as any)._timeoutManager._runnable?.type === 'afterAll') return type;
return 'afterAll';
} }
type StackFrame = { type StackFrame = {

4
packages/playwright-test/src/timeoutManager.ts
View file

@ -89,6 +89,10 @@ export class TimeoutManager {
this._timeoutRunner.updateTimeout(timeout); this._timeoutRunner.updateTimeout(timeout);
} }
currentRunnableType() {
return this._runnable.type;
}
private _currentSlot() { private _currentSlot() {
return this._fixture?.slot || this._runnable.slot || this._defaultSlot; return this._fixture?.slot || this._runnable.slot || this._defaultSlot;
} }

33
tests/playwright-test/playwright.reuse.spec.ts
View file

@ -114,29 +114,52 @@ test('should reuse context and disable video if mode=force', async ({ runInlineT
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-two', 'video.webm'))).toBeFalsy(); expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-two', 'video.webm'))).toBeFalsy();
}); });
test('should not reuse context with trace if mode=when-possible', async ({ runInlineTest }) => { test('should reuse context with trace if mode=when-possible', async ({ runInlineTest }, testInfo) => {
const result = await runInlineTest({ const result = await runInlineTest({
'playwright.config.ts': ` 'playwright.config.ts': `
export default { export default {
use: { trace: 'on' }, use: { trace: 'on' },
}; };
`, `,
'src/reuse.test.ts': ` 'reuse.spec.ts': `
const { test } = pwt; const { test } = pwt;
let lastContextGuid; let lastContextGuid;
test('one', async ({ context }) => { test('one', async ({ context, page }) => {
lastContextGuid = context._guid; lastContextGuid = context._guid;
await page.setContent('<button>Click</button>');
await page.click('button');
}); });
test('two', async ({ context }) => { test('two', async ({ context, page }) => {
expect(context._guid).not.toBe(lastContextGuid); expect(context._guid).toBe(lastContextGuid);
await page.setContent('<input>');
await page.fill('input', 'value');
await page.locator('input').click();
}); });
`, `,
}, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: 'when-possible' }); }, { workers: 1 }, { PW_TEST_REUSE_CONTEXT: 'when-possible' });
expect(result.exitCode).toBe(0); expect(result.exitCode).toBe(0);
expect(result.passed).toBe(2); expect(result.passed).toBe(2);
const trace1 = await parseTrace(testInfo.outputPath('test-results', 'reuse-one', 'trace.zip'));
expect(trace1.actions).toEqual([
'browserContext.newPage',
'page.setContent',
'page.click',
]);
expect(trace1.events.some(e => e.type === 'frame-snapshot')).toBe(true);
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-one', 'trace-1.zip'))).toBe(false);
const trace2 = await parseTrace(testInfo.outputPath('test-results', 'reuse-two', 'trace.zip'));
expect(trace2.actions).toEqual([
'page.setContent',
'page.fill',
'locator.click',
]);
expect(trace2.events.some(e => e.type === 'frame-snapshot')).toBe(true);
expect(fs.existsSync(testInfo.outputPath('test-results', 'reuse-two', 'trace-1.zip'))).toBe(false);
}); });
test('should work with manually closed pages', async ({ runInlineTest }) => { test('should work with manually closed pages', async ({ runInlineTest }) => {