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

Print attachment step text on error format #1041

Merged
merged 7 commits into from
Mar 21, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion features/error_formatting.feature
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Feature: Error formatting
"""
import {Given} from 'cucumber'

Given(/^a basic step$/, function() {})
Given(/^a basic step$/, function() { this.attach('Basic info.') })
Given(/^a step with a doc string$/, function(str) {})
Given(/^a pending step$/, function() { return 'pending' })
"""
Expand All @@ -65,6 +65,7 @@ Feature: Error formatting

1) Scenario: some scenario # features/a.feature:3
✔ Given a basic step # features/step_definitions/cucumber_steps.js:3
ℹ Basic info.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on this being: Attachment (text/plain): Basic info.
Then other non text attachments could be Attachment (other/type)
Currently the figures are only used for steps and not related messages and I'd like to keep that pattern

Copy link
Contributor Author

@DevSide DevSide Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, no figure. How should I stringify other media types ? I currently discard them.
I can eventually JSON.stringify application/json data but we need to indent them, maybe in an other PR ?

Would you like I write something in the doc about this feature ? In the attachment page ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the other media types, I was thinking the value could be suppressed. So for now the line would be Attachment (other/type). We can add json in another PR. Yes please add some docs to the attachment page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

✔ And a step with a doc string # features/step_definitions/cucumber_steps.js:4
\"\"\"
my doc string
Expand Down
12 changes: 12 additions & 0 deletions src/formatter/helpers/issue_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,18 @@ function formatStep({
}
text += '\n'

if (Array.isArray(testStep.attachments)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified to if (testStep.attachments) { as it should always be an array or null.

testStep.attachments
.filter(({ media }) => media.type === 'text/plain')
.forEach(({ data }) => {
text +=
indentString(
colorFns[Status.UNDEFINED](figures.info + ' ' + data),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm uncertain about attachments always being yellow. Thoughts on it being the same color as the step or not having any color?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for no color.

4
) + '\n'
})
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on attachments being added after the step argument so it appears after the docstring / data table and not in between?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be append always between step line and docstring/data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this was block was placed after the next block (that includes buildStepArgumentIterator) I believe it would appear after the step argument. Please add a test for this as well


if (pickleStep) {
let str
const iterator = buildStepArgumentIterator({
Expand Down
57 changes: 57 additions & 0 deletions src/formatter/helpers/issue_helpers_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,5 +235,62 @@ describe('IssueHelpers', () => {
)
})
})

describe('step with attachment text', () => {
beforeEach(function() {
this.testCase.steps[0].result = this.passedStepResult
this.testCase.steps[0].attachments = [
{
data: 'First info.',
media: {
type: 'text/plain',
},
},
{
data: 'Nop',
media: {
type: 'text/special',
},
},
{
data: 'Second info.',
media: {
type: 'text/plain',
},
},
]
this.testCase.steps[1] = {
actionLocation: { line: 3, uri: 'steps.js' },
sourceLocation: { line: 4, uri: 'a.feature' },
result: {
exception: 'error',
status: Status.FAILED,
},
}
this.testCase.steps[1].attachments = [
{
data: 'Third info.',
media: {
type: 'text/plain',
},
},
]
this.testCase.steps[2].result = this.skippedStepResult
this.formattedIssue = formatIssue(this.options)
})

it('prints the scenario', function() {
expect(this.formattedIssue).to.eql(
'1) Scenario: my scenario # a.feature:2\n' +
` ${figures.tick} Given step1 # steps.js:2\n` +
` ${figures.info} First info.\n` +
` ${figures.info} Second info.\n` +
` ${figures.cross} When step2 # steps.js:3\n` +
` ${figures.info} Third info.\n` +
' error\n' +
' - Then step3 # steps.js:4\n\n'
)
})
})
})
})