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: include exceptions in test step result messages #2229

Merged
merged 13 commits into from
Feb 6, 2023
9 changes: 9 additions & 0 deletions compatibility/features/cdata/cdata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import assert from 'assert'
import { Given } from '../../../src'

Given(
'I have {int} <![CDATA[cukes]]> in my belly',
mpkorstanje marked this conversation as resolved.
Show resolved Hide resolved
function (cukeCount: number) {
assert(cukeCount)
}
)
4 changes: 4 additions & 0 deletions features/fixtures/formatters/failed.message.json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ module.exports = [
},
status: 'FAILED',
message: 'Error: my error',
exception: {
type: 'Error',
message: 'my error',
},
},
timestamp: {
seconds: 0,
Expand Down
4 changes: 4 additions & 0 deletions features/fixtures/formatters/retried.message.json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ module.exports = [
},
status: 'FAILED',
message: 'Error: my error',
exception: {
type: 'Error',
message: 'my error',
},
},
timestamp: {
seconds: 0,
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
"yup": "^0.32.11"
},
"devDependencies": {
"@cucumber/compatibility-kit": "11.0.1",
"@cucumber/compatibility-kit": "11.2.0",
"@cucumber/query": "12.0.1",
"@microsoft/api-documenter": "7.19.27",
"@microsoft/api-extractor": "7.33.7",
Expand Down
42 changes: 25 additions & 17 deletions src/formatter/junit_formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@ interface IJUnitTestCase {
steps: IJUnitTestStep[]
}

interface IJUnitTestCaseFailure {
type: string
message?: string
detail: string
}

interface IJUnitTestCaseResult {
status: TestStepResultStatus
message?: string
failure?: IJUnitTestCaseFailure
}

interface IJUnitTestStep {
Expand All @@ -60,16 +66,6 @@ interface IBuildJUnitTestStepOptions {
testStepResult: messages.TestStepResult
}

const statusDescriptions: Record<TestStepResultStatus, string> = {
UNKNOWN: `A result couldn't be established`,
PASSED: 'Everything went fine',
SKIPPED: 'The test case was skipped',
PENDING: 'A step in the test case is not yet implemented',
UNDEFINED: 'A step in the test case is not defined',
AMBIGUOUS: 'Multiple definitions match one of the steps in the test case',
FAILED: 'A hook or step failed',
}

export default class JunitFormatter extends Formatter {
private readonly names: Record<string, string[]> = {}
public static readonly documentation: string = 'Outputs JUnit report'
Expand Down Expand Up @@ -134,8 +130,20 @@ export default class JunitFormatter extends Formatter {
}

private getTestCaseResult(steps: IJUnitTestStep[]): IJUnitTestCaseResult {
const worstResult = getWorstTestStepResult(steps.map((step) => step.result))
return worstResult
const { status, message, exception } = getWorstTestStepResult(
steps.map((step) => step.result)
)
return {
status,
failure:
message || exception
? {
type: exception?.type,
message: exception?.message,
detail: message,
}
: undefined,
}
}

private durationToSeconds(duration: Duration): number {
Expand Down Expand Up @@ -262,11 +270,11 @@ export default class JunitFormatter extends Formatter {
xmlTestCase.ele('skipped')
} else if (test.result.status !== TestStepResultStatus.PASSED) {
const xmlFailure = xmlTestCase.ele('failure', {
type: test.result.status,
message: statusDescriptions[test.result.status],
type: test.result.failure?.type,
message: test.result.failure?.message,
})
if (test.result.message) {
xmlFailure.cdata(test.result.message)
if (test.result?.failure) {
xmlFailure.cdata(test.result.failure.detail)
}
}
xmlTestCase.ele('system-out', {}).cdata(test.systemOutput)
Expand Down
10 changes: 5 additions & 5 deletions src/formatter/junit_formatter_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe('JunitFormatter', () => {
'<?xml version="1.0"?>\n' +
'<testsuite failures="1" skipped="0" name="cucumber-js" time="0.001" tests="1">\n' +
' <testcase classname="my feature" name="my scenario" time="0.001">\n' +
' <failure type="FAILED" message="A hook or step failed"><![CDATA[error]]></failure>\n' +
' <failure type="Error" message="error"><![CDATA[error]]></failure>\n' +
' <system-out><![CDATA[Given a failing step......................................................failed]]></system-out>\n' +
' </testcase>\n' +
'</testsuite>'
Expand Down Expand Up @@ -179,7 +179,7 @@ describe('JunitFormatter', () => {
'<?xml version="1.0"?>\n' +
'<testsuite failures="1" skipped="0" name="cucumber-js" time="0.001" tests="1">\n' +
' <testcase classname="my feature" name="my scenario" time="0.001">\n' +
' <failure type="FAILED" message="A hook or step failed"><![CDATA[Error: include invalid character]]></failure>\n' +
' <failure type="Error" message="Error: include invalid character"><![CDATA[Error: include invalid character]]></failure>\n' +
' <system-out><![CDATA[Given a failing step with invalid character...............................failed]]></system-out>\n' +
' </testcase>\n' +
'</testsuite>'
Expand Down Expand Up @@ -250,7 +250,7 @@ describe('JunitFormatter', () => {
'<?xml version="1.0"?>\n' +
'<testsuite failures="1" skipped="0" name="cucumber-js" time="0.001" tests="1">\n' +
' <testcase classname="my feature" name="my scenario" time="0.001">\n' +
' <failure type="PENDING" message="A step in the test case is not yet implemented"/>\n' +
' <failure/>\n' +
' <system-out><![CDATA[Given a pending step.....................................................pending]]></system-out>\n' +
' </testcase>\n' +
'</testsuite>'
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('JunitFormatter', () => {
'<?xml version="1.0"?>\n' +
'<testsuite failures="1" skipped="0" name="cucumber-js" time="0" tests="1">\n' +
' <testcase classname="my feature" name="my scenario" time="0">\n' +
' <failure type="UNDEFINED" message="A step in the test case is not defined"/>\n' +
' <failure/>\n' +
' <system-out><![CDATA[Given a passing step...................................................undefined]]></system-out>\n' +
' </testcase>\n' +
'</testsuite>'
Expand Down Expand Up @@ -409,7 +409,7 @@ describe('JunitFormatter', () => {
' <system-out><![CDATA[Given a passing step......................................................passed]]></system-out>\n' +
' </testcase>\n' +
' <testcase classname="my feature" name="my templated scenario [1]" time="0.001">\n' +
' <failure type="FAILED" message="A hook or step failed"><![CDATA[error]]></failure>\n' +
' <failure type="Error" message="error"><![CDATA[error]]></failure>\n' +
' <system-out><![CDATA[Given a failing step......................................................failed]]></system-out>\n' +
' </testcase>\n' +
'</testsuite>'
Expand Down
15 changes: 13 additions & 2 deletions src/runtime/format_error.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { TestStepResult } from '@cucumber/messages'
import { format } from 'assertion-error-formatter'
import errorStackParser from 'error-stack-parser'
import { filterStackTrace } from '../filter_stack_trace'

export function formatError(error: Error, filterStackTraces: boolean): string {
export function formatError(
error: Error,
filterStackTraces: boolean
): Pick<TestStepResult, 'message' | 'exception'> {
let filteredStack: string
if (filterStackTraces) {
try {
Expand All @@ -13,10 +17,17 @@ export function formatError(error: Error, filterStackTraces: boolean): string {
// if we weren't able to parse and filter, we'll settle for the original
}
}
return format(error, {
const message = format(error, {
colorFns: {
errorStack: (stack: string) =>
filteredStack ? `\n${filteredStack}` : stack,
},
})
return {
message,
exception: {
type: error.name || 'Error',
message: typeof error === 'string' ? error : error.message,
},
}
}
58 changes: 58 additions & 0 deletions src/runtime/format_error_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { expect } from 'chai'
import assert from 'assert'
import { formatError } from './format_error'

describe('formatError', () => {
function testFormatError(fn: () => void, filterStackTraces: boolean = false) {
try {
fn()
return undefined
} catch (error) {
return formatError(error, filterStackTraces)
}
}

it('should handle a custom error', () => {
expect(
testFormatError(() => {
assert.ok(false, 'Thing that should have been truthy was falsy!')
}).exception
).to.eql({
type: 'AssertionError',
message: 'Thing that should have been truthy was falsy!',
})
})

it('should handle a generic error', () => {
expect(
testFormatError(() => {
throw new Error('A generally bad thing happened!')
}).exception
).to.eql({
type: 'Error',
message: 'A generally bad thing happened!',
})
})

it('should handle an omitted message', () => {
expect(
testFormatError(() => {
throw new Error()
}).exception
).to.eql({
type: 'Error',
message: '',
})
})

it('should handle a thrown string', () => {
expect(
testFormatError(() => {
throw 'Yikes!'
}).exception
).to.eql({
type: 'Error',
message: 'Yikes!',
})
})
})
6 changes: 3 additions & 3 deletions src/runtime/step_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ export async function run({

const duration = stopwatch.stop().duration()
let status: messages.TestStepResultStatus
let message: string
let details = {}
if (result === 'skipped') {
status = messages.TestStepResultStatus.SKIPPED
} else if (result === 'pending') {
status = messages.TestStepResultStatus.PENDING
} else if (doesHaveValue(error)) {
message = formatError(error, filterStackTraces)
details = formatError(error, filterStackTraces)
status = messages.TestStepResultStatus.FAILED
} else {
status = messages.TestStepResultStatus.PASSED
Expand All @@ -77,7 +77,7 @@ export async function run({
return {
duration,
status,
message,
...details,
}
}

Expand Down
14 changes: 10 additions & 4 deletions src/runtime/test_case_runner_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ describe('TestCaseRunner', () => {
const passedTestResult: messages.TestStepResult = {
duration: messages.TimeConversion.millisecondsToDuration(1),
status: messages.TestStepResultStatus.PASSED,
message: undefined,
}

// Act
Expand Down Expand Up @@ -167,6 +166,10 @@ describe('TestCaseRunner', () => {
duration: messages.TimeConversion.millisecondsToDuration(0),
status: messages.TestStepResultStatus.FAILED,
message: 'fail',
exception: {
type: 'Error',
message: 'fail',
},
}

// Act
Expand Down Expand Up @@ -268,7 +271,7 @@ describe('TestCaseRunner', () => {
return
}
willPass = true
throw 'error' // eslint-disable-line @typescript-eslint/no-throw-literal
throw 'Oh no!' // eslint-disable-line @typescript-eslint/no-throw-literal
})
})
const {
Expand Down Expand Up @@ -309,7 +312,11 @@ describe('TestCaseRunner', () => {
testCaseStartedId: '2',
testStepResult: {
duration: messages.TimeConversion.millisecondsToDuration(1),
message: 'error',
message: 'Oh no!',
exception: {
type: 'Error',
message: 'Oh no!',
},
status: messages.TestStepResultStatus.FAILED,
},
testStepId: '1',
Expand Down Expand Up @@ -343,7 +350,6 @@ describe('TestCaseRunner', () => {
testCaseStartedId: '3',
testStepResult: {
duration: messages.TimeConversion.millisecondsToDuration(1),
message: undefined,
status: messages.TestStepResultStatus.PASSED,
},
testStepId: '1',
Expand Down