Deprecate save_and_open_page_path in favor of save_path

History.md

@@ -19,6 +19,8 @@ Release date: unreleased
 * :option selector supports :selected and :disabled filters [Thomas Walpole]
 * Element#matches_selector? and associated matchers (match_selector, match_css, etc) for comparing an element to a selector [Thomas Walpole]
 * Deprecated Driver#browser_initialized? - Driver#reset! is required to be synchronous [Jonas Nicklas, Thomas Walpole]
+* Deprecated Capybara.save_and_open_page_path in favor of Capybara.save_path with slightly different behavior when using relative paths with
+  save_page/save_screenshot
 
 #Version 2.6.2
 Relase date: 2016-01-27

lib/capybara.rb

@@ -23,9 +23,10 @@ module Capybara
     attr_accessor :asset_host, :app_host, :run_server, :default_host, :always_include_port
     attr_accessor :server_port, :exact, :match, :exact_options, :visible_text_only
     attr_accessor :default_selector, :default_max_wait_time, :ignore_hidden_elements
-    attr_accessor :save_and_open_page_path, :wait_on_first_by_default, :automatic_reload
+    attr_accessor :save_path, :wait_on_first_by_default, :automatic_reload
     attr_accessor :reuse_server, :raise_server_errors, :server_errors
     attr_writer :default_driver, :current_driver, :javascript_driver, :session_name, :server_host
+    attr_reader :save_and_open_page_path
     attr_accessor :app
 
     ##
@@ -49,7 +50,7 @@ module Capybara
     # [default_max_wait_time = Numeric]   The maximum number of seconds to wait for asynchronous processes to finish (Default: 2)
     # [ignore_hidden_elements = Boolean]  Whether to ignore hidden elements on the page (Default: true)
     # [automatic_reload = Boolean]        Whether to automatically reload elements as Capybara is waiting (Default: true)
-    # [save_and_open_page_path = String]  Where to put pages saved through save_and_open_page (Default: Dir.pwd)
+    # [save_path = String]  Where to put pages saved through save_(page|screenshot), save_and_open_(page|screenshot) (Default: Dir.pwd)
     # [wait_on_first_by_default = Boolean]   Whether Node#first defaults to Capybara waiting behavior for at least 1 element to match (Default: false)
     # [reuse_server = Boolean]  Reuse the server thread between multiple sessions using the same app object (Default: true)
     # === DSL Options
@@ -379,6 +380,12 @@ module Capybara
       self.default_max_wait_time = t
     end
 
+    def save_and_open_page_path=(path)
+      warn "DEPRECATED: #save_and_open_page_path is deprecated, please use #save_path instead. \m"\
+           "Note: behavior is slightly different with relative paths - see documentation" unless path.nil?
+      @save_and_open_page_path = path
+    end
+
     def included(base)
       base.send(:include, Capybara::DSL)
       warn "`include Capybara` is deprecated. Please use `include Capybara::DSL` instead."

lib/capybara/rails.rb

@@ -12,7 +12,7 @@ Capybara.app = Rack::Builder.new do
   end
 end.to_app
 
-Capybara.save_and_open_page_path = Rails.root.join('tmp/capybara')
+Capybara.save_path = Rails.root.join('tmp/capybara')
 
 # Override default rack_test driver to respect data-method attributes.
 Capybara.register_driver :rack_test do |app|

lib/capybara/session.rb

@@ -620,8 +620,11 @@ module Capybara
     # Save a snapshot of the page. If `Capybara.asset_host` is set it will inject `base` tag
     #   pointing to `asset_host`.
     #
-    # If invoked without arguments it will save file to `Capybara.save_and_open_page_path`
-    #   and file will be given randomly generated filename.
+    # If invoked without arguments it will save file to `Capybara.save_path`
+    #   and file will be given randomly generated filename. If invoked with a relative path
+    #   the path will be relative to `Capybara.save_path`, which is different from
+    #   the previous behavior with `Capybara.save_and_open_page_path` where the relative path was
+    #   relative to Dir.pwd
     #
     # @param [String] path  the path to where it should be saved
     # @return [String]      the path to which the file was saved
@@ -636,8 +639,11 @@ module Capybara
     #
     # Save a snapshot of the page and open it in a browser for inspection.
     #
-    # If invoked without arguments it will save file to `Capybara.save_and_open_page_path`
-    #   and file will be given randomly generated filename.
+    # If invoked without arguments it will save file to `Capybara.save_path`
+    #   and file will be given randomly generated filename. If invoked with a relative path
+    #   the path will be relative to `Capybara.save_path`, which is different from
+    #   the previous behavior with `Capybara.save_and_open_page_path` where the relative path was
+    #   relative to Dir.pwd
     #
     # @param [String] path  the path to where it should be saved
     #
@@ -650,8 +656,11 @@ module Capybara
     #
     # Save a screenshot of page.
     #
-    # If invoked without `path` argument it will save file to `Capybara.save_and_open_page_path`
-    #   and file will be given randomly generated filename.
+    # If invoked without arguments it will save file to `Capybara.save_path`
+    #   and file will be given randomly generated filename. If invoked with a relative path
+    #   the path will be relative to `Capybara.save_path`, which is different from
+    #   the previous behavior with `Capybara.save_and_open_page_path` where the relative path was
+    #   relative to Dir.pwd
     #
     # @param [String] path    the path to where it should be saved
     # @param [Hash] options   a customizable set of options
@@ -666,8 +675,11 @@ module Capybara
     #
     # Save a screenshot of the page and open it for inspection.
     #
-    # If invoked without `path` argument it will save file to `Capybara.save_and_open_page_path`
-    #   and file will be given randomly generated filename.
+    # If invoked without arguments it will save file to `Capybara.save_path`
+    #   and file will be given randomly generated filename. If invoked with a relative path
+    #   the path will be relative to `Capybara.save_path`, which is different from
+    #   the previous behavior with `Capybara.save_and_open_page_path` where the relative path was
+    #   relative to Dir.pwd
     #
     # @param [String] path    the path to where it should be saved
     # @param [Hash] options   a customizable set of options
@@ -715,15 +727,18 @@ module Capybara
     end
 
     def prepare_path(path, extension)
-      path = default_path(extension) if path.nil?
+      if Capybara.save_path || Capybara.save_and_open_page_path.nil?
+        path = File.expand_path(path || default_fn(extension), Capybara.save_path)
+      else
+        path = File.expand_path(default_fn(extension), Capybara.save_and_open_page_path) if path.nil?
+      end
       FileUtils.mkdir_p(File.dirname(path))
       path
     end
 
-    def default_path(extension)
+    def default_fn(extension)
       timestamp = Time.new.strftime("%Y%m%d%H%M%S")
       path = "capybara-#{timestamp}#{rand(10**10)}.#{extension}"
-      File.expand_path(path, Capybara.save_and_open_page_path)
     end
 
     def scopes

lib/capybara/spec/session/save_and_open_screenshot_spec.rb

@@ -26,8 +26,8 @@ Capybara::SpecHelper.spec '#save_and_open_screenshot' do
     @session.save_and_open_screenshot(custom_path)
 
     expect(@session.driver).to have_received(:save_screenshot).
-      with(custom_path, {})
-    expect(Launchy).to have_received(:open).with(custom_path)
+      with(/#{custom_path}$/, {})
+    expect(Launchy).to have_received(:open).with(/#{custom_path}$/)
   end
 
   context 'when launchy cannot be required' do

lib/capybara/spec/session/save_page_spec.rb

@@ -7,6 +7,7 @@ Capybara::SpecHelper.spec '#save_page' do
 
   after do
     Capybara.save_and_open_page_path = nil
+    Capybara.save_path = nil
     Dir.glob("capybara-*.html").each do |file|
       FileUtils.rm(file)
     end
@@ -26,7 +27,7 @@ Capybara::SpecHelper.spec '#save_page' do
   end
 
   it "can store files in a specified directory" do
-    Capybara.save_and_open_page_path = alternative_path
+    Capybara.save_path = alternative_path
     @session.save_page
     path = Dir.glob(alternative_path + "/capybara-*.html").first
     expect(File.read(path)).to include("Another World")
@@ -37,6 +38,20 @@ Capybara::SpecHelper.spec '#save_page' do
     expect(File.read("capybara-001122.html")).to include("Another World")
   end
 
+  it "can store files in a specified directory with a given filename" do
+    Capybara.save_path = alternative_path
+    @session.save_page("capybara-001133.html")
+    path = alternative_path + "/capybara-001133.html"
+    expect(File.read(path)).to include("Another World")
+  end
+
+  it "can store files in a specified directory with a given relative filename" do
+    Capybara.save_path = alternative_path
+    @session.save_page("tmp/capybara-001144.html")
+    path = alternative_path + "/tmp/capybara-001144.html"
+    expect(File.read(path)).to include("Another World")
+  end
+
   it "returns an absolute path in pwd" do
     result = @session.save_page
     path = File.expand_path(Dir.glob("capybara-*.html").first, Dir.pwd)
@@ -44,7 +59,7 @@ Capybara::SpecHelper.spec '#save_page' do
   end
 
   it "returns an absolute path in given directory" do
-    Capybara.save_and_open_page_path = alternative_path
+    Capybara.save_path = alternative_path
     result = @session.save_page
     path = File.expand_path(Dir.glob(alternative_path + "/capybara-*.html").first, alternative_path)
     expect(result).to eq(path)
@@ -89,4 +104,20 @@ Capybara::SpecHelper.spec '#save_page' do
       expect(result).to include("Bar")
     end
   end
+
+  context "with deprecated save_and_open_page_path" do
+    it "can store files in a specified directory" do
+      Capybara.save_and_open_page_path = alternative_path
+      @session.save_page
+      path = Dir.glob(alternative_path + "/capybara-*.html").first
+      expect(File.read(path)).to include("Another World")
+    end
+
+    it "returns an absolute path in given directory" do
+      Capybara.save_and_open_page_path = alternative_path
+      result = @session.save_page
+      path = File.expand_path(Dir.glob(alternative_path + "/capybara-*.html").first, alternative_path)
+      expect(result).to eq(path)
+    end
+  end
 end

lib/capybara/spec/session/save_screenshot_spec.rb

@@ -1,9 +1,16 @@
 # frozen_string_literal: true
 Capybara::SpecHelper.spec '#save_screenshot', requires: [:screenshot] do
+  let(:alternative_path) { File.join(Dir.pwd, "save_screenshot_tmp") }
   before do
     @session.visit '/foo'
   end
 
+  after do
+    Capybara.save_and_open_page_path = nil
+    Capybara.save_path = nil
+    FileUtils.rm_rf alternative_path
+  end
+
   it "generates sensible filename" do
     allow(@session.driver).to receive(:save_screenshot)
 
@@ -19,6 +26,28 @@ Capybara::SpecHelper.spec '#save_screenshot', requires: [:screenshot] do
     custom_path = 'screenshots/1.png'
     @session.save_screenshot(custom_path)
 
-    expect(@session.driver).to have_received(:save_screenshot).with(custom_path, {})
+    expect(@session.driver).to have_received(:save_screenshot).with(/#{custom_path}$/, {})
+  end
+
+  context "with Capybara.save_path" do
+    it "file is generated in the correct location" do
+      Capybara.save_path = alternative_path
+      allow(@session.driver).to receive(:save_screenshot)
+
+      @session.save_screenshot
+
+      regexp = Regexp.new(File.expand_path('capybara-\d+\.png', alternative_path))
+      expect(@session.driver).to have_received(:save_screenshot).with(regexp, {})
+    end
+
+    it "relative paths are relative to save_path" do
+      Capybara.save_path = alternative_path
+      allow(@session.driver).to receive(:save_screenshot)
+
+      custom_path = 'screenshots/2.png'
+      @session.save_screenshot(custom_path)
+
+      expect(@session.driver).to have_received(:save_screenshot).with(File.expand_path(custom_path, alternative_path), {})
+    end
   end
 end

spec/selenium_spec.rb

@@ -114,7 +114,7 @@ RSpec.describe Capybara::Session do
     end
 
     context "#fill_in with { clear: Array } fill_options" do
-      it 'should pass the array through to the element', tw: true do
+      it 'should pass the array through to the element' do
         #this is mainly for use with [[:ctrl, 'a'], :backspace] - however since that is platform dependant I'm testing with something less useful
         @session.visit('/form')
         @session.fill_in('form_first_name', :with => 'Harry',