From 887b59a31957784e4cecb5e69a4c0ec936d27101 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 21 Aug 2024 09:32:11 +0200 Subject: [PATCH] Migrate to megatest And add support for test parallelization, which speeds up the test suite quite significantly. --- .gitignore | 2 + .rubocop.yml | 3 ++ Gemfile | 2 +- Gemfile.lock | 9 +++- Rakefile | 36 ++++++------- bin/megatest | 27 ++++++++++ test/hiredis/test_helper.rb | 7 ++- test/redis_client/ractor_test.rb | 2 + test/sentinel/sentinel_test.rb | 38 +++++++++---- test/sentinel/test_helper.rb | 4 +- test/support/client_test_helper.rb | 13 ++++- test/support/server_manager.rb | 85 +++++++++++++++++++++++------- test/support/servers.rb | 76 +++++++++++++++++++------- test/test_config.rb | 26 +++++++++ test/test_helper.rb | 12 +---- 15 files changed, 259 insertions(+), 83 deletions(-) create mode 100755 bin/megatest create mode 100644 test/test_config.rb diff --git a/.gitignore b/.gitignore index 675d1d1..924c602 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,8 @@ /hiredis-client/pkg/ /spec/reports/ /tmp/ +/log/ + /hiredis-client/tmp/ /bin/toxiproxy-server *.so diff --git a/.rubocop.yml b/.rubocop.yml index be4eb79..3e0b1df 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -36,6 +36,9 @@ Lint/UnderscorePrefixedVariableName: Lint/EmptyBlock: Enabled: false +Lint/ShadowedException: + Enabled: false + Lint/DuplicateBranch: Enabled: false diff --git a/Gemfile b/Gemfile index 56d3d47..468a104 100644 --- a/Gemfile +++ b/Gemfile @@ -5,7 +5,7 @@ source "https://rubygems.org" # Specify your gem's dependencies in redis-client.gemspec gemspec name: "redis-client" -gem "minitest" +gem "megatest", github: "byroot/megatest" gem "rake", "~> 13.2" gem "rake-compiler" gem "rubocop" diff --git a/Gemfile.lock b/Gemfile.lock index 51e476c..613f32f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,9 @@ +GIT + remote: https://github.com/byroot/megatest.git + revision: 70e9948140eb888e02e810175d52d55b329db47d + specs: + megatest (0.1.1) + PATH remote: . specs: @@ -15,7 +21,6 @@ GEM hiredis (0.6.3-java) json (2.7.1) json (2.7.1-java) - minitest (5.25.1) parallel (1.24.0) parser (3.3.0.5) ast (~> 2.4.1) @@ -58,7 +63,7 @@ DEPENDENCIES benchmark-ips byebug hiredis - minitest + megatest! rake (~> 13.2) rake-compiler redis (~> 4.6) diff --git a/Rakefile b/Rakefile index 27c8bef..79196e7 100644 --- a/Rakefile +++ b/Rakefile @@ -1,7 +1,6 @@ # frozen_string_literal: true require "rake/extensiontask" -require "rake/testtask" require 'rubocop/rake_task' RuboCop::RakeTask.new @@ -21,29 +20,30 @@ Rake::ExtensionTask.new do |ext| CLEAN.add("#{ext.ext_dir}/vendor/*.{a,o}") end +require "megatest/test_task" + namespace :test do - Rake::TestTask.new(:ruby) do |t| - t.libs << "test" - t.libs << "lib" - t.test_files = FileList["test/**/*_test.rb"].exclude("test/sentinel/*_test.rb") - t.options = '-v' if ENV['CI'] || ENV['VERBOSE'] - end + jobs = ENV["JOBS"] || "4" + jobs = jobs && Process.respond_to?(:fork) ? ["-j", jobs] : [] + extra_args = ["--max-retries", "1"] + jobs - Rake::TestTask.new(:sentinel) do |t| - t.libs << "test/sentinel" - t.libs << "test" - t.libs << "lib" - t.test_files = FileList["test/sentinel/*_test.rb"] - t.options = '-v' if ENV['CI'] || ENV['VERBOSE'] + Megatest::TestTask.create(:ruby) do |t| + t.tests = FileList["test/**/*_test.rb"].exclude("test/sentinel/*_test.rb") + t.extra_args = extra_args end - Rake::TestTask.new(:hiredis) do |t| + Megatest::TestTask.create(:hiredis) do |t| t.libs << "test/hiredis" - t.libs << "test" t.libs << "hiredis-client/lib" - t.libs << "lib" - t.test_files = FileList["test/**/*_test.rb"].exclude("test/sentinel/*_test.rb") - t.options = '-v' if ENV['CI'] || ENV['VERBOSE'] + t.tests = FileList["test/hiredis/test_helper.rb"] + FileList["test/**/*_test.rb"].exclude("test/sentinel/*_test.rb") + t.extra_args = extra_args + t.deps << :compile + end + + Megatest::TestTask.create(:sentinel) do |t| + t.libs << "test/sentinel" + t.tests = ["test/sentinel"] + t.extra_args = extra_args end end diff --git a/bin/megatest b/bin/megatest new file mode 100755 index 0000000..d2c9e3b --- /dev/null +++ b/bin/megatest @@ -0,0 +1,27 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# +# This file was generated by Bundler. +# +# The application 'megatest' is installed as part of a gem, and +# this file is here to facilitate running it. +# + +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) + +bundle_binstub = File.expand_path("bundle", __dir__) + +if File.file?(bundle_binstub) + if File.read(bundle_binstub, 300) =~ /This file was generated by Bundler/ + load(bundle_binstub) + else + abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. +Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.") + end +end + +require "rubygems" +require "bundler/setup" + +load Gem.bin_path("megatest", "megatest") diff --git a/test/hiredis/test_helper.rb b/test/hiredis/test_helper.rb index 3b8b341..beb5565 100644 --- a/test/hiredis/test_helper.rb +++ b/test/hiredis/test_helper.rb @@ -8,7 +8,12 @@ unless RUBY_PLATFORM == "java" require "redis" require "hiredis" - Redis.new(host: Servers::HOST, port: Servers::REDIS.port, driver: :hiredis).ping + + begin + Redis.new(driver: :hiredis).ping + rescue + nil # does not matter, we just want to load the library + end end require "hiredis-client" diff --git a/test/redis_client/ractor_test.rb b/test/redis_client/ractor_test.rb index 5ee627f..18bcec9 100644 --- a/test/redis_client/ractor_test.rb +++ b/test/redis_client/ractor_test.rb @@ -3,6 +3,8 @@ require "test_helper" class RactorTest < RedisClientTestCase + tag isolated: true + def setup skip("Ractors are not supported on this Ruby version") unless defined?(::Ractor) skip("Hiredis is not Ractor safe") if RedisClient.default_driver.name == "RedisClient::HiredisConnection" diff --git a/test/sentinel/sentinel_test.rb b/test/sentinel/sentinel_test.rb index ff086bb..62fe02d 100644 --- a/test/sentinel/sentinel_test.rb +++ b/test/sentinel/sentinel_test.rb @@ -7,12 +7,30 @@ class SentinelTest < RedisClientTestCase include ClientTestHelper def setup + @config = new_config super @config = new_config end + def check_server + retried = false + begin + @redis = @config.new_client + @redis.call("FLUSHDB") + rescue + if retried + raise + else + retried = true + Servers::SENTINEL_TESTS.reset + retry + end + end + end + def teardown Servers::SENTINEL_TESTS.reset + super end def test_new_client @@ -86,9 +104,9 @@ def test_unknown_master def test_unresolved_config client = @config.new_client - @config.stub(:server_url, -> { raise ConnectionError.with_config('this should not be called', @config) }) do - @config.stub(:resolved?, false) do - client.stub(:call, ->(_) { raise ConnectionError.with_config('call error', @config) }) do + stub(@config, :server_url, -> { raise ConnectionError.with_config('this should not be called', @config) }) do + stub(@config, :resolved?, false) do + stub(client, :call, ->(_) { raise ConnectionError.with_config('call error', @config) }) do error = assert_raises ConnectionError do client.call('PING') end @@ -108,7 +126,7 @@ def test_master_failover_not_ready [["SENTINEL", "get-master-addr-by-name", "cache"], [Servers::REDIS_REPLICA.host, Servers::REDIS_REPLICA.port.to_s]], sentinel_refresh_command_mock, ]) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_raises FailoverError do client.call("PING") @@ -126,7 +144,7 @@ def test_master_failover_ready replica = RedisClient.new(host: Servers::REDIS_REPLICA.host, port: Servers::REDIS_REPLICA.port) assert_equal "OK", replica.call("REPLICAOF", "NO", "ONE") - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_equal "PONG", client.call("PING") @@ -145,7 +163,7 @@ def test_no_replicas sentinel_client_mock = SentinelClientMock.new([ [["SENTINEL", "replicas", "cache"], []], ] * tries) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do assert_raises ConnectionError do @config.new_client.call("PING") end @@ -159,7 +177,7 @@ def test_replica_failover_not_ready [["SENTINEL", "replicas", "cache"], [response_hash("ip" => Servers::REDIS.host, "port" => Servers::REDIS.port.to_s, "flags" => "slave")]], [["SENTINEL", "replicas", "cache"], [response_hash("ip" => Servers::REDIS.host, "port" => Servers::REDIS.port.to_s, "flags" => "slave")]], ]) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_equal "PONG", client.call("PING") @@ -180,7 +198,7 @@ def test_replica_failover_ready [["SENTINEL", "replicas", "cache"], [response_hash("ip" => Servers::REDIS_REPLICA.host, "port" => Servers::REDIS_REPLICA.port.to_s, "flags" => "slave")]], ]) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_equal "PONG", client.call("PING") end @@ -199,7 +217,7 @@ def test_successful_connection_refreshes_sentinels_list additional_sentinels: [response_hash("ip" => new_sentinel_ip, "port" => new_sentinel_port.to_s)], ), ]) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_equal "PONG", client.call("PING") end @@ -226,7 +244,7 @@ def test_sentinel_refresh_password ), ]) - @config.stub(:sentinel_client, ->(_config) { sentinel_client_mock }) do + stub(@config, :sentinel_client, ->(_config) { sentinel_client_mock }) do client = @config.new_client assert_equal "PONG", client.call("PING") end diff --git a/test/sentinel/test_helper.rb b/test/sentinel/test_helper.rb index 0e1c148..d965ea9 100644 --- a/test/sentinel/test_helper.rb +++ b/test/sentinel/test_helper.rb @@ -5,6 +5,4 @@ Servers.build_redis Servers::SENTINEL_TESTS.prepare - -require "minitest/autorun" -Minitest.after_run { Servers::TESTS.shutdown } +Servers.all = Servers::SENTINEL_TESTS diff --git a/test/support/client_test_helper.rb b/test/support/client_test_helper.rb index 9d35bd8..a9ea061 100644 --- a/test/support/client_test_helper.rb +++ b/test/support/client_test_helper.rb @@ -48,7 +48,18 @@ def setup private def check_server - @redis.call("flushdb", "async") + retried = false + begin + @redis.call("flushdb", "async") + rescue + if retried + raise + else + retried = true + Servers.reset + retry + end + end end def travel(seconds) diff --git a/test/support/server_manager.rb b/test/support/server_manager.rb index 68e47fb..737f806 100644 --- a/test/support/server_manager.rb +++ b/test/support/server_manager.rb @@ -3,6 +3,33 @@ require "pathname" class ServerManager + ROOT = Pathname.new(File.expand_path("../../", __dir__)) + + class << self + def kill_all + Dir[ROOT.join("tmp/**/*.pid").to_s].each do |pid_file| + pid = begin + Integer(File.read(pid_file)) + rescue ArgumentError + nil + end + + if pid + begin + Process.kill(:KILL, pid) + rescue Errno::ESRCH, Errno::ECHILD + nil # It's fine + end + end + + File.unlink(pid_file) + end + end + end + + @worker_index = nil + singleton_class.attr_accessor :worker_index + module NullIO extend self @@ -15,9 +42,7 @@ def print(_str) end end - ROOT = Pathname.new(File.expand_path("../../", __dir__)) - - attr_reader :name, :host, :port, :real_port, :command + attr_reader :name, :host, :command attr_accessor :out def initialize(name, port:, command: nil, real_port: port, host: "127.0.0.1") @@ -29,24 +54,42 @@ def initialize(name, port:, command: nil, real_port: port, host: "127.0.0.1") @out = $stderr end + def worker_index + ServerManager.worker_index + end + + def port_offset + worker_index.to_i * 200 + end + + def port + @port + port_offset + end + + def real_port + @real_port + port_offset + end + def spawn - if alive? - @out.puts "#{name} already running with pid=#{pid}" - else - pid_file.parent.mkpath - @out.print "starting #{name}... " - pid = Process.spawn(*command.map(&:to_s), out: log_file.to_s, err: log_file.to_s) - pid_file.write(pid.to_s) - @out.puts "started with pid=#{pid}" - end + shutdown + + pid_file.parent.mkpath + pid = Process.spawn(*command.map(&:to_s), out: log_file.to_s, err: log_file.to_s) + pid_file.write(pid.to_s) + @out.puts "started #{name}-#{worker_index.to_i} with pid=#{pid}" end def wait(timeout: 5) - @out.print "Waiting for #{name} (port #{real_port})..." - if wait_until_ready(timeout: timeout) - @out.puts " ready." + unless wait_until_ready(timeout: 1) + @out.puts "Waiting for #{name}-#{worker_index.to_i} (port #{real_port})..." + end + + if wait_until_ready(timeout: timeout - 1) + @out.puts "#{name}-#{worker_index.to_i} ready." + true else - @out.puts " timedout." + @out.puts "#{name}-#{worker_index.to_i} timedout." + false end end @@ -102,12 +145,16 @@ def alive? private + def dir + ROOT.join("tmp/#{name}-#{worker_index.to_i}").tap(&:mkpath) + end + def pid_file - @pid_file ||= ROOT.join("tmp/#{name}.pid") + dir.join("#{name}.pid") end def log_file - @log_file ||= ROOT.join("tmp/#{name}.log") + dir.join("#{name}.log") end end @@ -126,7 +173,7 @@ def silence def prepare shutdown @servers.each(&:spawn) - @servers.each(&:wait) + @servers.all?(&:wait) end def reset diff --git a/test/support/servers.rb b/test/support/servers.rb index a24711e..d8fb8ca 100644 --- a/test/support/servers.rb +++ b/test/support/servers.rb @@ -28,22 +28,29 @@ def redis_server_bin def redis_builder @redis_builder ||= RedisBuilder.new(ENV.fetch("REDIS", DEFAULT_REDIS_VERSION), ROOT.join("tmp").to_s) end + + attr_accessor :all + + def reset + all.reset + end end class RedisManager < ServerManager def spawn begin - ROOT.join("tmp/dump.rdb").rmtree + dir.join("dump.rdb").to_s rescue Errno::ENOENT end super end def socket_file - ServerManager::ROOT.join("tmp/redis.sock") + dir.join("redis.sock").to_s end def command + ROOT.join("tmp/redis-#{worker_index.to_i}").mkpath [ Servers.redis_server_bin, "--unixsocket", socket_file, @@ -55,10 +62,14 @@ def command "--tls-ca-cert-file", CERTS_PATH.join("ca.crt").to_s, "--save", "", "--appendonly", "no", - "--dir", "tmp/", + "--dir", dir, ] end + def dir + ROOT.join("tmp/redis-#{worker_index.to_i}") + end + def tls_port port + 10_000 end @@ -111,7 +122,7 @@ def generate_conf end def conf_file - ROOT.join("tmp/#{name}.conf") + ROOT.join("tmp/#{name}-#{worker_index.to_i}.conf") end def command @@ -132,20 +143,20 @@ def spawn SENTINELS = [ SentinelManager.new( "redis_sentinel_1", - port: 26480, - real_port: 26481, + port: 26_300, + real_port: 26_301, command: [], ), SentinelManager.new( "redis_sentinel_2", - port: 26580, - real_port: 26481, + port: 26_302, + real_port: 26_303, command: [], ), SentinelManager.new( "redis_sentinel_3", - port: 26680, - real_port: 26681, + port: 26_304, + real_port: 26_305, command: [], ), ].freeze @@ -158,14 +169,34 @@ def spawn super end + def command + [ + ToxiproxyManager::BIN.to_s, + "-port", + port.to_s, + ] + end + def on_ready Toxiproxy.host = "http://#{host}:#{port}" - Toxiproxy.populate(proxies) + retries = 3 + + begin + Toxiproxy.populate(proxies) + rescue SystemCallError, Net::HTTPError, Net::ProtocolError + retries -= 1 + if retries > 0 + sleep 0.5 + retry + else + raise + end + end end def proxies - proxies = [ + [ { name: "redis", upstream: "localhost:#{REDIS.real_port}", @@ -187,23 +218,32 @@ def proxies listen: ":#{REDIS_REPLICA_2.port}", }, ] + end + end - proxies += SENTINELS.map do |sentinel| + class ToxiproxySentinelManager < ToxiproxyManager + def proxies + sentinels = SENTINELS.map do |sentinel| { name: sentinel.name, upstream: "localhost:#{sentinel.real_port}", listen: ":#{sentinel.port}", } end - - proxies + super + sentinels end end TOXIPROXY = ToxiproxyManager.new( "toxiproxy", - port: 8474, - command: [ToxiproxyManager::BIN.to_s, "-port", 8474.to_s], + port: 8475, + command: [], + ) + + TOXIPROXY_SENTINELS = ToxiproxySentinelManager.new( + "toxiproxy", + port: 8475, + command: [], ) TESTS = ServerList.new( @@ -212,7 +252,7 @@ def proxies ) SENTINEL_TESTS = ServerList.new( - TOXIPROXY, + TOXIPROXY_SENTINELS, REDIS, REDIS_REPLICA, REDIS_REPLICA_2, diff --git a/test/test_config.rb b/test/test_config.rb new file mode 100644 index 0000000..2b65da1 --- /dev/null +++ b/test/test_config.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +Megatest.config do |c| + c.global_setup do + ServerManager.kill_all + $stderr.puts "Running test suite with driver: #{RedisClient.default_driver}" + end + + c.job_setup do |_, index| + Servers::TESTS.shutdown + Servers::SENTINEL_TESTS.shutdown + + ServerManager.worker_index = index + Toxiproxy.host = "http://#{Servers::HOST}:#{Servers::TOXIPROXY.port}" + unless Servers.all.prepare + puts "worker #{index} failed setup" + exit(1) + end + end + + c.job_teardown do + unless ENV["REDIS_CLIENT_RESTART_SERVER"] == "0" + Servers.all.shutdown + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 388bf31..47f2645 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -4,15 +4,7 @@ Servers.build_redis Servers::SENTINEL_TESTS.shutdown -Servers::TESTS.prepare +Servers.all = Servers::TESTS -require "minitest/autorun" - -at_exit { $stderr.puts "Running test suite with driver: #{RedisClient.default_driver}" } - -unless ENV["REDIS_CLIENT_RESTART_SERVER"] == "0" - Minitest.after_run { Servers::TESTS.shutdown } -end - -class RedisClientTestCase < Minitest::Test +class RedisClientTestCase < Megatest::Test end