-
Notifications
You must be signed in to change notification settings - Fork 7
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
Annotation error while running via worker #5
Comments
That's useful to understand. The scope for each annotation is a problem, then. I'll look into it. |
So, a few observations.
The way the name is generated depends on cucumber; cucumber re-creates World for each scenario, which provides us with a pre-scenario hook. A pre-line hook would require writing a custom formatter, which is more fragile. Per scenario, annotations are an array. That allows for plenty of annotate() calls without overwriting data. So for this: Scenario: something # say it's line 10
Given I foo
Then I bar Then(/^I foo$/) do
annotate("foo")
end
Then(/^I bar$/) do
annotate("bar")
end The expectation would be that for the key referencing line 10, there is one annotation, and it's an array that is The test case actually shows the value to be |
Reconsidering, this is actually as expected. You can call annotate "foo", "bar"
annotate "baz" In which case the contents would be In other words, it's a record of the argument lists passed to the annotate function, and that makes sense for best future processing. |
FWIW: # This works:
bundle exec cucumber features/annotation.feature:15
# This contains the error above
bundle exec cucumber --format json features/annotation.feature:15 |
Alright. Long story short, the JSON formatter bundled with cucumber can't accept embeds in the first step of a scenario. I'll reduce the test case and file an issue upstream. Then we can also consider a workaround for our case. |
It's a little more complex. A test scenario for pure cucumber works. The JSON formatter has two hooks:
Turns out that in the simple cucumber test case, the above is the order in which they're called. For our LL test case, it's the other way around. |
It is reproducible with stock cucumber and the |
Do you have a link to the upstream bug or are you still creating testcases? |
Not yet. I called it a day shortly after figuring out where the problems lie. |
TBH I don't think they're going to fix this. The JSON output - even when you don't use Not specifying Basically, we have some options (if you have more, let me know):
Neither of them really appeal. |
There's some good news. We don't actually need annotations, since screenshots are embedded in the JSON anyway. My best proposal currently is:
We could upload the entire JSON with screenshots, and let the reporting API sort it out, but real-time-ish reporting would be easier without that data. The downside is that the post-processing has to be in the same task, or use only local storage and be pinned to the same worker. |
Screenshots can be embedded, however this increases the JSON output with a few MB. Post-processing should remove the actual image data and replace it by an asset URL, but that is a generic annotation, however its outside cucumber. I like support for the vanilla cucumber screenshot embedding so that its easier to handle repositories that don't use LL |
Cucumber fix should be released in 1.3.20 https://github.com/cucumber/cucumber/blob/v1.3.x-bugfix/History.md |
I can confirm that embedding screenshots into JSON is not treated specially, i.e. this scenario also fails for screenshots. IMHO the most sensible solution is to require cucumber >= 1.3.20 with LL, once that has been released. |
cucumber-version independent choices possible
module for dealing with cucumber classes
Should work with cucumber 2.x, or 1.3.x when that is released. |
Logfile
Json embeddings for this scenario outline:
Decoded
So it also seems that line 16 is embedded with line 15
The text was updated successfully, but these errors were encountered: