Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(step.stack): expose step.stack for better current line in IDE #34547

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading