Cleanup save_* family of methods

* Better YARD documentation
* Make `path` argument of `save_screenshpt` `nil` by default
* Add `options` argument for `save_and_open_screenshot`

lib/capybara/helpers.rb

@@ -41,8 +41,8 @@ module Capybara
     #
     def inject_asset_host(html)
       if Capybara.asset_host && Nokogiri::HTML(html).css("base").empty? 
-         match = html.match(/<head[^<]*?>/)
-          html.clone.insert match.end(0), "<base href='#{Capybara.asset_host}' />"
+        match = html.match(/<head[^<]*?>/)
+        html.clone.insert match.end(0), "<base href='#{Capybara.asset_host}' />"
       else
         html
       end

lib/capybara/session.rb

@@ -616,54 +616,64 @@ module Capybara
 
     ##
     #
-    # Save a snapshot of the page.
+    # Save a snapshot of the page. If `Capybara.asset_host` is set it will inject `base` tag
+    #   pointing to `asset_host`.
     #
-    # @param  [String] path     The path to where it should be saved [optional]
+    # If invoked without arguments it will save file to `Capybara.save_and_open_page_path`
+    #   and file will be given randomly generated filename.
     #
-    def save_page(path=nil)
-      path ||= default_path('html')
-
-      FileUtils.mkdir_p(File.dirname(path))
-
-      File.open(path,'wb') { |f| f.write(Capybara::Helpers.inject_asset_host(body)) }
+    # @param [String] path  the path to where it should be saved
+    # @return [String]      the path to which the file was saved
+    #
+    def save_page(path = nil)
+      path = prepare_path(path, 'html')
+      File.write(path, Capybara::Helpers.inject_asset_host(body), mode: 'wb')
       path
     end
 
     ##
     #
-    # Save a snapshot of the page and open it in a browser for inspection
+    # Save a snapshot of the page and open it in a browser for inspection.
     #
-    # @param  [String] file_name  The path to where it should be saved [optional]
+    # If invoked without arguments it will save file to `Capybara.save_and_open_page_path`
+    #   and file will be given randomly generated filename.
     #
-    def save_and_open_page(file_name=nil)
-      file_name = save_page(file_name)
-      open_file(file_name)
+    # @param [String] path  the path to where it should be saved
+    #
+    def save_and_open_page(path = nil)
+      path = save_page(path)
+      open_file(path)
     end
 
     ##
     #
-    # Save a screenshot of page
+    # Save a screenshot of page.
     #
-    # @param  [String] path    A string of image path
-    # @option [Hash]   options Options for saving screenshot
-    def save_screenshot(path, options={})
-      path ||= default_path('png')
-
-      FileUtils.mkdir_p(File.dirname(path))
-
+    # If invoked without `path` argument it will save file to `Capybara.save_and_open_page_path`
+    #   and file will be given randomly generated filename.
+    #
+    # @param [String] path    the path to where it should be saved
+    # @param [Hash] options   a customizable set of options
+    # @return [String]        the path to which the file was saved
+    def save_screenshot(path = nil, options = {})
+      path = prepare_path(path, 'png')
       driver.save_screenshot(path, options)
       path
     end
 
     ##
     #
-    # Save a screenshot of the page and open it for inspection
+    # Save a screenshot of the page and open it for inspection.
     #
-    # @param  [String] file_name  The path to where it should be saved [optional]
+    # If invoked without `path` argument it will save file to `Capybara.save_and_open_page_path`
+    #   and file will be given randomly generated filename.
     #
-    def save_and_open_screenshot(file_name=nil)
-      file_name = save_screenshot(file_name)
-      open_file(file_name)
+    # @param [String] path    the path to where it should be saved
+    # @param [Hash] options   a customizable set of options
+    #
+    def save_and_open_screenshot(path = nil, options = {})
+      path = save_screenshot(path, options)
+      open_file(path)
     end
 
     def document
@@ -693,16 +703,22 @@ module Capybara
 
   private
 
-    def open_file(file_name)
+    def open_file(path)
       begin
         require "launchy"
-        Launchy.open(file_name)
+        Launchy.open(path)
       rescue LoadError
-        warn "File saved to #{file_name}."
+        warn "File saved to #{path}."
         warn "Please install the launchy gem to open the file automatically."
       end
     end
 
+    def prepare_path(path, extension)
+      path = default_path(extension) if path.nil?
+      FileUtils.mkdir_p(File.dirname(path))
+      path
+    end
+
     def default_path(extension)
       timestamp = Time.new.strftime("%Y%m%d%H%M%S")
       path = "capybara-#{timestamp}#{rand(10**10)}.#{extension}"

lib/capybara/spec/session/save_and_open_page_spec.rb

@@ -0,0 +1,19 @@
+require 'launchy'
+
+Capybara::SpecHelper.spec '#save_and_open_page' do
+  before do
+    @session.visit '/foo'
+  end
+
+  after do
+    Dir.glob("capybara-*.html").each do |file|
+      FileUtils.rm(file)
+    end
+  end
+
+  it "sends open method to launchy" do
+    allow(Launchy).to receive(:open)
+    @session.save_and_open_page
+    expect(Launchy).to have_received(:open).with(/capybara-\d+\.html/)
+  end
+end

lib/capybara/spec/session/save_page_spec.rb

@@ -20,9 +20,8 @@ Capybara::SpecHelper.spec '#save_page' do
 
   it "generates a sensible filename" do
     @session.save_page
-    path = Dir.glob("capybara-*.html").first
-    filename = path.split("/").last
-    expect(filename).to match /^capybara-\d+\.html$/
+    filename = Dir.glob("capybara-*.html").first
+    expect(filename).to match(/^capybara-\d+\.html$/)
   end
 
   it "can store files in a specified directory" do
@@ -68,6 +67,7 @@ Capybara::SpecHelper.spec '#save_page' do
       path = @session.save_page
 
       result = File.read(path)
+      expect(result).to include('<html')
       expect(result).not_to include("http://example.com")
     end
 
@@ -76,6 +76,7 @@ Capybara::SpecHelper.spec '#save_page' do
       path = @session.save_page
 
       result = File.read(path)
+      expect(result).to include('<html')
       expect(result).not_to include("http://example.com")
     end
   end

lib/capybara/spec/session/save_screenshot_spec.rb

@@ -0,0 +1,23 @@
+Capybara::SpecHelper.spec '#save_screenshot', requires: [:screenshot] do
+  before do
+    @session.visit '/foo'
+  end
+
+  it "generates sensible filename" do
+    allow(@session.driver).to receive(:save_screenshot)
+
+    @session.save_screenshot
+
+    regexp = Regexp.new(File.expand_path('capybara-\d+\.png'))
+    expect(@session.driver).to have_received(:save_screenshot).with(regexp, {})
+  end
+
+  it "allows to specify another path" do
+    allow(@session.driver).to receive(:save_screenshot)
+
+    custom_path = 'screenshots/1.png'
+    @session.save_screenshot(custom_path)
+
+    expect(@session.driver).to have_received(:save_screenshot).with(custom_path, {})
+  end
+end