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

(Ruby) Code for a fully embedded html screenshots fails when json format reporting enabled #651

Closed
Chuckv opened this issue Mar 12, 2014 · 12 comments

Comments

@Chuckv
Copy link

Chuckv commented Mar 12, 2014

We have the following code for creating a fully embedded screenshot in HTML format reports. We like this because you can pass around a single (abet large) file instead of having to send around an html report file plus the image files. If we enable JSON output format for reports (in addition to html) then the JSON reporting code fails with an error that it cannot locate a file. it seems to think we are sending it a filename, and not the actual file data.

(@browser is a watir-webdriver browser object, scenario_name is a string based on the name of the scenario or scenario-outline+example)
Works for HTML reports, fails if JSON reports enabled:

# create fully embedded (not just linked) screenshot
encoded_img = @browser.driver.screenshot_as(:base64)
embed("data:image/png;base64,#{encoded_img}",'image/png')

If we embed using a more standard way using the code below, then things work, but we have to pass around multiple files, which is a hassle

@browser.driver.save_screenshot("./results/#{scenario_name}_screenshot.png")
embed "./results/#{scenario_name}_screenshot.png", "image/png"

What I'd like is one of two things:

  1. fix the JSON report formatter so it works if the first code is used.
  2. if the first code is basically an abuse of the HTML report embed function, then can we have some kind of official option on embed that would create the same result as the first code, when code more like the second set is used.
@mattwynne
Copy link
Member

Let's try to fix the JSON formatter.

Is there any chance you could fork Cucumber and send us a PR (please use the 1.3.x branch to pull against) that reproduces this bug? That will make it much easier to fix it.

@Chuckv
Copy link
Author

Chuckv commented Mar 13, 2014

I'd try to work with the existing tests you have for embedding screenshots, but they don't actually use a real screenshot, you just have a file with like 'foo' in it or something.

But still, I'd think we could work with that code... I think.

Where is the code shown here https://www.relishapp.com/cucumber/cucumber/docs/json-output-formatter is that part of the cucumber project?

if so, then all you need to do is replace the two lines you have inside the "I embed a screenshot" step with the following

require 'base64'
screenshot = Base64.encode64('foo')
embed("data:image/png;base64,#{screenshot}",'image/png')

The only problem I see is that in your expected yaml file, you are looking for "Zm9v" as the data embedded in the YAML file, and when I look at the screenshot string produced above, it is "Zm9v\n"

@Chuckv
Copy link
Author

Chuckv commented Mar 13, 2014

Frankly the more I look at what that code I am using is doing, (can't claim credit for it, found it on someone's blog) I'm beginning to think that someone is abusing the HTML report code. I'll bet they just figured out that the code is creating a link that points to the filename specified, and discovered if they put the exact right thing into that string, (instead of the filename) that they could embed the entire thing into the report.

Maybe what we want is an official way to get the HTML formatter to do that for us, maybe a parameter on embed that would cause it to encode the contents of the file into base64 and embed it in the screenshot.

So I said all that, then discovered where I found the code and who posted it... it was here https://groups.google.com/forum/#!topic/cukes/EwOmLFtAFzs

So maybe I need to take back the 'abusing the html report' comment if one of the central people there at cuke HQ was telling folks to use that method for embedding screenshots

@Chuckv Chuckv closed this as completed Mar 13, 2014
@Chuckv
Copy link
Author

Chuckv commented Mar 13, 2014

stupid bad tester, hit wrong button

@Chuckv Chuckv reopened this Mar 13, 2014
@mattwynne
Copy link
Member

Who's that @aslakhellesoy guy again?

The JSON formatter is implemented in the Gherkin library.

@brasmusson
Copy link
Contributor

@mattwynne Yes, the JSON formatter is implemented in the Gherkin library, but the problem seems to be in the gherkin_formatter_adapter.rb, it is there the embed method assumes that a file is passed. The JSON formatter in the Gherkin library will store whetever data it receives by the embed method in the JSON file (see the spec).
(using old browser, my appologies for any bad markdown)

@Chuckv
Copy link
Author

Chuckv commented May 5, 2014

So it looks like we know where the problem is, any chance of a fix any time soon?

@mattwynne
Copy link
Member

If someone could send a PR that modifies https://github.com/cucumber/cucumber/blob/master/features/docs/formatters/json_formatter.feature with a failing test that describes what you want, that would be of great help to us.

@brasmusson
Copy link
Contributor

I took a stab at fixing this issue in #695 and #696. The "Let's try to fix the JSON formatter" part means that the protocol of the embed method available to step definitions is extended from:

embed "<filepath>", "<mime-type>"

to

embed "<filepath>", "<mime-type>"
embed <base64 encoded data>, "<mime-type>;base64"

The ";base64" part in the second form is important, since the gherkin JSON formatter applies base64 encoding on the data it is sent, so it is necessary to decode the base64 encoded data given as first argument to the embed method before passing it on the the gherkin JSON formatter.

This would be the preferred syntax for embedding screen shots into the HTML file (and the JSON file if also including that formatter):

encoded_img = @browser.driver.screenshot_as(:base64)
embed(encoded_img,'image/png;base64')

The trick:

encoded_img = @browser.driver.screenshot_as(:base64)
embed("data:image/png;base64,#{encoded_img}",'image/png')

still works with the HTML formatter also after #696, but the JSON file would not get the appropriate content even after #695. The JSON formatter will not choke on a non existing file, but base64 encoding will be applied to all of the content of the first argument to the embed method, before all of is is inserted into the JSON file, when what you really want is that the screen shot is inserted into it.

@mattwynne
Copy link
Member

@Chuckv thanks for being patient. This should get released in cucumer-2.0.0.beta.2

@mattwynne mattwynne added this to the 2.0 milestone Aug 25, 2014
@brasmusson
Copy link
Contributor

Now when both #695 and #696 are included in cucumer-2.0.0.beta.2, I think this issue can be closed.

@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants