Skip to content

Commit

Permalink
Merge pull request #1008 from gemini-testing/TESTPLANE-229.frames
Browse files Browse the repository at this point in the history 
fix: remove extra stack frames
  • Loading branch information
@KuznetsovRoman
KuznetsovRoman authored Sep 5, 2024
2 parents 05c0f80 + 4486481 commit d9884ea
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 18 deletions.
4 changes: 4 additions & 0 deletions src/browser/stacktrace/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ export const WDIO_IGNORED_STACK_FUNCTIONS = [
"Element.newCommand",
"Element.elementErrorHandlerCallbackFn",
];
// Other frames are being filtered out by error-stack-parser
// Declared at https://github.com/stacktracejs/error-stack-parser/blob/v2.1.4/error-stack-parser.js#L17
// Used at https://github.com/stacktracejs/error-stack-parser/blob/v2.1.4/error-stack-parser.js#L52-L54
export const STACK_FRAME_REG_EXP = /^\s*at .*(\S+:\d+|\(native\))/m;
57 changes: 45 additions & 12 deletions src/browser/stacktrace/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ErrorStackParser from "error-stack-parser";
import type { SetRequired } from "type-fest";
import logger from "../../utils/logger";
import { softFileURLToPath } from "../../utils/fs";
import { WDIO_IGNORED_STACK_FUNCTIONS, WDIO_STACK_TRACE_LIMIT } from "./constants";
import { STACK_FRAME_REG_EXP, WDIO_IGNORED_STACK_FUNCTIONS, WDIO_STACK_TRACE_LIMIT } from "./constants";

export type RawStackFrames = string;

Expand Down Expand Up @@ -47,7 +47,13 @@ const getErrorRawStackFrames = (e: ErrorWithStack): RawStackFrames => {
const errorMessageStackIndex = e.stack.indexOf(e.message);
const errorMessageEndsStackIndex = e.stack.indexOf("\n", errorMessageStackIndex + e.message.length);

return e.stack.slice(errorMessageEndsStackIndex + 1);
if (errorMessageStackIndex !== -1) {
return e.stack.slice(errorMessageEndsStackIndex + 1);
}

const stackTraceRegExpResult = STACK_FRAME_REG_EXP.exec(e.stack);

return stackTraceRegExpResult ? e.stack.slice(stackTraceRegExpResult.index) : "";
};

export const captureRawStackFrames = (filterFunc?: (...args: unknown[]) => unknown): RawStackFrames => {
Expand Down Expand Up @@ -144,35 +150,62 @@ export const applyStackTraceIfBetter = (error: Error, stack: RawStackFrames): Er
return error;
};

export const filterExtraWdioFrames = (error: Error): Error => {
export const filterExtraStackFrames = (error: Error): Error => {
if (!error || !error.message || !error.stack) {
return error;
}

try {
const rawFrames = getErrorRawStackFrames(error as ErrorWithStack);
const rawFramesArr = rawFrames.split("\n");
const rawFramesArr = rawFrames.split("\n").filter(frame => STACK_FRAME_REG_EXP.test(frame));
const framesParsed = ErrorStackParser.parse(error);

if (rawFramesArr.length !== framesParsed.length) {
return error;
}

const isWdioFrame = (frame: StackFrame): boolean => {
return Boolean(frame.fileName && frame.fileName.includes("/node_modules/webdriverio/"));
};
// If we found something more relevant (e.g. user's code), we can remove useless testplane internal frames
// If we haven't, we keep testplane internal frames
const shouldDropTestplaneInternalFrames =
getStackTraceRelevance(error) > FRAME_RELEVANCE.projectInternals.value;

const isIgnoredFunction = (frame: StackFrame): boolean => {
const funcName = frame.functionName;
const isIgnoredWebdriverioFrame = (frame: StackFrame): boolean => {
const isWebdriverioFrame = frame.fileName && frame.fileName.includes("/node_modules/webdriverio/");
const fnName = frame.functionName;

if (!funcName) {
if (!isWebdriverioFrame || !fnName) {
return false;
}

return Boolean(WDIO_IGNORED_STACK_FUNCTIONS.some(fn => fn === funcName || "async " + fn === funcName));
return WDIO_IGNORED_STACK_FUNCTIONS.some(fn => fn === fnName || "async " + fn === fnName);
};

const isWdioUtilsFrame = (frame: StackFrame): boolean => {
return Boolean(frame.fileName && frame.fileName.includes("/node_modules/@wdio/utils/"));
};

const isTestplaneExtraInternalFrame = (frame: StackFrame): boolean => {
const testplaneExtraInternalFramePaths = [
"/node_modules/testplane/src/browser/history/",
"/node_modules/testplane/src/browser/stacktrace/",
];

return Boolean(
frame.fileName && testplaneExtraInternalFramePaths.some(path => frame.fileName?.includes(path)),
);
};

const shouldIncludeFrame = (frame: StackFrame): boolean => !isWdioFrame(frame) || !isIgnoredFunction(frame);
const shouldIncludeFrame = (frame: StackFrame): boolean => {
if (isIgnoredWebdriverioFrame(frame) || isWdioUtilsFrame(frame)) {
return false;
}

if (shouldDropTestplaneInternalFrames && isTestplaneExtraInternalFrame(frame)) {
return false;
}

return true;
};

const framesFiltered = rawFramesArr.filter((_, i) => shouldIncludeFrame(framesParsed[i])).join("\n");

Expand Down
4 changes: 2 additions & 2 deletions src/worker/runner/test-runner/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const OneTimeScreenshooter = require("./one-time-screenshooter");
const { AssertViewError } = require("../../../browser/commands/assert-view/errors/assert-view-error");
const history = require("../../../browser/history");
const { SAVE_HISTORY_MODE } = require("../../../constants/config");
const { filterExtraWdioFrames } = require("../../../browser/stacktrace/utils");
const { filterExtraStackFrames } = require("../../../browser/stacktrace/utils");
const { extendWithCodeSnippet } = require("../../../error-snippets");
const { TestplaneInternalError } = require("../../../errors");

Expand Down Expand Up @@ -95,7 +95,7 @@ module.exports = class TestRunner extends Runner {
this._browserAgent.freeBrowser(this._browser);

if (error) {
filterExtraWdioFrames(error);
filterExtraStackFrames(error);

await extendWithCodeSnippet(error);

Expand Down
82 changes: 78 additions & 4 deletions test/src/browser/stacktrace/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
ShallowStackFrames,
applyStackTraceIfBetter,
captureRawStackFrames,
filterExtraWdioFrames,
filterExtraStackFrames,
} from "../../../../src/browser/stacktrace/utils";

type AnyFunc = (...args: any[]) => unknown; // eslint-disable-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -56,7 +56,7 @@ describe("stacktrace/utils", () => {
});
});

describe("filterExtraWdioFrames", () => {
describe("filterExtraStackFrames", () => {
it("should filter out internal wdio frames", () => {
const error = new Error("my\nmulti-line\nerror\nmessage");
const errorStack = [
Expand All @@ -72,7 +72,7 @@ describe("stacktrace/utils", () => {
].join("\n");

error.stack = `${error.name}: ${error.message}\n${errorStack}`;
filterExtraWdioFrames(error);
filterExtraStackFrames(error);

const expectedStack = [
"Error: my\nmulti-line\nerror\nmessage",
Expand All @@ -84,8 +84,82 @@ describe("stacktrace/utils", () => {
assert.equal(error.stack, expectedStack);
});

it("should filter out internal testplane frames", () => {
const error = new Error("my\nmulti-line\nerror\nmessage");
const errorStack = [
"at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)",
"at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)",
"at Object.<anonymous> (projectDir/features/feature/test-name.testplane.js:16:28)",
].join("\n");

error.stack = `${error.name}: ${error.message}\n${errorStack}`;
filterExtraStackFrames(error);

const expectedStack = [
"Error: my\nmulti-line\nerror\nmessage",
"at Object.<anonymous> (projectDir/features/feature/test-name.testplane.js:16:28)",
].join("\n");

assert.equal(error.stack, expectedStack);
});

it("should work with overwritten multiline error message", () => {
const error = new Error("foo");
const errorStack = [
"at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)",
"at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)",
"at Object.<anonymous> (projectDir/features/feature/test-name.testplane.js:16:28)",
].join("\n");

error.stack = `${error.name}: ${error.message}\n${errorStack}`;
error.message = "some/\nnew\nerror\nmessage";
filterExtraStackFrames(error);

const expectedStack = [
"Error: some/\nnew\nerror\nmessage",
"at Object.<anonymous> (projectDir/features/feature/test-name.testplane.js:16:28)",
].join("\n");

assert.equal(error.stack, expectedStack);
});

it("should not filter out internal testplane frames if there is no user's code frame", () => {
const error = new Error("my\nmulti-line\nerror\nmessage");
const errorStack = [
"at fn (projectDir/node_modules/testplane/src/browser/history/index.js:80:27)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithHistoryHooks (projectDir/node_modules/testplane/src/browser/history/index.js:32:12)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/history/index.js:77:20)",
"at fn (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:53:39)",
"at exports.runWithHooks (projectDir/node_modules/testplane/src/browser/history/utils.js:49:23)",
"at runWithStacktraceHooks (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:24:24)",
"at Browser.decoratedWrapper (projectDir/node_modules/testplane/src/browser/stacktrace/index.ts:51:46)",
"at Some.internal (projectDir/node_modules/foo/bar/baz/index.ts:51:46)",
"at Internal.metod (projectDir/node_modules/qux/index.ts:51:46)",
].join("\n");

error.stack = `${error.name}: ${error.message}\n${errorStack}`;
filterExtraStackFrames(error);

const expectedStack = "Error: my\nmulti-line\nerror\nmessage" + "\n" + errorStack;

assert.equal(error.stack, expectedStack);
});

it("should not throw on bad input", () => {
assert.doesNotThrow(() => filterExtraWdioFrames("error-message" as unknown as Error));
assert.doesNotThrow(() => filterExtraStackFrames("error-message" as unknown as Error));
});
});

Expand Down

0 comments on commit d9884ea

Please sign in to comment.