From 200609ccfd01b6855133dcd7a6f065950c8dc3bd Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Mon, 29 Apr 2024 20:22:56 +0900 Subject: [PATCH] fix(reporter): use default error formatter for JUnit (#5629) --- packages/vitest/src/node/error.ts | 23 +- .../src/node/reporters/github-actions.ts | 23 +- packages/vitest/src/node/reporters/junit.ts | 37 +--- test/reporters/fixtures/error.test.ts | 49 +++++ test/reporters/src/data-for-junit.ts | 63 ------ .../tests/__snapshots__/junit.test.ts.snap | 80 +++++++ .../__snapshots__/reporters.spec.ts.snap | 207 +----------------- test/reporters/tests/junit.test.ts | 5 + test/reporters/tests/reporters.spec.ts | 91 ++------ 9 files changed, 188 insertions(+), 390 deletions(-) create mode 100644 test/reporters/fixtures/error.test.ts delete mode 100644 test/reporters/src/data-for-junit.ts diff --git a/packages/vitest/src/node/error.ts b/packages/vitest/src/node/error.ts index d099bc482bb9..91ca3ea9e80b 100644 --- a/packages/vitest/src/node/error.ts +++ b/packages/vitest/src/node/error.ts @@ -1,5 +1,6 @@ /* eslint-disable prefer-template */ import { existsSync, readFileSync } from 'node:fs' +import { Writable } from 'node:stream' import { normalize, relative } from 'pathe' import c from 'picocolors' import cliTruncate from 'cli-truncate' @@ -13,7 +14,7 @@ import { TypeCheckError } from '../typecheck/typechecker' import { isPrimitive } from '../utils' import type { Vitest } from './core' import { divider } from './reporters/renderers/utils' -import type { Logger } from './logger' +import { Logger } from './logger' import type { WorkspaceProject } from './workspace' interface PrintErrorOptions { @@ -27,6 +28,26 @@ interface PrintErrorResult { nearest?: ParsedStack } +// use Logger with custom Console to capture entire error printing +export async function captuerPrintError( + error: unknown, + ctx: Vitest, + project: WorkspaceProject, +) { + let output = '' + const writable = new Writable({ + write(chunk, _encoding, callback) { + output += String(chunk) + callback() + }, + }) + const result = await printError(error, project, { + showCodeFrame: false, + logger: new Logger(ctx, writable, writable), + }) + return { nearest: result?.nearest, output } +} + export async function printError(error: unknown, project: WorkspaceProject | undefined, options: PrintErrorOptions): Promise { const { showCodeFrame = true, fullStack = false, type } = options const logger = options.logger diff --git a/packages/vitest/src/node/reporters/github-actions.ts b/packages/vitest/src/node/reporters/github-actions.ts index 8863bb7bbd50..7e65036dcd30 100644 --- a/packages/vitest/src/node/reporters/github-actions.ts +++ b/packages/vitest/src/node/reporters/github-actions.ts @@ -1,10 +1,8 @@ -import { Writable } from 'node:stream' import { getTasks } from '@vitest/runner/utils' import stripAnsi from 'strip-ansi' import type { File, Reporter, Vitest } from '../../types' import { getFullName } from '../../utils' -import { printError } from '../error' -import { Logger } from '../logger' +import { captuerPrintError } from '../error' import type { WorkspaceProject } from '../workspace' export class GithubActionsReporter implements Reporter { @@ -44,7 +42,7 @@ export class GithubActionsReporter implements Reporter { // format errors via `printError` for (const { project, title, error } of projectErrors) { - const result = await printErrorWrapper(error, this.ctx, project) + const result = await captuerPrintError(error, this.ctx, project) const stack = result?.nearest if (!stack) continue @@ -63,23 +61,6 @@ export class GithubActionsReporter implements Reporter { } } -// use Logger with custom Console to extract messgage from `processError` util -// TODO: maybe refactor `processError` to require single function `(message: string) => void` instead of full Logger? -async function printErrorWrapper(error: unknown, ctx: Vitest, project: WorkspaceProject) { - let output = '' - const writable = new Writable({ - write(chunk, _encoding, callback) { - output += String(chunk) - callback() - }, - }) - const result = await printError(error, project, { - showCodeFrame: false, - logger: new Logger(ctx, writable, writable), - }) - return { nearest: result?.nearest, output } -} - // workflow command formatting based on // https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message // https://github.com/actions/toolkit/blob/f1d9b4b985e6f0f728b4b766db73498403fd5ca3/packages/core/src/command.ts#L80-L85 diff --git a/packages/vitest/src/node/reporters/junit.ts b/packages/vitest/src/node/reporters/junit.ts index 6d1c5395bd29..3054aba84b95 100644 --- a/packages/vitest/src/node/reporters/junit.ts +++ b/packages/vitest/src/node/reporters/junit.ts @@ -3,13 +3,12 @@ import { hostname } from 'node:os' import { dirname, relative, resolve } from 'pathe' import type { Task } from '@vitest/runner' -import type { ErrorWithDiff } from '@vitest/utils' import { getSuites } from '@vitest/runner/utils' +import stripAnsi from 'strip-ansi' import type { Vitest } from '../../node' import type { Reporter } from '../../types/reporter' -import { parseErrorStacktrace } from '../../utils/source-map' -import { F_POINTER } from '../../utils/figures' import { getOutputFile } from '../../utils/config-helpers' +import { captuerPrintError } from '../error' import { IndentedLogger } from './renderers/indented-logger' export interface JUnitOptions { @@ -140,31 +139,6 @@ export class JUnitReporter implements Reporter { await this.logger.log(``) } - async writeErrorDetails(task: Task, error: ErrorWithDiff): Promise { - const errorName = error.name ?? error.nameStr ?? 'Unknown Error' - const errorDetails = `${errorName}: ${error.message}` - - // Be sure to escape any XML in the error Details - await this.baseLog(escapeXML(errorDetails)) - - const project = this.ctx.getProjectByTaskId(task.id) - const stack = parseErrorStacktrace(error, { - getSourceMap: file => project.getBrowserSourceMapModuleById(file), - frameFilter: this.ctx.config.onStackTrace, - }) - - // TODO: This is same as printStack but without colors. Find a way to reuse code. - for (const frame of stack) { - const path = relative(this.ctx.config.root, frame.file) - - await this.baseLog(escapeXML(` ${F_POINTER} ${[frame.method, `${path}:${frame.line}:${frame.column}`].filter(Boolean).join(' ')}`)) - - // reached at test file, skip the follow stack - if (frame.file in this.ctx.state.filesMap) - break - } - } - async writeLogs(task: Task, type: 'err' | 'out'): Promise { if (task.logs == null || task.logs.length === 0) return @@ -205,7 +179,12 @@ export class JUnitReporter implements Reporter { if (!error) return - await this.writeErrorDetails(task, error) + const result = await captuerPrintError( + error, + this.ctx, + this.ctx.getProjectByTaskId(task.id), + ) + await this.baseLog(escapeXML(stripAnsi(result.output.trim()))) }) } } diff --git a/test/reporters/fixtures/error.test.ts b/test/reporters/fixtures/error.test.ts new file mode 100644 index 000000000000..3b691c756cd5 --- /dev/null +++ b/test/reporters/fixtures/error.test.ts @@ -0,0 +1,49 @@ +import { afterAll, it, expect } from "vitest"; + +afterAll(() => { + throwSuite() +}) + +it('stack', () => { + throwDeep() +}) + +it('diff', () => { + expect({ hello: 'x' }).toEqual({ hello: 'y' }) +}) + +it('unhandled', () => { + (async () => throwSimple())() +}) + +it('no name object', () => { + throw { noName: 'hi' }; +}); + +it('string', () => { + throw "hi"; +}); + +it('number', () => { + throw 1234; +}); + +it('number name object', () => { + throw { name: 1234 }; +}); + +it('xml', () => { + throw new Error('error message that has XML in it
'); +}) + +function throwDeep() { + throwSimple() +} + +function throwSimple() { + throw new Error('throwSimple') +} + +function throwSuite() { + throw new Error('throwSuite') +} diff --git a/test/reporters/src/data-for-junit.ts b/test/reporters/src/data-for-junit.ts deleted file mode 100644 index f443155fe558..000000000000 --- a/test/reporters/src/data-for-junit.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { AssertionError } from 'node:assert' -import type { File, Suite, Task } from 'vitest' - -function createSuiteHavingFailedTestWithXmlInError(): File[] { - const file: File = { - id: '1223128da3', - name: 'test/core/test/basic.test.ts', - type: 'suite', - meta: {}, - mode: 'run', - filepath: '/vitest/test/core/test/basic.test.ts', - result: { state: 'fail', duration: 145.99284195899963 }, - tasks: [], - projectName: '', - } - - const suite: Suite = { - id: '', - type: 'suite', - name: 'suite', - mode: 'run', - meta: {}, - file, - result: { state: 'pass', duration: 1.90183687210083 }, - tasks: [], - projectName: '', - } - - const errorWithXml = new AssertionError({ - message: 'error message that has XML in it ', - }) - - errorWithXml.stack = 'Error: error message that has XML in it \n' - + ' at /vitest/test/core/test/basic.test.ts:8:32\n' - + ' at /vitest/test/core/test/.ts:3:11\n' - + ' at etc....' - - const tasks: Task[] = [ - { - id: '123_0', - type: 'test', - name: 'test with xml in error', - mode: 'run', - meta: {}, - suite, - fails: undefined, - file, - result: { - state: 'fail', - errors: [errorWithXml], - duration: 2.123123123, - }, - context: null as any, - }, - ] - - file.tasks = [suite] - suite.tasks = tasks - - return [file] -} - -export { createSuiteHavingFailedTestWithXmlInError } diff --git a/test/reporters/tests/__snapshots__/junit.test.ts.snap b/test/reporters/tests/__snapshots__/junit.test.ts.snap index a6c51a996401..7bcaeec8a22e 100644 --- a/test/reporters/tests/__snapshots__/junit.test.ts.snap +++ b/test/reporters/tests/__snapshots__/junit.test.ts.snap @@ -34,3 +34,83 @@ Error: afterAll error " `; + +exports[`format error 1`] = ` +" + + + + +Error: throwSimple + ❯ throwSimple error.test.ts:44:9 + ❯ throwDeep error.test.ts:40:3 + ❯ error.test.ts:8:3 + + + + +AssertionError: expected { hello: 'x' } to deeply equal { hello: 'y' } + +- Expected ++ Received + + Object { +- "hello": "y", ++ "hello": "x", + } + + ❯ error.test.ts:12:26 + + + + + + +{ + noName: 'hi', + expected: 'undefined', + actual: 'undefined', + stacks: [] +} +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ +Serialized Error: { noName: 'hi' } + + + + +Unknown Error: hi + + + + +Unknown Error: 1234 + + + + +{ + name: 1234, + nameStr: '1234', + expected: 'undefined', + actual: 'undefined', + stacks: [] +} + + + + +Error: error message that has XML in it <div><input/></div> + ❯ error.test.ts:36:9 + + + + +Error: throwSuite + ❯ throwSuite error.test.ts:48:9 + ❯ error.test.ts:4:3 + + + + +" +`; diff --git a/test/reporters/tests/__snapshots__/reporters.spec.ts.snap b/test/reporters/tests/__snapshots__/reporters.spec.ts.snap index 34ffd5c7262d..b4e8c95b87ec 100644 --- a/test/reporters/tests/__snapshots__/reporters.spec.ts.snap +++ b/test/reporters/tests/__snapshots__/reporters.spec.ts.snap @@ -2,72 +2,14 @@ exports[`JUnit reporter (no outputFile entry) 1`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - + " `; exports[`JUnit reporter 1`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - + " `; @@ -79,36 +21,7 @@ exports[`JUnit reporter with outputFile 1`] = ` exports[`JUnit reporter with outputFile 2`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - + " `; @@ -120,36 +33,7 @@ exports[`JUnit reporter with outputFile in non-existing directory 1`] = ` exports[`JUnit reporter with outputFile in non-existing directory 2`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - + " `; @@ -161,36 +45,7 @@ exports[`JUnit reporter with outputFile object 1`] = ` exports[`JUnit reporter with outputFile object 2`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - + " `; @@ -202,57 +57,7 @@ exports[`JUnit reporter with outputFile object in non-existing directory 1`] = ` exports[`JUnit reporter with outputFile object in non-existing directory 2`] = ` " - - - - -AssertionError: expected 2.23606797749979 to equal 2 - ❯ test/core/test/basic.test.ts:8:32 - - - - - - - - - - - - - - - - - -[33merror[39m - - - - - - - -" -`; - -exports[`JUnit reporter with outputFile with XML in error message 1`] = ` -"JUNIT report written to /report_escape_msg_xml.xml -" -`; - -exports[`JUnit reporter with outputFile with XML in error message 2`] = ` -" - - - - -AssertionError: error message that has XML in it <tag> - ❯ test/core/test/basic.test.ts:8:32 - ❯ test/core/test/<bracket-name>.ts:3:11 - - - + " `; diff --git a/test/reporters/tests/junit.test.ts b/test/reporters/tests/junit.test.ts index 72cdcbd4ab98..07b2698ce050 100644 --- a/test/reporters/tests/junit.test.ts +++ b/test/reporters/tests/junit.test.ts @@ -57,6 +57,11 @@ test('emits when beforeAll/afterAll failed', async () => { expect(xml).toMatchSnapshot() }) +test('format error', async () => { + const { stdout } = await runVitest({ reporters: 'junit', root }, ['error.test.ts']) + expect(stabilizeReport(stdout)).toMatchSnapshot() +}) + test('write testsuite name relative to root config', async () => { const { stdout } = await runVitest({ reporters: 'junit', root: './fixtures/better-testsuite-name' }) diff --git a/test/reporters/tests/reporters.spec.ts b/test/reporters/tests/reporters.spec.ts index 6245b034723c..d9a7af45457c 100644 --- a/test/reporters/tests/reporters.spec.ts +++ b/test/reporters/tests/reporters.spec.ts @@ -1,5 +1,5 @@ import { existsSync, readFileSync, rmSync } from 'node:fs' -import { afterEach, expect, test, vi } from 'vitest' +import { beforeEach, expect, test, vi } from 'vitest' import { normalize, resolve } from 'pathe' import { JsonReporter } from '../../../packages/vitest/src/node/reporters/json' import { JUnitReporter } from '../../../packages/vitest/src/node/reporters/junit' @@ -7,10 +7,16 @@ import { TapReporter } from '../../../packages/vitest/src/node/reporters/tap' import { TapFlatReporter } from '../../../packages/vitest/src/node/reporters/tap-flat' import { getContext } from '../src/context' import { files } from '../src/data' -import { createSuiteHavingFailedTestWithXmlInError } from '../src/data-for-junit' -afterEach(() => { - vi.useRealTimers() +vi.mock('os', () => ({ + hostname: () => 'hostname', +})) + +beforeEach(() => { + vi.setSystemTime(1642587001759) + return () => { + vi.useRealTimers() + } }) test('tap reporter', async () => { @@ -44,15 +50,9 @@ test('JUnit reporter', async () => { const reporter = new JUnitReporter({}) const context = getContext() - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(files) + await reporter.onFinished([]) // Assert expect(context.output).toMatchSnapshot() @@ -64,15 +64,9 @@ test('JUnit reporter (no outputFile entry)', async () => { const context = getContext() context.vitest.config.outputFile = {} - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(files) + await reporter.onFinished([]) // Assert expect(context.output).toMatchSnapshot() @@ -85,44 +79,9 @@ test('JUnit reporter with outputFile', async () => { const context = getContext() context.vitest.config.outputFile = outputFile - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - - // Act - await reporter.onInit(context.vitest) - await reporter.onFinished(files) - - // Assert - expect(normalizeCwd(context.output)).toMatchSnapshot() - expect(existsSync(outputFile)).toBe(true) - expect(readFileSync(outputFile, 'utf8')).toMatchSnapshot() - - // Cleanup - rmSync(outputFile) -}) - -test('JUnit reporter with outputFile with XML in error message', async () => { - // Arrange - const reporter = new JUnitReporter({}) - const outputFile = resolve('report_escape_msg_xml.xml') - const context = getContext() - context.vitest.config.outputFile = outputFile - - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - - // setup suite with failed test with xml - const filesWithTestHavingXmlInError = createSuiteHavingFailedTestWithXmlInError() - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(filesWithTestHavingXmlInError) + await reporter.onFinished([]) // Assert expect(normalizeCwd(context.output)).toMatchSnapshot() @@ -142,15 +101,9 @@ test('JUnit reporter with outputFile object', async () => { junit: outputFile, } - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(files) + await reporter.onFinished([]) // Assert expect(normalizeCwd(context.output)).toMatchSnapshot() @@ -169,15 +122,9 @@ test('JUnit reporter with outputFile in non-existing directory', async () => { const context = getContext() context.vitest.config.outputFile = outputFile - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(files) + await reporter.onFinished([]) // Assert expect(normalizeCwd(context.output)).toMatchSnapshot() @@ -198,15 +145,9 @@ test('JUnit reporter with outputFile object in non-existing directory', async () junit: outputFile, } - vi.mock('os', () => ({ - hostname: () => 'hostname', - })) - - vi.setSystemTime(1642587001759) - // Act await reporter.onInit(context.vitest) - await reporter.onFinished(files) + await reporter.onFinished([]) // Assert expect(normalizeCwd(context.output)).toMatchSnapshot()