Dont always display inline screenshots in system testing (#28133) #28185
Conversation
| output_type ||= "simple" if ENV["CI"] | ||
|
|
||
| # Default | ||
| output_type ||= "inline" |
There was a problem hiding this comment.
Can you move output type to a method? I also think RAILS_SYSTEM_TESTING_SCREENSHOTS_OUTPUT is a really long env var so maybe something like RAILS_SYSTEM_TESTING_SCREENSHOT
There was a problem hiding this comment.
Done in the last commit
| end | ||
|
|
||
| message |
There was a problem hiding this comment.
I can't test this locally right now so maybe I'm reading it wrong - but it seems like this no longer outputs the path to the image on failure? I use the tmp path a lot when developing locally to open and look at the screenshot.
There was a problem hiding this comment.
It does, I moved it to line 41. I needed to do this because I wanted to avoid puts display_message if display_message, so now display_message returns the whole message to be displayed.
For example:
[Screenshot]: tmp/screenshots/failure_test_home_page.png
<inline image if enabled>
renchap
force-pushed
the
system-testing-screenshot-output
branch
from
409b46a
to
3ecc3d9
Compare
|
@eileencodes I adressed your comments, could you please review? |
| @@ -38,14 +37,32 @@ def save_image | |||
| page.save_screenshot(Rails.root.join(image_path)) | |||
| end | |||
|
|
|||
| def output_type | |||
| # Environment variables have priority | |||
| output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] || ENV["CAPYBARA_INLINE_SCREENSHOT"] | |||
There was a problem hiding this comment.
This PR is looking good. I think however this does need to be documented in the take_screenshot method so folks know how to use these environment variables.
3 output types are supported: - simple: only display the screenshot path - artifact: display the screenshot in the terminal, using the artifact protocol (supported by some CI) - inline (default): display the screenshot in the terminal, inline (supported by some terminals) You can force the output type by setting the `RAILS_SYSTEM_TESTING_SCREENSHOT` environment variable
renchap
force-pushed
the
system-testing-screenshot-output
branch
from
3ecc3d9
to
79435c0
Compare
|
@eileencodes here you go, with documentation added to the |
|
Thanks for working on this @renchap! |
Fixes #28133
3 output types are supported:
You can force the output type by setting the
RAILS_SYSTEM_TESTING_SCREENSHOTS_OUTPUTenvironment variableI am not sure if the documentation should be updated.
Also inline screenshot (http://iterm2.com/documentation-images.html) is only supported by a few terminals. On a mac, Terminal.app displays an empty line and iTerm displays the inline image. Some terminals might display the Base64-encoded screenshot instead. I dont know if we want to add some kind of detection or this is even possible (without having a whitelist of
TERM_PROGRAMvalues).