Refactor how we patch node classes to support driver with browser: :r…

…emote

CHANGELOG.md

@@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file.
 
 This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+# Unreleased
+
+- Add support for drivers with `browser: :remote`.
+
 
 # 2.2.0
 

lib/capybara-lockstep/capybara_ext.rb

@@ -3,36 +3,73 @@
 module Capybara
   module Lockstep
     module SynchronizeMacros
+      # Helper module to avoid unnecessary double synchronization in case we patched a class and a subclass, i.e.:
+      # Capybara::Selenium::ChromeNode#click should run Lockstep.auto_synchronize, but it also might super to
+      # Capybara::Selenium::Node#click which should not also run Lockstep.auto_synchronize.
+      # However, we have to patch both classes, in case Capybara::Selenium::Node is used directly.
+      module Nestable
+        def self.skip_nested_calls(identifier, &block)
+          thread_local_key = self.thread_local_key(identifier)
+          if Thread.current[thread_local_key]
+            yield
+          else
+            begin
+              Thread.current[thread_local_key] = true
+              yield
+            ensure
+              Thread.current[thread_local_key] = nil
+            end
+          end
+        end
+
+        def self.nested_call?(identifier)
+          Thread.current[thread_local_key(identifier)]
+        end
+
+        private
+
+        def self.thread_local_key(identifier)
+          :"Capybara::Lockstep_skip_#{identifier}"
+        end
+      end
+
+      def self.extended(by)
+        by.instance_eval do
+          prepend(@before_synchronize_module = Module.new)
+          prepend(@synchronize_after_module = Module.new)
+          prepend(@unsynchronize_after_module = Module.new)
+        end
+      end
 
       def synchronize_before(meth, lazy:)
-        mod = Module.new do
+        @before_synchronize_module.module_eval do
           define_method meth do |*args, &block|
-            Lockstep.auto_synchronize(lazy: lazy, log: "Synchronizing before ##{meth}")
-            super(*args, &block)
+            Lockstep.auto_synchronize(lazy: lazy, log: "Synchronizing before ##{meth}") unless Nestable.nested_call?(:synchronize_before)
+            Nestable.skip_nested_calls(:synchronize_before) do
+              super(*args, &block)
+            end
           end
 
           ruby2_keywords meth
         end
-
-        prepend(mod)
       end
 
       def synchronize_after(meth)
-        mod = Module.new do
+        @synchronize_after_module.module_eval do
           define_method meth do |*args, &block|
-            super(*args, &block)
+            Nestable.skip_nested_calls(:synchronize_after) do
+              super(*args, &block)
+            end
           ensure
-            Lockstep.auto_synchronize
+            Lockstep.auto_synchronize(log: "Synchronizing after ##{meth}") unless Nestable.nested_call?(:synchronize_after)
           end
 
           ruby2_keywords meth
         end
-
-        prepend(mod)
       end
 
       def unsynchronize_after(meth)
-        mod = Module.new do
+        @unsynchronize_after_module.module_eval do
           define_method meth do |*args, &block|
             super(*args, &block)
           ensure
@@ -41,10 +78,7 @@ def unsynchronize_after(meth)
 
           ruby2_keywords meth
         end
-
-        prepend(mod)
       end
-
     end
   end
 end
@@ -158,23 +192,23 @@ def synchronize_around_script_method(meth)
   synchronize_around_script_method :evaluate_async_script
 end
 
-# Capybara 3 has driver-specific Node classes which sometimes
-# super to Capybara::Selenium::Node, but not always.
-node_classes = [
+# In Capybara 3 there are the specialized classes for nodes for most browers.
+# We need to patch relevant methods on all of these.
+driver_specific_node_classes = [
   (Capybara::Selenium::ChromeNode  if defined?(Capybara::Selenium::ChromeNode)),
   (Capybara::Selenium::FirefoxNode if defined?(Capybara::Selenium::FirefoxNode)),
   (Capybara::Selenium::SafariNode  if defined?(Capybara::Selenium::SafariNode)),
   (Capybara::Selenium::EdgeNode    if defined?(Capybara::Selenium::EdgeNode)),
   (Capybara::Selenium::IENode      if defined?(Capybara::Selenium::IENode)),
-].compact
+].compact.freeze
 
-if node_classes.empty?
-  # Capybara 2 has no driver-specific Node implementations,
-  # so we patch the shared base class.
-  node_classes = [Capybara::Selenium::Node]
-end
+# For other browsers (like the :remote browser) we instead get a generic node class.
+# This is also the case for Capybara 2.
+generic_node_classes = [
+  Capybara::Selenium::Node,
+].freeze
 
-node_classes.each do |node_class|
+[*driver_specific_node_classes, *generic_node_classes].each do |node_class|
   node_class.class_eval do
     extend Capybara::Lockstep::SynchronizeMacros
 

spec/capybara_ext_spec.rb

@@ -0,0 +1,100 @@
+describe Capybara::Lockstep::SynchronizeMacros do
+  after do
+    # called after each example, messes with our expectations
+    allow(Capybara).to receive(:reset_sessions!)
+  end
+
+  let(:example_class) do
+    Class.new do
+      def do_something
+      end
+
+      def call_do_something
+        do_something
+      end
+    end
+  end
+
+  describe 'synchronize_before' do
+    let(:patched_class) do
+      Class.new(example_class) do
+        extend Capybara::Lockstep::SynchronizeMacros
+
+        synchronize_before :call_do_something, lazy: false
+      end
+    end
+
+    let(:patched_sub_class) do
+      Class.new(patched_class) do
+        extend Capybara::Lockstep::SynchronizeMacros
+
+        synchronize_before :call_do_something, lazy: false
+      end
+    end
+
+    it 'runs auto_synchronize before the method' do
+      object = patched_class.new
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      expect(object).to receive(:do_something).ordered
+      object.call_do_something
+    end
+
+    it 'runs it only once, even if we patch multiple classes in the class hierarchy' do
+      object = patched_sub_class.new
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      expect(object).to receive(:do_something).ordered
+      object.call_do_something
+    end
+  end
+
+  describe 'synchronize_after' do
+    let(:patched_class) do
+      Class.new(example_class) do
+        extend Capybara::Lockstep::SynchronizeMacros
+
+        synchronize_after :call_do_something
+      end
+    end
+
+    let(:patched_sub_class) do
+      Class.new(patched_class) do
+        extend Capybara::Lockstep::SynchronizeMacros
+
+        synchronize_after :call_do_something
+      end
+    end
+
+    it 'runs auto_synchronize before the method' do
+      object = patched_class.new
+      expect(object).to receive(:do_something).ordered
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      object.call_do_something
+    end
+
+    it 'runs it only once, even if we patch multiple classes in the class hierarchy' do
+      object = patched_sub_class.new
+      expect(object).to receive(:do_something).ordered
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      object.call_do_something
+    end
+  end
+
+  describe 'synchronize_before and synchronize_after' do
+    let(:patched_class) do
+      Class.new(example_class) do
+        extend Capybara::Lockstep::SynchronizeMacros
+
+        synchronize_before :call_do_something, lazy: false
+        synchronize_after :call_do_something
+      end
+    end
+
+    it 'runs auto_synchronize before and after the method' do
+      object = patched_class.new
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      expect(object).to receive(:do_something).ordered
+      expect(Capybara::Lockstep).to receive(:auto_synchronize).exactly(:once).ordered
+      object.call_do_something
+    end
+  end
+end