Add tests to binder (#2156)

+ Make loopback_addresses private

+ Semi-bugfix: connected ports are unique

You can bind to the same port twice in ipv4 and ipv6, which probably wasnt what we intended to have those show up twice in the list

lib/puma/binder.rb

@@ -43,7 +43,8 @@ module Puma
       @ios = []
     end
 
-    attr_reader :ios, :listeners, :unix_paths, :proto_env, :envs
+    attr_reader :ios, :listeners, :unix_paths, :proto_env, :envs, :activated_sockets, :inherited_fds
+    attr_writer :ios, :listeners
 
     def env(sock)
       @envs.fetch(sock, @proto_env)
@@ -204,12 +205,6 @@ module Puma
       end
     end
 
-    def loopback_addresses
-      Socket.ip_address_list.select do |addrinfo|
-        addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback?
-      end.map { |addrinfo| addrinfo.ip_address }.uniq
-    end
-
     # Tell the server to listen on host +host+, port +port+.
     # If +optimize_for_latency+ is true (the default) then clients connecting
     # will be optimized for latency over throughput.
@@ -238,7 +233,7 @@ module Puma
     end
 
     def connected_ports
-      ios.map { |io| io.addr[1] }
+      ios.map { |io| io.addr[1] }.uniq
     end
 
     def inherit_tcp_listener(host, port, fd)
@@ -361,12 +356,12 @@ module Puma
     end
 
     def close_listeners
-      @listeners.each do |l, io|
+      listeners.each do |l, io|
         io.close unless io.closed? # Ruby 2.2 issue
         uri = URI.parse(l)
         next unless uri.scheme == 'unix'
         unix_path = "#{uri.host}#{uri.path}"
-        File.unlink unix_path if @unix_paths.include? unix_path
+        File.unlink unix_path if unix_paths.include? unix_path
       end
     end
 
@@ -378,5 +373,13 @@ module Puma
       end
       redirects
     end
+
+    private
+
+    def loopback_addresses
+      Socket.ip_address_list.select do |addrinfo|
+        addrinfo.ipv6_loopback? || addrinfo.ipv4_loopback?
+      end.map { |addrinfo| addrinfo.ip_address }.uniq
+    end
   end
 end

test/test_binder.rb

@@ -23,18 +23,44 @@ class TestBinderBase < Minitest::Test
 end
 
 class TestBinder < TestBinderBase
+  parallelize_me!
+
   def test_localhost_addresses_dont_alter_listeners_for_tcp_addresses
-    @binder.parse ["tcp://localhost:10001"], @events
+    @binder.parse ["tcp://localhost:0"], @events
 
     assert_empty @binder.listeners
   end
 
+  def test_home_alters_listeners_for_tcp_addresses
+    port = UniquePort.call
+    @binder.parse ["tcp://127.0.0.1:#{port}"], @events
+
+    assert_equal "tcp://127.0.0.1:#{port}", @binder.listeners[0][0]
+    assert_kind_of TCPServer, @binder.listeners[0][1]
+  end
+
+  def test_connected_ports
+    ports = (1..3).map { |_| UniquePort.call }
+
+    @binder.parse(ports.map { |p| "tcp://localhost:#{p}" }, @events)
+
+    assert_equal ports, @binder.connected_ports
+  end
+
   def test_localhost_addresses_dont_alter_listeners_for_ssl_addresses
-    @binder.parse ["ssl://localhost:10002?#{ssl_query}"], @events
+    @binder.parse ["ssl://localhost:0?#{ssl_query}"], @events
 
     assert_empty @binder.listeners
   end
 
+  def test_home_alters_listeners_for_ssl_addresses
+    port = UniquePort.call
+    @binder.parse ["ssl://127.0.0.1:#{port}?#{ssl_query}"], @events
+
+    assert_equal "ssl://127.0.0.1:#{port}?#{ssl_query}", @binder.listeners[0][0]
+    assert_kind_of TCPServer, @binder.listeners[0][1]
+  end
+
   def test_correct_zero_port
     @binder.parse ["tcp://localhost:0"], @events
 
@@ -59,7 +85,7 @@ class TestBinder < TestBinderBase
     @binder.parse ["tcp://localhost:0"], @events
 
     assert_match %r!tcp://127.0.0.1:(\d+)!, @events.stdout.string
-    if @binder.loopback_addresses.include?("::1")
+    if Socket.ip_address_list.any? {|i| i.ipv6_loopback? }
       assert_match %r!tcp://\[::1\]:(\d+)!, @events.stdout.string
     end
   end
@@ -69,7 +95,7 @@ class TestBinder < TestBinderBase
     @binder.parse ["ssl://localhost:0?#{ssl_query}"], @events
 
     assert_match %r!ssl://127.0.0.1:(\d+)!, @events.stdout.string
-    if @binder.loopback_addresses.include?("::1")
+    if Socket.ip_address_list.any? {|i| i.ipv6_loopback? }
       assert_match %r!ssl://\[::1\]:(\d+)!, @events.stdout.string
     end
   end
@@ -156,6 +182,54 @@ class TestBinder < TestBinderBase
     assert_equal @events.stderr, env_hash["rack.errors"]
   end
 
+  def test_close_calls_close_on_ios
+    @mocked_ios = [Minitest::Mock.new, Minitest::Mock.new]
+    @mocked_ios.each { |m| m.expect(:close, true) }
+    @binder.ios = @mocked_ios
+
+    @binder.close
+
+    assert @mocked_ios.each(&:verify)
+  end
+
+  # test redirect for restart
+
+  def test_close_listeners_closes_ios
+    @binder.parse ["tcp://127.0.0.1:#{UniquePort.call}"], @events
+
+    refute @binder.listeners.any? { |u, l| l.closed? }
+
+    @binder.close_listeners
+
+    assert @binder.listeners.all? { |u, l| l.closed? }
+  end
+
+  def test_close_listeners_closes_ios_unless_closed?
+    @binder.parse ["tcp://127.0.0.1:0"], @events
+
+    bomb = @binder.listeners.first[1]
+    bomb.close
+    def bomb.close; raise "Boom!"; end # the bomb has been planted
+
+    assert @binder.listeners.any? { |u, l| l.closed? }
+
+    @binder.close_listeners
+
+    assert @binder.listeners.all? { |u, l| l.closed? }
+  end
+
+  def test_listeners_file_unlink_if_unix_listener
+    skip UNIX_SKT_MSG unless UNIX_SKT_EXIST
+
+    @binder.parse ["unix://test/#{name}_server.sock"], @events
+
+    assert File.socket?("test/#{name}_server.sock")
+
+    @binder.close_listeners
+
+    refute File.socket?("test/#{name}_server.sock")
+  end
+
   private
 
   def assert_parsing_logs_uri(order = [:unix, :tcp])