From cf6549687cd682ce87d022dfd3d66b426c97a5f9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 1 Feb 2024 08:23:07 -0800 Subject: [PATCH] fix(trace viewer): reveal error location when it comes from the test (#29268) Errors coming from the test runner do not have an associated `action`, but have a `stack` that we can reveal. --- packages/trace-viewer/src/ui/errorsTab.tsx | 4 ++-- packages/trace-viewer/src/ui/sourceTab.tsx | 20 ++++++++-------- packages/trace-viewer/src/ui/stackTrace.tsx | 7 +++--- packages/trace-viewer/src/ui/workbench.tsx | 18 ++++++++++---- tests/playwright-test/ui-mode-trace.spec.ts | 26 +++++++++++++++++++++ 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/packages/trace-viewer/src/ui/errorsTab.tsx b/packages/trace-viewer/src/ui/errorsTab.tsx index e600bb4be2..05b99e59b6 100644 --- a/packages/trace-viewer/src/ui/errorsTab.tsx +++ b/packages/trace-viewer/src/ui/errorsTab.tsx @@ -45,7 +45,7 @@ export function useErrorsTabModel(model: modelUtil.MultiTraceModel | undefined): export const ErrorsTab: React.FunctionComponent<{ errorsModel: ErrorsTabModel, sdkLanguage: Language, - revealInSource: (action: modelUtil.ActionTraceEventInContext) => void, + revealInSource: (error: ErrorDescription) => void, }> = ({ errorsModel, sdkLanguage, revealInSource }) => { if (!errorsModel.errors.size) return ; @@ -70,7 +70,7 @@ export const ErrorsTab: React.FunctionComponent<{ }}> {error.action && renderAction(error.action, { sdkLanguage })} {location &&
- @ error.action && revealInSource(error.action)}>{location} + @ revealInSource(error)}>{location}
} diff --git a/packages/trace-viewer/src/ui/sourceTab.tsx b/packages/trace-viewer/src/ui/sourceTab.tsx index 9e26bf13a9..aebbbd872f 100644 --- a/packages/trace-viewer/src/ui/sourceTab.tsx +++ b/packages/trace-viewer/src/ui/sourceTab.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import type { ActionTraceEvent } from '@trace/trace'; import { SplitView } from '@web/components/splitView'; import * as React from 'react'; import { useAsyncMemo } from '@web/uiUtils'; @@ -23,26 +22,27 @@ import { StackTraceView } from './stackTrace'; import { CodeMirrorWrapper } from '@web/components/codeMirrorWrapper'; import type { SourceHighlight } from '@web/components/codeMirrorWrapper'; import type { SourceLocation, SourceModel } from './modelUtil'; +import type { StackFrame } from '@protocol/channels'; export const SourceTab: React.FunctionComponent<{ - action: ActionTraceEvent | undefined, + stack: StackFrame[] | undefined, sources: Map, hideStackFrames?: boolean, rootDir?: string, fallbackLocation?: SourceLocation, -}> = ({ action, sources, hideStackFrames, rootDir, fallbackLocation }) => { - const [lastAction, setLastAction] = React.useState(); +}> = ({ stack, sources, hideStackFrames, rootDir, fallbackLocation }) => { + const [lastStack, setLastStack] = React.useState(); const [selectedFrame, setSelectedFrame] = React.useState(0); React.useEffect(() => { - if (lastAction !== action) { - setLastAction(action); + if (lastStack !== stack) { + setLastStack(stack); setSelectedFrame(0); } - }, [action, lastAction, setLastAction, setSelectedFrame]); + }, [stack, lastStack, setLastStack, setSelectedFrame]); const { source, highlight, targetLine, fileName } = useAsyncMemo<{ source: SourceModel, targetLine?: number, fileName?: string, highlight: SourceHighlight[] }>(async () => { - const actionLocation = action?.stack?.[selectedFrame]; + const actionLocation = stack?.[selectedFrame]; const shouldUseFallback = !actionLocation?.file; if (shouldUseFallback && !fallbackLocation) return { source: { file: '', errors: [], content: undefined }, targetLine: 0, highlight: [] }; @@ -76,14 +76,14 @@ export const SourceTab: React.FunctionComponent<{ } } return { source, highlight, targetLine, fileName }; - }, [action, selectedFrame, rootDir, fallbackLocation], { source: { errors: [], content: 'Loading\u2026' }, highlight: [] }); + }, [stack, selectedFrame, rootDir, fallbackLocation], { source: { errors: [], content: 'Loading\u2026' }, highlight: [] }); return
{fileName &&
{fileName}
}
- + ; }; diff --git a/packages/trace-viewer/src/ui/stackTrace.tsx b/packages/trace-viewer/src/ui/stackTrace.tsx index 4e35946d84..6a0bad1672 100644 --- a/packages/trace-viewer/src/ui/stackTrace.tsx +++ b/packages/trace-viewer/src/ui/stackTrace.tsx @@ -16,18 +16,17 @@ import * as React from 'react'; import './stackTrace.css'; -import type { ActionTraceEvent } from '@trace/trace'; import { ListView } from '@web/components/listView'; import type { StackFrame } from '@protocol/channels'; const StackFrameListView = ListView; export const StackTraceView: React.FunctionComponent<{ - action: ActionTraceEvent | undefined, + stack: StackFrame[] | undefined, selectedFrame: number, setSelectedFrame: (index: number) => void -}> = ({ action, setSelectedFrame, selectedFrame }) => { - const frames = action?.stack || []; +}> = ({ stack, setSelectedFrame, selectedFrame }) => { + const frames = stack || []; return = ({ model, hideStackFrames, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status }) => { - const [selectedAction, setSelectedAction] = React.useState(undefined); + const [selectedAction, setSelectedActionImpl] = React.useState(undefined); + const [revealedStack, setRevealedStack] = React.useState(undefined); const [highlightedAction, setHighlightedAction] = React.useState(); const [highlightedEntry, setHighlightedEntry] = React.useState(); const [selectedNavigatorTab, setSelectedNavigatorTab] = React.useState('actions'); @@ -62,6 +64,11 @@ export const Workbench: React.FunctionComponent<{ const [selectedTime, setSelectedTime] = React.useState(); const [sidebarLocation, setSidebarLocation] = useSetting<'bottom' | 'right'>('propertiesSidebarLocation', 'bottom'); + const setSelectedAction = React.useCallback((action: ActionTraceEventInContext | undefined) => { + setSelectedActionImpl(action); + setRevealedStack(action?.stack); + }, [setSelectedActionImpl, setRevealedStack]); + const sources = React.useMemo(() => model?.sources || new Map(), [model]); React.useEffect(() => { @@ -137,8 +144,11 @@ export const Workbench: React.FunctionComponent<{ id: 'errors', title: 'Errors', errorCount: errorsModel.errors.size, - render: () => { - setSelectedAction(action); + render: () => { + if (error.action) + setSelectedAction(error.action); + else + setRevealedStack(error.stack); selectPropertiesTab('source'); }} /> }; @@ -146,7 +156,7 @@ export const Workbench: React.FunctionComponent<{ id: 'source', title: 'Source', render: () => { + const { page } = await runUITest({ + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('pass', async ({ page }) => { + throw new Error('Oh my'); + }); + `, + }); + + await page.getByText('pass').dblclick(); + const listItem = page.getByTestId('actions-tree').getByRole('listitem'); + + await expect( + listItem, + 'action list' + ).toContainText([ + /Before Hooks/, + /After Hooks/, + ]); + + await page.getByText('Errors', { exact: true }).click(); + await page.getByText('a.spec.ts:4', { exact: true }).click(); + await expect(page.locator('.source-line-running')).toContainText(`throw new Error('Oh my');`); +});