test: model one ephemeral directory per scenario in the unit fixture - #4701

Summary

The unit-test suite mocks Scenario.ephemeral_directory so tests do not touch a real per-project cache. The mock returns one shared directory for every scenario, so two Scenario objects in the same test see an identical ephemeral directory. The real property does not behave that way outside shared state: it derives a distinct path per scenario. This changes the mock to model what the code actually does, one directory per scenario, and cleans up a __repr__ assertion that was only passing by accident under the old mock.

This is test-only. Nothing under src/molecule changes, and no runtime behavior changes.

Problem

tests/conftest.py monkeypatches Scenario.ephemeral_directory onto the class with a function that ignores the scenario it is called on:

def mock_ephemeral_directory(_self: Scenario) -> str:
    return str(test_dir)

tests/unit/conftest.py wires config_instance to depend on that fixture, so most of the unit suite runs against this mock. Because it returns the same test_dir for every scenario, the suite cannot express or observe per-scenario ephemeral paths: two distinct Scenario instances return byte-identical directories.

The real Scenario.ephemeral_directory composes a per-scenario path (molecule.<checksum>.<scenario name>, with the name sanitized) and only collapses all scenarios onto one directory when shared_state is enabled. The old mock modeled that shared-state collapse unconditionally, for every test, which is the case the suite least needs as its default.

Root cause of the fallout

test_ephemeral_directory_property asserted the ephemeral directory appears in the scenario's repr():

# assert that scenario path is included in in repr (useful for debugging)
assert _instance.ephemeral_directory in repr(_instance)

But Scenario.__repr__ never mentions the ephemeral directory:

def __repr__(self) -> str:
    return f"<Scenario {self.name} from {self.config.project_directory}>"

The assertion passed only because config_instance chdirs into the same directory the shared mock returned, so ephemeral_directory and project_directory were the identical string at that moment. Once the mock returns a per-scenario subdirectory, the ephemeral directory becomes a strict subpath of project_directory rather than equal to it, the coincidence breaks, and the assertion fails, exposing that it was never checking the ephemeral directory at all. It was, at most, an incidental check of __repr__.

Changes

• tests/conftest.py: mock_ephemeral_directory now derives the directory from the scenario asking, test_dir / _self.name, creating it on demand exactly as the real property does with mkdir(parents=True, exist_ok=True). Teardown is unchanged: the fixture's existing shutil.rmtree(test_dir) removes the per-scenario subdirectories with the rest of the tree.

• tests/unit/test_scenario.py: test_ephemeral_directory_property now checks only what its name promises, that the ephemeral directory is writable. The repr() check moves to a dedicated test_scenario_repr that asserts the exact representation string, so it tests __repr__'s actual contract instead of relying on a chdir coincidence:

  def test_scenario_repr(_instance: Scenario) -> None:
      expected = f"<Scenario {_instance.name} from {_instance.config.project_directory}>"
      assert repr(_instance) == expected

• .config/pydoclint-baseline.txt: the mock gains an Args: entry for _self (and its Returns: type is corrected from Path to str to match the annotation), so its two DOC101/DOC103 baseline exceptions are no longer needed. The pydoclint pre-commit hook regenerated the file; the two removed lines are the only change.

What this does and does not claim

This is an obstacle removed, not a bug caught. It does not fix any user-facing behavior, and no pipeline will feel a change. The narrow claim is exactly this: the fixture should model one directory per scenario because that is what the code does, and a test whose comment claimed to check the ephemeral path never did. Stating it at that strength is the point.

Test / lint evidence

Base upstream/main at 66b70b0a. All runs in a clean clone, molecule dev environment.

tox -e lint (full prek/pre-commit suite, --all-files):

EXIT=0

All hooks pass, including mypy, pydoclint, pylint, codespell and cspell; the pydoclint baseline shows no drift once the regeneration is committed.

tox -e py, with a container engine available so the container-backed integration tests run:

909 passed, 7 skipped, 11 warnings in 538.74s

The full suite is green, including test_podman, test_native_inventory and test_with_backend_as_ansible_navigator, which build and drive real containers. test_scenario_repr and test_ephemeral_directory_property both pass, and total coverage holds at 90%.

Notes for reviewers

• Test-only by intent. The mock change is the piece that lets the unit suite express per-scenario ephemeral paths; the assertion move is the one bit of fallout it produced.
• The repr() check was relocated rather than dropped so Scenario.__repr__ stays covered, and rewritten as an exact-string assertion so it tests the representation contract directly rather than by coincidence.
• An alternative was to put the ephemeral directory into __repr__, which is what the old comment always intended and would be useful when debugging. That is a runtime change and out of scope for a test-only PR, so __repr__ is untouched.

Summary by CodeRabbit

• Tests
  • Improved test isolation by assigning each scenario its own ephemeral directory, including support for scenario names containing path separators.
  • Added coverage for creating required directories and formatting scenario-specific paths consistently.
  • Strengthened scenario representation tests to verify the complete expected display format.
  • Updated documentation checks to accurately reflect fixture arguments and return values.