fix(step.skip): show a skipped indicator in UI mode (#34407)

packages/playwright/src/worker/testInfo.ts

@@ -322,7 +322,7 @@ export class TestInfoImpl implements TestInfo {
       location: data.location,
     };
     this._onStepBegin(payload);
-    this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.apiName || data.title, data.params, data.location ? [data.location] : []);
+    this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.category, data.apiName || data.title, data.params, data.location ? [data.location] : []);
     return step;
   }
 
@@ -421,7 +421,7 @@ export class TestInfoImpl implements TestInfo {
     } else {
       // trace viewer has no means of representing attachments outside of a step, so we create an artificial action
       const callId = `attach@${++this._lastStepId}`;
-      this._tracing.appendBeforeActionForStep(callId, this._findLastStageStep(this._steps)?.stepId, `attach "${attachment.name}"`, undefined, []);
+      this._tracing.appendBeforeActionForStep(callId, this._findLastStageStep(this._steps)?.stepId, 'attach', `attach "${attachment.name}"`, undefined, []);
       this._tracing.appendAfterActionForStep(callId, undefined, [attachment]);
     }
 

packages/playwright/src/worker/testTracing.ts

@@ -245,14 +245,14 @@ export class TestTracing {
     });
   }
 
-  appendBeforeActionForStep(callId: string, parentId: string | undefined, apiName: string, params: Record<string, any> | undefined, stack: StackFrame[]) {
+  appendBeforeActionForStep(callId: string, parentId: string | undefined, category: string, apiName: string, params: Record<string, any> | undefined, stack: StackFrame[]) {
     this._appendTraceEvent({
       type: 'before',
       callId,
       parentId,
       startTime: monotonicTime(),
       class: 'Test',
-      method: 'step',
+      method: category,
       apiName,
       params: Object.fromEntries(Object.entries(params || {}).map(([name, value]) => [name, generatePreview(value)])),
       stack,

packages/trace-viewer/src/ui/actionList.css

@@ -41,6 +41,10 @@
   color: var(--vscode-editorCodeLens-foreground);
 }
 
+.action-skipped {
+  margin-right: 4px;
+}
+
 .action-icon {
   flex: none;
   display: flex;

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

@@ -15,7 +15,7 @@
 */
 
 import type { ActionTraceEvent, AfterActionTraceEventAttachment } from '@trace/trace';
-import { msToString } from '@web/uiUtils';
+import { clsx, msToString } from '@web/uiUtils';
 import * as React from 'react';
 import './actionList.css';
 import * as modelUtil from './modelUtil';
@@ -25,6 +25,7 @@ import { TreeView } from '@web/components/treeView';
 import type { ActionTraceEventInContext, ActionTreeItem } from './modelUtil';
 import type { Boundaries } from './geometry';
 import { ToolbarButton } from '@web/components/toolbarButton';
+import { testStatusIcon } from './testUtils';
 
 export interface ActionListProps {
   actions: ActionTraceEventInContext[],
@@ -119,6 +120,7 @@ export const renderAction = (
 
   const parameterString = actionParameterDisplayString(action, sdkLanguage || 'javascript');
 
+  const isSkipped = action.class === 'Test' && action.method === 'test.step.skip';
   let time: string = '';
   if (action.endTime)
     time = msToString(action.endTime - action.startTime);
@@ -149,9 +151,10 @@ export const renderAction = (
       {action.method === 'goto' && action.params.url && <div className='action-url' title={action.params.url}>{action.params.url}</div>}
       {action.class === 'APIRequestContext' && action.params.url && <div className='action-url' title={action.params.url}>{excludeOrigin(action.params.url)}</div>}
     </div>
-    {(showDuration || showBadges || showAttachments) && <div className='spacer'></div>}
+    {(showDuration || showBadges || showAttachments || isSkipped) && <div className='spacer'></div>}
     {showAttachments && <ToolbarButton icon='attach' title='Open Attachment' onClick={() => revealAttachment(action.attachments![0])} />}
-    {showDuration && <div className='action-duration'>{time || <span className='codicon codicon-loading'></span>}</div>}
+    {showDuration && !isSkipped && <div className='action-duration'>{time || <span className='codicon codicon-loading'></span>}</div>}
+    {isSkipped && <span className={clsx('action-skipped', 'codicon', testStatusIcon('skipped'))} title='skipped'></span>}
     {showBadges && <div className='action-icons' onClick={() => revealConsole?.()}>
       {!!errors && <div className='action-icon'><span className='codicon codicon-error'></span><span className='action-icon-value'>{errors}</span></div>}
       {!!warnings && <div className='action-icon'><span className='codicon codicon-warning'></span><span className='action-icon-value'>{warnings}</span></div>}

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

@@ -398,7 +398,7 @@ test('should show custom fixture titles in actions tree', async ({ runUITest })
   const { page } = await runUITest({
     'a.test.ts': `
       import { test as base, expect } from '@playwright/test';
-      
+
       const test = base.extend({
         fixture1: [async ({}, use) => {
           await use();
@@ -457,7 +457,7 @@ test('attachments tab shows all but top-level .push attachments', async ({ runUI
     - tree:
       - treeitem /step/:
         - group:
-          - treeitem /attach \\"foo-attach\\"/ 
+          - treeitem /attach \\"foo-attach\\"/
       - treeitem /attach \\"bar-push\\"/
       - treeitem /attach \\"bar-attach\\"/
   `);
@@ -470,3 +470,32 @@ test('attachments tab shows all but top-level .push attachments', async ({ runUI
       - button /bar-attach/
   `);
 });
+
+test('skipped steps should have an indicator', async ({ runUITest }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('test with steps', async ({}) => {
+        await test.step('outer', async () => {
+          await test.step.skip('skipped1', () => {});
+        });
+        await test.step.skip('skipped2', () => {});
+      });
+    `,
+  });
+
+  await page.getByRole('treeitem', { name: 'test with steps' }).dblclick();
+  const actionsTree = page.getByTestId('actions-tree');
+  await actionsTree.getByRole('treeitem', { name: 'outer' }).click();
+  await page.keyboard.press('ArrowRight');
+  await expect(actionsTree).toMatchAriaSnapshot(`
+    - tree:
+      - treeitem /outer/ [expanded]:
+        - group:
+          - treeitem /skipped1/
+      - treeitem /skipped2/
+  `);
+  const skippedMarker = actionsTree.getByRole('treeitem', { name: 'skipped1' }).locator('.action-skipped');
+  await expect(skippedMarker).toBeVisible();
+  await expect(skippedMarker).toHaveAccessibleName('skipped');
+});