Skip to content

Commit

Permalink
feat(step.stack): expose step.stack for better current line in IDE
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Jan 30, 2025
1 parent eff5cd6 commit 78a84a2
Show file tree
Hide file tree
Showing 17 changed files with 170 additions and 96 deletions.
6 changes: 6 additions & 0 deletions docs/src/test-reporter-api/class-teststep.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ Error thrown during the step execution, if any.

Parent step, if any.

## property: TestStep.stack
* since: v1.51
- type: <[Array]<[Location]>>

Call stack for this step.

## property: TestStep.startTime
* since: v1.10
- type: <[Date]>
Expand Down
9 changes: 7 additions & 2 deletions packages/playwright-core/src/utils/timeoutRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,19 @@
import { monotonicTime } from './';

export async function raceAgainstDeadline<T>(cb: () => Promise<T>, deadline: number): Promise<{ result: T, timedOut: false } | { timedOut: true }> {
// Avoid indirections to preserve better stacks.
if (deadline === 0) {
const result = await cb();
return { result, timedOut: false };
}

let timer: NodeJS.Timeout | undefined;
return Promise.race([
cb().then(result => {
return { result, timedOut: false };
}),
new Promise<{ timedOut: true }>(resolve => {
const kMaxDeadline = 2147483647; // 2^31-1
const timeout = (deadline || kMaxDeadline) - monotonicTime();
const timeout = deadline - monotonicTime();
timer = setTimeout(() => resolve({ timedOut: true }), timeout);
}),
]).finally(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright/src/common/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export type StepBeginPayload = {
title: string;
category: string;
wallTime: number; // milliseconds since unix epoch
location?: { file: string, line: number, column: number };
stack: { file: string, line: number, column: number }[];
};

export type StepEndPayload = {
Expand Down
16 changes: 8 additions & 8 deletions packages/playwright/src/common/testType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class TestTypeImpl {
this.test = test;
}

private _currentSuite(location: Location, title: string): Suite | undefined {
private _currentSuite(title: string): Suite | undefined {
const suite = currentlyLoadingFileSuite();
if (!suite) {
throw new Error([
Expand All @@ -86,7 +86,7 @@ export class TestTypeImpl {

private _createTest(type: 'default' | 'only' | 'skip' | 'fixme' | 'fail' | 'fail.only', location: Location, title: string, fnOrDetails: Function | TestDetails, fn?: Function) {
throwIfRunningInsideJest();
const suite = this._currentSuite(location, 'test()');
const suite = this._currentSuite('test()');
if (!suite)
return;

Expand Down Expand Up @@ -117,7 +117,7 @@ export class TestTypeImpl {

private _describe(type: 'default' | 'only' | 'serial' | 'serial.only' | 'parallel' | 'parallel.only' | 'skip' | 'fixme', location: Location, titleOrFn: string | Function, fnOrDetails?: TestDetails | Function, fn?: Function) {
throwIfRunningInsideJest();
const suite = this._currentSuite(location, 'test.describe()');
const suite = this._currentSuite('test.describe()');
if (!suite)
return;

Expand Down Expand Up @@ -169,7 +169,7 @@ export class TestTypeImpl {
}

private _hook(name: 'beforeEach' | 'afterEach' | 'beforeAll' | 'afterAll', location: Location, title: string | Function, fn?: Function) {
const suite = this._currentSuite(location, `test.${name}()`);
const suite = this._currentSuite(`test.${name}()`);
if (!suite)
return;
if (typeof title === 'function') {
Expand All @@ -182,7 +182,7 @@ export class TestTypeImpl {

private _configure(location: Location, options: { mode?: 'default' | 'parallel' | 'serial', retries?: number, timeout?: number }) {
throwIfRunningInsideJest();
const suite = this._currentSuite(location, `test.describe.configure()`);
const suite = this._currentSuite(`test.describe.configure()`);
if (!suite)
return;

Expand Down Expand Up @@ -252,7 +252,7 @@ export class TestTypeImpl {
}

private _use(location: Location, fixtures: Fixtures) {
const suite = this._currentSuite(location, `test.use()`);
const suite = this._currentSuite(`test.use()`);
if (!suite)
return;
suite._use.push({ fixtures, location });
Expand All @@ -263,11 +263,11 @@ export class TestTypeImpl {
if (!testInfo)
throw new Error(`test.step() can only be called from a test`);
if (expectation === 'skip') {
const step = testInfo._addStep({ category: 'test.step.skip', title, location: options.location, box: options.box });
const step = testInfo._addStep({ category: 'test.step.skip', title, box: options.box }, undefined, options.location ? [options.location] : undefined);
step.complete({});
return undefined as T;
}
const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
const step = testInfo._addStep({ category: 'test.step', title, box: options.box }, undefined, options.location ? [options.location] : undefined);
return await zones.run('stepZone', step, async () => {
try {
let result: Awaited<ReturnType<typeof raceAgainstDeadline<T>>> | undefined = undefined;
Expand Down
3 changes: 1 addition & 2 deletions packages/playwright/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,11 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
}
// In the general case, create a step for each api call and connect them through the stepId.
const step = testInfo._addStep({
location: data.frames[0],
category: 'pw:api',
title: renderApiCall(data.apiName, data.params),
apiName: data.apiName,
params: data.params,
}, tracingGroupSteps[tracingGroupSteps.length - 1]);
}, tracingGroupSteps[tracingGroupSteps.length - 1], data.frames);
data.userData = step;
data.stepId = step.stepId;
if (data.apiName === 'tracing.group')
Expand Down
17 changes: 10 additions & 7 deletions packages/playwright/src/isomorphic/teleReceiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ export type JsonTestStepStart = {
title: string;
category: string,
startTime: number;
location?: reporterTypes.Location;
// Best effort to keep step struct small.
stack?: reporterTypes.Location | reporterTypes.Location[];
};

export type JsonTestStepEnd = {
Expand Down Expand Up @@ -249,8 +250,8 @@ export class TeleReporterReceiver {
const result = test.results.find(r => r._id === resultId)!;
const parentStep = payload.parentStepId ? result._stepMap.get(payload.parentStepId) : undefined;

const location = this._absoluteLocation(payload.location);
const step = new TeleTestStep(payload, parentStep, location, result);
const stack = Array.isArray(payload.stack) ? payload.stack.map(l => this._absoluteLocation(l)) : this._absoluteLocation(payload.stack);
const step = new TeleTestStep(payload, parentStep, stack, result);
if (parentStep)
parentStep.steps.push(step);
else
Expand Down Expand Up @@ -426,8 +427,8 @@ export class TeleSuite implements reporterTypes.Suite {
}

allTests(): reporterTypes.TestCase[] {
const result: reporterTypes.TestCase[] = [];
const visit = (suite: reporterTypes.Suite) => {
const result: TeleTestCase[] = [];
const visit = (suite: TeleSuite) => {
for (const entry of suite.entries()) {
if (entry.type === 'test')
result.push(entry);
Expand Down Expand Up @@ -511,6 +512,7 @@ class TeleTestStep implements reporterTypes.TestStep {
title: string;
category: string;
location: reporterTypes.Location | undefined;
stack: reporterTypes.Location[];
parent: reporterTypes.TestStep | undefined;
duration: number = -1;
steps: reporterTypes.TestStep[] = [];
Expand All @@ -521,10 +523,11 @@ class TeleTestStep implements reporterTypes.TestStep {

private _startTime: number = 0;

constructor(payload: JsonTestStepStart, parentStep: reporterTypes.TestStep | undefined, location: reporterTypes.Location | undefined, result: TeleTestResult) {
constructor(payload: JsonTestStepStart, parentStep: reporterTypes.TestStep | undefined, stackOrLocation: reporterTypes.Location | reporterTypes.Location[] | undefined, result: TeleTestResult) {
this.title = payload.title;
this.category = payload.category;
this.location = location;
this.stack = Array.isArray(stackOrLocation) ? stackOrLocation : (stackOrLocation ? [stackOrLocation] : []);
this.location = this.stack[0];
this.parent = parentStep;
this._startTime = payload.startTime;
this._result = result;
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright/src/isomorphic/testServerConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class WebSocketTestServerTransport implements TestServerTransport {
}

onmessage(listener: (message: string) => void) {
this._ws.addEventListener('message', event => listener(event.data));
this._ws.addEventListener('message', event => listener(String(event.data)));
}

onopen(listener: () => void) {
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright/src/reporters/blob.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type BlobReporterOptions = {
_commandHash: string;
};

export const currentBlobReportVersion = 2;
export const currentBlobReportVersion = 3;

export type BlobReportMetadata = {
version: number;
Expand Down
14 changes: 12 additions & 2 deletions packages/playwright/src/reporters/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ export async function createMergedReport(config: FullConfigInternal, dir: string
await dispatchEvents(eventData.prologue);
for (const { reportFile, eventPatchers, metadata } of eventData.reports) {
const reportJsonl = await fs.promises.readFile(reportFile);
const events = parseTestEvents(reportJsonl);
let events = parseTestEvents(reportJsonl);
events = modernizer.modernize(metadata.version, events);
new JsonStringInternalizer(stringPool).traverse(events);
eventPatchers.patchers.push(new AttachmentPathPatcher(dir));
if (metadata.name)
Expand Down Expand Up @@ -480,7 +481,8 @@ class PathSeparatorPatcher {
}
if (jsonEvent.method === 'onStepBegin') {
const step = jsonEvent.params.step as JsonTestStepStart;
this._updateLocation(step.location);
for (const stackFrame of Array.isArray(step.stack) ? step.stack : [step.stack])
this._updateLocation(stackFrame);
return;
}
if (jsonEvent.method === 'onStepEnd') {
Expand Down Expand Up @@ -589,6 +591,14 @@ class BlobModernizer {
return event;
});
}

_modernize_2_to_3(events: JsonEvent[]): JsonEvent[] {
return events.map(event => {
if (event.method === 'onStepBegin')
(event.params.step as JsonTestStepStart).stack = event.params.step.location;
return event;
});
}
}

const modernizer = new BlobModernizer();
10 changes: 9 additions & 1 deletion packages/playwright/src/reporters/teleEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export class TeleReporterEmitter implements ReporterV2 {
title: step.title,
category: step.category,
startTime: +step.startTime,
location: this._relativeLocation(step.location),
stack: this._relativeStack(step.stack),
};
}

Expand All @@ -260,6 +260,14 @@ export class TeleReporterEmitter implements ReporterV2 {
};
}

private _relativeStack(stack: reporterTypes.Location[]): undefined | reporterTypes.Location | reporterTypes.Location[] {
if (!stack.length)
return undefined;
if (stack.length === 1)
return this._relativeLocation(stack[0]);
return stack.map(frame => this._relativeLocation(frame));
}

private _relativeLocation(location: reporterTypes.Location): reporterTypes.Location;
private _relativeLocation(location?: reporterTypes.Location): reporterTypes.Location | undefined;
private _relativeLocation(location: reporterTypes.Location | undefined): reporterTypes.Location | undefined {
Expand Down
3 changes: 2 additions & 1 deletion packages/playwright/src/runner/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ class JobDispatcher {
duration: -1,
steps: [],
attachments: [],
location: params.location,
location: params.stack[0],
stack: params.stack,
};
steps.set(params.stepId, step);
(parentStep || result).steps.push(step);
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ export function filterStackFile(file: string) {
return true;
}

export function filteredStackTrace(rawStack: RawStack): StackFrame[] {
export function filteredStackTrace(rawStack: RawStack): Location[] {
const frames: StackFrame[] = [];
for (const line of rawStack) {
const frame = parseStackTraceLine(line);
if (!frame || !frame.file)
continue;
if (!filterStackFile(frame.file))
continue;
frames.push(frame);
frames.push({ file: frame.file, line: frame.line, column: frame.column });
}
return frames;
}
Expand Down
21 changes: 11 additions & 10 deletions packages/playwright/src/worker/testInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import type { Annotation, FullConfigInternal, FullProjectInternal } from '../com
import type { FullConfig, Location } from '../../types/testReporter';
import { debugTest, filteredStackTrace, formatLocation, getContainedPath, normalizeAndSaveAttachment, trimLongString, windowsFilesystemFriendlyLength } from '../util';
import { TestTracing } from './testTracing';
import type { StackFrame } from '@protocol/channels';
import { testInfoError } from './util';

export interface TestStepInternal {
Expand All @@ -35,8 +34,8 @@ export interface TestStepInternal {
stepId: string;
title: string;
category: string;
location?: Location;
boxedStack?: StackFrame[];
stack: Location[];
boxedStack?: Location[];
steps: TestStepInternal[];
endWallTime?: number;
apiName?: string;
Expand Down Expand Up @@ -244,7 +243,7 @@ export class TestInfoImpl implements TestInfo {
?? this._findLastStageStep(this._steps); // If no parent step on stack, assume the current stage as parent.
}

_addStep(data: Omit<TestStepInternal, 'complete' | 'stepId' | 'steps' | 'attachmentIndices'>, parentStep?: TestStepInternal): TestStepInternal {
_addStep(data: Omit<TestStepInternal, 'complete' | 'stepId' | 'steps' | 'attachmentIndices' | 'stack'>, parentStep?: TestStepInternal, stackOverride?: Location[]): TestStepInternal {
const stepId = `${data.category}@${++this._lastStepId}`;

if (data.isStage) {
Expand All @@ -255,18 +254,20 @@ export class TestInfoImpl implements TestInfo {
parentStep = this._parentStep();
}

const filteredStack = filteredStackTrace(captureRawStack());
const filteredStack = stackOverride ? stackOverride : filteredStackTrace(captureRawStack());
data.boxedStack = parentStep?.boxedStack;
let stack = filteredStack;
if (!data.boxedStack && data.box) {
data.boxedStack = filteredStack.slice(1);
data.location = data.location || data.boxedStack[0];
// Only steps with box: true get boxed stack. Inner steps have original stack for better traceability.
stack = data.boxedStack;
}
data.location = data.location || filteredStack[0];

const attachmentIndices: number[] = [];
const step: TestStepInternal = {
stepId,
...data,
stack,
steps: [],
attachmentIndices,
complete: result => {
Expand Down Expand Up @@ -319,10 +320,10 @@ export class TestInfoImpl implements TestInfo {
title: data.title,
category: data.category,
wallTime: Date.now(),
location: data.location,
stack,
};
this._onStepBegin(payload);
this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.category, data.apiName || data.title, data.params, data.location ? [data.location] : []);
this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.category, data.apiName || data.title, data.params, stack);
return step;
}

Expand Down Expand Up @@ -351,7 +352,7 @@ export class TestInfoImpl implements TestInfo {
const location = stage.runnable?.location ? ` at "${formatLocation(stage.runnable.location)}"` : ``;
debugTest(`started stage "${stage.title}"${location}`);
}
stage.step = stage.stepInfo ? this._addStep({ ...stage.stepInfo, title: stage.title, isStage: true }) : undefined;
stage.step = stage.stepInfo ? this._addStep({ category: stage.stepInfo.category, title: stage.title, isStage: true }, undefined, stage.stepInfo.location ? [stage.stepInfo.location] : undefined) : undefined;

try {
await this._timeoutManager.withRunnable(stage.runnable, async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright/src/worker/workerMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ export class WorkerMain extends ProcessRunner {
let firstError: Error | undefined;
const hooks = suites.map(suite => this._collectHooksAndModifiers(suite, type, testInfo)).flat();
for (const hook of hooks) {
const runnable = { type: hook.type, location: hook.location, slot };
const runnable = { type: hook.type, stack: [hook.location], slot };
if (testInfo._timeoutManager.isTimeExhaustedFor(runnable)) {
// Do not run hooks that will timeout right away.
continue;
Expand Down
5 changes: 5 additions & 0 deletions packages/playwright/types/testReporter.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,11 @@ export interface TestStep {
*/
parent?: TestStep;

/**
* Call stack for this step.
*/
stack: Array<Location>;

/**
* Start time of this particular test step.
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/playwright-test/reporter-blob.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1432,7 +1432,7 @@ test('blob report should include version', async ({ runInlineTest }) => {

const events = await extractReport(test.info().outputPath('blob-report', 'report.zip'), test.info().outputPath('tmp'));
const metadataEvent = events.find(e => e.method === 'onBlobReportMetadata');
expect(metadataEvent.params.version).toBe(2);
expect(metadataEvent.params.version).toBe(3);
expect(metadataEvent.params.userAgent).toBe(getUserAgent());
});

Expand Down
Loading

0 comments on commit 78a84a2

Please sign in to comment.