Skip to content

Commit

Permalink
cherry-pick(#31768): fix(trace viewer): library-only trace should not…
Browse files Browse the repository at this point in the history
… 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: microsoft/playwright-java#1617
  • Loading branch information
yury-s authored Jul 22, 2024
1 parent d8a5f3b commit b129aba
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 4 deletions.
15 changes: 11 additions & 4 deletions packages/trace-viewer/src/ui/modelUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -211,12 +211,19 @@ function makeCallIdsUniqueAcrossTraceFiles(contexts: ContextEntry[], traceFileId
}
}

function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]) {
function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]): ActionTraceEventInContext[] {
const map = new Map<string, ActionTraceEventInContext>();

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
Expand Down Expand Up @@ -264,7 +271,7 @@ function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]) {
map.set(key, { ...action, context });
}
}
return map;
return [...map.values()];
}

function adjustMonotonicTime(contexts: ContextEntry[], monotonicTimeDelta: number) {
Expand Down
Binary file added tests/assets/trace-library-1.46.zip
Binary file not shown.
11 changes: 11 additions & 0 deletions tests/library/trace-viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1236,3 +1236,14 @@ test('should open snapshot in new browser context', async ({ browser, page, runA
await expect(newPage.getByText('hello')).toBeVisible();
await newPage.close();
});

test('should show similar actions from library-only trace', async ({ showTraceViewer, asset }) => {
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'\)/,
]);
});
33 changes: 33 additions & 0 deletions tests/playwright-test/playwright.trace.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' });

Expand Down

0 comments on commit b129aba

Please sign in to comment.