-
-
Notifications
You must be signed in to change notification settings - Fork 691
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
Comments
Sounds good. |
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 |
The idea is to have different methods has they have different purposes. TLDR:
Does that sound good ? |
That's exactly what I had in mind. @vincent-psarga - I think you meant I think a single polymorphic 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? |
Indeed :)
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 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 So after writing this down, I would rephrase my precedent text like this:
|
If you just want some debugging info printed to the console, couldn't you just use @vincent-psarga - do you expect |
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 And anyway, when developing the automation layer, I don't think people are going to use formatter such as |
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. |
Does anyone disagree with this summary?
|
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. |
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. |
@davidjgoss from the OP did we ever get around to renaming the 2 key methods for JS? |
JS methods are named |
In order to get a consistent API for attaching/logging images and text, we should rename the corresponding methods to
attach
andlog
Existing methods should be deprecated and removed in later major release.
embed
->attach
puts
->log
attach
- prints a warning when called with a single string argument, tells to calllog(string)
instead.attach(string)
->log(string)
embed
->attach
write
->log
All of the
log
implementation will delegate to attach with a media-typetext/x.cucumber.log+plain
@charlierudolph @mpkorstanje what do you think ?
The text was updated successfully, but these errors were encountered: