Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cherry-pick(#31768): fix(trace viewer): library-only trace should not merge actions #31780

Merged
merged 2 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading