diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts index 24f7ea2ffe..206e05e06c 100644 --- a/packages/playwright-test/src/index.ts +++ b/packages/playwright-test/src/index.ts @@ -325,13 +325,16 @@ const playwrightFixtures: Fixtures = ({ }; const startedCollectingArtifacts = Symbol('startedCollectingArtifacts'); - const stopTracing = async (tracing: Tracing) => { + const stopTracing = async (tracing: Tracing, contextTearDownStarted: boolean) => { if ((tracing as any)[startedCollectingArtifacts]) return; (tracing as any)[startedCollectingArtifacts] = true; if (captureTrace) { let tracePath; - if (preserveTrace()) { + // Create a trace file if we know that: + // - it is's going to be used due to the config setting and the test status or + // - we are inside a test or afterEach and the user manually closed the context. + if (preserveTrace() || !contextTearDownStarted) { tracePath = path.join(_artifactsDir(), createGuid() + '.zip'); temporaryTraceFiles.push(tracePath); } @@ -363,7 +366,7 @@ const playwrightFixtures: Fixtures = ({ // Do not record empty traces and useless screenshots for them. if (reusedContexts.has(context)) return; - await stopTracing(context.tracing); + await stopTracing(context.tracing, (context as any)[kStartedContextTearDown]); if (screenshotMode === 'on' || screenshotMode === 'only-on-failure') { // Capture screenshot for now. We'll know whether we have to preserve them // after the test finishes. @@ -373,7 +376,7 @@ const playwrightFixtures: Fixtures = ({ const onWillCloseRequestContext = async (context: APIRequestContext) => { const tracing = (context as any)._tracing as Tracing; - await stopTracing(tracing); + await stopTracing(tracing, (context as any)[kStartedContextTearDown]); }; // 1. Setup instrumentation and process existing contexts. @@ -435,7 +438,7 @@ const playwrightFixtures: Fixtures = ({ // 5. Collect artifacts from any non-closed contexts. await Promise.all(leftoverContexts.map(async context => { - await stopTracing(context.tracing); + await stopTracing(context.tracing, true); if (captureScreenshots) { await Promise.all(context.pages().map(async page => { if ((page as any)[screenshottedSymbol]) @@ -447,7 +450,7 @@ const playwrightFixtures: Fixtures = ({ } }).concat(leftoverApiRequests.map(async context => { const tracing = (context as any)._tracing as Tracing; - await stopTracing(tracing); + await stopTracing(tracing, true); }))); // 6. Save test trace. @@ -508,6 +511,7 @@ const playwrightFixtures: Fixtures = ({ let counter = 0; await Promise.all([...contexts.keys()].map(async context => { + (context as any)[kStartedContextTearDown] = true; await context.close(); const testFailed = testInfo.status !== testInfo.expectedStatus; @@ -568,6 +572,7 @@ const playwrightFixtures: Fixtures = ({ request: async ({ playwright }, use) => { const request = await playwright.request.newContext(); await use(request); + (request as any)[kStartedContextTearDown] = true; await request.dispose(); }, }); @@ -657,6 +662,7 @@ function attachConnectedHeaderIfNeeded(testInfo: TestInfo, browser: Browser | nu const kTracingStarted = Symbol('kTracingStarted'); const kIsReusedContext = Symbol('kReusedContext'); +const kStartedContextTearDown = Symbol('kStartedContextTearDown'); function connectOptionsFromEnv() { const wsEndpoint = process.env.PW_TEST_CONNECT_WS_ENDPOINT; diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index bf22c26818..9a2c349023 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -391,4 +391,66 @@ for (const mode of ['off', 'retain-on-failure', 'on-first-retry', 'on-all-retrie expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); }); -} \ No newline at end of file +} + +test(`trace:retain-on-failure should create trace if context is closed before failure in the test`, async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { use: { trace: 'retain-on-failure' } }; + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('passing test', async ({ page, context }) => { + await page.goto('about:blank'); + await context.close(); + expect(1).toBe(2); + }); + `, + }, { trace: 'retain-on-failure' }); + const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip'); + const trace = await parseTrace(tracePath); + expect(trace.actions).toContain('page.goto'); + expect(result.failed).toBe(1); +}); + +test(`trace:retain-on-failure should create trace if context is closed before failure in afterEach`, async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { use: { trace: 'retain-on-failure' } }; + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('passing test', async ({ page, context }) => { + }); + test.afterEach(async ({ page, context }) => { + await page.goto('about:blank'); + await context.close(); + expect(1).toBe(2); + }); + `, + }, { trace: 'retain-on-failure' }); + const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip'); + const trace = await parseTrace(tracePath); + expect(trace.actions).toContain('page.goto'); + expect(result.failed).toBe(1); +}); + +test(`trace:retain-on-failure should create trace if request context is disposed before failure`, async ({ runInlineTest, server }) => { + const result = await runInlineTest({ + 'playwright.config.ts': ` + module.exports = { use: { trace: 'retain-on-failure' } }; + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('passing test', async ({ request }) => { + expect(await request.get('${server.EMPTY_PAGE}')).toBeOK(); + await request.dispose(); + expect(1).toBe(2); + }); + `, + }, { trace: 'retain-on-failure' }); + const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip'); + const trace = await parseTrace(tracePath); + expect(trace.actions).toContain('apiRequestContext.get'); + expect(result.failed).toBe(1); +});