common icon indicating copy to clipboard operation
common copied to clipboard

Uniform method names for attachments and logs

Open vincent-psarga opened this issue 5 years ago • 11 comments

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.

  • [x] ruby: embed-> attach
  • [x] 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)
  • [x] Java: embed-> attach
  • [x] 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 ?

vincent-psarga avatar Feb 12 '20 12:02 vincent-psarga

Sounds good.

mpkorstanje avatar Feb 12 '20 17:02 mpkorstanje

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?

charlierudolph avatar Feb 14 '20 16:02 charlierudolph

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 ?

vincent-psarga avatar Feb 15 '20 08:02 vincent-psarga

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?

aslakhellesoy avatar Feb 15 '20 20:02 aslakhellesoy

@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)

vincent-psarga avatar Feb 17 '20 09:02 vincent-psarga

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).

aslakhellesoy avatar Feb 17 '20 17:02 aslakhellesoy

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.

vincent-psarga avatar Feb 18 '20 10:02 vincent-psarga

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.

mpkorstanje avatar Feb 18 '20 11:02 mpkorstanje

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.

aslakhellesoy avatar Feb 19 '20 08:02 aslakhellesoy

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[bot] avatar Apr 23 '20 05:04 stale[bot]

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[bot] avatar Jun 22 '20 13:06 stale[bot]

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

luke-hill avatar Sep 05 '23 11:09 luke-hill

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

davidjgoss avatar Sep 05 '23 11:09 davidjgoss