Skip to content

Commit

Permalink
cherry-pick(#23695): chore: miscellaneous trace viewer fixes
Browse files Browse the repository at this point in the history
- properly annotate continued requests
- nest `attach` steps inside the related `expect` step
- fix primary-id-to-non-primary-id mapping
- make sure images in trace are not draggable

Fixes #23693

---------

Signed-off-by: Andrey Lushnikov <aslushnikov@gmail.com>
Co-authored-by: Max Schmitt <max@schmitt.mx>
  • Loading branch information
aslushnikov and mxschmitt committed Jun 14, 2023
1 parent ba20378 commit 182a71f
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class CRServiceWorker extends Worker {
const r = new network.Route(request, route);
if (this._browserContext._requestInterceptor?.(r, request))
return;
r.continue();
r.continue({ isFallback: true });
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export class FrameManager {
frame.setPendingDocument({ documentId: request._documentId, request });
if (request._isFavicon) {
if (route)
route.continue(request, {});
route.continue(request, { isFallback: true });
return;
}
this._page.emitOnContext(BrowserContext.Events.Request, request);
Expand All @@ -306,7 +306,7 @@ export class FrameManager {
return;
if (this._page._browserContext._requestInterceptor?.(r, request))
return;
r.continue();
r.continue({ isFallback: true });
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export class Route extends SdkObject {
headers.push({ name: 'vary', value: 'Origin' });
}

async continue(overrides: types.NormalizedContinueOverrides = {}) {
async continue(overrides: types.NormalizedContinueOverrides) {
this._startHandling();
if (overrides.url) {
const newUrl = new URL(overrides.url);
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export type NormalizedContinueOverrides = {
method?: string,
headers?: HeadersArray,
postData?: Buffer,
isFallback?: boolean,
isFallback: boolean,
};

export type EmulatedSize = { viewport: Size, screen: Size };
Expand Down
4 changes: 3 additions & 1 deletion packages/playwright-test/src/worker/testInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ export class TestInfoImpl implements TestInfo {
const visit = (step: TestStepInternal) => {
// Never nest into under another lax element, it could be a series
// of no-reply actions, ala page.continue().
if (!step.endWallTime && step.category === data.category && !step.laxParent)
const canNest = step.category === data.category || step.category === 'expect' && data.category === 'attach';
if (!step.endWallTime && canNest && !step.laxParent)
parentStep = step;
step.steps.forEach(visit);
};
Expand Down Expand Up @@ -352,6 +353,7 @@ export class TestInfoImpl implements TestInfo {
title: `attach "${name}"`,
category: 'attach',
wallTime: Date.now(),
laxParent: true,
});
this._attachWithoutStep(attachment);
step.complete({});
Expand Down
2 changes: 1 addition & 1 deletion packages/trace-viewer/src/ui/attachmentsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const AttachmentsTab: React.FunctionComponent<{
{screenshots.size ? <div className='attachments-section'>Screenshots</div> : undefined}
{[...screenshots].map((a, i) => {
return <div className='attachment-item' key={`screenshot-${i}`}>
<div><img src={attachmentURL(traceUrl, a)} /></div>
<div><img draggable='false' src={attachmentURL(traceUrl, a)} /></div>
<div><a target='_blank' href={attachmentURL(traceUrl, a)}>{a.name}</a></div>
</div>;
})}
Expand Down
2 changes: 2 additions & 0 deletions packages/trace-viewer/src/ui/modelUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ function mergeActions(contexts: ContextEntry[]) {
existing.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
continue;
}
if (action.parentId)
action.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
map.set(key, { ...action, context });
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/trace-viewer/src/ui/networkResourceDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const NetworkResourceDetails: React.FunctionComponent<{
{resource.request.postData ? <div className='network-request-body'>{formatBody(requestBody, requestContentType)}</div> : ''}
<div className='network-request-details-header'>Response Body</div>
{!resource.response.content._sha1 ? <div className='network-request-response-body'>Response body is not available for this request.</div> : ''}
{responseBody !== null && responseBody.dataUrl ? <img src={responseBody.dataUrl} /> : ''}
{responseBody !== null && responseBody.dataUrl ? <img draggable='false' src={responseBody.dataUrl} /> : ''}
{responseBody !== null && responseBody.text ? <div className='network-request-response-body'>{formatBody(responseBody.text, resource.response.content.mimeType)}</div> : ''}
</div>
</Expandable>
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/components/imageDiffView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const ImageWithSize: React.FunctionComponent<{
<span style={{ flex: 'none', margin: '0 5px' }}>x</span>
<span style={{ flex: '1 1 0', textAlign: 'start' }}>{ size ? size.height : ''}</span>
</div>
<img src={src} onLoad={() => {
<img draggable='false' src={src} onLoad={() => {
onLoad?.();
if (ref.current)
setSize({ width: ref.current.naturalWidth, height: ref.current.naturalHeight });
Expand Down
5 changes: 3 additions & 2 deletions tests/library/trace-viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ test('should have network request overrides', async ({ page, server, runAndTrace
await traceViewer.selectAction('http://localhost');
await traceViewer.showNetworkTab();
await expect(traceViewer.networkRequests).toContainText([/200GET\/frame.htmltext\/html/]);
await expect(traceViewer.networkRequests).toContainText([/abort.*style.cssx-unknown/]);
await expect(traceViewer.networkRequests).toContainText([/aborted.*style.cssx-unknown/]);
await expect(traceViewer.networkRequests).not.toContainText([/continued/]);
});

test('should have network request overrides 2', async ({ page, server, runAndTrace }) => {
Expand All @@ -240,7 +241,7 @@ test('should have network request overrides 2', async ({ page, server, runAndTra
await traceViewer.selectAction('http://localhost');
await traceViewer.showNetworkTab();
await expect(traceViewer.networkRequests).toContainText([/200GET\/frame.htmltext\/html/]);
await expect(traceViewer.networkRequests).toContainText([/continue.*script.jsapplication\/javascript/]);
await expect(traceViewer.networkRequests).toContainText([/continued.*script.jsapplication\/javascript/]);
});

test('should show snapshot URL', async ({ page, runAndTrace, server }) => {
Expand Down
2 changes: 0 additions & 2 deletions tests/playwright-test/ui-mode-trace.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,13 @@ test('should merge screenshot assertions', async ({ runUITest }, testInfo) => {
await page.getByText('trace test').dblclick();

const listItem = page.getByTestId('action-list').getByRole('listitem');
// TODO: fixme.
await expect(
listItem,
'action list'
).toHaveText([
/Before Hooks[\d.]+m?s/,
/page.setContent[\d.]+m?s/,
/expect.toHaveScreenshot[\d.]+m?s/,
/attach "trace-test-1-actual\.png"[\d.]+m?s/,
/After Hooks[\d.]+m?s/,
/fixture: page[\d.]+m?s/,
/fixture: context[\d.]+m?s/,
Expand Down

0 comments on commit 182a71f

Please sign in to comment.