Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only reset sessions if they've been used? #632

Closed
jdelStrother opened this issue Feb 3, 2012 · 1 comment
Closed

Only reset sessions if they've been used? #632

jdelStrother opened this issue Feb 3, 2012 · 1 comment

Comments

@jdelStrother
Copy link
Contributor

Hi,

I introduced some :js specs into our spec suite recently and found our spec run times increased way more than I'd expect. After looking into it, I think it's caused by Capybara.reset_sessions! being a bit over-eager. For example, if I run a :js test (resulting in capybara adding a Selenium session to its session_pool), any subsequent controller specs (which will call session.reset! on all session_pool sessions) result in Selenium clearing cookies and visiting about:blank, on each individual example, despite them using the regular Rack driver rather than the selenium one.

How about keeping track of which sessions have been accessed and only resetting those ones at the end of each example? AFAICT, the only public way to access a session is via Capybara.current_session, in which case something like this would be sufficient -

diff --git a/lib/capybara.rb b/lib/capybara.rb
index f1203de..73a8fd9 100644
--- a/lib/capybara.rb
+++ b/lib/capybara.rb
@@ -248,7 +248,17 @@ module Capybara
     # @return [Capybara::Session]     The currently used session
     #
     def current_session
-      session_pool["#{current_driver}:#{session_name}:#{app.object_id}"] ||= Capybara::Session.new(current_driver, app)
+      session = (session_pool["#{current_driver}:#{session_name}:#{app.object_id}"] ||= Capybara::Session.new(current_driver, app))
+      sessions_to_reset << session
+      session
+    end
+
+    ##
+    #
+    # The list of sessions that will need resetting next time reset_sessions is called
+    #
+    def sessions_to_reset
+      @sessions_to_reset ||= Set.new
     end

     ##
@@ -257,7 +267,8 @@ module Capybara
     # as cookies.
     #
     def reset_sessions!
-      session_pool.each { |mode, session| session.reset! }
+      sessions_to_reset.each{ |session| session.reset! }
+      sessions_to_reset.clear
     end
     alias_method :reset!, :reset_sessions!

This cuts our 4-minute spec-suite back down to around 3 minutes, or a saving of nearly 0.1s per example. WDYT?

@jnicklas
Copy link
Collaborator

jnicklas commented Feb 9, 2012

I've noticed this problem as well, will take a look at it!

@lock lock bot locked and limited conversation to collaborators Aug 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants