Display pending requests (#2250)

* Backfill tests on pending requests behavior

* Implement parameterized wait time for wait_for_oending_requests

At some point, this might be worth being a top-level configuration.

* Re-implement counter mutex with a list of pending request envs

Set up for exposing more information to developers about pending requests.

* Expose pending requests in error message

This provides a bit more information to other developers about the state of their tests. Whether we include further information would require more feedback from other developers and how the community use the error message for debugging.

* Add safe fallback value to avoid passing `nil`

* Use a memory barrier on read operations for array

* Only store REQUEST_URI instead of env

* Revert back to fixed wait_time (60)

Keeping initial PR tidy and focused.

* Remove redundant attr reader

We already defined a getter for the value to ensure proper concurrent accessing.

* Ruby linter: single-line body for modifier if usage

* Ruby linter: single quotes

lib/capybara/server.rb

@@ -61,7 +61,7 @@ module Capybara
     def wait_for_pending_requests
       timer = Capybara::Helpers.timer(expire_in: 60)
       while pending_requests?
-        raise 'Requests did not finish in 60 seconds' if timer.expired?
+        raise "Requests did not finish in 60 seconds: #{middleware.pending_requests}" if timer.expired?
 
         sleep 0.01
       end

lib/capybara/server/middleware.rb

@@ -4,19 +4,25 @@ module Capybara
   class Server
     class Middleware
       class Counter
-        attr_reader :value
-
         def initialize
-          @value = 0
+          @value = []
           @mutex = Mutex.new
         end
 
-        def increment
+        def increment(uri)
-          @mutex.synchronize { @value += 1 }
+          @mutex.synchronize { @value.push(uri) }
         end
 
-        def decrement
+        def decrement(uri)
-          @mutex.synchronize { @value -= 1 }
+          @mutex.synchronize { @value.delete_at(@value.index(uri) || @value.length) }
+        end
+
+        def positive?
+          @mutex.synchronize { @value.length.positive? }
+        end
+
+        def value
+          @mutex.synchronize { @value }
         end
       end
 
@@ -31,8 +37,12 @@ module Capybara
         @server_errors = server_errors
       end
 
+      def pending_requests
+        @counter.value
+      end
+
       def pending_requests?
-        @counter.value.positive?
+        @counter.positive?
       end
 
       def clear_error
@@ -43,14 +53,14 @@ module Capybara
         if env['PATH_INFO'] == '/__identify__'
           [200, {}, [@app.object_id.to_s]]
         else
-          @counter.increment
+          @counter.increment(env['REQUEST_URI'])
           begin
             @extended_app.call(env)
           rescue *@server_errors => e
             @error ||= e
             raise e
           ensure
-            @counter.decrement
+            @counter.decrement(env['REQUEST_URI'])
           end
         end
       end

spec/server_spec.rb

@@ -239,6 +239,26 @@ RSpec.describe Capybara::Server do
     end
   end
 
+  it 'should raise an error when there are pending requests' do
+    app = proc do |env|
+      request = Rack::Request.new(env)
+      sleep request.params['wait_time'].to_f
+      [200, {}, ['Hello Server!']]
+    end
+
+    server = described_class.new(app).boot
+
+    expect do
+      start_request(server, 59.0)
+      server.wait_for_pending_requests
+    end.not_to raise_error
+
+    expect do
+      start_request(server, 61.0)
+      server.wait_for_pending_requests
+    end.to raise_error('Requests did not finish in 60 seconds: ["/?wait_time=61.0"]')
+  end
+
   it 'is not #responsive? when Net::HTTP raises a SystemCallError' do
     app = -> { [200, {}, ['Hello, world']] }
     server = described_class.new(app)