diff --git a/packages/playwright-test/src/reporters/base.ts b/packages/playwright-test/src/reporters/base.ts index 86e8fe023e..676b28e87a 100644 --- a/packages/playwright-test/src/reporters/base.ts +++ b/packages/playwright-test/src/reporters/base.ts @@ -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 { diff --git a/packages/playwright-test/src/reporters/github.ts b/packages/playwright-test/src/reporters/github.ts index 757589a6a7..3ee6578f5d 100644 --- a/packages/playwright-test/src/reporters/github.ts +++ b/packages/playwright-test/src/reporters/github.ts @@ -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); }); diff --git a/packages/playwright-test/src/reporters/json.ts b/packages/playwright-test/src/reporters/json.ts index 8d8553a09a..9f5f525ae2 100644 --- a/packages/playwright-test/src/reporters/json.ts +++ b/packages/playwright-test/src/reporters/json.ts @@ -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 & { @@ -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; } diff --git a/packages/playwright-test/src/reporters/raw.ts b/packages/playwright-test/src/reporters/raw.ts index 300600fdae..cd06bb6b48 100644 --- a/packages/playwright-test/src/reporters/raw.ts +++ b/packages/playwright-test/src/reporters/raw.ts @@ -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))) }; diff --git a/tests/playwright-test/blink-diff.spec.ts b/tests/playwright-test/blink-diff.spec.ts index 1779723d29..170caa0acb 100644 --- a/tests/playwright-test/blink-diff.spec.ts +++ b/tests/playwright-test/blink-diff.spec.ts @@ -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); }); diff --git a/tests/playwright-test/reporter-base.spec.ts b/tests/playwright-test/reporter-base.spec.ts index 6459b1a4ca..42fc23b94b 100644 --- a/tests/playwright-test/reporter-base.spec.ts +++ b/tests/playwright-test/reporter-base.spec.ts @@ -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 }) => {