Skip to content

Commit

Permalink
Refactor test suite so all test case inherit from a single class
Browse files Browse the repository at this point in the history
Also cleanup a few tests that were assertion-less etc.
  • Loading branch information
byroot committed Aug 23, 2024
1 parent 9f7bb8b commit 5252641
Show file tree
Hide file tree
Showing 20 changed files with 64 additions and 43 deletions.
1 change: 1 addition & 0 deletions test/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "redis_client/decorator"

require "toxiproxy"
require "stringio"

Dir[File.join(__dir__, "support/**/*.rb")].sort.each { |f| require f }
Dir[File.join(__dir__, "shared/**/*.rb")].sort.each { |f| require f }
2 changes: 1 addition & 1 deletion test/hiredis/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
unless RUBY_PLATFORM == "java"
require "redis"
require "hiredis"
Redis.new(host: Servers::HOST, port: Servers::REDIS_TCP_PORT, driver: :hiredis).ping
Redis.new(host: Servers::HOST, port: Servers::REDIS.port, driver: :hiredis).ping
end

require "hiredis-client"
Expand Down
3 changes: 2 additions & 1 deletion test/redis_client/circuit_breaker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
require "test_helper"

class RedisClient
class CircuitBreakerTest < Minitest::Test
class CircuitBreakerTest < RedisClientTestCase
include ClientTestHelper

def setup
super
@circuit_breaker = CircuitBreaker.new(
error_threshold: 3,
error_threshold_timeout: 2,
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/command_builder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "test_helper"

class RedisClient
class CommandBuilderTest < Minitest::Test
class CommandBuilderTest < RedisClientTestCase
def test_positional
assert_equal ["a", "b", "c"], call("a", "b", "c")
end
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "test_helper"

class RedisClient
class ConfigTest < Minitest::Test
class ConfigTest < RedisClientTestCase
def test_simple_uri
config = Config.new(url: "redis://example.com")
assert_equal "example.com", config.host
Expand Down
8 changes: 4 additions & 4 deletions test/redis_client/connection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def test_oom_errors

private

def assert_timeout(error, faster_than = 0.5, &block)
def assert_timeout(error, faster_than = ClientTestHelper::DEFAULT_TIMEOUT + 0.2, &block)
realtime = Benchmark.realtime do
assert_raises(error, &block)
end
Expand All @@ -291,7 +291,7 @@ def assert_timeout(error, faster_than = 0.5, &block)
end
end

class TCPConnectionTest < Minitest::Test
class TCPConnectionTest < RedisClientTestCase
include ClientTestHelper
include ConnectionTests

Expand Down Expand Up @@ -431,7 +431,7 @@ def new_client(**overrides)
end
end

class SSLConnectionTest < Minitest::Test
class SSLConnectionTest < RedisClientTestCase
include ClientTestHelper
include ConnectionTests

Expand All @@ -449,7 +449,7 @@ def new_client(**overrides)
end
end

class UnixConnectionTest < Minitest::Test
class UnixConnectionTest < RedisClientTestCase
include ClientTestHelper

def test_connection_working
Expand Down
4 changes: 2 additions & 2 deletions test/redis_client/decorator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_client_methods_not_available_on_pipelines
end
end

class DecoratorTest < Minitest::Test
class DecoratorTest < RedisClientTestCase
include DecoratorTests

private
Expand All @@ -51,7 +51,7 @@ def new_client(**overrides)
end
end

class PooledDecoratorTest < Minitest::Test
class PooledDecoratorTest < RedisClientTestCase
include DecoratorTests

private
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/middlewares_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "test_helper"

class RedisClient
class MiddlewaresTest < Minitest::Test
class MiddlewaresTest < RedisClientTestCase
include ClientTestHelper

def setup
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/pooled_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require "test_helper"

class RedisPooledClientTest < Minitest::Test
class RedisPooledClientTest < RedisClientTestCase
include ClientTestHelper
include RedisClientTests

Expand Down
15 changes: 11 additions & 4 deletions test/redis_client/ractor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require "test_helper"

class RactorTest < Minitest::Test
class RactorTest < RedisClientTestCase
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"
Expand All @@ -11,32 +11,39 @@ def setup
rescue Ractor::RemoteError
skip("Ractor implementation is too limited (MRI 3.0?)")
end
super
end

def test_get_and_set_within_ractor
ractor = Ractor.new do
within_ractor_redis = ClientTestHelper.new_client
config = Ractor.receive
within_ractor_redis = RedisClient.new(**config)
within_ractor_redis.call("SET", "foo", "bar")
within_ractor_redis.call("GET", "foo")
end
ractor.send(ClientTestHelper.tcp_config.freeze)

assert_equal("bar", ractor.take)
end

def test_multiple_ractors
ractor1 = Ractor.new do
within_ractor_redis = ClientTestHelper.new_client
config = Ractor.receive
within_ractor_redis = RedisClient.new(**config)
within_ractor_redis.call("SET", "foo", "bar")
within_ractor_redis.call("GET", "foo")
end
ractor1.send(ClientTestHelper.tcp_config.freeze)

ractor1.take # We do this to ensure that the SET has been processed

ractor2 = Ractor.new do
config = Ractor.receive
within_ractor_redis = RedisClient.new(**config)
key = Ractor.receive
within_ractor_redis = ClientTestHelper.new_client
within_ractor_redis.call("GET", key)
end
ractor2.send(ClientTestHelper.tcp_config.freeze)
ractor2.send("foo")

assert_equal("bar", ractor2.take)
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/resp3_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
require "redis_client/ruby_connection/resp3"

class RedisClient
class RESP3Test < Minitest::Test
class RESP3Test < RedisClientTestCase
class StringIO < ::StringIO
def skip(offset)
seek(offset, IO::SEEK_CUR)
Expand Down
2 changes: 1 addition & 1 deletion test/redis_client/subscriptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "test_helper"

class RedisClient
class SubscriptionsTest < Minitest::Test
class SubscriptionsTest < RedisClientTestCase
include ClientTestHelper

def setup
Expand Down
10 changes: 5 additions & 5 deletions test/redis_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require "test_helper"

class RedisClientTest < Minitest::Test
class RedisClientTest < RedisClientTestCase
include ClientTestHelper
include RedisClientTests

Expand Down Expand Up @@ -127,11 +127,11 @@ def test_measure_round_trip_delay
end

def test_server_url
assert_equal "redis://#{Servers::HOST}:#{Servers::REDIS_TCP_PORT}", @redis.server_url
assert_equal "redis://#{Servers::HOST}:#{Servers::REDIS.port}", @redis.server_url
end

def test_timeout
assert_equal 0.1, @redis.timeout
assert_equal ClientTestHelper::DEFAULT_TIMEOUT, @redis.timeout
end

def test_db
Expand All @@ -147,12 +147,12 @@ def test_host
end

def test_port
assert_equal Servers::REDIS_TCP_PORT, @redis.port
assert_equal Servers::REDIS.port, @redis.port
end

def test_path
client = new_client(**unix_config)
assert_equal Servers::REDIS_SOCKET_FILE.to_s, client.path
assert_equal Servers::REDIS.socket_file.to_s, client.path
end

def test_username
Expand Down
2 changes: 1 addition & 1 deletion test/sentinel/sentinel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require "test_helper"

class RedisClient
class SentinelTest < Minitest::Test
class SentinelTest < RedisClientTestCase
include ClientTestHelper

def setup
Expand Down
1 change: 1 addition & 0 deletions test/sentinel/test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative "../env"
require_relative "../test_helper"

Servers.build_redis
Servers::SENTINEL_TESTS.prepare
Expand Down
10 changes: 6 additions & 4 deletions test/shared/redis_client_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,20 +188,22 @@ def test_empty_pipeline
end

def test_large_read_pipelines
@redis.call("LPUSH", "list", *1000.times.to_a)
@redis.pipelined do |pipeline|
assert_equal 1000, @redis.call("LPUSH", "list", *1000.times.to_a)
results = @redis.pipelined do |pipeline|
100.times do
pipeline.call("LRANGE", "list", 0, -1)
end
end
assert_equal 100, results.size
end

def test_large_write_pipelines
@redis.pipelined do |pipeline|
results = @redis.pipelined do |pipeline|
10.times do |i|
pipeline.call("SET", i, i.to_s * 10485760)
end
end
assert_equal ["OK"] * 10, results
end

def test_get_set
Expand Down Expand Up @@ -570,7 +572,7 @@ def test_timeouts_are_adjustable_on_the_client
@redis.read_timeout = 1
@redis.write_timeout = 1

@redis.call("PING")
assert_equal "PONG", @redis.call("PING")
@redis.connect_timeout = 2
@redis.read_timeout = 2
@redis.write_timeout = 2
Expand Down
22 changes: 15 additions & 7 deletions test/support/client_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,18 @@ def write_multi(commands)
end

def setup
super

@redis = new_client
@redis.call("FLUSHDB")
check_server
end

private

def check_server
@redis.call("flushdb", "async")
end

def travel(seconds)
original_now = RedisClient.singleton_class.instance_method(:now)
original_now_ms = RedisClient.singleton_class.instance_method(:now_ms)
Expand Down Expand Up @@ -90,19 +96,21 @@ def simulate_network_errors(client, failures)

module_function

DEFAULT_TIMEOUT = 0.1

def tcp_config
{
host: Servers::HOST,
port: Servers::REDIS_TCP_PORT,
timeout: 0.1,
port: Servers::REDIS.port,
timeout: DEFAULT_TIMEOUT,
}
end

def ssl_config
{
host: Servers::HOST,
port: Servers::REDIS_TLS_PORT,
timeout: 0.1,
port: Servers::REDIS.tls_port,
timeout: DEFAULT_TIMEOUT,
ssl: true,
ssl_params: {
cert: Servers::CERTS_PATH.join("client.crt").to_s,
Expand All @@ -114,8 +122,8 @@ def ssl_config

def unix_config
{
path: Servers::REDIS_SOCKET_FILE.to_s,
timeout: 0.1,
path: Servers::REDIS.socket_file.to_s,
timeout: DEFAULT_TIMEOUT,
}
end

Expand Down
2 changes: 1 addition & 1 deletion test/support/server_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def spawn
else
pid_file.parent.mkpath
@out.print "starting #{name}... "
pid = Process.spawn(*command, out: log_file.to_s, err: log_file.to_s)
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
Expand Down
12 changes: 5 additions & 7 deletions test/support/servers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ module Servers
HOST = "127.0.0.1"
CERTS_PATH = ServerManager::ROOT.join("test/fixtures/certs")

REDIS_TCP_PORT = 16379
REDIS_TLS_PORT = 26379
REDIS_REAL_TCP_PORT = 16380
REDIS_REAL_TLS_PORT = 26380
REDIS_SOCKET_FILE = ServerManager::ROOT.join("tmp/redis.sock")

SENTINEL_CONF_PATH = ServerManager::ROOT.join("tmp/sentinel.conf")
SENTINEL_NAME = "cache"

Expand Down Expand Up @@ -45,10 +39,14 @@ def spawn
super
end

def socket_file
ServerManager::ROOT.join("tmp/redis.sock")
end

def command
[
Servers.redis_server_bin,
"--unixsocket", REDIS_SOCKET_FILE.to_s,
"--unixsocket", socket_file,
"--unixsocketperm", "700",
"--port", real_port.to_s,
"--tls-port", real_tls_port.to_s,
Expand Down
3 changes: 3 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@
unless ENV["REDIS_CLIENT_RESTART_SERVER"] == "0"
Minitest.after_run { Servers::TESTS.shutdown }
end

class RedisClientTestCase < Minitest::Test
end

0 comments on commit 5252641

Please sign in to comment.