From b129abab045582075337b84519e6d2237ad75f05 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 22 Jul 2024 10:00:20 -0700 Subject: [PATCH] cherry-pick(#31768): fix(trace viewer): library-only trace should not merge actions (#31780) This PR cherry-picks two trace viewer fixes: cherry-pick(#31768): fix(trace viewer): library-only trace should not merge actions cherry-pick(#31564): fix(trace): do not corrupt test runner actions when no library trace is present Reference: https://github.com/microsoft/playwright-java/issues/1617 --- packages/trace-viewer/src/ui/modelUtil.ts | 15 +++++--- tests/assets/trace-library-1.46.zip | Bin 0 -> 1229 bytes tests/library/trace-viewer.spec.ts | 11 ++++++ .../playwright-test/playwright.trace.spec.ts | 33 ++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tests/assets/trace-library-1.46.zip diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts index ba866ad7caf29..7993699e3f344 100644 --- a/packages/trace-viewer/src/ui/modelUtil.ts +++ b/packages/trace-viewer/src/ui/modelUtil.ts @@ -183,8 +183,8 @@ function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) { if (traceFileToContexts.size > 1) makeCallIdsUniqueAcrossTraceFiles(contexts, ++traceFileId); // Align action times across runner and library contexts within each trace file. - const map = mergeActionsAndUpdateTimingSameTrace(contexts); - result.push(...map.values()); + const actions = mergeActionsAndUpdateTimingSameTrace(contexts); + result.push(...actions); } result.sort((a1, a2) => { if (a2.parentId === a1.callId) @@ -211,12 +211,19 @@ function makeCallIdsUniqueAcrossTraceFiles(contexts: ContextEntry[], traceFileId } } -function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]) { +function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]): ActionTraceEventInContext[] { const map = new Map(); const libraryContexts = contexts.filter(context => context.origin === 'library'); const testRunnerContexts = contexts.filter(context => context.origin === 'testRunner'); + // With library-only or test-runner-only traces there is nothing to match. + if (!testRunnerContexts.length || !libraryContexts.length) { + return contexts.map(context => { + return context.actions.map(action => ({ ...action, context })); + }).flat(); + } + // Library actions are replaced with corresponding test runner steps. Matching with // the test runner steps enables us to find parent steps. // - In the newer versions the actions are matched by explicit step id stored in the @@ -264,7 +271,7 @@ function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]) { map.set(key, { ...action, context }); } } - return map; + return [...map.values()]; } function adjustMonotonicTime(contexts: ContextEntry[], monotonicTimeDelta: number) { diff --git a/tests/assets/trace-library-1.46.zip b/tests/assets/trace-library-1.46.zip new file mode 100644 index 0000000000000000000000000000000000000000..46d7d99bb49dbe050a6fde9622f66a6be211314d GIT binary patch literal 1229 zcmWIWW@Zs#;Nak3D0TWA!GHv~f$Wl^#N<>xFnM)~fBtO?furyLiX4i$#TpSd#V1gx zOX;W>E0<2y%lX0jTU1 zDNPJ<{9Ilua{6DX@7uzhb-P+2VQyY2zpBLEl1s77jY~I6ufOBIbC%u9Ik7LlYF&A^ zPb+`H_V*#m*V*=GZIxW(tRbDbM@eC|7w?aAc~!}&Zx`O#%gFS2-#0e1UB~#mradui z^n1C@S;JN6{yiayo+HP0iB1Xd-hRXm!jS*^)1sh;@gyC55L_!+xqmh zRU7>n8@;4HFXZdU)-&2&Jq2}^`B?n_l5bXOZeONlrL;L z;FXwuxR*V&xunQ+=ADP995~l!+wOZg#qitL)gLT&F8t`-ZzaP%TXBb4wCP!Kxpl8X zmg@9fb-H5HJGn&g^RiPc$x4rJ25mf08T4)C^yc?E@0P~B{c_gEV4H0w!@DiwCZ3-c z9RI!cZk*z)eZ8|5n5^1%%R}TC-}>u`UtZic`DH$%d=jr#`KIZwUeCQc=Q8X06Whwn z{(bp<$H?^)nAR%KYO$0 zOt+k5pSnLw^7I?Ns|xik&fiqrvnu#%zd*NP$3?ftr75$G87+PDvy2Z<4^~Y3k}xH0 z?KibKij^x~C{B54c&e#w!~gu#`)YorroVLE@{%XlvGR}0YrFm7N2YZC34inZ%2PW- zrDdLz?5ZB}->uY8`=gS8vsVb5AIJt^H{H%`F+O1s<5*&4p?)I8VZPTyx zw^*j7@kZxDob&Q0$)9)Znsw?Jd)9GB=B-nA_s4cEuPc*KT6(Mf_MbHh9Jh9V`t-7_ z` { + const traceViewer = await showTraceViewer([asset('trace-library-1.46.zip')]); + await expect(traceViewer.actionTitles).toHaveText([ + /page.setContent/, + /locator.getAttributelocator\('div'\)/, + /locator.isVisiblelocator\('div'\)/, + /locator.getAttributelocator\('div'\)/, + /locator.isVisiblelocator\('div'\)/, + ]); +}); diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index 3e441fbf19ce6..9a692245f04e0 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -1113,6 +1113,39 @@ test('trace:retain-on-first-failure should create trace if request context is di expect(result.failed).toBe(1); }); +test('should not corrupt actions when no library trace is present', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test as base, expect } from '@playwright/test'; + const test = base.extend({ + foo: async ({}, use) => { + expect(1).toBe(1); + await use(); + expect(2).toBe(2); + }, + }); + test('fail', async ({ foo }) => { + expect(1).toBe(2); + }); + `, + }, { trace: 'on' }); + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + + const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip'); + const trace = await parseTrace(tracePath); + expect(trace.actionTree).toEqual([ + 'Before Hooks', + ' fixture: foo', + ' expect.toBe', + 'expect.toBe', + 'After Hooks', + ' fixture: foo', + ' expect.toBe', + 'Worker Cleanup', + ]); +}); + test('should record trace for manually created context in a failed test', async ({ runInlineTest }) => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31541' });