From 3c0fab489bebe0901c90d159b74785f8cc2c3643 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 14 Jun 2023 09:37:19 -0700 Subject: [PATCH] chore: miscellaneous trace viewer fixes (#23695) - 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 Co-authored-by: Max Schmitt --- .../playwright-core/src/server/chromium/crServiceWorker.ts | 2 +- packages/playwright-core/src/server/frames.ts | 4 ++-- packages/playwright-core/src/server/network.ts | 2 +- packages/playwright-core/src/server/types.ts | 2 +- packages/playwright-test/src/worker/testInfo.ts | 4 +++- packages/trace-viewer/src/ui/attachmentsTab.tsx | 2 +- packages/trace-viewer/src/ui/modelUtil.ts | 2 ++ packages/trace-viewer/src/ui/networkResourceDetails.tsx | 2 +- packages/web/src/components/imageDiffView.tsx | 2 +- tests/library/trace-viewer.spec.ts | 5 +++-- tests/playwright-test/ui-mode-trace.spec.ts | 2 -- 11 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crServiceWorker.ts b/packages/playwright-core/src/server/chromium/crServiceWorker.ts index 064afd5130..a9cddac9f9 100644 --- a/packages/playwright-core/src/server/chromium/crServiceWorker.ts +++ b/packages/playwright-core/src/server/chromium/crServiceWorker.ts @@ -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 }); } } diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index de1a231295..8e6c3a9875 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -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); @@ -306,7 +306,7 @@ export class FrameManager { return; if (this._page._browserContext._requestInterceptor?.(r, request)) return; - r.continue(); + r.continue({ isFallback: true }); } } diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index 122001f4c7..5717d5ef0e 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -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); diff --git a/packages/playwright-core/src/server/types.ts b/packages/playwright-core/src/server/types.ts index ccf8238938..3983037626 100644 --- a/packages/playwright-core/src/server/types.ts +++ b/packages/playwright-core/src/server/types.ts @@ -146,7 +146,7 @@ export type NormalizedContinueOverrides = { method?: string, headers?: HeadersArray, postData?: Buffer, - isFallback?: boolean, + isFallback: boolean, }; export type EmulatedSize = { viewport: Size, screen: Size }; diff --git a/packages/playwright-test/src/worker/testInfo.ts b/packages/playwright-test/src/worker/testInfo.ts index 771dac2c3d..39a5ba3906 100644 --- a/packages/playwright-test/src/worker/testInfo.ts +++ b/packages/playwright-test/src/worker/testInfo.ts @@ -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); }; @@ -352,6 +353,7 @@ export class TestInfoImpl implements TestInfo { title: `attach "${name}"`, category: 'attach', wallTime: Date.now(), + laxParent: true, }); this._attachWithoutStep(attachment); step.complete({}); diff --git a/packages/trace-viewer/src/ui/attachmentsTab.tsx b/packages/trace-viewer/src/ui/attachmentsTab.tsx index 14b066979e..55abaf7546 100644 --- a/packages/trace-viewer/src/ui/attachmentsTab.tsx +++ b/packages/trace-viewer/src/ui/attachmentsTab.tsx @@ -45,7 +45,7 @@ export const AttachmentsTab: React.FunctionComponent<{ {screenshots.size ?
Screenshots
: undefined} {[...screenshots].map((a, i) => { return
-
+
{a.name}
; })} diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts index e895e4fd05..7b6b0e9926 100644 --- a/packages/trace-viewer/src/ui/modelUtil.ts +++ b/packages/trace-viewer/src/ui/modelUtil.ts @@ -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 }); } } diff --git a/packages/trace-viewer/src/ui/networkResourceDetails.tsx b/packages/trace-viewer/src/ui/networkResourceDetails.tsx index 0e7fba8f6c..a5e8f48f2e 100644 --- a/packages/trace-viewer/src/ui/networkResourceDetails.tsx +++ b/packages/trace-viewer/src/ui/networkResourceDetails.tsx @@ -103,7 +103,7 @@ export const NetworkResourceDetails: React.FunctionComponent<{ {resource.request.postData ?
{formatBody(requestBody, requestContentType)}
: ''}
Response Body
{!resource.response.content._sha1 ?
Response body is not available for this request.
: ''} - {responseBody !== null && responseBody.dataUrl ? : ''} + {responseBody !== null && responseBody.dataUrl ? : ''} {responseBody !== null && responseBody.text ?
{formatBody(responseBody.text, resource.response.content.mimeType)}
: ''} diff --git a/packages/web/src/components/imageDiffView.tsx b/packages/web/src/components/imageDiffView.tsx index 2ef7e356a4..525338f357 100644 --- a/packages/web/src/components/imageDiffView.tsx +++ b/packages/web/src/components/imageDiffView.tsx @@ -161,7 +161,7 @@ const ImageWithSize: React.FunctionComponent<{ x { size ? size.height : ''} - { + { onLoad?.(); if (ref.current) setSize({ width: ref.current.naturalWidth, height: ref.current.naturalHeight }); diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index f331314959..ff748852ee 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -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 }) => { @@ -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 }) => { diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts index fa919d5dc7..9ead20c58b 100644 --- a/tests/playwright-test/ui-mode-trace.spec.ts +++ b/tests/playwright-test/ui-mode-trace.spec.ts @@ -91,7 +91,6 @@ 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' @@ -99,7 +98,6 @@ test('should merge screenshot assertions', async ({ runUITest }, testInfo) => { /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/,