`#save_path` only works for pages, not screenshots

[f3ndot]

Unsure which gem is responsible for this behaviour: https://github.com/jnicklas/capybara/issues/1683

[f3ndot]

One of the contributors of capybara confirmed that the issue is with this gem:

capybara-screenshot passes absolute paths to #save_screenshot and has not been updated to use #save_path - If you call page.save_screenshot yourself you will see that it does use #save_path when passed a relative path (or nothing).

[mattheworiordan]

Thanks @f3ndot

[threadhead]

+1 would really like to see this merged

[sridgma]

+1 to see this merged, screenshots are placed up all over the directory

[mattheworiordan]

Ok will merge now, sorry for the delay chaps

[mattheworiordan]

I have merged, can you confirm this fixes your issue when using master and I will tag & release a new version if so

[f3ndot]

It appears to now nest:

Capybara.save_path = 'foo/bar'

The .png screenshot shows up in ./foo/bar whereas the .html ends up in ./foo/bar/foo/bar/

What I meant by "insufficient' with my PR, is that there are other references to save_and_open_page_path in this gem and wanted to ensure all of it was refactored to the new, undeprecated call.

[jessebye]

@mattheworiordan I merged in the fix and it works great for absolute paths, however relative paths have the issue as described by @f3ndot.

[mattheworiordan]

@f3ndot not had time to look at this, given you fixed this issue partially, any chance you have time for a full fix. If not, I will do my best to make some time to fix in the next few days.

[mattheworiordan]

Any one here able to help out with a fix? I will merge in quickly, but am just out of time recently so unfortunately not been able to fix this for you guys.

[nruth]

Perhaps I missed something, but it looks like the path code could be deleted from this gem now? Likely means a major version bump though?

[dylanlive]

I dug into the .html nesting issue described a few posts up. When calling capybara.save_page, after Capybara 2.7, it's passing a path that becomes relative to what a user sets in Capybara.save_path

That said, I concur with the above that paths don't seem necessary anymore.

However, I was intrigued why save_screenshot didn't have the same issue.

save_screenshot looks to call the Selenium Webdriver method, while save_html calls the Capybara method.

I'm not sure the history as to why the save_screenshot method for Capybara wouldn't be used. I think it would be clearer if it were consistent.

I can look into opening a PR. Modifying the path being returned for html_path to just a filename would be easy (basing on Capybara version), but I'm curious around the history of not using Capybara for saving screenshots. Does anyone know?

Creating differing paths between html and screenshot seems confusing.

[romikoops]

Monkey patched temporary until a good fix

require 'capybara-screenshot/saver'
module Capybara
  module Screenshot
    class Saver
      # Bug https://github.com/mattheworiordan/capybara-screenshot/issues/164
      def save_html
        path = html_path
        clear_save_and_open_page_path do
          if Capybara::VERSION.match(/^\d+/)[0] == '1'
            capybara.save_page(page.body, path.to_s)
          else
            capybara.save_page("#{file_base_name}.html")
          end
        end
        @html_saved = true
      end
    end
  end
end

[mattheworiordan]

I can look into opening a PR. Modifying the path being returned for html_path to just a filename would be easy (basing on Capybara version), but I'm curious around the history of not using Capybara for saving screenshots. Does anyone know?

I can't recall unfortunately. It's plausible it was a mistake, or at the time, due to this Gem being around for so long, that that option was more predictable. Of course as gems have moved on and not provided the same behaviour, we're now seeing odd things happening.

I would love to see a PR :)

@romikoops thanks for the monkey patch input.

[romikoops]

I will do PR these week. I need a little bit time to review all code and dependencies in order to support compatibility with old version capybara gems, frameworks like rails, etc

[mattheworiordan]

Thanks @romikoops, appreciated

[mattheworiordan]

hey @romikoops, any update?

[lokki007]

Any updates?

[lokki007]

My workaround was instead of doing this:

Capybara.save_and_open_page_path = File.join(Integrator.root, 'tmp', 'failures')
Capybara::Screenshot.register_filename_prefix_formatter(:cucumber) do |scenario|
  scenario.title.gsub(/\s+/, '_')
end

I simply do this:

Capybara::Screenshot.register_filename_prefix_formatter(:cucumber) do |scenario|
  scenario.title.gsub(/\s+/, '_')
  'tmp/failures/"+scenario.title.gsub(/\s+/, '_')

end

This one does not require any "save_path" method and works perfectly for storing both html and jpeg into a folder I needed. Hope it helps