From 688c88cac3df41ccf4dbe34d621d0c80433a2778 Mon Sep 17 00:00:00 2001
From: Sam Bostock <sam.bostock@shopify.com>
Date: Fri, 8 Mar 2024 01:02:39 -0500
Subject: [PATCH] Dynamically choose available ports in tests

Many tests appear to be flakey due to hard coded ports already being
taken.

This refactors all ports to be chosen dynamically by checking for an
available port first.
---
 test/benchmark_test.rb                     |  2 +-
 test/helpers/memcached.rb                  | 11 +++++--
 test/integration/test_authentication.rb    |  2 +-
 test/integration/test_compressor.rb        | 14 +++++----
 test/integration/test_failover.rb          | 34 +++++++---------------
 test/integration/test_network.rb           | 10 +++----
 test/integration/test_operations.rb        |  6 ++--
 test/integration/test_serializer.rb        |  6 ++--
 test/protocol/test_server_config_parser.rb |  4 +--
 test/test_rack_session.rb                  |  2 +-
 test/test_ring.rb                          | 19 +++++++-----
 test/utils/memcached_mock.rb               | 12 ++++++--
 12 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/test/benchmark_test.rb b/test/benchmark_test.rb
index c3880343..ce013c1a 100644
--- a/test/benchmark_test.rb
+++ b/test/benchmark_test.rb
@@ -23,7 +23,7 @@ def profile(&block)
     @value = []
     @marshalled = Marshal.dump(@value)
 
-    @port = 23_417
+    @port = find_available_port
     @servers = ["127.0.0.1:#{@port}", "localhost:#{@port}"]
     @key1 = 'Short'
     @key2 = 'Sym1-2-3::45' * 8
diff --git a/test/helpers/memcached.rb b/test/helpers/memcached.rb
index 5923ecee..b8b4bdd2 100644
--- a/test/helpers/memcached.rb
+++ b/test/helpers/memcached.rb
@@ -46,13 +46,14 @@ def memcached(protocol, port_or_socket, args = '', client_options = {}, terminat
     # but sets terminate_process to false ensuring that the process persists
     # past execution of the block argument.
     # rubocop:disable Metrics/ParameterLists
-    def memcached_persistent(protocol = :binary, port_or_socket = 21_345, args = '', client_options = {}, &block)
+    def memcached_persistent(protocol = :binary, port_or_socket = find_available_port, args = '', client_options = {},
+                             &block)
       memcached(protocol, port_or_socket, args, client_options, terminate_process: false, &block)
     end
     # rubocop:enable Metrics/ParameterLists
 
     # Launches a persistent memcached process, configured to use SSL
-    def memcached_ssl_persistent(protocol = :binary, port_or_socket = rand(21_397..21_896), &block)
+    def memcached_ssl_persistent(protocol = :binary, port_or_socket = find_available_port, &block)
       memcached_persistent(protocol,
                            port_or_socket,
                            CertificateGenerator.ssl_args,
@@ -67,7 +68,7 @@ def memcached_kill(port_or_socket)
     end
 
     # Launches a persistent memcached process, configured to use SASL authentication
-    def memcached_sasl_persistent(port_or_socket = 21_398, &block)
+    def memcached_sasl_persistent(port_or_socket = find_available_port, &block)
       memcached_persistent(:binary, port_or_socket, '-S', sasl_credentials, &block)
     end
 
@@ -95,5 +96,9 @@ def kill_process(pid)
     def supports_fork?
       Process.respond_to?(:fork)
     end
+
+    def find_available_port
+      MemcachedMock.find_available_port
+    end
   end
 end
diff --git a/test/integration/test_authentication.rb b/test/integration/test_authentication.rb
index a0145f63..949e9061 100644
--- a/test/integration/test_authentication.rb
+++ b/test/integration/test_authentication.rb
@@ -7,7 +7,7 @@
     let(:username) { SecureRandom.hex(5) }
     it 'raises an error if the username is set' do
       err = assert_raises Dalli::DalliError do
-        memcached_persistent(:meta, 21_345, '', username: username) do |dc|
+        memcached_persistent(:meta, find_available_port, '', username: username) do |dc|
           dc.flush
           dc.set('key1', 'abcd')
         end
diff --git a/test/integration/test_compressor.rb b/test/integration/test_compressor.rb
index 26cb378a..113cab7b 100644
--- a/test/integration/test_compressor.rb
+++ b/test/integration/test_compressor.rb
@@ -17,7 +17,7 @@ def self.decompress(data)
   MemcachedManager.supported_protocols.each do |p|
     describe "using the #{p} protocol" do
       it 'default to Dalli::Compressor' do
-        memcached(p, 29_199) do |dc|
+        memcached(p, find_available_port) do |dc|
           dc.set 1, 2
 
           assert_equal Dalli::Compressor, dc.instance_variable_get(:@ring).servers.first.compressor
@@ -25,14 +25,15 @@ def self.decompress(data)
       end
 
       it 'support a custom compressor' do
-        memcached(p, 29_199) do |_dc|
-          memcache = Dalli::Client.new('127.0.0.1:29199', { compressor: NoopCompressor })
+        port = find_available_port
+        memcached(p, port) do |_dc|
+          memcache = Dalli::Client.new("127.0.0.1:#{port}", { compressor: NoopCompressor })
           memcache.set 1, 2
           begin
             assert_equal NoopCompressor,
                          memcache.instance_variable_get(:@ring).servers.first.compressor
 
-            memcached(p, 19_127) do |newdc|
+            memcached(p, port) do |newdc|
               assert newdc.set('string-test', 'a test string')
               assert_equal('a test string', newdc.get('string-test'))
             end
@@ -42,8 +43,9 @@ def self.decompress(data)
 
       describe 'GzipCompressor' do
         it 'compress and uncompress data using Zlib::GzipWriter/Reader' do
-          memcached(p, 19_127) do |_dc|
-            memcache = Dalli::Client.new('127.0.0.1:19127', { compress: true, compressor: Dalli::GzipCompressor })
+          port = find_available_port
+          memcached(p, port) do |_dc|
+            memcache = Dalli::Client.new("127.0.0.1:#{port}", { compress: true, compressor: Dalli::GzipCompressor })
             data = (0...1025).map { rand(65..90).chr }.join
 
             assert memcache.set('test', data)
diff --git a/test/integration/test_failover.rb b/test/integration/test_failover.rb
index 85de7e7d..633a741f 100644
--- a/test/integration/test_failover.rb
+++ b/test/integration/test_failover.rb
@@ -33,8 +33,7 @@
 
       describe 'assuming some bad servers' do
         it 'silently reconnect if server hiccups' do
-          server_port = 30_124
-          memcached_persistent(p, server_port) do |dc, port|
+          memcached_persistent(p, find_available_port) do |dc, port|
             dc.set 'foo', 'bar'
             foo = dc.get 'foo'
 
@@ -52,11 +51,8 @@
         end
 
         it 'reconnects if server idles the connection' do
-          port1 = 32_112
-          port2 = 37_887
-
-          memcached(p, port1, '-o idle_timeout=1') do |_, first_port|
-            memcached(p, port2, '-o idle_timeout=1') do |_, second_port|
+          memcached(p, find_available_port, '-o idle_timeout=1') do |_, first_port|
+            memcached(p, find_available_port, '-o idle_timeout=1') do |_, second_port|
               dc = Dalli::Client.new ["localhost:#{first_port}", "localhost:#{second_port}"]
               dc.set 'foo', 'bar'
               dc.set 'foo2', 'bar2'
@@ -75,10 +71,8 @@
         end
 
         it 'handle graceful failover' do
-          port1 = 31_777
-          port2 = 32_113
-          memcached_persistent(p, port1) do |_first_dc, first_port|
-            memcached_persistent(p, port2) do |_second_dc, second_port|
+          memcached_persistent(p, find_available_port) do |_first_dc, first_port|
+            memcached_persistent(p, find_available_port) do |_second_dc, second_port|
               dc = Dalli::Client.new ["localhost:#{first_port}", "localhost:#{second_port}"]
               dc.set 'foo', 'bar'
               foo = dc.get 'foo'
@@ -102,10 +96,8 @@
         end
 
         it 'handle them gracefully in get_multi' do
-          port1 = 32_971
-          port2 = 34_312
-          memcached_persistent(p, port1) do |_first_dc, first_port|
-            memcached(p, port2) do |_second_dc, second_port|
+          memcached_persistent(p, find_available_port) do |_first_dc, first_port|
+            memcached(p, find_available_port) do |_second_dc, second_port|
               dc = Dalli::Client.new ["localhost:#{first_port}", "localhost:#{second_port}"]
               dc.set 'a', 'a1'
               result = dc.get_multi ['a']
@@ -122,10 +114,8 @@
         end
 
         it 'handle graceful failover in get_multi' do
-          port1 = 34_541
-          port2 = 33_044
-          memcached_persistent(p, port1) do |_first_dc, first_port|
-            memcached_persistent(p, port2) do |_second_dc, second_port|
+          memcached_persistent(p, find_available_port) do |_first_dc, first_port|
+            memcached_persistent(p, find_available_port) do |_second_dc, second_port|
               dc = Dalli::Client.new ["localhost:#{first_port}", "localhost:#{second_port}"]
               dc.set 'foo', 'foo1'
               dc.set 'bar', 'bar1'
@@ -151,10 +141,8 @@
         end
 
         it 'stats it still properly report' do
-          port1 = 34_547
-          port2 = 33_219
-          memcached_persistent(p, port1) do |_first_dc, first_port|
-            memcached_persistent(p, port2) do |_second_dc, second_port|
+          memcached_persistent(p, find_available_port) do |_first_dc, first_port|
+            memcached_persistent(p, find_available_port) do |_second_dc, second_port|
               dc = Dalli::Client.new ["localhost:#{first_port}", "localhost:#{second_port}"]
               result = dc.stats
 
diff --git a/test/integration/test_network.rb b/test/integration/test_network.rb
index cac8a876..69229d8e 100644
--- a/test/integration/test_network.rb
+++ b/test/integration/test_network.rb
@@ -122,7 +122,7 @@
 
       it 'handles timeout error during pipelined get' do
         with_nil_logger do
-          memcached(p, 19_191) do |dc|
+          memcached(p, find_available_port) do |dc|
             dc.send(:ring).server_for_key('abc').sock.stub(:write, proc { raise Timeout::Error }) do
               assert_empty dc.get_multi(['abc'])
             end
@@ -132,7 +132,7 @@
 
       it 'handles asynchronous Thread#raise' do
         with_nil_logger do
-          memcached(p, 19_191) do |dc|
+          memcached(p, find_available_port) do |dc|
             10.times do |i|
               thread = Thread.new do
                 loop do
@@ -156,7 +156,7 @@
 
       it 'handles asynchronous Thread#raise during pipelined get' do
         with_nil_logger do
-          memcached(p, 19_191) do |dc|
+          memcached(p, find_available_port) do |dc|
             10.times do |i|
               expected_response = 100.times.to_h { |x| ["key:#{i}:#{x}", x.to_s] }
               expected_response.each do |key, val|
@@ -185,7 +185,7 @@
 
       it 'handles asynchronous Thread#kill' do
         with_nil_logger do
-          memcached(p, 19_191) do |dc|
+          memcached(p, find_available_port) do |dc|
             10.times do |i|
               thread = Thread.new do
                 loop do
@@ -209,7 +209,7 @@
 
       it 'handles asynchronous Thread#kill during pipelined get' do
         with_nil_logger do
-          memcached(p, 19_191) do |dc|
+          memcached(p, find_available_port) do |dc|
             10.times do |i|
               expected_response = 100.times.to_h { |x| ["key:#{i}:#{x}", x.to_s] }
               expected_response.each do |key, val|
diff --git a/test/integration/test_operations.rb b/test/integration/test_operations.rb
index f47962b5..d1b46e39 100644
--- a/test/integration/test_operations.rb
+++ b/test/integration/test_operations.rb
@@ -221,7 +221,9 @@
         end
 
         it 'supports with nil values when cache_nils: true' do
-          memcached_persistent(p, 21_345, '', cache_nils: true) do |dc|
+          port = find_available_port
+
+          memcached_persistent(p, port, '', cache_nils: true) do |dc|
             dc.flush
 
             dc.set('fetch_key', nil)
@@ -230,7 +232,7 @@
             assert_nil res
           end
 
-          memcached_persistent(p, 21_345, '', cache_nils: false) do |dc|
+          memcached_persistent(p, port, '', cache_nils: false) do |dc|
             dc.flush
             dc.set('fetch_key', nil)
             executed = false
diff --git a/test/integration/test_serializer.rb b/test/integration/test_serializer.rb
index 46dff8ac..83ffe2a6 100644
--- a/test/integration/test_serializer.rb
+++ b/test/integration/test_serializer.rb
@@ -7,7 +7,7 @@
   MemcachedManager.supported_protocols.each do |p|
     describe "using the #{p} protocol" do
       it 'defaults to Marshal' do
-        memcached(p, 29_198) do |dc|
+        memcached(p, find_available_port) do |dc|
           dc.set 1, 2
 
           assert_equal Marshal, dc.instance_variable_get(:@ring).servers.first.serializer
@@ -15,13 +15,13 @@
       end
 
       it 'supports a custom serializer' do
-        memcached(p, 29_198) do |_dc, port|
+        memcached(p, find_available_port) do |_dc, port|
           memcache = Dalli::Client.new("127.0.0.1:#{port}", serializer: JSON)
           memcache.set 1, 2
           begin
             assert_equal JSON, memcache.instance_variable_get(:@ring).servers.first.serializer
 
-            memcached(p, 21_956) do |newdc|
+            memcached(p, find_available_port) do |newdc|
               assert newdc.set('json_test', { 'foo' => 'bar' })
               assert_equal({ 'foo' => 'bar' }, newdc.get('json_test'))
             end
diff --git a/test/protocol/test_server_config_parser.rb b/test/protocol/test_server_config_parser.rb
index 4e120392..3ff34717 100644
--- a/test/protocol/test_server_config_parser.rb
+++ b/test/protocol/test_server_config_parser.rb
@@ -4,7 +4,7 @@
 
 describe Dalli::Protocol::ServerConfigParser do
   describe 'parse' do
-    let(:port) { rand(9999..99_999) }
+    let(:port) { find_available_port }
     let(:weight) { rand(1..5) }
 
     describe 'when the string is not an memcached URI' do
@@ -90,7 +90,7 @@
     describe 'when the string is a memcached URI' do
       let(:user) { SecureRandom.hex(5) }
       let(:password) { SecureRandom.hex(5) }
-      let(:port) { rand(15_000..16_023) }
+      let(:port) { find_available_port }
       let(:hostname) { "a#{SecureRandom.hex(3)}.b#{SecureRandom.hex(3)}.com" }
 
       describe 'when the URI is properly formed and includes all values' do
diff --git a/test/test_rack_session.rb b/test/test_rack_session.rb
index acb3de06..57d135ae 100644
--- a/test/test_rack_session.rb
+++ b/test/test_rack_session.rb
@@ -8,7 +8,7 @@
 require 'rack/mock'
 describe Rack::Session::Dalli do
   before do
-    @port = 19_129
+    @port = find_available_port
     memcached_persistent(:binary, @port)
     Rack::Session::Dalli::DEFAULT_DALLI_OPTIONS[:memcache_server] = "localhost:#{@port}"
 
diff --git a/test/test_ring.rb b/test/test_ring.rb
index 41264fe6..d64188b9 100644
--- a/test/test_ring.rb
+++ b/test/test_ring.rb
@@ -43,9 +43,10 @@ def weight
       end
 
       it "return the server when it's alive" do
-        servers = ['localhost:19191']
+        port = find_available_port
+        servers = ["localhost:#{find_available_port}"]
         ring = Dalli::Ring.new(servers, Dalli::Protocol::Binary, {})
-        memcached(:binary, 19_191) do |mc|
+        memcached(:binary, port) do |mc|
           ring = mc.send(:ring)
 
           assert_equal ring.servers.first.port, ring.server_for_key('test').port
@@ -63,9 +64,10 @@ def weight
       end
 
       it 'return an alive server when at least one is alive' do
-        servers = ['localhost:12346', 'localhost:19191']
+        port = find_available_port
+        servers = ['localhost:12346', "localhost:#{port}"]
         ring = Dalli::Ring.new(servers, Dalli::Protocol::Binary, {})
-        memcached(:binary, 19_191) do |mc|
+        memcached(:binary, port) do |mc|
           ring = mc.send(:ring)
 
           assert_equal ring.servers.first.port, ring.server_for_key('test').port
@@ -74,13 +76,16 @@ def weight
     end
 
     it 'detect when a dead server is up again' do
-      memcached(:binary, 19_997) do
+      first_port = find_available_port
+      second_port = find_available_port
+      memcached(:binary, first_port) do
         down_retry_delay = 0.5
-        dc = Dalli::Client.new(['localhost:19997', 'localhost:19998'], down_retry_delay: down_retry_delay)
+        dc = Dalli::Client.new(["localhost:#{first_port}", "localhost:#{second_port}"],
+                               down_retry_delay: down_retry_delay)
 
         assert_equal 1, dc.stats.values.compact.count
 
-        memcached(:binary, 19_998) do
+        memcached(:binary, second_port) do
           assert_equal 2, dc.stats.values.compact.count
         end
       end
diff --git a/test/utils/memcached_mock.rb b/test/utils/memcached_mock.rb
index 1724b980..18fcbcd3 100644
--- a/test/utils/memcached_mock.rb
+++ b/test/utils/memcached_mock.rb
@@ -12,7 +12,7 @@ module MemcachedMock
                       f.close
                       f.path)
 
-  def self.start(port = 19_123)
+  def self.start(port = find_available_port)
     server = TCPServer.new('localhost', port)
     session = server.accept
     yield(session)
@@ -29,9 +29,17 @@ def self.start_unix(path = UNIX_SOCKET_PATH)
     yield(session)
   end
 
-  def self.delayed_start(port = 19_123, wait = 1)
+  def self.delayed_start(port = find_available_port, wait = 1)
     server = TCPServer.new('localhost', port)
     sleep wait
     yield(server)
   end
+
+  def self.find_available_port
+    socket = Socket.new(:INET, :STREAM, 0)
+    socket.bind(Addrinfo.tcp('127.0.0.1', 0))
+    port = socket.local_address.ip_port
+    socket.close
+    port
+  end
 end