Merge pull request #2318 from MSP-Greg/test-port-0

Test fixups, port 0 and timeout handling [changelog skip]

test/helper.rb

@@ -16,6 +16,7 @@ require "minitest/autorun"
 require "minitest/pride"
 require "minitest/proveit"
 require "minitest/stub_const"
+require "net/http"
 require_relative "helpers/apps"
 
 Thread.abort_on_exception = true
@@ -29,7 +30,6 @@ require "puma/detect"
 # Either takes a string to do a get request against, or a tuple of [URI, HTTP] where
 # HTTP is some kind of Net::HTTP request object (POST, HEAD, etc.)
 def hit(uris)
-  require "net/http"
   uris.map do |u|
     response =
       if u.kind_of? String
@@ -58,11 +58,27 @@ module TimeoutEveryTestCase
   class TestTookTooLong < Timeout::Error
   end
 
-  def time_it
-    t0 = Minitest.clock_time
-    ::Timeout.timeout(RUBY_ENGINE == 'ruby' ? 60 : 120, TestTookTooLong) { yield }
-  ensure
-    self.time = Minitest.clock_time - t0
+  def run
+    with_info_handler do
+      time_it do
+        capture_exceptions do
+          before_setup; setup; after_setup
+
+          # wrap timeout around test method only
+          ::Timeout.timeout(RUBY_ENGINE == 'ruby' ? 60 : 120, TestTookTooLong) {
+            self.send self.name
+          }
+        end
+
+        Minitest::Test::TEARDOWN_METHODS.each do |hook|
+          capture_exceptions do
+            self.send hook
+          end
+        end
+      end
+    end
+
+    Minitest::Result.from self # per contract
   end
 end
 

test/helpers/integration.rb

@@ -114,8 +114,8 @@ class TestIntegration < Minitest::Test
     s
   end
 
-  def read_body(connection)
-    Timeout.timeout(10) do
+  def read_body(connection, time_out = 10)
+    Timeout.timeout(time_out) do
       loop do
         response = connection.readpartial(1024)
         body = response.split("\r\n\r\n", 2).last

test/test_busy_worker.rb

@@ -15,7 +15,7 @@ class TestBusyWorker < Minitest::Test
   end
 
   def new_connection
-    TCPSocket.new('127.0.0.1', @server.connected_ports[0]).tap {|s| @ios << s}
+    TCPSocket.new('127.0.0.1', @port).tap {|s| @ios << s}
   rescue IOError
     Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
     retry
@@ -56,7 +56,7 @@ class TestBusyWorker < Minitest::Test
     @server = Puma::Server.new request_handler, Puma::Events.strings, **options
     @server.min_threads = options[:min_threads] || 0
     @server.max_threads = options[:max_threads] || 10
-    @server.add_tcp_listener '127.0.0.1', 0
+    @port = (@server.add_tcp_listener '127.0.0.1', 0).addr[1]
     @server.run
   end
 

test/test_events.rb

@@ -167,10 +167,9 @@ class TestEvents < Minitest::Test
     events = Puma::Events.strings
     server = Puma::Server.new app, events
 
-    server.add_tcp_listener host, port
+    port = (server.add_tcp_listener host, 0).addr[1]
     server.run
 
-    port = server.connected_ports[0]
     sock = TCPSocket.new host, port
     path = "/"
     params = "a"*1024*10

test/test_integration_cluster.rb

@@ -448,7 +448,7 @@ RUBY
     begin
       sleep delay
       s = connect "sleep#{sleep_time}", unix: unix
-      body = read_body(s)
+      body = read_body(s, 20)
       mutex.synchronize { replies << body }
     rescue Errno::ECONNRESET
       # connection was accepted but then closed

test/test_out_of_band_server.rb

@@ -19,7 +19,7 @@ class TestOutOfBandServer < Minitest::Test
   end
 
   def new_connection
-    TCPSocket.new('127.0.0.1', @server.connected_ports[0]).tap {|s| @ios << s}
+    TCPSocket.new('127.0.0.1', @port).tap {|s| @ios << s}
   rescue IOError
     Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
     retry
@@ -62,7 +62,7 @@ class TestOutOfBandServer < Minitest::Test
     @server = Puma::Server.new app, Puma::Events.strings, out_of_band: [oob], **options
     @server.min_threads = options[:min_threads] || 1
     @server.max_threads = options[:max_threads] || 1
-    @server.add_tcp_listener '127.0.0.1', 0
+    @port = (@server.add_tcp_listener '127.0.0.1', 0).addr[1]
     @server.run
   end
 

test/test_persistent.rb

@@ -23,10 +23,8 @@ class TestPersistent < Minitest::Test
       [status, @headers, @body]
     end
 
-    @port = UniquePort.call
-
     @server = Puma::Server.new @simple
-    @server.add_tcp_listener HOST, @port
+    @port = (@server.add_tcp_listener HOST, 0).addr[1]
     @server.max_threads = 1
     @server.run
 

test/test_puma_server.rb

@@ -6,7 +6,6 @@ class TestPumaServer < Minitest::Test
   parallelize_me!
 
   def setup
-    @port = 0
     @host = "127.0.0.1"
 
     @ios = []
@@ -24,7 +23,7 @@ class TestPumaServer < Minitest::Test
 
   def server_run(app: @app, early_hints: false)
     @server.app = app
-    @server.add_tcp_listener @host, @port
+    @port = (@server.add_tcp_listener @host, 0).addr[1]
     @server.early_hints = true if early_hints
     @server.run
   end
@@ -49,8 +48,7 @@ class TestPumaServer < Minitest::Test
   end
 
   def new_connection
-    port = @server.connected_ports[0]
-    TCPSocket.new(@host, port).tap {|sock| @ios << sock}
+    TCPSocket.new(@host, @port).tap {|sock| @ios << sock}
   end
 
   def test_proper_stringio_body
@@ -138,8 +136,7 @@ class TestPumaServer < Minitest::Test
     req = Net::HTTP::Get.new '/'
     req['HOST'] = 'example.com'
 
-    port = @server.connected_ports[0]
-    res = Net::HTTP.start @host, port do |http|
+    res = Net::HTTP.start @host, @port do |http|
       http.request(req)
     end
 
@@ -155,8 +152,7 @@ class TestPumaServer < Minitest::Test
     req['HOST'] = "example.com"
     req['X-FORWARDED-PROTO'] = "https,http"
 
-    port = @server.connected_ports[0]
-    res = Net::HTTP.start @host, port do |http|
+    res = Net::HTTP.start @host, @port do |http|
       http.request(req)
     end
 
@@ -1054,7 +1050,7 @@ EOF
   end
 
   def stub_accept_nonblock(error)
-    @server.add_tcp_listener @host, @port
+    @port = (@server.add_tcp_listener @host, 0).addr[1]
     io = @server.binder.ios.last
     accept_old = io.method(:accept_nonblock)
     accept_stub = -> do

test/test_puma_server_ssl.rb

@@ -54,7 +54,6 @@ class TestPumaServerSSL < Minitest::Test
 
   # yields ctx to block, use for ctx setup & configuration
   def start_server
-    @port = 0
     @host = "127.0.0.1"
 
     app = lambda { |env| [200, {}, [env['rack.url_scheme']]] }
@@ -75,10 +74,10 @@ class TestPumaServerSSL < Minitest::Test
 
     @events = SSLEventsHelper.new STDOUT, STDERR
     @server = Puma::Server.new app, @events
-    @ssl_listener = @server.add_ssl_listener @host, @port, ctx
+    @port = (@server.add_ssl_listener @host, 0, ctx).addr[1]
     @server.run
 
-    @http = Net::HTTP.new @host, @server.connected_ports[0]
+    @http = Net::HTTP.new @host, @port
     @http.use_ssl = true
     @http.verify_mode = OpenSSL::SSL::VERIFY_NONE
   end

test/test_rack_server.rb

@@ -35,8 +35,8 @@ class TestRackServer < Minitest::Test
   def setup
     @simple = lambda { |env| [200, { "X-Header" => "Works" }, ["Hello"]] }
     @server = Puma::Server.new @simple
-    @server.add_tcp_listener "127.0.0.1", 0
-
+    port = (@server.add_tcp_listener "127.0.0.1", 0).addr[1]
+    @tcp = "http://127.0.0.1:#{port}"
     @stopped = false
   end
 
@@ -55,7 +55,7 @@ class TestRackServer < Minitest::Test
 
     @server.run
 
-    hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"])
+    hit(["#{@tcp}/test"])
 
     stop
 
@@ -70,7 +70,7 @@ class TestRackServer < Minitest::Test
 
     big = "x" * (1024 * 16)
 
-    Net::HTTP.post_form URI.parse("http://127.0.0.1:#{ @server.connected_ports[0] }/test"),
+    Net::HTTP.post_form URI.parse("#{@tcp}/test"),
                  { "big" => big }
 
     stop
@@ -83,7 +83,7 @@ class TestRackServer < Minitest::Test
     @server.app = lambda { |env| input = env; @simple.call(env) }
     @server.run
 
-    hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test/a/b/c"])
+    hit(["#{@tcp}/test/a/b/c"])
 
     stop
 
@@ -100,7 +100,7 @@ class TestRackServer < Minitest::Test
 
     @server.run
 
-    hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"])
+    hit(["#{@tcp}/test"])
 
     stop
 
@@ -116,7 +116,7 @@ class TestRackServer < Minitest::Test
 
     @server.run
 
-    hit(["http://127.0.0.1:#{ @server.connected_ports[0] }/test"])
+    hit(["#{@tcp}/test"])
 
     stop
 

test/test_web_server.rb

@@ -24,8 +24,8 @@ class WebServerTest < Minitest::Test
   def setup
     @tester = TestHandler.new
     @server = Puma::Server.new @tester, Puma::Events.strings
-    @server.add_tcp_listener "127.0.0.1", 0
-
+    @port = (@server.add_tcp_listener "127.0.0.1", 0).addr[1]
+    @tcp = "http://127.0.0.1:#{@port}"
     @server.run
   end
 
@@ -34,14 +34,14 @@ class WebServerTest < Minitest::Test
   end
 
   def test_simple_server
-    hit(["http://127.0.0.1:#{@server.connected_ports[0]}/test"])
+    hit(["#{@tcp}/test"])
     assert @tester.ran_test, "Handler didn't really run"
   end
 
   def test_requests_count
     assert_equal @server.requests_count, 0
     3.times do
-      hit(["http://127.0.0.1:#{@server.connected_ports[0]}/test"])
+      hit(["#{@tcp}/test"])
     end
     assert_equal @server.requests_count, 3
   end
@@ -83,7 +83,7 @@ class WebServerTest < Minitest::Test
 
   def do_test(string, chunk)
     # Do not use instance variables here, because it needs to be thread safe
-    socket = TCPSocket.new("127.0.0.1", @server.connected_ports[0]);
+    socket = TCPSocket.new("127.0.0.1", @port);
     request = StringIO.new(string)
     chunks_out = 0
 
@@ -96,7 +96,7 @@ class WebServerTest < Minitest::Test
 
   def do_test_raise(string, chunk, close_after = nil)
     # Do not use instance variables here, because it needs to be thread safe
-    socket = TCPSocket.new("127.0.0.1", @server.connected_ports[0]);
+    socket = TCPSocket.new("127.0.0.1", @port);
     request = StringIO.new(string)
     chunks_out = 0