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: location info for journey and step #474

Merged
merged 2 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions __tests__/helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
rewriteErrorStack,
findPWLogsIndexes,
microSecsToSeconds,
wrapFnWithLocation,
} from '../src/helpers';

it('indent message with seperator', () => {
Expand Down Expand Up @@ -133,3 +134,10 @@ it('does not rewrite non playwright errors', () => {
const newNormalStack = rewriteErrorStack(normalStack, indexes);
expect(normalStack).toStrictEqual(newNormalStack);
});

it('location info on execution', () => {
const checkLoc = wrapFnWithLocation(location => {
return location;
});
expect(checkLoc().file).toBe(__filename);
});
13 changes: 8 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"author": "",
"license": "MIT",
"dependencies": {
"@cspotcode/source-map-support": "^0.7.0",
"commander": "^9.0.0",
"deepmerge": "^4.2.2",
"expect": "^27.0.2",
Expand All @@ -48,8 +49,7 @@
"sharp": "^0.30.1",
"snakecase-keys": "^3.2.1",
"sonic-boom": "^2.6.0",
"source-map-support": "^0.5.21",
"ts-node": "^10.5.0",
"ts-node": "^10.7.0",
"typescript": "^4.5.5"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ async function prepareSuites(inputs: string[]) {
compilerOptions: {
esModuleInterop: true,
allowJs: true,
target: 'es2018',
target: 'es2020',
},
});

Expand Down
19 changes: 13 additions & 6 deletions src/common_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ import { Step } from './dsl';
import { reporters } from './reporters';

export type VoidCallback = () => void;
export type Params = Record<string, any>;
export type NetworkConditions = {
offline: boolean;
downloadThroughput: number;
uploadThroughput: number;
latency: number;
export type Location = {
file: string;
line: number;
column: number;
};

export type Params = Record<string, any>;
export type HooksArgs = {
env: string;
params: Params;
Expand All @@ -51,6 +51,13 @@ export type HooksCallback = (args: HooksArgs) => void;
export type StatusValue = 'succeeded' | 'failed' | 'skipped';
export type Reporters = keyof typeof reporters;

export type NetworkConditions = {
offline: boolean;
downloadThroughput: number;
uploadThroughput: number;
latency: number;
};

export type Driver = {
browser: ChromiumBrowser;
context: ChromiumBrowserContext;
Expand Down
38 changes: 22 additions & 16 deletions src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@

import { Journey, JourneyCallback, JourneyOptions } from '../dsl';
import Runner from './runner';
import { VoidCallback, HooksCallback } from '../common_types';
import { VoidCallback, HooksCallback, Location } from '../common_types';
import { wrapFnWithLocation } from '../helpers';
import { log } from './logger';

/**
Expand All @@ -39,23 +40,28 @@ if (!global[SYNTHETICS_RUNNER]) {

export const runner: Runner = global[SYNTHETICS_RUNNER];

export const journey = (
options: JourneyOptions | string,
callback: JourneyCallback
) => {
log(`register journey: ${JSON.stringify(options)}`);
if (typeof options === 'string') {
options = { name: options, id: options };
export const journey = wrapFnWithLocation(
(
location: Location,
options: JourneyOptions | string,
callback: JourneyCallback
) => {
log(`register journey: ${JSON.stringify(options)}`);
if (typeof options === 'string') {
options = { name: options, id: options };
}
const j = new Journey(options, callback, location);
runner.addJourney(j);
return j;
}
const j = new Journey(options, callback);
runner.addJourney(j);
return j;
};
);

export const step = (name: string, callback: VoidCallback) => {
log(`register step: ${name}`);
return runner.currentJourney?.addStep(name, callback);
};
export const step = wrapFnWithLocation(
(location: Location, name: string, callback: VoidCallback) => {
log(`register step: ${name}`);
return runner.currentJourney?.addStep(name, callback, location);
}
);

export const beforeAll = (callback: HooksCallback) => {
runner.addHook('beforeAll', callback);
Expand Down
14 changes: 10 additions & 4 deletions src/dsl/journey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import { Browser, Page, BrowserContext, CDPSession } from 'playwright-chromium';
import micromatch, { isMatch } from 'micromatch';
import { Step } from './step';
import { VoidCallback, HooksCallback, Params } from '../common_types';
import { VoidCallback, HooksCallback, Params, Location } from '../common_types';

export type JourneyOptions = {
name: string;
Expand All @@ -49,18 +49,24 @@ export class Journey {
id?: string;
tags?: string[];
callback: JourneyCallback;
location?: Location;
steps: Step[] = [];
hooks: Hooks = { before: [], after: [] };

constructor(options: JourneyOptions, callback: JourneyCallback) {
constructor(
options: JourneyOptions,
callback: JourneyCallback,
location?: Location
) {
this.name = options.name;
this.id = options.id || options.name;
this.tags = options.tags;
this.callback = callback;
this.location = location;
}

addStep(name: string, callback: VoidCallback) {
const step = new Step(name, this.steps.length + 1, callback);
addStep(name: string, callback: VoidCallback, location?: Location) {
const step = new Step(name, this.steps.length + 1, callback, location);
this.steps.push(step);
return step;
}
Expand Down
11 changes: 9 additions & 2 deletions src/dsl/step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,23 @@
*
*/

import { VoidCallback } from '../common_types';
import { Location, VoidCallback } from '../common_types';

export class Step {
name: string;
index: number;
callback: VoidCallback;
location?: Location;

constructor(name: string, index: number, callback: VoidCallback) {
constructor(
name: string,
index: number,
callback: VoidCallback,
location: Location
) {
this.name = name;
this.index = index;
this.callback = callback;
this.location = location;
}
}
41 changes: 40 additions & 1 deletion src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ import { resolve, join, dirname } from 'path';
import fs from 'fs';
import { promisify } from 'util';
import { performance } from 'perf_hooks';
import { HooksArgs, HooksCallback, NetworkConditions } from './common_types';
import sourceMapSupport from '@cspotcode/source-map-support';
import {
HooksArgs,
HooksCallback,
NetworkConditions,
Location,
} from './common_types';

const lstatAsync = promisify(fs.lstat);
const readdirAsync = promisify(fs.readdir);
Expand Down Expand Up @@ -315,3 +321,36 @@ export function parseNetworkConditions(args: string): NetworkConditions {

return networkConditions;
}

// default stack trace limit
const dstackTraceLimit = 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we allow setting this value through an environment variable as Jest and others do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, interesting point. Do you think it provides value? In the past, I had used them when V8 async promise stack trace was not done correctly and even if you do increase the stack trace limit, its only going to show the Jest stack traces which seems not so useful to me. WDYT?

Copy link
Contributor

@lucasfcosta lucasfcosta Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've previously used this on Deno and I use a similar option to debug errors on test diffs, although that's not exactly a stack trace, it's similar.

I don't think it's a very important change, it just came to mind because it was helpful for me in the past, although in very very few situations.

Also, even if it shows our stack frames, it could be useful to help users report bugs to us (that is, if we don't cut our stack frames straightaway).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, if its useful let's do it. Let me follow up with a PR.


// Uses the V8 Stacktrace API to get the function location
// information - https://v8.dev/docs/stack-trace-api#customizing-stack-traces
export function wrapFnWithLocation<A extends unknown[], R>(
func: (location: Location, ...args: A) => R
): (...args: A) => R {
return (...args) => {
const _prepareStackTrace = Error.prepareStackTrace;
Error.prepareStackTrace = (_, stackFrames) => {
// Deafult CallSite would not map to the original transpiled source
// from ts-node properly, So we wrap it with the library that knows
// how to retrive those source-map for the transpiled code
const frame: NodeJS.CallSite = sourceMapSupport.wrapCallSite(
stackFrames[1]
);
return {
file: frame.getFileName(),
line: frame.getLineNumber(),
column: frame.getColumnNumber(),
};
};
Error.stackTraceLimit = 2;
const obj: { stack: Location } = {} as any;
Error.captureStackTrace(obj);
const location = obj.stack;
Error.stackTraceLimit = dstackTraceLimit;
Error.prepareStackTrace = _prepareStackTrace;
return func(location, ...args);
};
}
8 changes: 0 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,8 @@

import { runner } from './core';
import { RunOptions } from './core/runner';
import sourceMapSupport from 'source-map-support';

export async function run(options: RunOptions) {
/**
* Install source map support
*/
sourceMapSupport.install({
environment: 'node',
});

try {
return await runner.run(options);
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"noImplicitOverride": true,
"sourceMap": true,
"outDir": "dist",
"target": "es2018",
"target": "es2020",
"module": "commonjs"
},
"include": ["src/**/*"]
Expand Down