From 481be8ad8bb4395aa19438905fcee305aab4e598 Mon Sep 17 00:00:00 2001 From: Xabier Arbulu Insausti Date: Thu, 3 Nov 2022 09:04:24 +0100 Subject: [PATCH] Replace mock usage for dispatch (#946) * Use CommandAuditMiddleware to test the commands * Use adapter on commanded dispatch to improve testability * Inactivate heartbeat checker scheduler in test mode --- config/config.exs | 2 + config/test.exs | 7 + .../event_handlers/rollup_event_handler.ex | 5 +- .../application/integration/checks/checks.ex | 7 +- .../integration/discovery/discovery.ex | 7 +- .../application/usecases/clusters/clusters.ex | 9 +- .../application/usecases/hosts/heartbeats.ex | 5 +- test/support/factory.ex | 8 + test/test_helper.exs | 4 + .../integration/checks/checks_test.exs | 214 +++++++++--------- .../application/usecases/clusters_test.exs | 34 +-- .../application/usecases/heartbeats_test.exs | 95 +++++--- 12 files changed, 238 insertions(+), 159 deletions(-) diff --git a/config/config.exs b/config/config.exs index dcc4ab0317..ff4d3e4b16 100644 --- a/config/config.exs +++ b/config/config.exs @@ -53,6 +53,8 @@ config :logger, :console, # Use Jason for JSON parsing in Phoenix config :phoenix, :json_library, Jason +config :trento, Trento.Commanded, adapter: Trento.Commanded + config :trento, Trento.Commanded, event_store: [ adapter: Commanded.EventStore.Adapters.EventStore, diff --git a/config/test.exs b/config/test.exs index be965ee2f4..c793d3e83f 100644 --- a/config/test.exs +++ b/config/test.exs @@ -70,6 +70,13 @@ config :trento, Trento.Integration.Checks.Wanda.Messaging.AMQP, ] ] +config :trento, Trento.Scheduler, + jobs: [ + heartbeat_check: [ + state: :inactive + ] + ] + config :trento, extra_children: [ Trento.Messaging.Adapters.AMQP.Publisher, diff --git a/lib/trento/application/event_handlers/rollup_event_handler.ex b/lib/trento/application/event_handlers/rollup_event_handler.ex index 3433c7d0db..3255f5b701 100644 --- a/lib/trento/application/event_handlers/rollup_event_handler.ex +++ b/lib/trento/application/event_handlers/rollup_event_handler.ex @@ -26,6 +26,9 @@ defmodule Trento.RollupEventHandler do defp after_max_retries_reached(%ClusterRolledUp{cluster_id: cluster_id}, _, _) do %{cluster_id: cluster_id} |> AbortClusterRollup.new!() - |> Trento.Commanded.dispatch() + |> commanded().dispatch() end + + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] end diff --git a/lib/trento/application/integration/checks/checks.ex b/lib/trento/application/integration/checks/checks.ex index cb7ae0ac63..9d07fe83cd 100644 --- a/lib/trento/application/integration/checks/checks.ex +++ b/lib/trento/application/integration/checks/checks.ex @@ -59,7 +59,7 @@ defmodule Trento.Integration.Checks do }) do case StartChecksExecution.new(%{cluster_id: cluster_id}) do {:ok, command} -> - Trento.Commanded.dispatch(command, correlation_id: execution_id) + commanded().dispatch(command, correlation_id: execution_id) error -> error @@ -74,12 +74,15 @@ defmodule Trento.Integration.Checks do with {:ok, execution_completed_event} <- ExecutionCompletedEventDto.new(payload), {:ok, command} <- build_complete_checks_execution_command(execution_completed_event) do - Trento.Commanded.dispatch(command, correlation_id: execution_id) + commanded().dispatch(command, correlation_id: execution_id) end end def handle_callback(_), do: {:error, :invalid_payload} + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] + defp adapter, do: Application.fetch_env!(:trento, __MODULE__)[:adapter] diff --git a/lib/trento/application/integration/discovery/discovery.ex b/lib/trento/application/integration/discovery/discovery.ex index fea3233ebd..cd73f21eff 100644 --- a/lib/trento/application/integration/discovery/discovery.ex +++ b/lib/trento/application/integration/discovery/discovery.ex @@ -127,7 +127,7 @@ defmodule Trento.Integration.Discovery do @spec dispatch(command | [command]) :: :ok | {:error, any} defp dispatch(commands) when is_list(commands) do Enum.reduce(commands, :ok, fn command, acc -> - case {Trento.Commanded.dispatch(command), acc} do + case {commanded().dispatch(command), acc} do {:ok, :ok} -> :ok @@ -140,5 +140,8 @@ defmodule Trento.Integration.Discovery do end) end - defp dispatch(command), do: Trento.Commanded.dispatch(command) + defp dispatch(command), do: commanded().dispatch(command) + + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] end diff --git a/lib/trento/application/usecases/clusters/clusters.ex b/lib/trento/application/usecases/clusters/clusters.ex index 2ee59c2833..3a59af9679 100644 --- a/lib/trento/application/usecases/clusters/clusters.ex +++ b/lib/trento/application/usecases/clusters/clusters.ex @@ -29,21 +29,21 @@ defmodule Trento.Clusters do host_id: host_id, checks_results: build_check_results(checks_results) }) do - Trento.Commanded.dispatch(command) + commanded().dispatch(command) end end @spec select_checks(String.t(), [String.t()]) :: :ok | {:error, any} def select_checks(cluster_id, checks) do with {:ok, command} <- SelectChecks.new(%{cluster_id: cluster_id, checks: checks}) do - Trento.Commanded.dispatch(command) + commanded().dispatch(command) end end @spec request_checks_execution(String.t()) :: :ok | {:error, any} def request_checks_execution(cluster_id) do with {:ok, command} <- RequestChecksExecution.new(%{cluster_id: cluster_id}) do - Trento.Commanded.dispatch(command) + commanded().dispatch(command) end end @@ -96,6 +96,9 @@ defmodule Trento.Clusters do end) end + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] + @spec build_check_results([String.t()]) :: {:ok, [CheckResult.t()]} | {:error, any} defp build_check_results(checks_results) do Enum.map(checks_results, fn c -> diff --git a/lib/trento/application/usecases/hosts/heartbeats.ex b/lib/trento/application/usecases/hosts/heartbeats.ex index 48d2fb2db4..b5a9f9d339 100644 --- a/lib/trento/application/usecases/hosts/heartbeats.ex +++ b/lib/trento/application/usecases/hosts/heartbeats.ex @@ -56,7 +56,7 @@ defmodule Trento.Heartbeats do defp dispatch_command(agent_id, heartbeat) do case %{host_id: agent_id, heartbeat: heartbeat} |> UpdateHeartbeat.new!() - |> Trento.Commanded.dispatch() do + |> commanded().dispatch() do :ok -> {:ok, :done} @@ -64,4 +64,7 @@ defmodule Trento.Heartbeats do error end end + + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] end diff --git a/test/support/factory.ex b/test/support/factory.ex index d3fc886232..ace89cd3f6 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -41,6 +41,7 @@ defmodule Trento.Factory do ClusterReadModel, DatabaseInstanceReadModel, DatabaseReadModel, + Heartbeat, HostChecksExecutionsReadModel, HostConnectionSettings, HostReadModel, @@ -102,6 +103,13 @@ defmodule Trento.Factory do } end + def heartbeat_factory do + %Heartbeat{ + agent_id: Faker.UUID.v4(), + timestamp: DateTime.utc_now() + } + end + def host_connection_settings_factory do %HostConnectionSettings{ id: Faker.UUID.v4(), diff --git a/test/test_helper.exs b/test/test_helper.exs index aebc5afe25..fa6f42ccd7 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,3 +1,7 @@ +Mox.defmock(Trento.Commanded.Mock, for: Commanded.Application) + +Application.put_env(:trento, Trento.Commanded, adapter: Trento.Commanded.Mock) + Mox.defmock(Trento.Integration.Telemetry.Mock, for: Trento.Integration.Telemetry.Gen) Application.put_env(:trento, Trento.Integration.Telemetry, diff --git a/test/trento/application/integration/checks/checks_test.exs b/test/trento/application/integration/checks/checks_test.exs index a7b41169c4..a6aa17d86c 100644 --- a/test/trento/application/integration/checks/checks_test.exs +++ b/test/trento/application/integration/checks/checks_test.exs @@ -3,8 +3,6 @@ defmodule Trento.Integration.ChecksTest do use Trento.DataCase import Mox - # TODO: Remove Mock usage - import Mock alias Trento.Integration.Checks @@ -29,6 +27,8 @@ defmodule Trento.Integration.ChecksTest do @runner_fixtures_path File.cwd!() <> "/test/fixtures/runner" + setup [:set_mox_from_context, :verify_on_exit!] + def load_runner_fixture(name) do @runner_fixtures_path |> Path.join("#{name}.json") @@ -355,112 +355,122 @@ defmodule Trento.Integration.ChecksTest do end test "should handle execution started event properly" do - with_mock Trento.Commanded, dispatch: fn _, _ -> :ok end do - execution_id = Faker.UUID.v4() - cluster_id = Faker.UUID.v4() + execution_id = Faker.UUID.v4() + cluster_id = Faker.UUID.v4() - Checks.handle_callback(%{ - "event" => "execution_started", - "execution_id" => execution_id, - "payload" => %{ - "cluster_id" => cluster_id - } - }) + expect( + Trento.Commanded.Mock, + :dispatch, + fn command, opts -> + assert %StartChecksExecution{ + cluster_id: ^cluster_id + } = command + + assert [correlation_id: ^execution_id] = opts + + :ok + end + ) - assert_called Trento.Commanded.dispatch( - %StartChecksExecution{ - cluster_id: cluster_id - }, - correlation_id: execution_id - ) - end + Checks.handle_callback(%{ + "event" => "execution_started", + "execution_id" => execution_id, + "payload" => %{ + "cluster_id" => cluster_id + } + }) end test "should handle execution completed event properly" do - with_mock Trento.Commanded, dispatch: fn _, _ -> :ok end do - execution_id = Faker.UUID.v4() - cluster_id = Faker.UUID.v4() - host_id_1 = Faker.UUID.v4() - host_id_2 = Faker.UUID.v4() + execution_id = Faker.UUID.v4() + cluster_id = Faker.UUID.v4() + host_id_1 = Faker.UUID.v4() + host_id_2 = Faker.UUID.v4() - Checks.handle_callback(%{ - "event" => "execution_completed", - "execution_id" => execution_id, - "payload" => %{ - "cluster_id" => cluster_id, - "hosts" => [ - %{ - "host_id" => host_id_1, - "reachable" => true, - "msg" => "", - "results" => [ - %{ - "check_id" => "check1", - "result" => "passing" - }, - %{ - "check_id" => "check2", - "result" => "warning" - } - ] - }, - %{ - "host_id" => host_id_2, - "reachable" => true, - "msg" => "", - "results" => [ - %{ - "check_id" => "check1", - "result" => "critical" - }, - %{ - "check_id" => "check2", - "result" => "warning" - } - ] - } - ] - } - }) + expect( + Trento.Commanded.Mock, + :dispatch, + fn command, opts -> + assert %CompleteChecksExecution{ + cluster_id: ^cluster_id, + hosts_executions: [ + %HostExecution{ + checks_results: [ + %CheckResult{ + check_id: "check1", + result: :passing + }, + %CheckResult{ + check_id: "check2", + result: :warning + } + ], + host_id: ^host_id_1, + msg: nil, + reachable: true + }, + %HostExecution{ + checks_results: [ + %CheckResult{ + check_id: "check1", + result: :critical + }, + %CheckResult{ + check_id: "check2", + result: :warning + } + ], + host_id: ^host_id_2, + msg: nil, + reachable: true + } + ] + } = command + + assert [correlation_id: ^execution_id] = opts + + :ok + end + ) - assert_called Trento.Commanded.dispatch( - %CompleteChecksExecution{ - cluster_id: cluster_id, - hosts_executions: [ - %HostExecution{ - checks_results: [ - %CheckResult{ - check_id: "check1", - result: :passing - }, - %CheckResult{ - check_id: "check2", - result: :warning - } - ], - host_id: host_id_1, - msg: nil, - reachable: true - }, - %HostExecution{ - checks_results: [ - %CheckResult{ - check_id: "check1", - result: :critical - }, - %CheckResult{ - check_id: "check2", - result: :warning - } - ], - host_id: host_id_2, - msg: nil, - reachable: true - } - ] - }, - correlation_id: execution_id - ) - end + Checks.handle_callback(%{ + "event" => "execution_completed", + "execution_id" => execution_id, + "payload" => %{ + "cluster_id" => cluster_id, + "hosts" => [ + %{ + "host_id" => host_id_1, + "reachable" => true, + "msg" => "", + "results" => [ + %{ + "check_id" => "check1", + "result" => "passing" + }, + %{ + "check_id" => "check2", + "result" => "warning" + } + ] + }, + %{ + "host_id" => host_id_2, + "reachable" => true, + "msg" => "", + "results" => [ + %{ + "check_id" => "check1", + "result" => "critical" + }, + %{ + "check_id" => "check2", + "result" => "warning" + } + ] + } + ] + } + }) end end diff --git a/test/trento/application/usecases/clusters_test.exs b/test/trento/application/usecases/clusters_test.exs index d4494631fb..85abfe1601 100644 --- a/test/trento/application/usecases/clusters_test.exs +++ b/test/trento/application/usecases/clusters_test.exs @@ -3,8 +3,9 @@ defmodule Trento.ClustersTest do use ExUnit.Case, async: true use Trento.DataCase + import Mox + import Trento.Factory - import Mock alias Trento.Clusters @@ -13,20 +14,27 @@ defmodule Trento.ClustersTest do alias Trento.Domain.Commands.RequestChecksExecution + setup [:set_mox_from_context, :verify_on_exit!] + describe "checks execution" do test "should dispatch checks execution requests for each cluster" do - # TODO: use Mox and beavhiours to test this - with_mock Trento.Commanded, dispatch: fn _ -> :ok end do - clusters = Enum.map(0..4, fn _ -> insert(:cluster) end) - - :ok = Clusters.request_clusters_checks_execution() - - Enum.each(clusters, fn cluster -> - assert_called Trento.Commanded.dispatch(%RequestChecksExecution{ - cluster_id: cluster.id - }) - end) - end + clusters = Enum.map(0..4, fn _ -> insert(:cluster) end) + + Enum.each(clusters, fn %{id: cluster_id} -> + expect( + Trento.Commanded.Mock, + :dispatch, + fn command -> + assert %RequestChecksExecution{ + cluster_id: ^cluster_id + } = command + + :ok + end + ) + end) + + :ok = Clusters.request_clusters_checks_execution() end end diff --git a/test/trento/application/usecases/heartbeats_test.exs b/test/trento/application/usecases/heartbeats_test.exs index e44d7370ee..cde56e0f5f 100644 --- a/test/trento/application/usecases/heartbeats_test.exs +++ b/test/trento/application/usecases/heartbeats_test.exs @@ -2,8 +2,11 @@ defmodule Trento.HeartbeatsTest do use ExUnit.Case use Trento.DataCase + import Mox import Mock + import Trento.Factory + alias Trento.{ Heartbeat, Heartbeats @@ -15,75 +18,97 @@ defmodule Trento.HeartbeatsTest do @moduletag :integration + setup [:set_mox_from_context, :verify_on_exit!] + test "create new heartbeat" do now = DateTime.utc_now() + agent_id = Faker.UUID.v4() + + with_mock DateTime, [:passthrough], utc_now: fn -> now end do + expect( + Trento.Commanded.Mock, + :dispatch, + fn command -> + assert %UpdateHeartbeat{ + host_id: ^agent_id, + heartbeat: :passing + } = command + + :ok + end + ) - with_mocks [ - {DateTime, [:passthrough], utc_now: fn -> now end}, - {Trento.Commanded, [:passthrough], dispatch: fn _ -> :ok end} - ] do - agent_id = Faker.UUID.v4() Heartbeats.heartbeat(agent_id) [heartbeat] = Repo.all(Heartbeat) assert heartbeat.agent_id == agent_id assert heartbeat.timestamp == now - - assert_called Trento.Commanded.dispatch(%UpdateHeartbeat{ - host_id: agent_id, - heartbeat: :passing - }) end end test "update existing heartbeat" do agent_id = Faker.UUID.v4() + insert(:host, id: agent_id, heartbeat: :critical) + %{timestamp: now} = insert(:heartbeat, agent_id: agent_id) - with_mock Trento.Commanded, [:passthrough], dispatch: fn _ -> :ok end do - Heartbeats.heartbeat(agent_id) - end + updated_time = + DateTime.add( + now, + Application.get_env(:trento, Heartbeats)[:interval] + 1, + :millisecond + ) - now = DateTime.utc_now() + with_mock DateTime, [:passthrough], utc_now: fn -> updated_time end do + expect( + Trento.Commanded.Mock, + :dispatch, + fn command -> + assert %UpdateHeartbeat{ + host_id: ^agent_id, + heartbeat: :passing + } = command + + :ok + end + ) - with_mocks [ - {DateTime, [:passthrough], utc_now: fn -> now end}, - {Trento.Commanded, [:passthrough], dispatch: fn _ -> :ok end} - ] do Heartbeats.heartbeat(agent_id) [heartbeat] = Repo.all(Heartbeat) - assert heartbeat.timestamp == now - - assert_called Trento.Commanded.dispatch(:_) + assert heartbeat.timestamp == updated_time end end test "dispatch commands on heartbeat expiration" do agent_id = Faker.UUID.v4() - with_mock Trento.Commanded, [:passthrough], dispatch: fn _ -> :ok end do - Heartbeats.heartbeat(agent_id) - end + insert(:host, id: agent_id, heartbeat: :passing) + %{timestamp: now} = insert(:heartbeat, agent_id: agent_id) - now = + expired_time = DateTime.add( - DateTime.utc_now(), + now, Application.get_env(:trento, Heartbeats)[:interval] + 1, :millisecond ) - with_mocks [ - {DateTime, [:passthrough], utc_now: fn -> now end}, - {Trento.Commanded, [:passthrough], dispatch: fn _ -> :ok end} - ] do - Heartbeats.dispatch_heartbeat_failed_commands() + with_mock DateTime, [:passthrough], utc_now: fn -> expired_time end do + expect( + Trento.Commanded.Mock, + :dispatch, + fn command -> + assert %UpdateHeartbeat{ + host_id: ^agent_id, + heartbeat: :critical + } = command + + :ok + end + ) - assert_called Trento.Commanded.dispatch(%UpdateHeartbeat{ - host_id: agent_id, - heartbeat: :critical - }) + Heartbeats.dispatch_heartbeat_failed_commands() end end end