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

Opt out option please: Print attachment step text on error format #1136

Closed
Izhaki opened this issue Sep 18, 2018 · 11 comments · Fixed by #1721
Closed

Opt out option please: Print attachment step text on error format #1136

Izhaki opened this issue Sep 18, 2018 · 11 comments · Fixed by #1721
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ⚡ enhancement Request for new functionality

Comments

@Izhaki
Copy link
Contributor

Izhaki commented Sep 18, 2018

So #1041 was merged a while back (@DevSide @charlierudolph), but we have just upgraded to version 4 to see its effect.

Our use case:

  • Cucumber used in e2e tests.
  • After each scenario we attach the browser logs.
  • In dev mode our app spits a LOT of log data (~5 per second); some include JSON dumps.

And so, every time something fails (even cannot read property of undefined), we get this ginormous print out and have to scroll all the way up to see what the issue was.


Would be handy for us to be able to opt-out of this behaviour (or maybe even have it as opt-in?).

It makes certain assumptions with regards to attachments that are not valid with some use-cases.

@Izhaki
Copy link
Contributor Author

Izhaki commented Sep 19, 2018

More:

   ✔ Given my language is set to "EN" # step-definitions/page.js:47
   ✔ When I go to the "start" page # step-definitions/page.js:15
   ✖ Then I should see "count" containing "Online now" # step-definitions/page.js:35
       Error: function timed out, ensure the promise resolves within 16000 milliseconds
           at Timeout._onTimeout (/Development/bdd-tests/node_modules/cucumber/src/user_code_runner.js:61:18)
           at ontimeout (timers.js:498:11)
           at tryOnTimeout (timers.js:323:5)
           at Timer.listOnTimeout (timers.js:290:5)
   ✔ After # step-definitions/common.js:12
       Attachment (application/json)
   ✔ After # step-definitions/common.js:6
       Attachment (image/png)
   ✔ After # node_modules/protractor-cucumber-framework/lib/resultsCapturer.js:25

See the Attachment (application/json) and Attachment (image/png)?

@DevSide
Copy link
Contributor

DevSide commented Sep 19, 2018

Sorry about that.
How and when do print/treat your logs ?
It's strange you get json and png types from browser logs, how do you do ?

@Izhaki
Copy link
Contributor Author

Izhaki commented Sep 19, 2018

We use multiple-cucumber-html-reporter. It saves a json report that it then parse and display in the UI.

// Attach screenshot to the scenario.
After(async function() {
  const png = await browser.takeScreenshot();
  return this.attach(png, 'image/png');
});

// Attach browser logs to the scenario.
After(async function() {
  const logs = await browser
    .manage()
    .logs()
    .get('browser');
  return this.attach(JSON.stringify(logs));
});

The default progress reporter is still useful since when something fails we can tell where/why without having to go to the html report page.

@goofballLogic
Copy link

Almost identical issue (duplicate #1160) which I am now closing. Note that I would prefer the ability to not just opt-in or opt-out but to have control over which attachments display and which do not (e.g. a tagging mechanism)

@goofballLogic
Copy link

At the moment we are hacking around the behaviour by specifying a custom content type (like "text/plain-large") to get the attachment ignored for CLI purposes.

@aslakhellesoy aslakhellesoy added ⚡ enhancement Request for new functionality 🙏 help wanted Help wanted - not prioritized by core team labels Feb 2, 2021
@aurelien-reeves
Copy link
Contributor

I am discovering that issue.

If I understand well, it is requested a new format options to disable step attachments?

Something like that:
--format-options '{"printStepAttachments": true|false}' which would result into, if set to "false", the formatters not printing the attachments?

@DevSide
Copy link
Contributor

DevSide commented Jul 7, 2021

It could also be an option in the attach function.
At the time, the function can have two arguments:

this.attach('Some info.') // Simple text
this.attach('{"some", "JSON"}}', 'application/json') // Typed data

Why not using the 2nd argument as a map of options (and keep backward compatibility for string param)

this.attach('{"some", "JSON"}}', { 
  mime: 'application/json',
  print: false, // This would disable logging attachments
}) 

That way, we could mix print and hidden attachements on the same run.

@aurelien-reeves
Copy link
Contributor

aurelien-reeves commented Jul 7, 2021

It could also be an option in the attach function.
At the time, the function can have two arguments:

this.attach('Some info.') // Simple text
this.attach('{"some", "JSON"}}', 'application/json') // Typed data

Why not using the 2nd argument as a map of options (and keep backward compatibility for string param)

this.attach('{"some", "JSON"}}', { 
  mime: 'application/json',
  print: false, // This would disable logging attachments
}) 

That way, we could mix print and hidden attachements on the same run.

That is interesting

I am more puzzled about it.

Why would we attach some data to not print those ?
If we don't want to print it, may we just avoid attaching it?

That may also lead in big reports embedding a lot of invisible attached data because it would be less easy to notice that some attachments are not printed out.

@DevSide
Copy link
Contributor

DevSide commented Jul 7, 2021

Yes, the purpose of attachments is to store information during the scenario run for two use cases:

  • used by the user to export data
  • used temporarily by cucumber to pass information to the formatter

The log function may be a better place to work on. It is based on attachment api, so the print option could be pass internally and avoid ambiguity.

@aurelien-reeves
Copy link
Contributor

I suggest I continue to work on the first global formatOption as it is a good topic for a tutorial for a future new contributor first contribution.

But your ideas are still interesting and we may continue discuss it, and then open a distinct PR once we have a better idea of what we may do.

What do you think?

@aurelien-reeves
Copy link
Contributor

A step by step tutorial has been added to #1721

You would like to contribute to cucumber, but don't really know where to begin? Take a look on the PR #1721!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team ⚡ enhancement Request for new functionality
Projects
None yet
6 participants