Capybara.using_session should use using_session_with_screenshot automatically

[TylerRick]

@trusche was kind enough in to add a note in the Readme (in #226) about how you need to use Capybara.using_session_with_screenshot instead of Capybara.using_session if you want it to correctly take a screenshot using the correct session. (Unfortunately I didn't see that note until I'd wasted an hour or so trying to figure out why it wasn't working. :) )

The thing is, though, that's just a workaround for the underlying bug: you shouldn't have to switch all your tests to use Capybara.using_session_with_screenshot instead of Capybara.using_session. It should just work.

I believe that was the original intention, too. Otherwise, why was there a full alias method chain in the original PR code (#91)?

alias_method :using_session, :using_session_with_screenshot

Also, the tests demonstrate that it should work (and does work) using using_session:

# spec/unit/capybara_spec.rb

    it 'saves the name of the final session' do
      expect(Capybara::Screenshot).to receive(:final_session_name=).with(:different_session)
      expect {
        using_session :different_session do
          expect(0).to eq 1
        end
      }.to raise_exception ::RSpec::Expectations::ExpectationNotMetError        
    end

and

# spec/unit/capybara_spec.rb:23:

    it 'saves a screenshot for the correct session for failures using_session' do
      run_failing_case <<-RUBY, %r{Unable to find (visible )?link or button "you'll never find me"}
        feature 'screenshot with failure' do
          scenario 'click on a missing link' do
            visit '/'
            expect(page.body).to include('This is the root page')
            using_session :different_session do
              visit '/different_page'
              expect(page.body).to include('This is a different page')
              click_on "you'll never find me"
            end
          end
        end
      RUBY
      expect('tmp/screenshot.html').to have_file_content(/is/)
    end

So how/why did it work in the tests but not in an actual Rails app?? I even made a simple demo app to prove that it wasn't anything specific about my app's configuration/gemset.

What the problem was

Using the trusty byebug debugger and some putss, I finally figured out what the difference was. Here's what I found...

I discovered that while the source location of plain old using_session was our version from the capybara-screenshot gem, Capybara.using_session was still the original version from the capybara gem:

         method(:using_session).source_location[0]=".../capybara-screenshot-1.0.22/lib/capybara-screenshot/capybara.rb"
Capybara.method(:using_session).source_location[0]=".../capybara-3.17.0/lib/capybara.rb

Everything actually works fine if you just change Capybara.using_session to bare word using_session. Then it actually does use the version defined in DSL module.

But we should be able to fix it so Capybara.using_session works too. We just can't do so (override it) from within the DSL module (see section below for demonstration).

All the using_session method in DSL does is simply delegate to the Capybara.using_session class method:

    def using_session(name_or_session, &block)
      Capybara.using_session(name_or_session, &block)
    end

So the Capybara.using_session class method is the one we need to override/extend, not the DSL using_session instance method that is just a wrapper for it. Then it will work from both entrypoints. PR coming soon which does that...

Aren't the methods in Capybara::DSL added as class methods on Capybara?

"But!", you say, "Capybara gets extended with Capybara::DSL, so all instance methods from Capybara::DSL should be available as class methods on Capybara!"

And you would be correct. capybara-3.17.0/lib/capybara/dsl.rb does indeed have:

module Capybara
  module DSL
    # …
  end

  extend(Capybara::DSL)
end

The only problem is that capybara-3.17.0/lib/capybara.rb (where Capybara.using_session class method is defined) gets loaded after capybara-3.17.0/lib/capybara/dsl.rb extends Capybara with Capybara::DSL, which of course overrides/replaces our method with the one in lib/capybara.rb:

# defining_class_methods_overrides_class_methods_added_by_extend.rb

module Capybara
  module DSL
    def using_session_with_screenshot
      'version from DSL'
    end

    alias_method :using_session, :using_session_with_screenshot
  end

  extend(Capybara::DSL)
end

p Capybara.using_session # => "version from DSL"

module Capybara
  class << self
    def using_session
      'original version'
    end
  end
end

p Capybara.using_session_with_screenshot # => "version from DSL
p Capybara.using_session                 # => "original version"

That explains why Capybara.using_session_with_screenshot was working but not Capybara.using_session.

But even if the order were reversed, I believe the result would be the same. I just learned today that extend apparently doesn't override class methods that are already defined, even though the extend happens later:

# extend_doesnt_override_existing_class_methods.rb 

module Capybara
  class << self
    def using_session
      'original version'
    end
  end

  module DSL
    def using_session
      'version from DSL'
    end
  end

  extend(Capybara::DSL)
end

p Capybara.using_session # => "original version"