Support Multiple Property Sources With Precedence #220

digia · 2025-12-23T03:21:17Z

Adds read_property_sources/1 to read configuration from multiple sources with proper precedence: env vars > user file > project file. Matches the behavior of Java and Go implementations.

Supports TESTCONTAINERS_* environment variables (converted to dot notation)
Reads from ~/.testcontainers.properties and .testcontainers.properties
Optional keyword args for overriding paths/prefix
Keeps read_property_file/1 for backward compat

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

Copilot

Pull request overview

This PR adds support for reading Testcontainers configuration from multiple property sources with proper precedence ordering, aligning with Java and Go implementations. The implementation introduces read_property_sources/1 which merges configuration from environment variables, user files, and project files, with environment variables having the highest priority.

Key Changes:

Introduces read_property_sources/1 that reads from three sources: env vars (TESTCONTAINERS_*), user file (~/.testcontainers.properties), and project file (.testcontainers.properties)
Environment variables are converted from TESTCONTAINERS_PROPERTY_NAME format to property.name format
Maintains backward compatibility with existing read_property_file/1 function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
lib/util/properties.ex	Adds new `read_property_sources/1` function with helper methods for reading multiple property sources and environment variable conversion
test/util/properties_test.exs	Adds test coverage for the new multi-source reading functionality and environment variable precedence

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T03:24:38Z

test/util/properties_test.exs

+
+  alias Testcontainers.Util.PropertiesParser
+
+  describe "read_property_sources/0" do


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 · 2025-12-23T03:24:38Z

test/util/properties_test.exs

+  describe "read_property_sources/0" do
+    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


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 · 2025-12-23T03:24:38Z

test/util/properties_test.exs

+  describe "read_property_sources/0" do
+    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


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 · 2025-12-23T03:24:38Z

lib/util/properties.ex

+  @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.
+  """


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 · 2025-12-23T03:24:39Z

lib/util/properties.ex

+  defp env_to_property({key, value}, prefix) do
+    property_key =
+      key
+      |> String.replace_prefix(prefix, "")
+      |> String.downcase()
+      |> String.replace("_", ".")


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 · 2025-12-23T03:24:39Z

test/util/properties_test.exs

+  describe "read_property_sources/0" do
+    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


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 · 2025-12-23T03:24:39Z

test/util/properties_test.exs

+    end
+  end
+
+  describe "read_property_file/0" do


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

feat: support multiple property sources with precedence

5d63c7f

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

Copilot AI review requested due to automatic review settings December 23, 2025 03:21

Copilot started reviewing on behalf of digia December 23, 2025 03:21 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

digia mentioned this pull request Dec 23, 2025

Support Container.with_privileged for Ryuk Privileged Mode #221

Open

jarlah marked this pull request as draft December 24, 2025 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support Multiple Property Sources With Precedence #220

Support Multiple Property Sources With Precedence #220

digia commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		alias Testcontainers.Util.PropertiesParser

		describe "read_property_sources/0" do

	describe "read_property_sources/0" do
	describe "read_property_sources/1" do

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

Support Multiple Property Sources With Precedence #220

Are you sure you want to change the base?

Support Multiple Property Sources With Precedence #220

Conversation

digia commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant