chore: fix ui mode to show screenshots (#26563)

packages/trace-viewer/src/sw.ts

@@ -80,12 +80,11 @@ async function doFetch(event: FetchEvent): Promise<Response> {
       return new Response(null, { status: 200 });
     }
 
-    const traceUrl = url.searchParams.get('trace')!;
+    const traceUrl = url.searchParams.get('trace');
-    const { snapshotServer } = loadedTraces.get(traceUrl) || {};
 
     if (relativePath === '/contexts') {
       try {
-        const traceModel = await loadTrace(traceUrl, url.searchParams.get('traceFileName'), event.clientId, (done: number, total: number) => {
+        const traceModel = await loadTrace(traceUrl!, url.searchParams.get('traceFileName'), event.clientId, (done: number, total: number) => {
           client.postMessage({ method: 'progress', params: { done, total } });
         });
         return new Response(JSON.stringify(traceModel!.contextEntries), {
@@ -101,12 +100,14 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     }
 
     if (relativePath.startsWith('/snapshotInfo/')) {
+      const { snapshotServer } = loadedTraces.get(traceUrl!) || {};
       if (!snapshotServer)
         return new Response(null, { status: 404 });
       return snapshotServer.serveSnapshotInfo(relativePath, url.searchParams);
     }
 
     if (relativePath.startsWith('/snapshot/')) {
+      const { snapshotServer } = loadedTraces.get(traceUrl!) || {};
       if (!snapshotServer)
         return new Response(null, { status: 404 });
       const response = snapshotServer.serveSnapshot(relativePath, url.searchParams, url.href);
@@ -116,13 +117,13 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     }
 
     if (relativePath.startsWith('/sha1/')) {
+      const download = url.searchParams.has('download');
       // Sha1 for sources is based on the file path, can't load it of a random model.
-      const traceUrls = clientIdToTraceUrls.get(event.clientId);
+      const sha1 = relativePath.slice('/sha1/'.length);
-      for (const [trace, { traceModel }] of loadedTraces) {
+      for (const trace of loadedTraces.values()) {
-        // We will accept explicit ?trace= value as well as the clientId associated with the trace.
+        const blob = await trace.traceModel.resourceForSha1(sha1);
-        if (traceUrl !== trace && !traceUrls.includes(trace))
+        if (blob)
-          continue;
+          return new Response(blob, { status: 200, headers: download ? downloadHeadersForAttachment(trace.traceModel, sha1) : undefined });
-        return await serveResource(traceModel, relativePath.slice('/sha1/'.length));
       }
       return new Response(null, { status: 404 });
     }
@@ -143,14 +144,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
   return snapshotServer.serveResource(lookupUrls, request.method, snapshotUrl);
 }
 
-async function serveResource(traceModel: TraceModel, sha1: string): Promise<Response> {
+function downloadHeadersForAttachment(traceModel: TraceModel, sha1: string): Headers | undefined {
-  const blob = await traceModel!.resourceForSha1(sha1);
-  if (blob)
-    return new Response(blob, { status: 200, headers: headersForResource(traceModel, sha1) });
-  return new Response(null, { status: 404 });
-}
-
-function headersForResource(traceModel: TraceModel, sha1: string): Headers | undefined {
   const attachment = traceModel.attachmentForSha1(sha1);
   if (!attachment)
     return;

packages/trace-viewer/src/ui/attachmentsTab.tsx

@@ -54,15 +54,16 @@ export const AttachmentsSection: React.FunctionComponent<{
     }} />}
     {screenshots.size ? <div className='attachments-section'>Screenshots</div> : undefined}
     {[...screenshots].map((a, i) => {
+      const url = attachmentURL(traceUrl, a);
       return <div className='attachment-item' key={`screenshot-${i}`}>
-        <div><img draggable='false' src={attachmentURL(traceUrl, a)} /></div>
+        <div><img draggable='false' src={url} /></div>
-        <div><a href={attachmentURL(traceUrl, a)}>{a.name}</a></div>
+        <div><a target='_blank' href={url}>{a.name}</a></div>
       </div>;
     })}
     {otherAttachments.size ? <div className='attachments-section'>Attachments</div> : undefined}
     {[...otherAttachments].map((a, i) => {
       return <div className='attachment-item' key={`attachment-${i}`}>
-        <a href={attachmentURL(traceUrl, a)}>{a.name}</a>
+        <a href={attachmentURL(traceUrl, a) + '&download'}>{a.name}</a>
       </div>;
     })}
   </>;

packages/trace-viewer/src/ui/workbench.tsx

@@ -54,6 +54,10 @@ export const Workbench: React.FunctionComponent<{
 
   const sources = React.useMemo(() => model?.sources || new Map(), [model]);
 
+  React.useEffect(() => {
+    setSelectedTime(undefined);
+  }, [model]);
+
   React.useEffect(() => {
     if (selectedAction && model?.actions.includes(selectedAction))
       return;

packages/web/src/common.css

@@ -51,6 +51,10 @@ body {
   -webkit-font-smoothing: antialiased;
 }
 
+a {
+  color: var(--vscode-textLink-foreground);
+}
+
 * {
   box-sizing: border-box;
   min-width: 0;

tests/playwright-test/ui-mode-test-attachments.spec.ts

@@ -14,12 +14,11 @@
  * limitations under the License.
  */
 
-import fs from 'fs';
 import { test, expect, retries } from './ui-mode-fixtures';
 
 test.describe.configure({ mode: 'parallel', retries });
 
-test('should contain file attachment', async ({ runUITest }) => {
+test('should contain text attachment', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
       import { test } from '@playwright/test';
@@ -40,6 +39,27 @@ test('should contain file attachment', async ({ runUITest }) => {
   expect((await readAllFromStream(await download.createReadStream())).toString()).toContain('attach test');
 });
 
+test('should contain binary attachment', async ({ runUITest }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test } from '@playwright/test';
+      test('attach test', async () => {
+        await test.info().attach('data', { body: Buffer.from([1, 2, 3]), contentType: 'application/octet-stream' });
+      });
+    `,
+  });
+  await page.getByText('attach test').click();
+  await page.getByTitle('Run all').click();
+  await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
+  await page.getByText('Attachments').click();
+  await page.getByText('attach "data"', { exact: true }).click();
+  const downloadPromise = page.waitForEvent('download');
+  await page.getByRole('link', { name: 'data' }).click();
+  const download = await downloadPromise;
+  expect(download.suggestedFilename()).toBe('data');
+  expect(await readAllFromStream(await download.createReadStream())).toEqual(Buffer.from([1, 2, 3]));
+});
+
 test('should contain string attachment', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
@@ -61,27 +81,6 @@ test('should contain string attachment', async ({ runUITest }) => {
   expect((await readAllFromStream(await download.createReadStream())).toString()).toEqual('text42');
 });
 
-test('should contain attachment with filename and extension', async ({ runUITest, asset }) => {
-  const { page } = await runUITest({
-    'a.test.ts': `
-      import { test } from '@playwright/test';
-      test('attach test', async () => {
-        await test.info().attach('screenshot.png', { path: ${JSON.stringify(asset('pptr.png'))} });
-      });
-    `,
-  });
-  await page.getByText('attach test').click();
-  await page.getByTitle('Run all').click();
-  await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
-  await page.getByText('Attachments').click();
-  await page.getByText('attach "screenshot.png"', { exact: true }).click();
-  const downloadPromise = page.waitForEvent('download');
-  await page.getByRole('link', { name: 'screenshot.png' }).click();
-  const download = await downloadPromise;
-  expect(download.suggestedFilename()).toBe('screenshot.png');
-  expect(await readAllFromStream(await download.createReadStream())).toEqual(fs.readFileSync(asset('pptr.png')));
-});
-
 function readAllFromStream(stream: NodeJS.ReadableStream): Promise<Buffer> {
   return new Promise(resolve => {
     const chunks: Buffer[] = [];

tests/playwright-test/ui-mode-test-screencast.spec.ts

@@ -0,0 +1,49 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { test, expect, retries } from './ui-mode-fixtures';
+
+test.describe.configure({ mode: 'parallel', retries });
+
+test('should show screenshots', async ({ runUITest }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test } from '@playwright/test';
+      test('test 1', async ({ page }) => {
+        await page.setContent('<div style="background: red; width: 100%; height: 100%"></div>');
+        await page.waitForTimeout(1000);
+      });
+      test('test 2', async ({ page }) => {
+        await page.setContent('<div style="background: blue; width: 100%; height: 100%"></div>');
+        await page.waitForTimeout(1000);
+      });
+    `,
+  });
+  await page.getByTitle('Run all').click();
+  await expect(page.getByTestId('status-line')).toHaveText('2/2 passed (100%)');
+
+  await page.getByText('test 1', { exact: true }).click();
+  await expect(
+      page.locator('.CodeMirror .source-line-running'),
+  ).toContainText(`test('test 1', async ({ page }) => {`);
+  await expect(page.locator('.film-strip-frame')).toBeVisible();
+
+  await page.getByText('test 2', { exact: true }).click();
+  await expect(
+      page.locator('.CodeMirror .source-line-running'),
+  ).toContainText(`test('test 2', async ({ page }) => {`);
+  await expect(page.locator('.film-strip-frame')).toBeVisible();
+});