-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
70a16e7
to
9ea6b03
Compare
5803c91
to
7b23413
Compare
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool idea. Thanks for the PR!
@@ -93,6 +93,18 @@ function formatStep({ | |||
} | |||
text += '\n' | |||
|
|||
if (Array.isArray(testStep.attachments)) { |
There was a problem hiding this comment.
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.
.forEach(({ data }) => { | ||
text += | ||
indentString( | ||
colorFns[Status.UNDEFINED](figures.info + ' ' + data), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for no color.
features/error_formatting.feature
Outdated
@@ -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. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
4 | ||
) + '\n' | ||
}) | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
e2231c9
to
b910d36
Compare
docs/support_files/attachments.md
Outdated
They appears right after the step and only `text/plain` content is visible. | ||
It can be used to debug scenarios, especially in parallel mode. | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please use javascript syntax highlighting here? The result format could be entirely commented out or put in its own block
Hi @DevSide, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I found hard to log information right after a step without rewriting a whole custom formatter.
I suggest formatting each step attachment (text only) as line in the error summary.
What do you think ?