From d151fc1ccd4321f025bc649ebc74264ba3bed182 Mon Sep 17 00:00:00 2001 From: Adam Parod Date: Fri, 7 Apr 2023 16:55:33 -0500 Subject: [PATCH 1/3] Make sure connections get closed for commands that failed their first attempt --- lib/faktory_worker/connection_manager.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/faktory_worker/connection_manager.ex b/lib/faktory_worker/connection_manager.ex index 36d65d0..8a5029a 100644 --- a/lib/faktory_worker/connection_manager.ex +++ b/lib/faktory_worker/connection_manager.ex @@ -32,14 +32,14 @@ defmodule FaktoryWorker.ConnectionManager do {Connection.response(), ConnectionManager.t()} def send_command(state, command, allow_retry \\ true) do case try_send_command(state, command) do - {{:error, reason}, _} when reason in @connection_errors -> + {{:error, reason}, state} when reason in @connection_errors -> # Close dangling port close_connection(state) error = {:error, "Failed to connect to Faktory"} state = %{state | conn: nil} if allow_retry, - do: send_command(%{state | conn: nil}, command, false), + do: send_command(state, command, false), else: {error, state} # Handle errors from Faktory that should not be tried again From 4def66ffb4383f5177032e3b2f0e74c1ddc51562 Mon Sep 17 00:00:00 2001 From: Adam Parod Date: Fri, 7 Apr 2023 16:58:23 -0500 Subject: [PATCH 2/3] Update tests to expect the socket being closed --- .../connection_manager_test.exs | 22 ++++------ .../worker/heartbeat_server_test.exs | 16 +++---- test/faktory_worker/worker_test.exs | 44 +++++++------------ 3 files changed, 29 insertions(+), 53 deletions(-) diff --git a/test/faktory_worker/connection_manager_test.exs b/test/faktory_worker/connection_manager_test.exs index d6a5592..7a782d5 100644 --- a/test/faktory_worker/connection_manager_test.exs +++ b/test/faktory_worker/connection_manager_test.exs @@ -97,13 +97,10 @@ defmodule FaktoryWorker.ConnectionManagerTest do test "should unset the connection when there is a socket failure" do connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, "INFO\r\n" -> - {:error, :closed} - end) - - expect(FaktoryWorker.SocketMock, :connect, fn _, _, _ -> - {:error, :econnrefused} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, "INFO\r\n" -> {:error, :closed} end) + |> expect(:connect, fn _, _, _ -> {:error, :econnrefused} end) + |> expect(:close, fn _ -> :ok end) opts = [socket_handler: FaktoryWorker.SocketMock] state = ConnectionManager.new(opts) @@ -161,13 +158,10 @@ defmodule FaktoryWorker.ConnectionManagerTest do connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, _ -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:ok, "+OK\r\n"} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, _ -> :ok end) + |> expect(:recv, fn _ -> {:ok, "+OK\r\n"} end) + |> expect(:close, fn _ -> :ok end) opts = [socket_handler: FaktoryWorker.SocketMock] diff --git a/test/faktory_worker/worker/heartbeat_server_test.exs b/test/faktory_worker/worker/heartbeat_server_test.exs index acaa5ac..aae2162 100644 --- a/test/faktory_worker/worker/heartbeat_server_test.exs +++ b/test/faktory_worker/worker/heartbeat_server_test.exs @@ -241,17 +241,11 @@ defmodule FaktoryWorker.Worker.HeartbeatServerTest do worker_connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, ^beat_command -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:error, :closed} - end) - - expect(FaktoryWorker.SocketMock, :connect, fn _, _, _ -> - {:error, :econnrefused} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, ^beat_command -> :ok end) + |> expect(:recv, fn _ -> {:error, :closed} end) + |> expect(:connect, fn _, _, _ -> {:error, :econnrefused} end) + |> expect(:close, fn _ -> :ok end) opts = [socket_handler: FaktoryWorker.SocketMock, is_worker: true, process_wid: process_wid] diff --git a/test/faktory_worker/worker_test.exs b/test/faktory_worker/worker_test.exs index 4b01680..38242a9 100644 --- a/test/faktory_worker/worker_test.exs +++ b/test/faktory_worker/worker_test.exs @@ -792,24 +792,18 @@ defmodule FaktoryWorker.WorkerTest do worker_connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, ^ack_command -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:error, :closed} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, ^ack_command -> :ok end) + |> expect(:recv, fn _ -> {:error, :closed} end) + |> expect(:close, fn _ -> :ok end) # the connection manager retries a failed request once worker_connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, ^ack_command -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:error, :closed} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, ^ack_command -> :ok end) + |> expect(:recv, fn _ -> {:error, :closed} end) + |> expect(:close, fn _ -> :ok end) opts = [ process_wid: Random.process_wid(), @@ -858,24 +852,18 @@ defmodule FaktoryWorker.WorkerTest do worker_connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, ^fail_command -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:error, :closed} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, ^fail_command -> :ok end) + |> expect(:recv, fn _ -> {:error, :closed} end) + |> expect(:close, fn _ -> :ok end) # the connection manager retries a failed request one more time worker_connection_mox() - expect(FaktoryWorker.SocketMock, :send, fn _, ^fail_command -> - :ok - end) - - expect(FaktoryWorker.SocketMock, :recv, fn _ -> - {:error, :closed} - end) + FaktoryWorker.SocketMock + |> expect(:send, fn _, ^fail_command -> :ok end) + |> expect(:recv, fn _ -> {:error, :closed} end) + |> expect(:close, fn _ -> :ok end) opts = [ process_wid: Random.process_wid(), From 680e00aef9ba29f22cbdea3d2d9867fe11f7bb6d Mon Sep 17 00:00:00 2001 From: Adam Parod Date: Fri, 7 Apr 2023 16:58:57 -0500 Subject: [PATCH 3/3] Update inaccurate README testing instructions --- README.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 992a47e..e323d2f 100644 --- a/README.md +++ b/README.md @@ -137,15 +137,14 @@ Creating faktory_worker_test_tls ... done Creating faktory_worker_password_test ... done ``` +Faktory has a free, open-source solution and an enterprise edition. By default, tests for the enterprise edition of Faktory are excluded: -Faktory have free open-source solution and enterprise edition. - -If you don't have enterprise license then tests will fail on enterprise features (batching operations etc). In this case you can exclude them by tag `:enterprise` ```sh -$ mix test --exclude enterprise +$ mix test ``` -If you are enterprise user all tests should pass +If you wish to run the enterprise tests, you may include them, like so: + ```sh -$ mix test +$ mix test --include enterprise ```