From b105400cec9c59adac5d815cd5286f27d074e94e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarl=20Andr=C3=A9=20H=C3=BCbenthal?= Date: Mon, 18 Nov 2024 20:35:11 +0100 Subject: [PATCH] remove custom logger and improve mix task --- config/test.exs | 2 - lib/connection/connection.ex | 7 ++-- .../docker_socket_path.ex | 5 ++- lib/container.ex | 4 +- lib/ecto.ex | 5 ++- lib/mix/tasks/testcontainers/test.ex | 40 ++++++++++--------- lib/testcontainers.ex | 21 +++++----- lib/util/logger.ex | 20 ---------- lib/wait_strategy/command_wait_strategy.ex | 6 ++- lib/wait_strategy/log_wait_strategy.ex | 5 ++- lib/wait_strategy/port_wait_strategy.ex | 6 ++- test/generic_container_test.exs | 4 +- 12 files changed, 59 insertions(+), 66 deletions(-) delete mode 100644 lib/util/logger.ex diff --git a/config/test.exs b/config/test.exs index 518c075..63787d4 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,5 +1,3 @@ import Config config :logger, level: :warning - -config :testcontainers, log_level: :info diff --git a/lib/connection/connection.ex b/lib/connection/connection.ex index 328495e..966c8d3 100644 --- a/lib/connection/connection.ex +++ b/lib/connection/connection.ex @@ -2,9 +2,10 @@ defmodule Testcontainers.Connection do @moduledoc false + require Logger + alias Testcontainers.Constants alias Testcontainers.DockerUrl - alias Testcontainers.Logger alias Testcontainers.DockerHostStrategyEvaluator alias Testcontainers.DockerSocketPathStrategy alias Testcontainers.DockerHostFromPropertiesStrategy @@ -15,9 +16,7 @@ defmodule Testcontainers.Connection do def get_connection(options \\ []) do {docker_host_url, docker_host} = - get_docker_host_url() |> IO.inspect(label: "Testcontainers using") - - Logger.log("Using docker host url: #{docker_host_url}") + get_docker_host_url() |> tap(&Logger.info("Docker host: #{inspect(&1, pretty: false)}")) options = Keyword.merge(options, diff --git a/lib/connection/docker_host_strategy/docker_socket_path.ex b/lib/connection/docker_host_strategy/docker_socket_path.ex index f0fcc24..861dc91 100644 --- a/lib/connection/docker_host_strategy/docker_socket_path.ex +++ b/lib/connection/docker_host_strategy/docker_socket_path.ex @@ -2,11 +2,12 @@ defmodule Testcontainers.DockerSocketPathStrategy do @moduledoc false + require Logger + defstruct socket_paths: [] defimpl Testcontainers.DockerHostStrategy do alias Testcontainers.DockerUrl - alias Testcontainers.Logger defp default_socket_paths do [ @@ -43,7 +44,7 @@ defmodule Testcontainers.DockerSocketPathStrategy do {:halt, {:ok, path_with_scheme}} {:error, reason} -> - Logger.log("Docker socket path #{path} failed: #{reason}") + Logger.debug("Docker socket path #{path} failed: #{reason}") {:cont, {:error, {:docker_socket_not_found, tried_paths ++ [path]}}} end else diff --git a/lib/container.ex b/lib/container.ex index 688fa06..587beb7 100644 --- a/lib/container.ex +++ b/lib/container.ex @@ -6,6 +6,8 @@ defmodule Testcontainers.Container do A struct with builder functions for creating a definition of a container. """ + require Logger + @enforce_keys [:image] defstruct [ :image, @@ -212,7 +214,7 @@ defmodule Testcontainers.Container do mode = String.downcase(mode) if mode == "host" and not is_os(:linux) do - Testcontainers.Logger.log( + Logger.warning( "To use host network mode on non-linux hosts, please see https://docs.docker.com/network/drivers/host" ) end diff --git a/lib/ecto.ex b/lib/ecto.ex index 3f12670..649ea21 100644 --- a/lib/ecto.ex +++ b/lib/ecto.ex @@ -7,7 +7,8 @@ defmodule Testcontainers.Ecto do This module simplifies the process of launching a real Postgres or MySql database instance within a Docker container for testing purposes. It leverages the `Testcontainers` library to instantiate a Postgres or MySql container with the desired configuration, providing an isolated database environment for each test session. """ - alias Testcontainers.Logger + require Logger + alias Testcontainers.PostgresContainer alias Testcontainers.MySqlContainer @@ -283,7 +284,7 @@ defmodule Testcontainers.Ecto do :ok = case File.exists?(absolute_migrations_path) do false -> - Logger.log("Migrations directory does not exist, this will be ignored") + Logger.debug("Migrations directory does not exist, this will be ignored") _ -> :ok diff --git a/lib/mix/tasks/testcontainers/test.ex b/lib/mix/tasks/testcontainers/test.ex index 578f77a..3242458 100644 --- a/lib/mix/tasks/testcontainers/test.ex +++ b/lib/mix/tasks/testcontainers/test.ex @@ -11,7 +11,7 @@ defmodule Mix.Tasks.Testcontainers.Test do {:ok, _} = Testcontainers.start_link() - {opts, _, _} = + {opts, args, _} = OptionParser.parse(args, switches: [ database: :string, @@ -24,10 +24,10 @@ defmodule Mix.Tasks.Testcontainers.Test do if Enum.empty?(folder_to_watch) do IO.puts("No folders specified. Only running tests.") - run_tests_and_exit(database) + run_tests_and_exit(database, args) else check_folders_exist(folder_to_watch) - run_tests_and_watch(database, folder_to_watch) + run_tests_and_watch(database, args, folder_to_watch) end end @@ -39,15 +39,15 @@ defmodule Mix.Tasks.Testcontainers.Test do end) end - @spec run_tests_and_exit(String.t()) :: no_return() - defp run_tests_and_exit(database) do + @spec run_tests_and_exit(String.t(), list(String.t())) :: no_return() + defp run_tests_and_exit(database, args) do {container, env} = setup_container(database) - exit_code = run_tests(env) + exit_code = run_tests(env, args) Testcontainers.stop_container(container.container_id) System.halt(exit_code) end - defp run_tests_and_watch(database, folders) do + defp run_tests_and_watch(database, args, folders) do {container, env} = setup_container(database) Enum.each(folders, fn folder -> @@ -55,8 +55,8 @@ defmodule Mix.Tasks.Testcontainers.Test do :fs.subscribe(String.to_atom("watcher_" <> folder)) end) - run_tests(env) - loop(env, container) + run_tests(env, args) + loop(env, args, container) end defp setup_container(database) do @@ -99,8 +99,12 @@ defmodule Mix.Tasks.Testcontainers.Test do ] end - defp run_tests(env) do - case System.cmd("mix", ["test"], env: env, into: IO.stream(:stdio, :line)) do + defp run_tests(env, args) do + case System.cmd("mix", ["test"] ++ args, + env: env, + into: IO.stream(), + stderr_to_stdout: false + ) do {_, exit_code} -> if exit_code == 0 do IO.puts("Test process completed successfully") @@ -112,27 +116,27 @@ defmodule Mix.Tasks.Testcontainers.Test do end end - defp loop(env, container) do + defp loop(env, args, container) do receive do {_watcher_process, {:fs, :file_event}, {changed_file, _type}} -> IO.puts("#{changed_file} was updated, waiting for more changes...") - wait_for_changes(env, container) + wait_for_changes(env, args, container) after 5000 -> - loop(env, container) + loop(env, args, container) end end - defp wait_for_changes(env, container) do + defp wait_for_changes(env, args, container) do receive do {_watcher_process, {:fs, :file_event}, {changed_file, _type}} -> IO.puts("#{changed_file} was updated, waiting for more changes...") - wait_for_changes(env, container) + wait_for_changes(env, args, container) after 1000 -> IO.ANSI.clear() - run_tests(env) - loop(env, container) + run_tests(env, args) + loop(env, args, container) end end end diff --git a/lib/testcontainers.ex b/lib/testcontainers.ex index 3b6de60..267c58b 100644 --- a/lib/testcontainers.ex +++ b/lib/testcontainers.ex @@ -7,9 +7,10 @@ defmodule Testcontainers do This is a GenServer that needs to be started before anything can happen. """ + require Logger + alias Testcontainers.Constants alias Testcontainers.WaitStrategy - alias Testcontainers.Logger alias Testcontainers.Docker.Api alias Testcontainers.Connection alias Testcontainers.Container @@ -64,7 +65,7 @@ defmodule Testcontainers do :ok <- register_ryuk_filter(session_id, socket), {:ok, docker_hostname} <- get_docker_hostname(docker_host_url, conn), {:ok, properties} <- PropertiesParser.read_property_file() do - Logger.log("Testcontainers initialized") + Logger.info("Testcontainers initialized") {:ok, %{ @@ -170,17 +171,17 @@ defmodule Testcontainers do uri when uri.scheme == "http+unix" -> if File.exists?("/.dockerenv") do - Logger.log("Running in docker environment, trying to get bridge network gateway") + Logger.debug("Running in docker environment, trying to get bridge network gateway") with {:ok, gateway} <- Api.get_bridge_gateway(conn) do {:ok, gateway} else {:error, reason} -> - Logger.log("Failed to get bridge gateway: #{inspect(reason)}. Using localhost") + Logger.debug("Failed to get bridge gateway: #{inspect(reason)}. Using localhost") {:ok, "localhost"} end else - Logger.log("Not running in docker environment, using localhost") + Logger.debug("Not running in docker environment, using localhost") {:ok, "localhost"} end end @@ -206,7 +207,7 @@ defmodule Testcontainers do {:ok, connected} {:error, :econnrefused} -> - Logger.log("Connection refused. Retrying... Attempt #{reattempt_count + 1}/3") + Logger.debug("Connection refused. Retrying... Attempt #{reattempt_count + 1}/3") :timer.sleep(5000) create_ryuk_socket(container, reattempt_count + 1) @@ -216,7 +217,7 @@ defmodule Testcontainers do end defp create_ryuk_socket(%Container{} = _container, _reattempt_count) do - Logger.log("Ryuk host refused to connect") + Logger.debug("Ryuk host refused to connect") {:error, :econnrefused} end @@ -244,7 +245,7 @@ defmodule Testcontainers do {:reuse, config, hash} -> case Api.get_container_by_hash(hash, state.conn) do {:error, :no_container} -> - Logger.log("Container does not exist with hash: #{hash}") + Logger.debug("Container does not exist with hash: #{hash}") create_and_start_container( config, @@ -253,11 +254,11 @@ defmodule Testcontainers do ) {:error, error} -> - Logger.log("Failed to get container by hash: #{inspect(error)}") + Logger.debug("Failed to get container by hash: #{inspect(error)}") {:error, error} {:ok, container} -> - Logger.log("Container already exists with hash: #{hash}") + Logger.debug("Container already exists with hash: #{hash}") {:ok, container} end diff --git a/lib/util/logger.ex b/lib/util/logger.ex deleted file mode 100644 index 82c2fc6..0000000 --- a/lib/util/logger.ex +++ /dev/null @@ -1,20 +0,0 @@ -# SPDX-License-Identifier: MIT -defmodule Testcontainers.Logger do - @moduledoc """ - Defines an abstraction on top of Elixir.Logger - """ - - require Logger - - import Testcontainers.Constants - - def log(message) when is_binary(message) do - case get_log_level() do - nil -> :ok - level -> Logger.log(level, message) - end - end - - defp get_log_level, - do: Application.get_env(library_name(), :log_level, nil) -end diff --git a/lib/wait_strategy/command_wait_strategy.ex b/lib/wait_strategy/command_wait_strategy.ex index 448f002..e3f8316 100644 --- a/lib/wait_strategy/command_wait_strategy.ex +++ b/lib/wait_strategy/command_wait_strategy.ex @@ -6,6 +6,8 @@ defmodule Testcontainers.CommandWaitStrategy do Considers a container ready as soon as a command runs successfully inside it. """ + require Logger + @retry_delay 200 defstruct [:command, :timeout, retry_delay: @retry_delay] @@ -22,7 +24,7 @@ defmodule Testcontainers.CommandWaitStrategy do # Private functions and implementations defimpl Testcontainers.WaitStrategy do - alias Testcontainers.{Docker, Logger} + alias Testcontainers.Docker @impl true def wait_until_container_is_ready(wait_strategy, container, conn) do @@ -96,7 +98,7 @@ defmodule Testcontainers.CommandWaitStrategy do end defp log_retry_message(container_id, exit_code, delay) do - Logger.log( + Logger.debug( "Command execution in container #{container_id} failed with exit_code #{exit_code}, retrying in #{delay}ms." ) end diff --git a/lib/wait_strategy/log_wait_strategy.ex b/lib/wait_strategy/log_wait_strategy.ex index b179c63..25565a8 100644 --- a/lib/wait_strategy/log_wait_strategy.ex +++ b/lib/wait_strategy/log_wait_strategy.ex @@ -3,6 +3,7 @@ defmodule Testcontainers.LogWaitStrategy do @moduledoc """ Considers the container as ready as soon as a specific log message is detected in the container's log stream. """ + require Logger @retry_delay 500 defstruct [:log_regex, :timeout, retry_delay: @retry_delay] @@ -20,7 +21,7 @@ defmodule Testcontainers.LogWaitStrategy do # Private functions and implementations defimpl Testcontainers.WaitStrategy do - alias Testcontainers.{Docker, Logger} + alias Testcontainers.Docker @impl true def wait_until_container_is_ready(wait_strategy, container, conn) do @@ -68,7 +69,7 @@ defmodule Testcontainers.LogWaitStrategy do end defp log_retry_message(container_id, log_regex, delay) do - Logger.log( + Logger.debug( "Logs in container #{container_id} didn't match regex #{inspect(log_regex)}, retrying in #{delay}ms." ) end diff --git a/lib/wait_strategy/port_wait_strategy.ex b/lib/wait_strategy/port_wait_strategy.ex index 1925b9f..5082ec8 100644 --- a/lib/wait_strategy/port_wait_strategy.ex +++ b/lib/wait_strategy/port_wait_strategy.ex @@ -4,6 +4,8 @@ defmodule Testcontainers.PortWaitStrategy do Considers the container as ready when it successfully accepts connections on the specified port. """ + require Logger + @retry_delay 200 defstruct [:ip, :port, :timeout, retry_delay: @retry_delay] @@ -19,7 +21,7 @@ defmodule Testcontainers.PortWaitStrategy do # Private functions and implementations defimpl Testcontainers.WaitStrategy do - alias Testcontainers.{Container, Logger} + alias Testcontainers.Container @impl true def wait_until_container_is_ready(wait_strategy, container, _conn) do @@ -78,7 +80,7 @@ defmodule Testcontainers.PortWaitStrategy do end defp log_retry_message(wait_strategy, host_port) do - Logger.log( + Logger.debug( "Port #{wait_strategy.port} (host port #{host_port}) not open on IP #{wait_strategy.ip}, retrying in #{wait_strategy.retry_delay}ms." ) end diff --git a/test/generic_container_test.exs b/test/generic_container_test.exs index f390957..f020ba8 100644 --- a/test/generic_container_test.exs +++ b/test/generic_container_test.exs @@ -1,6 +1,8 @@ defmodule Testcontainers.GenericContainerTest do use ExUnit.Case, async: true + require Logger + import Testcontainers.Container, only: [is_os: 1] import TestHelper, only: [port_open?: 2] @@ -15,7 +17,7 @@ defmodule Testcontainers.GenericContainerTest do @tag :needs_root test "can start and stop generic container with network mode set to host" do if not is_os(:linux) do - Testcontainers.Logger.log("Host is not Linux, therefore not running network_mode test") + Logger.warning("Host is not Linux, therefore not running network_mode test") else config = %Testcontainers.Container{image: "redis:latest", network_mode: "host"} assert {:ok, container} = Testcontainers.start_container(config)