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

chore: miscellaneous trace viewer fixes #23695

Merged
merged 2 commits into from Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
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
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
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
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>
aslushnikov marked this conversation as resolved.
Show resolved Hide resolved
<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
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
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} /> : ''}
aslushnikov marked this conversation as resolved.
Show resolved Hide resolved
{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
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={() => {
aslushnikov marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Up @@ -230,7 +230,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 @@ -241,7 +242,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
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