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.

Sign up for GitHub

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

Jump to bottom

Add possibility to opt-out from printing step attachments when scenario is not successful #1721

Merged
merged 17 commits into from
Feb 17, 2022
Merged

Add possibility to opt-out from printing step attachments when scenario is not successful #1721

merged 17 commits into from
Feb 17, 2022

Conversation

aurelien-reeves
Copy link
Contributor

@aurelien-reeves aurelien-reeves commented Jul 1, 2021 •
edited
Loading

Description

Refs. #1136

I've added a scenario to show a new --format-options that would let the possibility to opt-out from the attachments being printed when a scenario is not successful.

That option is named printAttachments and is presumably true per default.

New contributors, welcome!

This is an easy one, and to help you through it, you'll find all the steps to fix that PR here: https://github.com/cucumber/cucumber-js/blob/17527667f70c64600adf1ac924336c64b65df66e/docs/first-time-contributor-tutorial.md

Motivation & context

Fixes #1136

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new behaviour)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • CHANGELOG has been updated

@aurelien-reeves aurelien-reeves marked this pull request as draft July 1, 2021 09:54
@davidjgoss
Copy link
Contributor

That option is named printStepAttachments and is presumably true per default.

Given you can attach from hooks and these are also printed:

image

Potentially just printAttachments is a better name?

If the fix is not too complex, I plan to use this PR as a step-by-step tutorial for new contributors.

That sounds really good!

@aurelien-reeves aurelien-reeves added ⚡ enhancement Request for new functionality beginners mob candidate good first issue Good for newcomers labels Aug 5, 2021
@aurelien-reeves aurelien-reeves mentioned this pull request Aug 5, 2021
Closed
@aurelien-reeves
Copy link
Contributor Author

aurelien-reeves commented Sep 13, 2021 •
edited
Loading

@mattwynne @16sheep @artismarti any plan to go further with that one?

@aurelien-reeves
Copy link
Contributor Author

@mattwynne @16sheep @artismarti do you plan to continue working on this?
If not, any hard feeling if I take care of it?

@artismarti
Copy link

@mattwynne @16sheep @artismarti do you plan to continue working on this? If not, any hard feeling if I take care of it?

None whatsoever, from me! :)

I had kinda forgotten about it, tbh.

@aurelien-reeves
Copy link
Contributor Author

I'll finish implementation for that PR as it would be now difficult for a new comer to continue with this as it has been partially implemented

I hope there won't be hard feelings for this!?

@coveralls
Copy link

coveralls commented Feb 16, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.003%) to 98.13% when pulling 8bdef73 on opt-out-print-attachments-1136 into 5295a45 on main.

@artismarti
Copy link

None whatsoever! Thanks for finishing the work :)

@aurelien-reeves aurelien-reeves marked this pull request as ready for review February 16, 2022 14:16
davidjgoss
davidjgoss reviewed Feb 16, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
docs/cli.md
@@ -74,6 +74,13 @@ By default, cucumber works in _strict_ mode, meaning it will fail if there are p

See [Parallel](./parallel.md).

## Printing Attachments Details

Printing attachments details can be disabled with
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this content would sit better in formatters.md under the "Options" heading; this doc will shrink over time as we decouple config from the CLI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added some docs in formatters.md.
However I think keeping the one in cli.md could still be relevant?

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem!

src/formatter/helpers/issue_helpers.ts Outdated Show resolved Hide resolved
davidjgoss
davidjgoss approved these changes Feb 17, 2022
View reviewed changes
docs/cli.md
@@ -74,6 +74,13 @@ By default, cucumber works in _strict_ mode, meaning it will fail if there are p

See [Parallel](./parallel.md).

## Printing Attachments Details

Printing attachments details can be disabled with
Copy link
Contributor

Choose a reason for hiding this comment

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

No problem!

@aurelien-reeves aurelien-reeves merged commit be75a6e into main Feb 17, 2022
@aurelien-reeves aurelien-reeves deleted the opt-out-print-attachments-1136 branch February 17, 2022 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidjgoss davidjgoss davidjgoss approved these changes

Assignees
No one assigned
Labels
good first issue Good for newcomers ⚡ enhancement Request for new functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Opt out option please: Print attachment step text on error format
6 participants
@aurelien-reeves @davidjgoss @artismarti @coveralls @16sheep @mattwynne