From 9c3523d9d11b7431f96bc36d5f53b9664452bf34 Mon Sep 17 00:00:00 2001 From: Mike Cifelli Date: Fri, 19 Apr 2024 11:34:37 -0400 Subject: [PATCH] Refactor mocks --- config/test.exs | 7 +++-- lib/chronoscope/nts.ex | 20 ++++++++------ lib/chronoscope/nts/certificate.ex | 8 +++--- lib/chronoscope/nts/client.ex | 7 ++--- .../nts/key_establishment_client.ex | 17 +++++------- .../v1/nts/key_establishment_controller.ex | 7 ++--- test/chronoscope/nts_test.exs | 26 ++++++++++++++++++- test/support/behaviours.ex | 23 ++++++++++++++++ test/support/mocks.ex | 19 +++----------- test/support/stubs.ex | 4 +++ 10 files changed, 86 insertions(+), 52 deletions(-) create mode 100644 test/support/behaviours.ex create mode 100644 test/support/stubs.ex diff --git a/config/test.exs b/config/test.exs index fc6404a..4e2df64 100644 --- a/config/test.exs +++ b/config/test.exs @@ -21,5 +21,8 @@ config :phoenix, :plug_init_mode, :runtime config :chronoscope, Chronoscope.NTS, behaviour: Chronoscope.NTS.BehaviourMock, - datetime: Chronoscope.NTS.DateTimeMock, - ssl: Chronoscope.NTS.SSLMock + date_time: Chronoscope.NTS.DateTimeMock, + ssl: Chronoscope.NTS.SSLMock, + registry: Chronoscope.NTS.RegistryMock, + dynamic_supervisor: Chronoscope.NTS.DynamicSupervisorMock, + gen_server: Chronoscope.NTS.GenServerMock diff --git a/lib/chronoscope/nts.ex b/lib/chronoscope/nts.ex index 6d211d6..ccd8ed8 100644 --- a/lib/chronoscope/nts.ex +++ b/lib/chronoscope/nts.ex @@ -9,22 +9,26 @@ defmodule Chronoscope.NTS do alias Chronoscope.NTS + @registry Application.compile_env(:chronoscope, Chronoscope.NTS)[:registry] || Registry + @genserver Application.compile_env(:chronoscope, Chronoscope.NTS)[:gen_server] || GenServer + @dynamic_supervisor Application.compile_env(:chronoscope, Chronoscope.NTS)[:dynamic_supervisor] || DynamicSupervisor + def healthy?() do true end def list() do NTS.DynamicSupervisor - |> DynamicSupervisor.which_children() - |> Enum.map(fn {_, pid, _, _} -> GenServer.call(pid, :list) end) + |> @dynamic_supervisor.which_children() + |> Enum.map(fn {_, pid, _, _} -> @genserver.call(pid, :list) end) end def remove(host, port) do name = client_name(%{host: host, port: port}) - case Registry.lookup(NTS.Registry, name) do - [{pid, _}] -> GenServer.call(pid, :terminate) - [] -> {:error, :notfound} + case @registry.lookup(NTS.Registry, name) do + [{pid, _}] -> {:ok, @genserver.call(pid, :terminate)} + [] -> {:error, :not_found} end end @@ -32,13 +36,13 @@ defmodule Chronoscope.NTS do def key_establishment(host, port) do %{host: host, port: port} |> client_pid() - |> GenServer.call(:key_establishment) + |> @genserver.call(:key_establishment) end defp client_pid(server) do name = client_name(server) - case Registry.lookup(NTS.Registry, name) do + case @registry.lookup(NTS.Registry, name) do [{pid, _}] -> pid [] -> start_client(server, name) end @@ -50,7 +54,7 @@ defmodule Chronoscope.NTS do defp start_client(server, name) do NTS.DynamicSupervisor - |> DynamicSupervisor.start_child({NTS.Client, server: server, name: {:via, Registry, {NTS.Registry, name}}}) + |> @dynamic_supervisor.start_child({NTS.Client, server: server, name: {:via, @registry, {NTS.Registry, name}}}) |> then(fn {:ok, pid} -> pid end) end end diff --git a/lib/chronoscope/nts/certificate.ex b/lib/chronoscope/nts/certificate.ex index 31000f2..e9f4e76 100644 --- a/lib/chronoscope/nts/certificate.ex +++ b/lib/chronoscope/nts/certificate.ex @@ -1,4 +1,6 @@ defmodule Chronoscope.NTS.Certificate do + @date_time Application.compile_env(:chronoscope, Chronoscope.NTS)[:date_time] || DateTime + def expiration_date(certificate) do {:Validity, _, {:utcTime, expiration}} = certificate @@ -20,7 +22,7 @@ defmodule Chronoscope.NTS.Certificate do defp short_year_to_full_year(short_year) do {century, current_year} = - datetime().utc_now().year + @date_time.utc_now().year |> to_string() |> String.split_at(-2) @@ -32,8 +34,4 @@ defmodule Chronoscope.NTS.Certificate do |> then(&"#{&1 + 1}#{short_year}") end end - - defp datetime() do - Application.get_env(:chronoscope, Chronoscope.NTS)[:datetime] || DateTime - end end diff --git a/lib/chronoscope/nts/client.ex b/lib/chronoscope/nts/client.ex index d1fff9c..c887c32 100644 --- a/lib/chronoscope/nts/client.ex +++ b/lib/chronoscope/nts/client.ex @@ -5,6 +5,7 @@ defmodule Chronoscope.NTS.Client do alias Chronoscope.NTS.KeyEstablishmentClient @interval_in_seconds 30 + @date_time Application.compile_env(:chronoscope, Chronoscope.NTS)[:date_time] || DateTime def start_link(server: server, name: name) do GenServer.start_link(__MODULE__, server, name: name) @@ -70,10 +71,6 @@ defmodule Chronoscope.NTS.Client do end defp utc_now() do - datetime().utc_now() - end - - defp datetime() do - Application.get_env(:chronoscope, Chronoscope.NTS)[:datetime] || DateTime + @date_time.utc_now() end end diff --git a/lib/chronoscope/nts/key_establishment_client.ex b/lib/chronoscope/nts/key_establishment_client.ex index 4612b78..a72e2c8 100644 --- a/lib/chronoscope/nts/key_establishment_client.ex +++ b/lib/chronoscope/nts/key_establishment_client.ex @@ -6,6 +6,7 @@ defmodule Chronoscope.NTS.KeyEstablishmentClient do alias Chronoscope.NTS.KeyEstablishmentResponse @timeout_in_milliseconds 3000 + @ssl Application.compile_env(:chronoscope, Chronoscope.NTS)[:ssl] || :ssl def key_establishment(%{host: host, port: port}) do case ssl_connect(host, port) do @@ -21,7 +22,7 @@ defmodule Chronoscope.NTS.KeyEstablishmentClient do defp ssl_connect(host, port) do host |> String.to_charlist() - |> ssl().connect(port, tls_options(host), @timeout_in_milliseconds) + |> @ssl.connect(port, tls_options(host), @timeout_in_milliseconds) end defp tls_options(host) do @@ -31,21 +32,21 @@ defmodule Chronoscope.NTS.KeyEstablishmentClient do end defp perform_key_establishment(socket) do - :ok = ssl().send(socket, KeyEstablishmentRequest.create()) - {:ok, peercert} = ssl().peercert(socket) + :ok = @ssl.send(socket, KeyEstablishmentRequest.create()) + {:ok, peercert} = @ssl.peercert(socket) receive do {:ssl, _socket, response} -> - ssl().close(socket) + @ssl.close(socket) parse_response(response, peercert) msg -> - ssl().close(socket) + @ssl.close(socket) Logger.error("received unexpected message: #{inspect(msg)}") {:error, :no_response} after @timeout_in_milliseconds -> - ssl().close(socket) + @ssl.close(socket) Logger.error("timed out waiting for response") {:error, :timeout} end @@ -67,8 +68,4 @@ defmodule Chronoscope.NTS.KeyEstablishmentClient do {:error, String.trim(error)} end end - - defp ssl() do - Application.get_env(:chronoscope, Chronoscope.NTS)[:ssl] || :ssl - end end diff --git a/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex b/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex index 3a1f4e0..ffe5c94 100644 --- a/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex +++ b/lib/chronoscope_web/controllers/api/v1/nts/key_establishment_controller.ex @@ -7,6 +7,7 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do @default_port 4460 @max_host_length 255 + @nts Application.compile_env(:chronoscope, NTS)[:behaviour] || NTS def get(conn, %{"host" => host, "port" => port}) do try do @@ -41,7 +42,7 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do defp key_establishment_response(host, port) do host |> String.slice(0, @max_host_length) - |> nts_behaviour().key_establishment(port) + |> @nts.key_establishment(port) end defp format_response(response) do @@ -55,8 +56,4 @@ defmodule ChronoscopeWeb.API.V1.NTS.KeyEstablishmentController do |> put_status(:bad_request) |> json(%{error: message}) end - - defp nts_behaviour() do - Application.get_env(:chronoscope, NTS)[:behaviour] || NTS - end end diff --git a/test/chronoscope/nts_test.exs b/test/chronoscope/nts_test.exs index c8fbefc..08b3b25 100644 --- a/test/chronoscope/nts_test.exs +++ b/test/chronoscope/nts_test.exs @@ -1,7 +1,14 @@ defmodule Chronoscope.NTSTest do use Chronoscope.Case + alias Chronoscope.NTS.DynamicSupervisorMock + alias Chronoscope.NTS.GenServerMock + alias Chronoscope.NTS.RegistryMock + import Chronoscope.NTS + import Mox + + setup :verify_on_exit! describe "Chronoscope.NTS.healthy?()" do test "is healthy" do @@ -11,13 +18,30 @@ defmodule Chronoscope.NTSTest do describe "Chronoscope.NTS.list()" do test "shows empty client list" do + DynamicSupervisorMock + |> expect(:which_children, fn _ -> [] end) + assert list() == [] end + + test "lists all children" do + DynamicSupervisorMock + |> expect(:which_children, fn _ -> [{1, 2, 3, 4}, {5, 6, 7, 8}] end) + + GenServerMock + |> expect(:call, fn 2, :list -> :one end) + |> expect(:call, fn 6, :list -> :two end) + + assert list() == [:one, :two] + end end describe "Chronoscope.NTS.remove()" do test "does nothing if the client doesn't exist" do - assert remove("localhost", 1111) == {:error, :notfound} + RegistryMock + |> expect(:lookup, fn _, _ -> [] end) + + assert remove("localhost", 1111) == {:error, :not_found} end end end diff --git a/test/support/behaviours.ex b/test/support/behaviours.ex new file mode 100644 index 0000000..b5003f6 --- /dev/null +++ b/test/support/behaviours.ex @@ -0,0 +1,23 @@ +defmodule Chronoscope.DateTime.Behaviour do + @callback utc_now :: DateTime.t() +end + +defmodule Chronoscope.SSL.Behaviour do + @callback connect(any(), any(), any(), any()) :: {:ok, any()} | {:error, any()} + @callback send(any(), any()) :: :ok | {:error, any()} + @callback peercert(any()) :: {:ok, any()} | {:error, any()} + @callback close(any()) :: :ok | {:error, any()} +end + +defmodule Chronoscope.Registry.Behaviour do + @callback lookup(atom(), any()) :: [{pid(), any()}] +end + +defmodule Chronoscope.DynamicSupervisor.Behaviour do + @callback start_child(Supervisor.supervisor(), any()) :: any() + @callback which_children(Supervisor.supervisor()) :: [any()] +end + +defmodule Chronoscope.GenServer.Behaviour do + @callback call(pid(), any()) :: any() +end diff --git a/test/support/mocks.ex b/test/support/mocks.ex index 37ee853..53a182e 100644 --- a/test/support/mocks.ex +++ b/test/support/mocks.ex @@ -1,19 +1,6 @@ -defmodule Chronoscope.DateTime.Behaviour do - @callback utc_now :: DateTime.t() -end - -defmodule Chronoscope.DateTime.Stub do - @behaviour Chronoscope.DateTime.Behaviour - def utc_now(), do: DateTime.utc_now() -end - -defmodule Chronoscope.SSL.Behaviour do - @callback connect(any(), any(), any(), any()) :: {:ok, any()} | {:error, any()} - @callback send(any(), any()) :: :ok | {:error, any()} - @callback peercert(any()) :: {:ok, any()} | {:error, any()} - @callback close(any()) :: :ok | {:error, any()} -end - Mox.defmock(Chronoscope.NTS.BehaviourMock, for: Chronoscope.NTS.Behaviour) Mox.defmock(Chronoscope.NTS.DateTimeMock, for: Chronoscope.DateTime.Behaviour) Mox.defmock(Chronoscope.NTS.SSLMock, for: Chronoscope.SSL.Behaviour) +Mox.defmock(Chronoscope.NTS.RegistryMock, for: Chronoscope.Registry.Behaviour) +Mox.defmock(Chronoscope.NTS.DynamicSupervisorMock, for: Chronoscope.DynamicSupervisor.Behaviour) +Mox.defmock(Chronoscope.NTS.GenServerMock, for: Chronoscope.GenServer.Behaviour) diff --git a/test/support/stubs.ex b/test/support/stubs.ex new file mode 100644 index 0000000..92ec93e --- /dev/null +++ b/test/support/stubs.ex @@ -0,0 +1,4 @@ +defmodule Chronoscope.DateTime.Stub do + @behaviour Chronoscope.DateTime.Behaviour + def utc_now(), do: DateTime.utc_now() +end