Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions lib/util/properties.ex
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
defmodule Testcontainers.Util.PropertiesParser do
@moduledoc false

@file_path "~/.testcontainers.properties"
@user_file "~/.testcontainers.properties"
@project_file ".testcontainers.properties"
@env_prefix "TESTCONTAINERS_"

def read_property_file(file_path \\ @file_path) do
def read_property_file(file_path \\ @user_file) do
if File.exists?(Path.expand(file_path)) do
with {:ok, content} <- File.read(Path.expand(file_path)),
properties <- parse_properties(content) do
Expand All @@ -18,6 +20,76 @@ defmodule Testcontainers.Util.PropertiesParser do
end
end

@doc """
Reads properties from all sources with proper precedence.

Configuration is read from three sources with the following precedence
(highest to lowest):

1. Environment variables (TESTCONTAINERS_* prefix)
2. User file (~/.testcontainers.properties)
3. Project file (.testcontainers.properties)

Environment variables are converted from TESTCONTAINERS_PROPERTY_NAME format
to property.name format (uppercase to lowercase, underscores to dots, prefix removed).

## Options

- `:user_file` - path to user properties file (default: ~/.testcontainers.properties)
- `:project_file` - path to project properties file (default: .testcontainers.properties)
- `:env_prefix` - environment variable prefix (default: TESTCONTAINERS_)

Returns `{:ok, map}` with merged properties.
"""
Comment on lines +23 to +43
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says the function accepts opts with keyword options, but the title uses "read_property_sources/0" in the doc string. Since the function signature is read_property_sources(opts \\ []), the function documentation should be consistent and use the actual arity. Update the internal doc reference or clarify that it can be called as read_property_sources/0 OR read_property_sources/1.

Copilot uses AI. Check for mistakes.
def read_property_sources(opts \\ []) do
user_file = Keyword.get(opts, :user_file, @user_file)
project_file = Keyword.get(opts, :project_file, @project_file)
env_prefix = Keyword.get(opts, :env_prefix, @env_prefix)

project_props = read_file_silent(project_file)
user_props = read_file_silent(user_file)
env_props = read_env_vars(env_prefix)

# Merge in order of lowest to highest precedence
merged =
project_props
|> Map.merge(user_props)
|> Map.merge(env_props)

{:ok, merged}
end

defp read_file_silent(file_path) do
expanded = Path.expand(file_path)

if File.exists?(expanded) do
case File.read(expanded) do
{:ok, content} -> parse_properties(content)
{:error, _} -> %{}
end
else
%{}
end
end

defp read_env_vars(prefix) do
System.get_env()
|> Enum.filter(fn {key, _value} -> String.starts_with?(key, prefix) end)
|> Enum.map(&env_to_property(&1, prefix))
|> Map.new()
end

# Converts TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED to ryuk.container.privileged
defp env_to_property({key, value}, prefix) do
property_key =
key
|> String.replace_prefix(prefix, "")
|> String.downcase()
|> String.replace("_", ".")
Comment on lines +83 to +88
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The env_to_property function does not handle the case where a property name could have multiple consecutive underscores. For example, "TESTCONTAINERS_RYUK__PRIVILEGED" would be converted to "ryuk..privileged" (with double dots). While this may be an edge case, consider whether consecutive underscores should be treated differently or if the current behavior is acceptable for this use case.

Suggested change
defp env_to_property({key, value}, prefix) do
property_key =
key
|> String.replace_prefix(prefix, "")
|> String.downcase()
|> String.replace("_", ".")
# Also handles consecutive underscores by treating them as a single separator,
# e.g. TESTCONTAINERS_RYUK__PRIVILEGED -> ryuk.privileged
defp env_to_property({key, value}, prefix) do
property_key =
key
|> String.replace_prefix(prefix, "")
|> String.downcase()
|> String.split("_")
|> Enum.reject(&(&1 == ""))
|> Enum.join(".")

Copilot uses AI. Check for mistakes.

{property_key, value}
end

defp parse_properties(content) do
content
|> String.split("\n")
Expand Down
70 changes: 70 additions & 0 deletions test/util/properties_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
defmodule Testcontainers.Util.PropertiesParserTest do
use ExUnit.Case, async: false

alias Testcontainers.Util.PropertiesParser

describe "read_property_sources/0" do
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The describe block header says "read_property_sources/0" but the function signature in the implementation is "read_property_sources(opts \ [])". This is technically read_property_sources/1, not /0. Update the describe block to use "read_property_sources/1" for consistency with Elixir naming conventions.

Suggested change
describe "read_property_sources/0" do
describe "read_property_sources/1" do

Copilot uses AI. Check for mistakes.
test "returns empty map when no files or env vars exist" do
# Clean env vars that might interfere
System.get_env()
|> Enum.filter(fn {k, _} -> String.starts_with?(k, "TESTCONTAINERS_") end)
|> Enum.each(fn {k, _} -> System.delete_env(k) end)

{:ok, props} = PropertiesParser.read_property_sources()

# Should at least return a map (may have project file props)
assert is_map(props)
end

test "reads environment variables with TESTCONTAINERS_ prefix" do
System.put_env("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED", "true")
System.put_env("TESTCONTAINERS_SOME_OTHER_PROPERTY", "value")

{:ok, props} = PropertiesParser.read_property_sources()

assert props["ryuk.container.privileged"] == "true"
assert props["some.other.property"] == "value"

# Cleanup
System.delete_env("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED")
System.delete_env("TESTCONTAINERS_SOME_OTHER_PROPERTY")
end

test "environment variables take precedence over file properties" do
# Set env var
System.put_env("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED", "from_env")

{:ok, props} = PropertiesParser.read_property_sources()

# Env should win over any file-based setting
assert props["ryuk.container.privileged"] == "from_env"

# Cleanup
System.delete_env("TESTCONTAINERS_RYUK_CONTAINER_PRIVILEGED")
end
end
Comment on lines +6 to +45
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for the precedence order between user file and project file. The current tests only verify env vars vs files, but don't test that user file takes precedence over project file when both exist. Consider adding a test that uses custom file paths (via opts) to verify the complete precedence chain: env vars > user file > project file.

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +45
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for the custom options (:user_file, :project_file, :env_prefix) documented in the function. These options are part of the public API and should be tested to ensure they work correctly. Consider adding tests that verify custom file paths and custom env prefix work as expected.

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +45
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for error handling in read_file_silent when File.read returns an error. The function silently returns an empty map when File.read fails (line 68), but there's no test verifying this behavior. Consider adding a test that verifies the function gracefully handles file read errors (e.g., permission denied).

Copilot uses AI. Check for mistakes.

describe "read_property_file/0" do
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The describe block says "read_property_file/0" but tests the default behavior. Since read_property_file has a default argument, it's actually read_property_file/1 with an optional argument. Consider using "read_property_file/0 (default path)" or "read_property_file with defaults" to be clearer about what's being tested.

Suggested change
describe "read_property_file/0" do
describe "read_property_file/0 (default path)" do

Copilot uses AI. Check for mistakes.
test "defaults to user file path" do
{:ok, props} = PropertiesParser.read_property_file()

# Should return a map (empty if user file doesn't exist)
assert is_map(props)
end
end

describe "read_property_file/1" do
test "reads properties from specified file" do
{:ok, props} = PropertiesParser.read_property_file("test/fixtures/.testcontainers.properties")

assert is_map(props)
assert props["tc.host"] == "tcp://localhost:9999"
end

test "returns empty map for nonexistent file" do
{:ok, props} = PropertiesParser.read_property_file("/nonexistent/path/.testcontainers.properties")

assert props == %{}
end
end
end
Loading