fix(test runner): show codeframe and location from the error top stack frame (#11179)

Previously, reporter would look for a stack frame directly in the test file.
Often times, that is not a top stack frame, especially when the test uses
some helper functions.

This changes error snippets and locations to use the top frame. When top
frame does not match the test file, we additionally show the location
to avoid confusion:

```
  1) a.spec.ts:7:7 › foobar ========================================================================

    Error: oh my

       at helper.ts:5

      3 |
      4 |       export function ohMy() {
    > 5 |         throw new Error('oh my');
        |               ^
      6 |       }
      7 |

        at ohMy (.../reporter-base-should-print-codeframe-from-a-helper/helper.ts:5:15)
        at .../reporter-base-should-print-codeframe-from-a-helper/a.spec.ts:8:9
        at FixtureRunner.resolveParametersAndRunHookOrTest (.../src/fixtures.ts:281:12)
```
This commit is contained in:
Dmitry Gozman 2022-01-04 16:00:55 -08:00 committed by GitHub
parent 9fcb8ace4e
commit 16a779a5ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 104 additions and 113 deletions

View File

@ -14,35 +14,33 @@
* limitations under the License.
*/
import { BabelCodeFrameOptions, codeFrameColumns } from '@babel/code-frame';
import { codeFrameColumns } from '@babel/code-frame';
import colors from 'colors/safe';
import fs from 'fs';
import milliseconds from 'ms';
import path from 'path';
import StackUtils from 'stack-utils';
import { FullConfig, TestCase, Suite, TestResult, TestError, Reporter, FullResult, TestStep } from '../../types/testReporter';
import { FullConfig, TestCase, Suite, TestResult, TestError, Reporter, FullResult, TestStep, Location } from '../../types/testReporter';
const stackUtils = new StackUtils();
export type TestResultOutput = { chunk: string | Buffer, type: 'stdout' | 'stderr' };
export const kOutputSymbol = Symbol('output');
export type PositionInFile = { column: number; line: number };
type Annotation = {
filePath: string;
title: string;
message: string;
position?: PositionInFile;
location?: Location;
};
type FailureDetails = {
tokens: string[];
position?: PositionInFile;
location?: Location;
};
type ErrorDetails = {
message: string;
position?: PositionInFile;
location?: Location;
};
type TestSummary = {
@ -102,7 +100,7 @@ export class BaseReporter implements Reporter {
}
onError(error: TestError) {
console.log(formatError(error, colors.enabled).message);
console.log(formatError(this.config, error, colors.enabled).message);
}
async onEnd(result: FullResult) {
@ -224,11 +222,11 @@ export class BaseReporter implements Reporter {
}
}
export function formatFailure(config: FullConfig, test: TestCase, options: {index?: number, includeStdio?: boolean, includeAttachments?: boolean, filePath?: string} = {}): {
export function formatFailure(config: FullConfig, test: TestCase, options: {index?: number, includeStdio?: boolean, includeAttachments?: boolean} = {}): {
message: string,
annotations: Annotation[]
} {
const { index, includeStdio, includeAttachments = true, filePath } = options;
const { index, includeStdio, includeAttachments = true } = options;
const lines: string[] = [];
const title = formatTestTitle(config, test);
const annotations: Annotation[] = [];
@ -236,7 +234,7 @@ export function formatFailure(config: FullConfig, test: TestCase, options: {inde
lines.push(colors.red(header));
for (const result of test.results) {
const resultLines: string[] = [];
const { tokens: resultTokens, position } = formatResultFailure(test, result, ' ', colors.enabled);
const { tokens: resultTokens, location } = formatResultFailure(config, test, result, ' ', colors.enabled);
if (!resultTokens.length)
continue;
if (result.retry) {
@ -281,14 +279,11 @@ export function formatFailure(config: FullConfig, test: TestCase, options: {inde
resultLines.push('');
resultLines.push(colors.gray(pad('--- Test output', '-')) + '\n\n' + outputText + '\n' + pad('', '-'));
}
if (filePath) {
annotations.push({
filePath,
position,
title,
message: [header, ...resultLines].join('\n'),
});
}
annotations.push({
location,
title,
message: [header, ...resultLines].join('\n'),
});
lines.push(...resultLines);
}
lines.push('');
@ -298,7 +293,7 @@ export function formatFailure(config: FullConfig, test: TestCase, options: {inde
};
}
export function formatResultFailure(test: TestCase, result: TestResult, initialIndent: string, highlightCode: boolean): FailureDetails {
export function formatResultFailure(config: FullConfig, test: TestCase, result: TestResult, initialIndent: string, highlightCode: boolean): FailureDetails {
const resultTokens: string[] = [];
if (result.status === 'timedOut') {
resultTokens.push('');
@ -310,17 +305,21 @@ export function formatResultFailure(test: TestCase, result: TestResult, initialI
}
let error: ErrorDetails | undefined = undefined;
if (result.error !== undefined) {
error = formatError(result.error, highlightCode, test.location.file);
error = formatError(config, result.error, highlightCode, test.location.file);
resultTokens.push(indent(error.message, initialIndent));
}
return {
tokens: resultTokens,
position: error?.position,
location: error?.location,
};
}
function relativeFilePath(config: FullConfig, file: string): string {
return path.relative(config.rootDir, file) || path.basename(file);
}
function relativeTestPath(config: FullConfig, test: TestCase): string {
return path.relative(config.rootDir, test.location.file) || path.basename(test.location.file);
return relativeFilePath(config, test.location.file);
}
function stepSuffix(step: TestStep | undefined) {
@ -342,32 +341,39 @@ function formatTestHeader(config: FullConfig, test: TestCase, indent: string, in
return pad(header, '=');
}
export function formatError(error: TestError, highlightCode: boolean, file?: string): ErrorDetails {
export function formatError(config: FullConfig, error: TestError, highlightCode: boolean, file?: string): ErrorDetails {
const stack = error.stack;
const tokens = [''];
let positionInFile: PositionInFile | undefined;
let location: Location | undefined;
if (stack) {
const { message, stackLines, position } = prepareErrorStack(
stack,
file
);
positionInFile = position;
tokens.push(message);
const codeFrame = generateCodeFrame({ highlightCode }, file, position);
if (codeFrame) {
tokens.push('');
tokens.push(codeFrame);
const parsed = prepareErrorStack(stack);
tokens.push(parsed.message);
location = parsed.location;
if (location) {
try {
// Stack will have /private/var/folders instead of /var/folders on Mac.
const realFile = fs.realpathSync(location.file);
const source = fs.readFileSync(realFile, 'utf8');
const codeFrame = codeFrameColumns(source, { start: location }, { highlightCode });
if (!file || file !== realFile) {
tokens.push('');
tokens.push(colors.gray(` at `) + `${relativeFilePath(config, realFile)}:${location.line}`);
}
tokens.push('');
tokens.push(codeFrame);
} catch (e) {
// Failed to read the source file - that's ok.
}
}
tokens.push('');
tokens.push(colors.dim(stackLines.join('\n')));
tokens.push(colors.dim(parsed.stackLines.join('\n')));
} else if (error.message) {
tokens.push(error.message);
} else if (error.value) {
tokens.push(error.value);
}
return {
position: positionInFile,
location,
message: tokens.join('\n'),
};
}
@ -382,48 +388,26 @@ function indent(lines: string, tab: string) {
return lines.replace(/^(?=.+$)/gm, tab);
}
export function generateCodeFrame(options: BabelCodeFrameOptions, file?: string, position?: PositionInFile): string | undefined {
if (!position || !file)
return;
const source = fs.readFileSync(file!, 'utf8');
const codeFrame = codeFrameColumns(
source,
{ start: position },
options
);
return codeFrame;
}
export function prepareErrorStack(stack: string, file?: string): {
export function prepareErrorStack(stack: string): {
message: string;
stackLines: string[];
position?: PositionInFile;
location?: Location;
} {
const lines = stack.split('\n');
let firstStackLine = lines.findIndex(line => line.startsWith(' at '));
if (firstStackLine === -1) firstStackLine = lines.length;
if (firstStackLine === -1)
firstStackLine = lines.length;
const message = lines.slice(0, firstStackLine).join('\n');
const stackLines = lines.slice(firstStackLine);
const position = file ? positionInFile(stackLines, file) : undefined;
return {
message,
stackLines,
position,
};
}
function positionInFile(stackLines: string[], file: string): PositionInFile | undefined {
// Stack will have /private/var/folders instead of /var/folders on Mac.
file = fs.realpathSync(file);
let location: Location | undefined;
for (const line of stackLines) {
const parsed = stackUtils.parseLine(line);
if (!parsed || !parsed.file)
continue;
if (path.resolve(process.cwd(), parsed.file) === file)
return { column: parsed.column || 0, line: parsed.line || 0 };
if (parsed && parsed.file) {
location = { file: path.join(process.cwd(), parsed.file), column: parsed.column || 0, line: parsed.line || 0 };
break;
}
}
return { message, stackLines, location };
}
function monotonicTime(): number {

View File

@ -69,7 +69,7 @@ export class GitHubReporter extends BaseReporter {
}
override onError(error: TestError) {
const errorMessage = formatError(error, false).message;
const errorMessage = formatError(this.config, error, false).message;
this.githubLogger.error(errorMessage);
}
@ -100,21 +100,19 @@ export class GitHubReporter extends BaseReporter {
private _printFailureAnnotations(failures: TestCase[]) {
failures.forEach((test, index) => {
const filePath = workspaceRelativePath(test.location.file);
const { annotations } = formatFailure(this.config, test, {
filePath,
index: index + 1,
includeStdio: true,
includeAttachments: false,
});
annotations.forEach(({ filePath, title, message, position }) => {
annotations.forEach(({ location, title, message }) => {
const options: GitHubLogOptions = {
file: filePath,
file: workspaceRelativePath(location?.file || test.location.file),
title,
};
if (position) {
options.line = position.line;
options.col = position.column;
if (location) {
options.line = location.line;
options.col = location.column;
}
this.githubLogger.error(message, options);
});

View File

@ -17,7 +17,7 @@
import fs from 'fs';
import path from 'path';
import { FullConfig, TestCase, Suite, TestResult, TestError, TestStep, FullResult, TestStatus, Location, Reporter } from '../../types/testReporter';
import { PositionInFile, prepareErrorStack } from './base';
import { prepareErrorStack } from './base';
export interface JSONReport {
config: Omit<FullConfig, 'projects'> & {
@ -77,7 +77,7 @@ export interface JSONReportTestResult {
body?: string;
contentType: string;
}[];
errorLocation?: PositionInFile
errorLocation?: Location;
}
export interface JSONReportTestStep {
title: string;
@ -229,12 +229,12 @@ class JSONReporter implements Reporter {
annotations: test.annotations,
expectedStatus: test.expectedStatus,
projectName: test.titlePath()[1],
results: test.results.map(r => this._serializeTestResult(r, test.location.file)),
results: test.results.map(r => this._serializeTestResult(r)),
status: test.outcome(),
};
}
private _serializeTestResult(result: TestResult, file: string): JSONReportTestResult {
private _serializeTestResult(result: TestResult): JSONReportTestResult {
const steps = result.steps.filter(s => s.category === 'test.step');
const jsonResult: JSONReportTestResult = {
workerIndex: result.workerIndex,
@ -252,14 +252,8 @@ class JSONReporter implements Reporter {
body: a.body?.toString('base64')
})),
};
if (result.error?.stack) {
const { position } = prepareErrorStack(
result.error.stack,
file
);
if (position)
jsonResult.errorLocation = position;
}
if (result.error?.stack)
jsonResult.errorLocation = prepareErrorStack(result.error.stack).location;
return jsonResult;
}

View File

@ -219,7 +219,7 @@ class RawReporter {
startTime: result.startTime.toISOString(),
duration: result.duration,
status: result.status,
error: formatResultFailure(test, result, '', true).tokens.join('').trim(),
error: formatResultFailure(this.config, test, result, '', true).tokens.join('').trim(),
attachments: this._createAttachments(result),
steps: dedupeSteps(result.steps.map(step => this._serializeStep(test, step)))
};

View File

@ -730,7 +730,6 @@ it.describe('Blink-Diff', () => {
it('should crop image-a', async () => {
instance._cropImageA = { width: 1, height: 2 };
console.log('A');
const result = instance.runSync();
expect(result.dimension).toBe(2);
});

View File

@ -64,26 +64,15 @@ test('print should print the error name without a message', async ({ runInlineTe
expect(result.output).toContain('FooBarError');
});
test('print an error in a codeframe', async ({ runInlineTest }) => {
test('should print an error in a codeframe', async ({ runInlineTest }) => {
const result = await runInlineTest({
'my-lib.ts': `
const foobar = () => {
const error = new Error('my-message');
error.name = 'FooBarError';
throw error;
}
export default () => {
foobar();
}
`,
'a.spec.ts': `
const { test } = pwt;
import myLib from './my-lib';
test('foobar', async ({}) => {
const error = new Error('my-message');
error.name = 'FooBarError';
throw error;
});
const { test } = pwt;
test('foobar', async ({}) => {
const error = new Error('my-message');
error.name = 'FooBarError';
throw error;
});
`
}, {}, {
FORCE_COLOR: '0',
@ -91,9 +80,36 @@ test('print an error in a codeframe', async ({ runInlineTest }) => {
expect(result.exitCode).toBe(1);
expect(result.failed).toBe(1);
expect(result.output).toContain('FooBarError: my-message');
expect(result.output).toContain('test(\'foobar\', async');
expect(result.output).toContain('throw error;');
expect(result.output).toContain('import myLib from \'./my-lib\';');
expect(result.output).not.toContain('at a.spec.ts:7');
expect(result.output).toContain(` 5 | const { test } = pwt;`);
expect(result.output).toContain(` 6 | test('foobar', async ({}) => {`);
expect(result.output).toContain(`> 7 | const error = new Error('my-message');`);
});
test('should print codeframe from a helper', async ({ runInlineTest }) => {
const result = await runInlineTest({
'helper.ts': `
export function ohMy() {
throw new Error('oh my');
}
`,
'a.spec.ts': `
import { ohMy } from './helper';
const { test } = pwt;
test('foobar', async ({}) => {
ohMy();
});
`
}, {}, {
FORCE_COLOR: '0',
});
expect(result.exitCode).toBe(1);
expect(result.failed).toBe(1);
expect(result.output).toContain('Error: oh my');
expect(result.output).toContain(` at helper.ts:5`);
expect(result.output).toContain(` 4 | export function ohMy() {`);
expect(result.output).toContain(`> 5 | throw new Error('oh my');`);
expect(result.output).toContain(` | ^`);
});
test('should print slow tests', async ({ runInlineTest }) => {