From 45606864276c98a3ba9c2a5c69c1b9b80de55e0a Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Mon, 4 Nov 2024 16:18:36 +0100 Subject: [PATCH 1/3] test: re-add headless-new fixme's (#33426) --- tests/library/emulation-focus.spec.ts | 3 +-- tests/library/screenshot.spec.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/library/emulation-focus.spec.ts b/tests/library/emulation-focus.spec.ts index e9a0480a67..5bbef6b164 100644 --- a/tests/library/emulation-focus.spec.ts +++ b/tests/library/emulation-focus.spec.ts @@ -104,8 +104,7 @@ it('should change document.activeElement', async ({ page, server }) => { it('should not affect screenshots', async ({ page, server, browserName, headless, isWindows, channel }) => { it.skip(browserName === 'webkit' && isWindows && !headless, 'WebKit/Windows/headed has a larger minimal viewport. See https://github.com/microsoft/playwright/issues/22616'); it.skip(browserName === 'firefox' && !headless, 'Firefox headed produces a different image'); - // TODO: We want to see test results - // it.fixme(browserName === 'chromium' && channel !== 'chromium-headless-shell', 'https://github.com/microsoft/playwright/issues/33330'); + it.fixme(browserName === 'chromium' && channel !== 'chromium-headless-shell', 'https://github.com/microsoft/playwright/issues/33330'); const page2 = await page.context().newPage(); await Promise.all([ diff --git a/tests/library/screenshot.spec.ts b/tests/library/screenshot.spec.ts index 3e0b1921fc..2f8e516963 100644 --- a/tests/library/screenshot.spec.ts +++ b/tests/library/screenshot.spec.ts @@ -23,8 +23,7 @@ browserTest.describe('page screenshot', () => { browserTest.skip(({ browserName, headless }) => browserName === 'firefox' && !headless, 'Firefox headed produces a different image.'); browserTest('should run in parallel in multiple pages', async ({ server, contextFactory, browserName, channel }) => { - // TODO: We want to see test results - // browserTest.fixme(browserName === 'chromium' && channel !== 'chromium-headless-shell', 'https://github.com/microsoft/playwright/issues/33330'); + browserTest.fixme(browserName === 'chromium' && channel !== 'chromium-headless-shell', 'https://github.com/microsoft/playwright/issues/33330'); const context = await contextFactory(); const N = 5; const pages = await Promise.all(Array(N).fill(0).map(async () => { From edf1eb154dfc1f2f6080438a3af3cc5ecccd37bb Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 4 Nov 2024 16:25:44 +0100 Subject: [PATCH 2/3] chore(trace viewer): always format trace location as URL, not file path (#33344) --- .../src/server/trace/viewer/traceViewer.ts | 13 ++++++++-- .../trace-viewer/src/sw/traceModelBackends.ts | 24 +++++++------------ .../src/ui/embeddedWorkbenchLoader.tsx | 11 ++++++++- .../trace-viewer/src/ui/uiModeTraceView.tsx | 22 +++++++++++++---- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index b8dc3e5314..cc404c18ba 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -106,8 +106,17 @@ export async function installRootRedirect(server: HttpServer, traceUrls: string[ const params = new URLSearchParams(); if (path.sep !== path.posix.sep) params.set('pathSeparator', path.sep); - for (const traceUrl of traceUrls) - params.append('trace', traceUrl); + for (const traceUrl of traceUrls) { + if (traceUrl.startsWith('http://') || traceUrl.startsWith('https://')) { + params.append('trace', traceUrl); + continue; + } + + // /trace/file?path=/path/to/trace.zip + const url = new URL('/trace/file', server.urlPrefix('precise')); + url.searchParams.set('path', traceUrl); + params.append('trace', url.toString()); + } if (server.wsGuid()) params.append('ws', server.wsGuid()!); if (options?.isServer) diff --git a/packages/trace-viewer/src/sw/traceModelBackends.ts b/packages/trace-viewer/src/sw/traceModelBackends.ts index 19c5fc2dee..1318b298a7 100644 --- a/packages/trace-viewer/src/sw/traceModelBackends.ts +++ b/packages/trace-viewer/src/sw/traceModelBackends.ts @@ -30,9 +30,8 @@ export class ZipTraceModelBackend implements TraceModelBackend { constructor(traceURL: string, progress: Progress) { this._traceURL = traceURL; - zipjs.configure({ baseURL: self.location.href } as any); this._zipReader = new zipjs.ZipReader( - new zipjs.HttpReader(formatUrl(traceURL), { mode: 'cors', preventHeadRequest: true } as any), + new zipjs.HttpReader(traceURL, { mode: 'cors', preventHeadRequest: true } as any), { useWebWorkers: false }); this._entriesPromise = this._zipReader.getEntries({ onprogress: progress }).then(entries => { const map = new Map(); @@ -87,8 +86,8 @@ export class FetchTraceModelBackend implements TraceModelBackend { constructor(traceURL: string) { this._traceURL = traceURL; - this._entriesPromise = fetch('/trace/file?path=' + encodeURIComponent(traceURL)).then(async response => { - const json = JSON.parse(await response.text()); + this._entriesPromise = fetch(traceURL).then(async response => { + const json = await response.json(); const entries = new Map(); for (const entry of json.entries) entries.set(entry.name, entry.path); @@ -126,17 +125,12 @@ export class FetchTraceModelBackend implements TraceModelBackend { private async _readEntry(entryName: string): Promise { const entries = await this._entriesPromise; - const fileName = entries.get(entryName); - if (!fileName) + const filePath = entries.get(entryName); + if (!filePath) return; - return fetch('/trace/file?path=' + encodeURIComponent(fileName)); + + const url = new URL(this.traceURL()); + url.searchParams.set('path', filePath); + return fetch(url); } } - -function formatUrl(trace: string) { - let url = trace.startsWith('http') || trace.startsWith('blob') ? trace : `file?path=${encodeURIComponent(trace)}`; - // Dropbox does not support cors. - if (url.startsWith('https://www.dropbox.com/')) - url = 'https://dl.dropboxusercontent.com/' + url.substring('https://www.dropbox.com/'.length); - return url; -} diff --git a/packages/trace-viewer/src/ui/embeddedWorkbenchLoader.tsx b/packages/trace-viewer/src/ui/embeddedWorkbenchLoader.tsx index c8b8aa216c..23801f95da 100644 --- a/packages/trace-viewer/src/ui/embeddedWorkbenchLoader.tsx +++ b/packages/trace-viewer/src/ui/embeddedWorkbenchLoader.tsx @@ -21,6 +21,7 @@ import './embeddedWorkbenchLoader.css'; import { Workbench } from './workbench'; import { currentTheme, toggleTheme } from '@web/theme'; import type { SourceLocation } from './modelUtil'; +import { filePathToTraceURL } from './uiModeTraceView'; function openPage(url: string, target?: string) { if (url) @@ -40,7 +41,15 @@ export const EmbeddedWorkbenchLoader: React.FunctionComponent = () => { React.useEffect(() => { window.addEventListener('message', async ({ data: { method, params } }) => { if (method === 'loadTraceRequested') { - setTraceURLs(params.traceUrl ? [params.traceUrl] : []); + if (params.traceUrl) { + // the param is called URL, but VS Code sends a path + const url = params.traceUrl.startsWith('http') + ? params.traceUrl + : filePathToTraceURL(params.traceUrl).toString(); + setTraceURLs([url]); + } else { + setTraceURLs([]); + } setProcessingErrorMessage(null); } else if (method === 'applyTheme') { if (currentTheme() !== params.theme) diff --git a/packages/trace-viewer/src/ui/uiModeTraceView.tsx b/packages/trace-viewer/src/ui/uiModeTraceView.tsx index cf35d89007..8e98a8ccd7 100644 --- a/packages/trace-viewer/src/ui/uiModeTraceView.tsx +++ b/packages/trace-viewer/src/ui/uiModeTraceView.tsx @@ -54,7 +54,7 @@ export const TraceView: React.FC<{ // Test finished. const attachment = result && result.duration >= 0 && result.attachments.find(a => a.name === 'trace'); if (attachment && attachment.path) { - loadSingleTraceFile(attachment.path).then(model => setModel({ model, isLive: false })); + loadSingleTraceFile(filePathToTraceURL(attachment.path)).then(model => setModel({ model, isLive: false })); return; } @@ -72,7 +72,7 @@ export const TraceView: React.FC<{ // Start polling running test. pollTimer.current = setTimeout(async () => { try { - const model = await loadSingleTraceFile(traceLocation); + const model = await loadSingleTraceFile(filePathToTraceURL(traceLocation)); setModel({ model, isLive: true }); } catch { setModel(undefined); @@ -108,11 +108,25 @@ const outputDirForTestCase = (testCase: reporterTypes.TestCase): string | undefi return undefined; }; -async function loadSingleTraceFile(url: string): Promise { +async function loadSingleTraceFile(traceURL: URL): Promise { const params = new URLSearchParams(); - params.set('trace', url); + params.set('trace', formatUrl(traceURL).toString()); params.set('limit', '1'); const response = await fetch(`contexts?${params.toString()}`); const contextEntries = await response.json() as ContextEntry[]; return new MultiTraceModel(contextEntries); } + +function formatUrl(traceURL: URL) { + // Dropbox does not support cors. + if (traceURL.hostname === 'dropbox.com') + traceURL.hostname = 'dl.dropboxusercontent.com'; + + return traceURL; +} + +export function filePathToTraceURL(path: string) { + const url = new URL('file', location.href); + url.searchParams.set('path', path); + return url; +} From cb0171e5715ec96d35794d205ff0520e6b0887bd Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 4 Nov 2024 16:58:20 +0100 Subject: [PATCH 3/3] chore(expect): clarify message() for custom matchers (#33321) --- docs/src/test-assertions-js.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/test-assertions-js.md b/docs/src/test-assertions-js.md index defbc3a293..162d0351eb 100644 --- a/docs/src/test-assertions-js.md +++ b/docs/src/test-assertions-js.md @@ -254,7 +254,7 @@ Note that by default `toPass` has timeout 0 and does not respect custom [expect You can extend Playwright assertions by providing custom matchers. These matchers will be available on the `expect` object. -In this example we add a custom `toHaveAmount` function. Custom matcher should return a `message` callback and a `pass` flag indicating whether the assertion passed. +In this example we add a custom `toHaveAmount` function. Custom matcher should return a `pass` flag indicating whether the assertion passed, and a `message` callback that's used when the assertion fails. ```js title="fixtures.ts" import { expect as baseExpect } from '@playwright/test'; @@ -279,7 +279,7 @@ export const expect = baseExpect.extend({ ? () => this.utils.matcherHint(assertionName, undefined, undefined, { isNot: this.isNot }) + '\n\n' + `Locator: ${locator}\n` + - `Expected: ${this.isNot ? 'not' : ''}${this.utils.printExpected(expected)}\n` + + `Expected: not ${this.utils.printExpected(expected)}\n` + (matcherResult ? `Received: ${this.utils.printReceived(matcherResult.actual)}` : '') : () => this.utils.matcherHint(assertionName, undefined, undefined, { isNot: this.isNot }) + '\n\n' +