Skip to content

Conversation

@jackdewinter
Copy link
Owner

@jackdewinter jackdewinter commented Jan 24, 2026

Summary by Sourcery

Allow TOML configuration loading to support quoted keys with periods and align explicit and implicit pyproject.toml handling, while adding supporting tests, utilities, and documentation updates.

New Features:

  • Support loading TOML configurations with quoted keys that contain period separators without rejecting them as invalid keys.
  • Allow MultisourceConfigurationLoader to take an optional section header for TOML files via MultisourceConfigurationLoaderOptions.
  • Provide reusable patch utilities for mocking builtins.open in tests and capturing file contents or exceptions.
  • Add a clean.sh flag to skip dependency upgrade checks during project maintenance runs.

Bug Fixes:

  • Fix TOML key parsing to correctly handle keys with quoted separators and to surface clear errors when invalid unquoted keys are used.
  • Ensure TOML configuration loaded via an explicit --config file respects the same section header behavior as implicit pyproject.toml loading.

Enhancements:

  • Extend ApplicationProperties dictionary loading to optionally allow periods in keys by quoting them when flattening into the property map.
  • Improve TOML loader integration to always enable support for keys with periods when loading from TOML maps.

Documentation:

  • Update the changelog with fixes for TOML parsing, pyproject.toml config handling, and historical version notes including JSON5 and YAML support, utility moves, and other project changes.

Tests:

  • Add TOML loader tests covering quoted and unquoted keys with periods and slashes and unsupported type conversions for such keys.
  • Add TOML loading tests for both implicit pyproject.toml discovery and explicitly specified configuration files using the multisource loader.
  • Introduce tests using the new builtin.open patch helpers to simulate filesystem-based TOML loading without real files.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 24, 2026

Reviewer's Guide

Support TOML keys containing periods by allowing ApplicationProperties to optionally accept separator characters when loading from dicts and wiring TOML loading to use that path, add section-header-aware TOML loading in MultisourceConfigurationLoader (including implicit and explicit pyproject.toml), and introduce reusable file I/O patch helpers plus minor tooling/docs updates.

Sequence diagram for TOML loading with section headers and period-containing keys

sequenceDiagram
    actor User
    participant Loader as MultisourceConfigurationLoader
    participant Options as MultisourceConfigurationLoaderOptions
    participant TomlLoader as ApplicationPropertiesTomlLoader
    participant Props as ApplicationProperties

    User->>Loader: add_specified_configuration_file(file_name, config_file_type, section_header_if_toml)
    Loader->>Options: set section_header_if_toml

    User->>Loader: _load_config(options, application_properties, handle_error_fn)
    Loader->>Loader: detect ConfigurationFileType as TOML
    Loader->>Loader: __load_as_toml(file_name, options, application_properties, handle_error_fn)

    Loader->>TomlLoader: load_and_set(properties_object=Props, file_name, section_header=options.section_header_if_toml, handle_error_fn, clear_property_map=False, check_for_file_presence=True, is_optional=True)
    TomlLoader->>TomlLoader: parse TOML and apply section_header
    TomlLoader->>Props: load_from_dict(configuration_map, clear_map=clear_property_map, allow_periods_in_keys=True)
    Props->>Props: __scan_map(config_map, current_prefix, allow_periods_in_keys=True)
    Props->>Props: validate keys and preserve quoted keys with periods
    Props-->>TomlLoader: properties updated
    TomlLoader-->>Loader: (did_apply_map, did_have_one_error)
    Loader-->>User: (did_apply_map, did_have_one_error)
Loading

Class diagram for updated configuration and TOML loading types

classDiagram
    class ApplicationProperties {
        -__flat_property_map: Dict[Any, Any]
        -__assignment_operator: str
        -__separator: str
        +clear(): void
        +load_from_dict(config_map: Dict[Any, Any], clear_map: bool, allow_periods_in_keys: bool): void
        +verify_full_part_form(property_key: str): str
        +property_names_under(key_name: str): List[str]
        -__scan_map(config_map: Dict[Any, Any], current_prefix: str, allow_periods_in_keys: bool): void
    }

    class ApplicationPropertiesTomlLoader {
        +load_and_set(properties_object: ApplicationProperties, file_name: str, section_header: Optional[str], handle_error_fn: Callable[[str, Optional[Exception]], None], clear_property_map: bool, check_for_file_presence: bool, is_optional: bool): Tuple[bool, bool]
    }

    class MultisourceConfigurationLoaderOptions {
        +json_as_json5: bool
        +section_header_if_toml: Optional[str]
    }

    class MultisourceConfigurationLoader {
        -__load_as_toml(file_name: str, options: MultisourceConfigurationLoaderOptions, application_properties: ApplicationProperties, handle_error_fn: Callable[[str, Optional[Exception]], None]): Tuple[bool, bool]
        +add_specified_configuration_file(file_name: str, config_file_type: ConfigurationFileType, section_header_if_toml: Optional[str]): MultisourceConfigurationLoader
        +_load_config(options: MultisourceConfigurationLoaderOptions, application_properties: ApplicationProperties, handle_error_fn: Callable[[str, Optional[Exception]], None]): Tuple[bool, bool]
    }

    ApplicationPropertiesTomlLoader ..> ApplicationProperties
    MultisourceConfigurationLoader ..> MultisourceConfigurationLoaderOptions
    MultisourceConfigurationLoader ..> ApplicationPropertiesTomlLoader
    MultisourceConfigurationLoader ..> ApplicationProperties
Loading

Flow diagram for clean.sh look_for_upgrades with no-upgrades option

flowchart TD
    A[look_for_upgrades] --> B{NO_UPGRADE_MODE != 0}
    B -- yes --> C[verbose_echo Skipping check for Python package upgrades by request.]
    C --> D[return]
    B -- no --> E[verbose_echo Looking for Python package upgrades in Pre-Commit and Pipenv.]
    E --> F[run ./check_project_dependencies.sh]
    F -->|success| G[continue script]
    F -->|failure| H[complete_process 1 One or more project dependencies can be updated. Please run ./check_project_dependencies.sh --upgrade to update them.]
Loading

File-Level Changes

Change Details Files
Allow ApplicationProperties to optionally accept TOML keys containing separator characters while still validating and normalizing keys.
  • Extend load_from_dict to take an allow_periods_in_keys flag that is propagated into the internal scanning routine.
  • Update __scan_map to conditionally allow separator characters when allow_periods_in_keys is True and quote keys containing separators before flattening.
  • Preserve existing key validation for whitespace, assignment operator, and separator characters when periods are not explicitly allowed.
application_properties/application_properties.py
Ensure TOML-based configuration loading uses the relaxed-key behavior for periods and add section-header-aware TOML loading from multisource configuration files.
  • Update ApplicationPropertiesTomlLoader.load_and_set to call load_from_dict with allow_periods_in_keys=True so quoted TOML keys with separators are preserved.
  • Extend MultisourceConfigurationLoaderOptions with an optional section_header_if_toml field and thread it into TOML loading.
  • Refactor __load_as_toml and its caller to pass options through and use section_header_if_toml when invoking ApplicationPropertiesTomlLoader.
  • Clarify add_specified_configuration_file docstring to mention the new TOML section header option.
application_properties/application_properties_toml_loader.py
application_properties/multisource_configuration_loader.py
Add focused tests for TOML keys with quoted/unquoted separators and for implicit/explicit pyproject.toml loading via MultisourceConfigurationLoader.
  • Import MultisourceConfigurationLoader, its options, and the new patch_builtin_open helper into the TOML loader test module.
  • Add tests that verify successful loading of quoted TOML keys with periods and slashes, rejection of invalid unquoted keys containing separators, and behavior when the value type is unsupported for a requested property accessor.
  • Add tests that exercise implicit pyproject.toml loading and explicit configuration file loading with a TOML section header via MultisourceConfigurationLoader.
  • Minorly tidy existing tests with explicit Assert comments for readability.
test/test_application_properties_toml_loader.py
Introduce reusable patch helpers for builtins.open to simulate text, binary, and exceptional file I/O in tests.
  • Create a PatchBase utility that centralizes unittest.mock.patch setup/teardown and side-effect wiring with optional action logging.
  • Implement PatchBuiltinOpen to patch builtins.open, providing maps for registered text content, binary content, and per-file-mode exceptions plus pass-through behavior for unregistered files.
  • Expose context managers path_builtin_open_with_binary_content and path_builtin_open_with_exception for concise test usage of the patching behavior.
test/patches/patch_base.py
test/patches/patch_builtin_open.py
Document TOML parsing and pyproject configuration fixes and adjust project tooling to manage upgrades more flexibly.
  • Expand the changelog with entries for improper TOML parsing, pyproject TOML config flag behavior, and historical release notes for multiple versions.
  • Extend clean.sh with a --no-upgrades flag (NO_UPGRADE_MODE) and make look_for_upgrades respect that flag before running dependency checks.
newdocs/src/changelog.md
clean.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 3 issues, and left some high level feedback:

  • The validation error message in ApplicationProperties.__scan_map still states that keys cannot contain a '.' even when allow_periods_in_keys=True; consider updating the text so it accurately reflects the relaxed constraint when that flag is enabled.
  • In PatchBuiltinOpen, the docstrings and parameter names for register_binary_content_for_file and path_builtin_open_with_binary_content refer to text content or exceptions, but the functions actually handle binary content; aligning these names/docstrings with their behavior would make the helpers clearer and less confusing.
  • PatchBuiltinOpen defines attributes such as mock_patcher, patched_open, and open_file_args that are not referenced anywhere; removing or wiring these up would reduce dead code and clarify how the patching is intended to work.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The validation error message in ApplicationProperties.__scan_map still states that keys cannot contain a '.' even when allow_periods_in_keys=True; consider updating the text so it accurately reflects the relaxed constraint when that flag is enabled.
- In PatchBuiltinOpen, the docstrings and parameter names for register_binary_content_for_file and path_builtin_open_with_binary_content refer to text content or exceptions, but the functions actually handle binary content; aligning these names/docstrings with their behavior would make the helpers clearer and less confusing.
- PatchBuiltinOpen defines attributes such as mock_patcher, patched_open, and open_file_args that are not referenced anywhere; removing or wiring these up would reduce dead code and clarify how the patching is intended to work.

## Individual Comments

### Comment 1
<location> `test/patches/patch_builtin_open.py:11-20` </location>
<code_context>
+class PatchBuiltinOpen(PatchBase):
</code_context>

<issue_to_address>
**suggestion (testing):** The new `PatchBuiltinOpen` helper itself is not covered by tests

This helper has several behaviors (content maps, binary handling, exception map, passthrough, logging) and is currently untested. Please add a focused test module (e.g. `test/test_patch_builtin_open.py`) that covers at least:

- Text content registration and line iteration
- Binary content registration without treating bytes as text
- Exceptions raised only for matching modes, with non-matching modes delegating to real `open`
- Passthrough for unknown files delegating to real `open` and resuming patching afterward

This will help ensure tests that depend on this utility remain reliable and debuggable.

Suggested implementation:

```python
import io
import builtins
import pytest

from test.patches.patch_builtin_open import PatchBuiltinOpen


class DummyLogger:
    """Simple logger to capture log messages in tests."""

    def __init__(self) -> None:
        self.messages = []

    def info(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("info", msg, args, kwargs))

    def warning(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("warning", msg, args, kwargs))

    def error(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("error", msg, args, kwargs))


def _read_all_lines(f):
    # helper to support both text and binary modes
    if "b" in getattr(getattr(f, "mode", ""), ""):
        return f.read()
    return f.read().splitlines()


def test_text_content_registration_and_line_iteration(monkeypatch):
    """
    Register text content for a file and ensure that:
    - open returns a text file-like object
    - iteration/line-reading works as expected
    - the real builtins.open is not used for the registered path/mode
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    # If your implementation has a logger injection, adapt this:
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # This assumes you have an API like `register_text` or `register_content`.
    # Adjust to the actual method name/signature in PatchBuiltinOpen.
    if hasattr(patch, "register_text"):
        patch.register_text("foo.txt", "line1\nline2\nline3\n")
    elif hasattr(patch, "register_content"):
        patch.register_content("foo.txt", "line1\nline2\nline3\n", mode="r")
    else:
        # Fallback: populate a known public mapping if that exists
        if hasattr(patch, "content_map"):
            patch.content_map[("foo.txt", "r")] = "line1\nline2\nline3\n"
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register text content")

    with patch:
        with open("foo.txt", "r") as f:
            # support both iteration protocol and readlines
            lines_via_iter = [line.rstrip("\n") for line in f]
        with open("foo.txt", "r") as f:
            lines_via_readlines = [line.rstrip("\n") for line in f.readlines()]

    assert lines_via_iter == ["line1", "line2", "line3"]
    assert lines_via_readlines == ["line1", "line2", "line3"]

    # Ensure real open was not called for the registered path
    assert all(call[0][0] != "foo.txt" for call in real_open_calls)


def test_binary_content_registration_without_text_treatment(monkeypatch):
    """
    Register binary content and ensure:
    - returned object is opened in binary mode
    - data is bytes, not str
    - no implicit text decoding is performed
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    payload = b"\x00\x01binary\xff"
    if hasattr(patch, "register_binary"):
        patch.register_binary("bin.dat", payload)
    elif hasattr(patch, "register_content"):
        patch.register_content("bin.dat", payload, mode="rb")
    else:
        if hasattr(patch, "content_map"):
            patch.content_map[("bin.dat", "rb")] = payload
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register binary content")

    with patch:
        with open("bin.dat", "rb") as f:
            data = f.read()

    assert isinstance(data, (bytes, bytearray))
    assert bytes(data) == payload

    # Ensure real open was not called for the registered path
    assert all(call[0][0] != "bin.dat" for call in real_open_calls)


def test_exceptions_only_for_matching_modes(monkeypatch):
    """
    Register an exception for a file/mode and ensure:
    - the exception is raised when path+mode match
    - a different mode for the same path delegates to real open
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # Create a real file to verify passthrough when mode doesn't match
    with real_open("real.txt", "w", encoding="utf-8") as f:
        f.write("real content")

    exc = IOError("forced failure")

    # This assumes you have something like `register_exception`.
    if hasattr(patch, "register_exception"):
        patch.register_exception("real.txt", exc, mode="r")
    else:
        if hasattr(patch, "exception_map"):
            patch.exception_map[("real.txt", "r")] = exc
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register exceptions")

    with patch:
        # Matching mode should raise the registered exception
        with pytest.raises(IOError) as ctx:
            open("real.txt", "r")
        assert str(ctx.value) == "forced failure"

        # Non-matching mode should delegate to the real open
        with open("real.txt", "w", encoding="utf-8") as f:
            f.write("updated")

    # Ensure real open was called at least once for the non-matching mode
    assert any(call[0][0] == "real.txt" and call[0][1] == "w" for call in real_open_calls)


def test_passthrough_for_unknown_files_and_resuming_patching(monkeypatch, tmp_path):
    """
    For unknown files, PatchBuiltinOpen should:
    - delegate to real open (passthrough)
    - keep patching active afterward for known files
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # Register a known file
    if hasattr(patch, "register_text"):
        patch.register_text("known.txt", "known")
    elif hasattr(patch, "register_content"):
        patch.register_content("known.txt", "known", mode="r")
    else:
        if hasattr(patch, "content_map"):
            patch.content_map[("known.txt", "r")] = "known"
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register text content")

    # Unknown file path that should go to real open
    unknown_path = tmp_path / "unknown.txt"

    with patch:
        # Unknown file should be created by the real open (passthrough)
        with open(str(unknown_path), "w", encoding="utf-8") as f:
            f.write("passthrough")

        # After the passthrough, the patch must still work for known files
        with open("known.txt", "r") as f:
            known_data = f.read()

    assert known_data == "known"

    # Ensure real open was used for the unknown path
    assert any(call[0][0] == str(unknown_path) for call in real_open_calls)

    # Ensure the patch did *not* delegate to real open for the known file
    assert all(call[0][0] != "known.txt" for call in real_open_calls)

```

The above tests assume a few public APIs on `PatchBuiltinOpen`:

1. Registration methods:
   - `register_text(path: str, content: str)` *or* a more generic `register_content(path: str, content: Union[str, bytes], mode: str = "r")`.
   - `register_binary(path: str, content: bytes)` for explicit binary registration, *or* using `register_content(..., mode="rb")`.
   - `register_exception(path: str, exc: BaseException, mode: str = "r")` to define exceptions for particular path+mode combinations.
   If your helper instead exposes mappings like `content_map` and `exception_map`, keep them keyed by `(path, mode)` and adjust the tests to use the actual names.

2. Context management:
   - `PatchBuiltinOpen` should be usable as a context manager (`with PatchBuiltinOpen(): ...`) to apply and restore the patch around a block. If your current API uses an explicit `start()`/`stop()` or `__enter__`/`__exit__` on a base class, ensure `PatchBuiltinOpen` inherits and supports this, and adjust the tests if the usage differs.

3. Logging:
   - If the helper logs operations, expose a `.logger` attribute (or an injection point used at initialization) that supports `.info/.warning/.error`. The tests provide a `DummyLogger` that you can use to assert on logs if desired; currently the tests just ensure that assigning a custom logger doesn't break behavior.

4. Passthrough behavior:
   - When an open request doesn’t match any registered content/exception, `PatchBuiltinOpen` should call the real `builtins.open` with the same arguments and then resume patching for subsequent calls.
   - Make sure your implementation saves the original `builtins.open` before patching and restores it on context exit to avoid leaking the patch across tests.

Update the test code above to match your actual method/attribute names where they differ, and ensure any necessary imports (e.g., `pytest`) are present in your test environment.
</issue_to_address>

### Comment 2
<location> `newdocs/src/changelog.md:36` </location>
<code_context>
+### Fixed and Added
+
+- Added proper documentation.
+- Hosting documentation at ReadTheDocs.
+- [Issue 148](https://github.com/jackdewinter/application_properties/issues/148)
+    - Added JSON5 support, which includes support for JSON comments.
</code_context>

<issue_to_address>
**suggestion (typo):** Consider using the official "Read the Docs" naming instead of "ReadTheDocs".

Please update this reference to use the official "Read the Docs" spelling for accuracy.

```suggestion
- Hosting documentation at Read the Docs.
```
</issue_to_address>

### Comment 3
<location> `newdocs/src/changelog.md:77` </location>
<code_context>
+### Fixed and Added
+
+- [Issue 137](https://github.com/jackdewinter/application_properties/issues/137)
+    - to support multiple configurations, added clear_map flag to Json loader
+    - refactoring to clean things up for upcoming changes
+- [Issue 2](https://github.com/jackdewinter/application_properties/issues/2)
</code_context>

<issue_to_address>
**suggestion (typo):** Correct capitalization of "Json" to "JSON" for consistency.

For consistency with other entries (e.g., "JSON5"), please change "Json loader" to "JSON loader".

```suggestion
    - to support multiple configurations, added clear_map flag to JSON loader
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +11 to +20
class PatchBuiltinOpen(PatchBase):
"""
Class to patch the "builtin.open" function.
"""

def __init__(self) -> None:
super().__init__("builtins.open")
self.mock_patcher = None
self.patched_open = None
self.open_file_args = None
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): The new PatchBuiltinOpen helper itself is not covered by tests

This helper has several behaviors (content maps, binary handling, exception map, passthrough, logging) and is currently untested. Please add a focused test module (e.g. test/test_patch_builtin_open.py) that covers at least:

  • Text content registration and line iteration
  • Binary content registration without treating bytes as text
  • Exceptions raised only for matching modes, with non-matching modes delegating to real open
  • Passthrough for unknown files delegating to real open and resuming patching afterward

This will help ensure tests that depend on this utility remain reliable and debuggable.

Suggested implementation:

import io
import builtins
import pytest

from test.patches.patch_builtin_open import PatchBuiltinOpen


class DummyLogger:
    """Simple logger to capture log messages in tests."""

    def __init__(self) -> None:
        self.messages = []

    def info(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("info", msg, args, kwargs))

    def warning(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("warning", msg, args, kwargs))

    def error(self, msg: str, *args: object, **kwargs: object) -> None:
        self.messages.append(("error", msg, args, kwargs))


def _read_all_lines(f):
    # helper to support both text and binary modes
    if "b" in getattr(getattr(f, "mode", ""), ""):
        return f.read()
    return f.read().splitlines()


def test_text_content_registration_and_line_iteration(monkeypatch):
    """
    Register text content for a file and ensure that:
    - open returns a text file-like object
    - iteration/line-reading works as expected
    - the real builtins.open is not used for the registered path/mode
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    # If your implementation has a logger injection, adapt this:
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # This assumes you have an API like `register_text` or `register_content`.
    # Adjust to the actual method name/signature in PatchBuiltinOpen.
    if hasattr(patch, "register_text"):
        patch.register_text("foo.txt", "line1\nline2\nline3\n")
    elif hasattr(patch, "register_content"):
        patch.register_content("foo.txt", "line1\nline2\nline3\n", mode="r")
    else:
        # Fallback: populate a known public mapping if that exists
        if hasattr(patch, "content_map"):
            patch.content_map[("foo.txt", "r")] = "line1\nline2\nline3\n"
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register text content")

    with patch:
        with open("foo.txt", "r") as f:
            # support both iteration protocol and readlines
            lines_via_iter = [line.rstrip("\n") for line in f]
        with open("foo.txt", "r") as f:
            lines_via_readlines = [line.rstrip("\n") for line in f.readlines()]

    assert lines_via_iter == ["line1", "line2", "line3"]
    assert lines_via_readlines == ["line1", "line2", "line3"]

    # Ensure real open was not called for the registered path
    assert all(call[0][0] != "foo.txt" for call in real_open_calls)


def test_binary_content_registration_without_text_treatment(monkeypatch):
    """
    Register binary content and ensure:
    - returned object is opened in binary mode
    - data is bytes, not str
    - no implicit text decoding is performed
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    payload = b"\x00\x01binary\xff"
    if hasattr(patch, "register_binary"):
        patch.register_binary("bin.dat", payload)
    elif hasattr(patch, "register_content"):
        patch.register_content("bin.dat", payload, mode="rb")
    else:
        if hasattr(patch, "content_map"):
            patch.content_map[("bin.dat", "rb")] = payload
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register binary content")

    with patch:
        with open("bin.dat", "rb") as f:
            data = f.read()

    assert isinstance(data, (bytes, bytearray))
    assert bytes(data) == payload

    # Ensure real open was not called for the registered path
    assert all(call[0][0] != "bin.dat" for call in real_open_calls)


def test_exceptions_only_for_matching_modes(monkeypatch):
    """
    Register an exception for a file/mode and ensure:
    - the exception is raised when path+mode match
    - a different mode for the same path delegates to real open
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # Create a real file to verify passthrough when mode doesn't match
    with real_open("real.txt", "w", encoding="utf-8") as f:
        f.write("real content")

    exc = IOError("forced failure")

    # This assumes you have something like `register_exception`.
    if hasattr(patch, "register_exception"):
        patch.register_exception("real.txt", exc, mode="r")
    else:
        if hasattr(patch, "exception_map"):
            patch.exception_map[("real.txt", "r")] = exc
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register exceptions")

    with patch:
        # Matching mode should raise the registered exception
        with pytest.raises(IOError) as ctx:
            open("real.txt", "r")
        assert str(ctx.value) == "forced failure"

        # Non-matching mode should delegate to the real open
        with open("real.txt", "w", encoding="utf-8") as f:
            f.write("updated")

    # Ensure real open was called at least once for the non-matching mode
    assert any(call[0][0] == "real.txt" and call[0][1] == "w" for call in real_open_calls)


def test_passthrough_for_unknown_files_and_resuming_patching(monkeypatch, tmp_path):
    """
    For unknown files, PatchBuiltinOpen should:
    - delegate to real open (passthrough)
    - keep patching active afterward for known files
    """
    real_open_calls = []

    real_open = builtins.open

    def tracking_open(*args, **kwargs):
        real_open_calls.append((args, kwargs))
        return real_open(*args, **kwargs)

    monkeypatch.setattr(builtins, "open", tracking_open)

    patch = PatchBuiltinOpen()
    if hasattr(patch, "logger"):
        patch.logger = DummyLogger()

    # Register a known file
    if hasattr(patch, "register_text"):
        patch.register_text("known.txt", "known")
    elif hasattr(patch, "register_content"):
        patch.register_content("known.txt", "known", mode="r")
    else:
        if hasattr(patch, "content_map"):
            patch.content_map[("known.txt", "r")] = "known"
        else:
            pytest.skip("PatchBuiltinOpen has no visible API to register text content")

    # Unknown file path that should go to real open
    unknown_path = tmp_path / "unknown.txt"

    with patch:
        # Unknown file should be created by the real open (passthrough)
        with open(str(unknown_path), "w", encoding="utf-8") as f:
            f.write("passthrough")

        # After the passthrough, the patch must still work for known files
        with open("known.txt", "r") as f:
            known_data = f.read()

    assert known_data == "known"

    # Ensure real open was used for the unknown path
    assert any(call[0][0] == str(unknown_path) for call in real_open_calls)

    # Ensure the patch did *not* delegate to real open for the known file
    assert all(call[0][0] != "known.txt" for call in real_open_calls)

The above tests assume a few public APIs on PatchBuiltinOpen:

  1. Registration methods:

    • register_text(path: str, content: str) or a more generic register_content(path: str, content: Union[str, bytes], mode: str = "r").
    • register_binary(path: str, content: bytes) for explicit binary registration, or using register_content(..., mode="rb").
    • register_exception(path: str, exc: BaseException, mode: str = "r") to define exceptions for particular path+mode combinations.
      If your helper instead exposes mappings like content_map and exception_map, keep them keyed by (path, mode) and adjust the tests to use the actual names.
  2. Context management:

    • PatchBuiltinOpen should be usable as a context manager (with PatchBuiltinOpen(): ...) to apply and restore the patch around a block. If your current API uses an explicit start()/stop() or __enter__/__exit__ on a base class, ensure PatchBuiltinOpen inherits and supports this, and adjust the tests if the usage differs.
  3. Logging:

    • If the helper logs operations, expose a .logger attribute (or an injection point used at initialization) that supports .info/.warning/.error. The tests provide a DummyLogger that you can use to assert on logs if desired; currently the tests just ensure that assigning a custom logger doesn't break behavior.
  4. Passthrough behavior:

    • When an open request doesn’t match any registered content/exception, PatchBuiltinOpen should call the real builtins.open with the same arguments and then resume patching for subsequent calls.
    • Make sure your implementation saves the original builtins.open before patching and restores it on context exit to avoid leaking the patch across tests.

Update the test code above to match your actual method/attribute names where they differ, and ensure any necessary imports (e.g., pytest) are present in your test environment.

### Fixed and Added

- Added proper documentation.
- Hosting documentation at ReadTheDocs.
Copy link

Choose a reason for hiding this comment

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

suggestion (typo): Consider using the official "Read the Docs" naming instead of "ReadTheDocs".

Please update this reference to use the official "Read the Docs" spelling for accuracy.

Suggested change
- Hosting documentation at ReadTheDocs.
- Hosting documentation at Read the Docs.

### Fixed and Added

- [Issue 137](https://github.com/jackdewinter/application_properties/issues/137)
- to support multiple configurations, added clear_map flag to Json loader
Copy link

Choose a reason for hiding this comment

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

suggestion (typo): Correct capitalization of "Json" to "JSON" for consistency.

For consistency with other entries (e.g., "JSON5"), please change "Json loader" to "JSON loader".

Suggested change
- to support multiple configurations, added clear_map flag to Json loader
- to support multiple configurations, added clear_map flag to JSON loader

@codecov
Copy link

codecov bot commented Jan 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (14141cb) to head (eb55a48).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #319   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          657       661    +4     
  Branches        89        90    +1     
=========================================
+ Hits           657       661    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jackdewinter jackdewinter merged commit a657c04 into main Jan 24, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants