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

Uniform method names for attachments and logs #897

Closed
6 tasks done
vincent-psarga opened this issue Feb 12, 2020 · 13 comments
Closed
6 tasks done

Uniform method names for attachments and logs #897

vincent-psarga opened this issue Feb 12, 2020 · 13 comments
Labels
language: java language: javascript language: ruby 🧷 pinned Tells Stalebot not to close this issue ⚡ enhancement Request for new functionality

Comments

@vincent-psarga
Copy link
Contributor

vincent-psarga commented Feb 12, 2020

In order to get a consistent API for attaching/logging images and text, we should rename the corresponding methods to attach and log
Existing methods should be deprecated and removed in later major release.

  • ruby: embed-> attach
  • ruby: puts-> log
  • JavaScript: attach - prints a warning when called with a single string argument, tells to call log(string) instead.
  • JavaScript: attach(string) -> log(string)
  • Java: embed-> attach
  • Java: write-> log

All of the log implementation will delegate to attach with a media-type text/x.cucumber.log+plain

@charlierudolph @mpkorstanje what do you think ?

@mpkorstanje
Copy link
Contributor

Sounds good.

@charlierudolph
Copy link
Member

Hmm. What's the reason for having separate methods for attaching images / text? Why not have a single method and if given a string default to media type text/x.cucumber.log+plain?

@vincent-psarga
Copy link
Contributor Author

The idea is to have different methods has they have different purposes.
embed should be used to attach any data that would be interesting to understand the test status (screenshots, maybe console logs, well anything that could help the dev team see what went wrong)
log is more meant for debugging while implementing the scenarios/step definitions.

TLDR:

  • embed attach information about the system under test
  • log add information about the testing framework

Does that sound good ?

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented Feb 15, 2020

What's the reason for having separate methods for attaching images / text? Why not have a single method and if given a string default to media type text/x.cucumber.log+plain

That's exactly what I had in mind.

@vincent-psarga - I think you meant attach rather than embed.

I think a single polymorphic attach method would be sufficient. The only reason to provide a log alias would be that it's maybe a bit more idiomatic for text. We "log" text and "attach" media. I had this implementation in mind:

def log(text)
  raise "Can only log text" unless text.is_a?(String)
  this.attach(text)
end

@vincent-psarga what do you mean by adding information about the testing framework? Can you give an example?

@vincent-psarga
Copy link
Contributor Author

@vincent-psarga - I think you meant attach rather than embed.

Indeed :)

@vincent-psarga what do you mean by adding information about the testing framework? Can you give an example?

So, as I see it, log would be useful when writing/debugging the support code. Let's imagine we have this step:

Given the following items are available for sale:
  | name             | price |
  | strap cutter     | $25   |
  | groover          | $20   |
  | edge beveller #0 | $10   |

and I implement it this way(more or less, just to give an idea).

Then('the following items are available for sale:') do |name_and_prices|
  name_and_prices.each do |name, price|
    item = Item.find_by_name(name)
    item.update_attributes(price: price, for_sale: true)
  end
end

Now, I try this and it fails. As I'm pretty new to Cucumber, I want to know what exactly is the content of the name_and_prices object.
So what I'll do is log(name_and_prices.to_s). Is has no interest for people except me who is trying to implement the step definitions (and is likely to disappear once I have this up and running).

Then('the following items are available for sale:') do |name_and_prices|
  name_and_prices.each do |name, price|
    item = Item.find_by_name(name)
    item.update_attributes(price: price, for_sale: true)
  end
  on_sale = Items.where(for_sale: true).map { | item | "| #{item.name} | #{item.price} |" }.join("\n")
  attach(on_sale, 'text/plain')
end

I wouldn't use log here, the information is not meant for those who will implement the testing code, it is meant for people who will read the test results/living documentation (even if is is text too, it's a different audience).

So after writing this down, I would rephrase my precedent text like this:

  • attach add any information useful for people reading the test results/living documentation
  • log add information useful for the developers (in a broad meaning, QA engineers in there too, anyone writing support code)

@aslakhellesoy
Copy link
Contributor

So what I'll do is log(name_and_prices.to_s)

If you just want some debugging info printed to the console, couldn't you just use Kernel.puts (or console.log for JavaScript and System.out.println for Java).

@vincent-psarga - do you expect log to end up as Attachment messages, and ultimately in reports that support it, or do you just want it to end up in the console? (Hence my question above).

@vincent-psarga
Copy link
Contributor Author

I guess you're right. I thought it could be interesting not to polute the test reports with those informations but we can rely on built-in solutions (as puts and console.log)

And anyway, when developing the automation layer, I don't think people are going to use formatter such as json or html but simply the default console/progress one.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Feb 18, 2020

If you just want some debugging info printed to the console, couldn't you just use Kernel.puts (or console.log for JavaScript and System.out.println for Java).

This quickly loses value when used in combination with parallel execution because different executions will collide.

I'm currently attaching allot of debug information to reports so when a scenario fails its easy to see what exactly went wrong. Examples would be user and session ids, log correlation values, raw requests and responses, ect.

The most important feature is the ability to attach plain text and have it rendered as plain text by reporting tools. The naming log/embed/attach doesn't matter much.

@aslakhellesoy
Copy link
Contributor

Does anyone disagree with this summary?

  • log is an alias for attach (for text attachments)
  • attach does not print anything, just emits an Attachment message
  • Console formatters progress and pretty only output attachments with a text/plain compatible media type
  • Rich formatters (JSON, HTM) output all attachments (including text/plain)
  • The platform’s console logging can be used for single-threaded executions when the logged text is not wanted in formatter output
  • for parallel execution, the log method is preferred. The drawback is that logged text will be outputted by all formatters, which may not always be desired.

@stale
Copy link

stale bot commented Apr 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added ⌛ stale Will soon be closed by stalebot unless there is activity and removed ⌛ stale Will soon be closed by stalebot unless there is activity labels Apr 23, 2020
@stale
Copy link

stale bot commented Jun 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Jun 22, 2020
@mpkorstanje mpkorstanje added the 🧷 pinned Tells Stalebot not to close this issue label Jun 22, 2020
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Jun 22, 2020
@luke-hill
Copy link
Contributor

@davidjgoss from the OP did we ever get around to renaming the 2 key methods for JS?

@davidjgoss
Copy link
Contributor

JS methods are named attach and log respectively - so yeah I think we're done here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language: java language: javascript language: ruby 🧷 pinned Tells Stalebot not to close this issue ⚡ enhancement Request for new functionality
Projects
Status: No status
Development

No branches or pull requests

6 participants