Repository navigation
fix: remove git-ignore-based scenario filter - #4665
Merged
tanwigeetika1618 merged 4 commits intoJul 31, 2026
Merged
tanwigeetika1618 merged 4 commits into
tanwigeetika1618 merged 4 commits into
Conversation
📝 WalkthroughWalkthroughScenario discovery no longer uses ChangesScenario discovery
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
The All three are in Each fails on a corrupted ephemeral
|
Scenario discovery is anchored: `molecule/*/molecule.yml` is non-recursive and the collection glob is rooted under `extensions/molecule/**`. Neither pattern descends into a virtualenv, so installed scenarios under `site-packages` are already excluded by where discovery looks. The `git check-ignore` filter added in ansible#3996 is therefore redundant, and it actively breaks projects that keep their own `molecule/` directory out of version control by dropping those scenarios (ansible#4659). Remove `filter_ignored_scenarios()` and its call in `get_configs()`, drop the now-unused `subprocess` and `contextlib` imports, and remove the test-only monkeypatches that existed solely to defeat the filter on git-ignored cache paths. Rework the integration test to assert the new contract (a scenario in a git-ignored directory is discovered) and add a regression test that default discovery does not descend into a `site-packages`-shaped nested path. Fixes ansible#4659 Assisted-by: Claude (Anthropic) Signed-off-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com>
jeffcpullen
force-pushed
the
fix/remove-gitignore-scenario-filter
branch
from
July 24, 2026 03:59
cf50212 to
57957b6
Compare
This was referenced Jul 25, 2026
tanwigeetika1618
added a commit
that referenced
this pull request
Jul 31, 2026
Refs #4666. First step of the phased plan in #4666: make the scenario `state.yml` write atomic, which removes the partial-read crash. It does not attempt the larger shared-state design change (defect 2 in the issue); that stays tracked in #4666. ## Problem Molecule runs a collection's scenarios in parallel with `--workers`. The parallel-testing guide (`docs/guides/parallel.md:30-32`) pairs that with `shared_state: true`, so every scenario running at once reads and writes the *same* `state.yml`. That file is written non-atomically, so a scenario can read it mid-rewrite, get a truncated document, and abort with a YAML scanner error. It surfaces as the intermittent `test_workers` failure on the `py314-milestone` job (py3.14 timing just widens the window): ``` molecule.util.MoleculeError: while scanning a quoted scalar ... yaml.scanner.ScannerError: could not find expected ':' ``` ## Root cause The state write is a truncate-then-rewrite with no atomicity: - `State._write_state_file` (`src/molecule/state.py:234`) → `util.write_file` → `Path.write_text(content)` (`src/molecule/util.py:222`). `write_text` truncates the file, then writes — a window in which a concurrent reader sees an empty or partial document. - `State._load_file` (`src/molecule/state.py:231`) → `util.safe_load_file` → `util.safe_load`, which turns the resulting `yaml.scanner.ScannerError` into a `MoleculeError` (`src/molecule/util.py:285-286`) and the worker aborts. - A truncation that lands on an *empty* read is quieter but worse: `safe_load` returns `{}` (`util.py:347`, `... or {}`), so `State._get_data` (`state.py:214-218`) falls back to `_default_data()` and the worker silently resets shared state (e.g. `prepared` → `False`) with no error at all. Atomicity removes both this silent reset and the `ScannerError` crash. `SharedState` (`src/molecule/state.py:241-249`) points every opted-in scenario at one `shared_ephemeral_directory/state.yml`, and molecule's own `test_workers` fixture sets `shared_state: true` (`tests/fixtures/integration/test_workers/extensions/molecule/config.yml:13`) — so CI exercises exactly this path. ## Fix - Add `util.atomic_write_file`: write the content to a temp file in the target's **own directory**, then `Path.replace` onto the target. The replace is a same-filesystem rename — POSIX guarantees `rename(2)` is atomic — so a concurrent reader always opens either the complete old file or the complete new file, never a truncated one. The temp file is removed if the write fails. - Point `State._write_state_file` at it. The file *content* is identical (`safe_dump` output with the same header); the only practically observable change is that `state.yml` is now created owner-only (`0600`) via `mkstemp` instead of umask-derived (`0644`) — harmless for this per-run ephemeral file, and if anything tighter. `write_file` and `atomic_write_file` share a small `_prepare_write` helper for the header/`Path` prep, so the new function is not a copy-paste of the old; `write_file`'s write path itself is unchanged. Scoped deliberately to the state write, not a global `write_file` swap: in the normal `--workers` layout only `state.yml` is written concurrently — every other `write_file` caller writes under a per-scenario `ephemeral_directory` — so widening it would add churn without fixing anything new. ## Evidence **Reproduction of the race and its removal.** Six workers (3 writers / 3 readers) hammer one shared `state.yml` through the exact functions `State` uses — `util.safe_load_file` to read, and `util.write_file` (before) vs. `util.atomic_write_file` (after) to write. Run against this branch on Python `3.10.20` (the race reproduces on every supported version; 3.14 timing widens the window): ``` ===== BEFORE (util.write_file) ===== util.write_file -- py 3.10.20, 3000 iters x 6 workers reader outcome: ok x8560 reader outcome: TRUNCATED_DICT(missing keys) x440 CORRUPTION: 440 corrupt read(s) ===== AFTER (util.atomic_write_file) ===== util.atomic_write_file -- py 3.10.20, 3000 iters x 6 workers reader outcome: ok x9000 clean: 0 corrupt read(s) summary: before=440 corrupt reads, after=0 corrupt reads ``` (The script is in #4666; earlier runs also caught the genuine `ScannerError` — which partial read you hit depends on where the truncation lands.) **Unit tests** (`tests/unit/test_util.py`), three new, all deterministic: - `test_atomic_write_file` — same content as `write_file`, and no temp file survives in the directory. - `test_atomic_write_file_never_truncates_target` — spies on `Path.replace` to assert the live file still holds the complete *old* content right up to the atomic swap (proves the target is never truncated in place). - `test_atomic_write_file_cleans_up_on_error` — a failing replace leaves the original intact and drops no temp file behind. The last two patch `Path.replace` — exactly the call the code makes — so they exercise the real swap on every supported interpreter (verified on 3.10 and 3.14). ``` tests/unit/test_util.py::test_atomic_write_file PASSED tests/unit/test_util.py::test_atomic_write_file_never_truncates_target PASSED tests/unit/test_util.py::test_atomic_write_file_cleans_up_on_error PASSED ``` **Suite / lint:** ``` tox -e py -- tests/unit: full unit suite green (Fedora container, py3.14) python3.10 -m pytest tests/unit/test_util.py tests/unit/test_state.py: 98 passed ``` The single-process state flow (`tests/unit/test_state.py`) is unchanged and green, and the new `test_util.py` cases are included above. `tox -e lint` — `ruff`, `mypy`, `pydoclint`, `cspell` all pass on the diff. ## Regression / no-behavior-change checks - `util.write_file`'s write path is unchanged (it now shares the `_prepare_write` header/`Path` helper); `test_write_file` still passes. - `atomic_write_file` reproduces `write_file`'s output exactly (header prepend included), asserted by `test_atomic_write_file` — the on-disk *content* is byte-identical; the write path and the resulting file mode (`0600`, see Fix) are the only differences. - The single-process state flow is unaffected: `tests/unit/test_state.py` passes unchanged. ## Notes for reviewers - **This is intentionally step 1 of #4666, not the whole issue.** It removes the crash (defect 1). The second defect in #4666 — under `--workers`, each worker rewrites the whole shared file from its own in-memory snapshot, so one can silently overwrite another's flags — is a *design* question (who owns `prepare`, per-worker state vs. one shared file) that atomic writes do not solve. I left #4666 open for it rather than closing it here. - **Why scoped to the state write** and not a global `atomic_write_file` swap: in the normal `--workers` layout only `state.yml` is written concurrently, so widening it would add churn without fixing anything new. `atomic_write_file` is left public (not `_private`) because it is a general helper worth reusing if another file ever needs the same guarantee. - **Leftover temp files:** if a process is killed between `mkstemp` and the replace, a hidden `.<name>.*.tmp` can remain in the ephemeral dir. The `finally` unlinks it on any in-process failure; only a hard kill mid-write could leave one, and it's inert. ## Related - #4666 — the issue; the full mechanism, both defects, and the phased plan. - #4616 — added `--workers`; where the shared `state.yml` began being written concurrently. - #4664, #4665 — where the `py314-milestone` flake was observed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved state-file updates to write changes atomically, preventing readers from seeing partially written content. * Preserved existing state files and ensured temporary files are cleaned up if an update fails. * **Reliability** * State persistence now safely handles concurrent updates and interruptions by using atomic write behavior. * **Tests** * Added unit tests covering atomic write correctness, concurrency safety, and failure/cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: tanwigeetika1618 <84617407+tanwigeetika1618@users.noreply.github.com>
tanwigeetika1618
approved these changes
Jul 31, 2026
tanwigeetika1618
added a commit
to TejasAmle/molecule
that referenced
this pull request
Aug 4, 2026
Refs ansible#4666. First step of the phased plan in ansible#4666: make the scenario `state.yml` write atomic, which removes the partial-read crash. It does not attempt the larger shared-state design change (defect 2 in the issue); that stays tracked in ansible#4666. ## Problem Molecule runs a collection's scenarios in parallel with `--workers`. The parallel-testing guide (`docs/guides/parallel.md:30-32`) pairs that with `shared_state: true`, so every scenario running at once reads and writes the *same* `state.yml`. That file is written non-atomically, so a scenario can read it mid-rewrite, get a truncated document, and abort with a YAML scanner error. It surfaces as the intermittent `test_workers` failure on the `py314-milestone` job (py3.14 timing just widens the window): ``` molecule.util.MoleculeError: while scanning a quoted scalar ... yaml.scanner.ScannerError: could not find expected ':' ``` ## Root cause The state write is a truncate-then-rewrite with no atomicity: - `State._write_state_file` (`src/molecule/state.py:234`) → `util.write_file` → `Path.write_text(content)` (`src/molecule/util.py:222`). `write_text` truncates the file, then writes — a window in which a concurrent reader sees an empty or partial document. - `State._load_file` (`src/molecule/state.py:231`) → `util.safe_load_file` → `util.safe_load`, which turns the resulting `yaml.scanner.ScannerError` into a `MoleculeError` (`src/molecule/util.py:285-286`) and the worker aborts. - A truncation that lands on an *empty* read is quieter but worse: `safe_load` returns `{}` (`util.py:347`, `... or {}`), so `State._get_data` (`state.py:214-218`) falls back to `_default_data()` and the worker silently resets shared state (e.g. `prepared` → `False`) with no error at all. Atomicity removes both this silent reset and the `ScannerError` crash. `SharedState` (`src/molecule/state.py:241-249`) points every opted-in scenario at one `shared_ephemeral_directory/state.yml`, and molecule's own `test_workers` fixture sets `shared_state: true` (`tests/fixtures/integration/test_workers/extensions/molecule/config.yml:13`) — so CI exercises exactly this path. ## Fix - Add `util.atomic_write_file`: write the content to a temp file in the target's **own directory**, then `Path.replace` onto the target. The replace is a same-filesystem rename — POSIX guarantees `rename(2)` is atomic — so a concurrent reader always opens either the complete old file or the complete new file, never a truncated one. The temp file is removed if the write fails. - Point `State._write_state_file` at it. The file *content* is identical (`safe_dump` output with the same header); the only practically observable change is that `state.yml` is now created owner-only (`0600`) via `mkstemp` instead of umask-derived (`0644`) — harmless for this per-run ephemeral file, and if anything tighter. `write_file` and `atomic_write_file` share a small `_prepare_write` helper for the header/`Path` prep, so the new function is not a copy-paste of the old; `write_file`'s write path itself is unchanged. Scoped deliberately to the state write, not a global `write_file` swap: in the normal `--workers` layout only `state.yml` is written concurrently — every other `write_file` caller writes under a per-scenario `ephemeral_directory` — so widening it would add churn without fixing anything new. ## Evidence **Reproduction of the race and its removal.** Six workers (3 writers / 3 readers) hammer one shared `state.yml` through the exact functions `State` uses — `util.safe_load_file` to read, and `util.write_file` (before) vs. `util.atomic_write_file` (after) to write. Run against this branch on Python `3.10.20` (the race reproduces on every supported version; 3.14 timing widens the window): ``` ===== BEFORE (util.write_file) ===== util.write_file -- py 3.10.20, 3000 iters x 6 workers reader outcome: ok x8560 reader outcome: TRUNCATED_DICT(missing keys) x440 CORRUPTION: 440 corrupt read(s) ===== AFTER (util.atomic_write_file) ===== util.atomic_write_file -- py 3.10.20, 3000 iters x 6 workers reader outcome: ok x9000 clean: 0 corrupt read(s) summary: before=440 corrupt reads, after=0 corrupt reads ``` (The script is in ansible#4666; earlier runs also caught the genuine `ScannerError` — which partial read you hit depends on where the truncation lands.) **Unit tests** (`tests/unit/test_util.py`), three new, all deterministic: - `test_atomic_write_file` — same content as `write_file`, and no temp file survives in the directory. - `test_atomic_write_file_never_truncates_target` — spies on `Path.replace` to assert the live file still holds the complete *old* content right up to the atomic swap (proves the target is never truncated in place). - `test_atomic_write_file_cleans_up_on_error` — a failing replace leaves the original intact and drops no temp file behind. The last two patch `Path.replace` — exactly the call the code makes — so they exercise the real swap on every supported interpreter (verified on 3.10 and 3.14). ``` tests/unit/test_util.py::test_atomic_write_file PASSED tests/unit/test_util.py::test_atomic_write_file_never_truncates_target PASSED tests/unit/test_util.py::test_atomic_write_file_cleans_up_on_error PASSED ``` **Suite / lint:** ``` tox -e py -- tests/unit: full unit suite green (Fedora container, py3.14) python3.10 -m pytest tests/unit/test_util.py tests/unit/test_state.py: 98 passed ``` The single-process state flow (`tests/unit/test_state.py`) is unchanged and green, and the new `test_util.py` cases are included above. `tox -e lint` — `ruff`, `mypy`, `pydoclint`, `cspell` all pass on the diff. ## Regression / no-behavior-change checks - `util.write_file`'s write path is unchanged (it now shares the `_prepare_write` header/`Path` helper); `test_write_file` still passes. - `atomic_write_file` reproduces `write_file`'s output exactly (header prepend included), asserted by `test_atomic_write_file` — the on-disk *content* is byte-identical; the write path and the resulting file mode (`0600`, see Fix) are the only differences. - The single-process state flow is unaffected: `tests/unit/test_state.py` passes unchanged. ## Notes for reviewers - **This is intentionally step 1 of ansible#4666, not the whole issue.** It removes the crash (defect 1). The second defect in ansible#4666 — under `--workers`, each worker rewrites the whole shared file from its own in-memory snapshot, so one can silently overwrite another's flags — is a *design* question (who owns `prepare`, per-worker state vs. one shared file) that atomic writes do not solve. I left ansible#4666 open for it rather than closing it here. - **Why scoped to the state write** and not a global `atomic_write_file` swap: in the normal `--workers` layout only `state.yml` is written concurrently, so widening it would add churn without fixing anything new. `atomic_write_file` is left public (not `_private`) because it is a general helper worth reusing if another file ever needs the same guarantee. - **Leftover temp files:** if a process is killed between `mkstemp` and the replace, a hidden `.<name>.*.tmp` can remain in the ephemeral dir. The `finally` unlinks it on any in-process failure; only a hard kill mid-write could leave one, and it's inert. ## Related - ansible#4666 — the issue; the full mechanism, both defects, and the phased plan. - ansible#4616 — added `--workers`; where the shared `state.yml` began being written concurrently. - ansible#4664, ansible#4665 — where the `py314-milestone` flake was observed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved state-file updates to write changes atomically, preventing readers from seeing partially written content. * Preserved existing state files and ensured temporary files are cleaned up if an update fails. * **Reliability** * State persistence now safely handles concurrent updates and interruptions by using atomic write behavior. * **Tests** * Added unit tests covering atomic write correctness, concurrency safety, and failure/cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: tanwigeetika1618 <84617407+tanwigeetika1618@users.noreply.github.com>
tanwigeetika1618
added a commit
to TejasAmle/molecule
that referenced
this pull request
Aug 4, 2026
## What Removes `filter_ignored_scenarios()` and its call in `get_configs()`. Scenario discovery no longer consults `git check-ignore`, so it is decoupled from git entirely. Fixes ansible#4659. Reverts the mechanism added in ansible#3996 (for ansible#3992). ## Why the git filter is vestigial The filter was added in ansible#3996 to stop `molecule list` from discovering scenarios that a `pip install` had dropped inside a project-local virtualenv (`venv/lib/pythonX.Y/site-packages/.../molecule/*/molecule.yml`). Because `venv/` is virtually always git-ignored, "is this git-ignored?" was used as a proxy for "this is installed, not part of my project." Discovery has since become anchored and collection-aware, and that already keeps installed scenarios out before the git filter is ever consulted. `get_configs()` finds scenarios through a single source, `util.get_effective_molecule_glob()`, and both patterns it returns are rooted where a project actually keeps its scenarios: - `molecule/*/molecule.yml` for a normal project. The single `*` does not cross directory separators, so it only matches `./molecule/<name>/molecule.yml` at the top of the project. - `extensions/molecule/**/molecule.yml` once a collection is detected (ansible#4499). This one is recursive but rooted under `extensions/molecule/`, where a virtualenv never lives. Neither pattern can descend into `venv/lib/.../site-packages/`, so the installed scenarios ansible#3992 was about are excluded by *where discovery looks*. The filter's only remaining effect on real projects is a bug: it drops a user's own scenarios when they keep their `molecule/` directory out of version control (ansible#4659). | Scenario on disk | When ansible#3996 was added | Today, filter in place | With the filter removed | |---|---|---|---| | Vendored scenarios in a git-ignored virtualenv, *should be ignored* | excluded by the git filter | excluded by discovery; filter redundant | excluded by discovery | | Your own scenarios in a git-ignored `molecule/` dir, *should be found* | dropped by the git filter | dropped by the git filter, `glob failed` (ansible#4659) | found | ## The one behavior change, and its guardrail Removing the filter changes behavior in exactly one, opt-in situation. If a user has *both* deliberately set a **recursive** custom `MOLECULE_GLOB` (e.g. `**/molecule.yml`) *and* a project-local virtualenv with vendored scenarios in it, then `list` and `matrix` will now surface those vendored scenarios, because git-ignore is no longer there to prune them. That combination is an explicit opt-in to scanning everything under the project root; the default globs are rooted (see above) and are unaffected. The default path is pinned by test: `test_get_configs_default_glob_ignores_vendored_venv_scenarios` places a real `molecule/default` scenario alongside a vendored `venv/lib/python3.12/site-packages/.../molecule/default/molecule.yml` and asserts default discovery returns only the project's own scenario. So the ansible#3992 case stays fixed without the git heuristic; only the recursive-custom-glob-plus-venv combination changes. Molecule already has a git-independent way to prune under a recursive glob: the `--exclude` option (`_is_excluded()`), which today applies to `run` but not to `list`/`matrix`. Extending that coverage is the natural git-independent replacement for this narrow case. ## The fix, reproduced `molecule 26.6.1.dev9`, python 3.14. Base `upstream/main` at `ab625b40`; fix commit `cf502126`. A project whose `molecule/` directory is git-ignored: ```console $ git init -q && printf 'molecule/\n' > .gitignore $ mkdir -p molecule/default && $EDITOR molecule/default/molecule.yml $ git check-ignore molecule/default/molecule.yml molecule/default/molecule.yml ``` Before (on `ab625b40`), the scenario is dropped and there is nothing left to discover: ```console $ molecule list CRITICAL 'molecule/*/molecule.yml' glob failed. Exiting. ERROR 'molecule/*/molecule.yml' glob failed. Exiting. $ echo $? 1 ``` After (this branch), the same git-ignored scenario is discovered: ```console $ molecule list INFO default ➜ list: Executing INFO default ➜ list: Executed: Successful Instance Driver Name Provisioner Scenario Created Converged Name Name Name instance default ansible default false false $ echo $? 0 ``` ## Tests - Removed `test_with_and_without_gitignore` (the direct behavior test for the filter) and reworked it into `test_gitignored_scenario_is_discovered`, which asserts the new contract: a scenario whose directory is git-ignored is discovered. Its docstring states this contract explicitly (discovery is now git-agnostic; the test does not consult git-ignore status). It reuses the pre-existing git-ignored `.extensions/` fixture path. - Removed the two test-only monkeypatches of `filter_ignored_scenarios` (in `tests/unit/conftest.py` and `tests/unit/test_nested_scenarios.py`) that existed only to defeat the filter on the git-ignored test cache path. Those tests now rely on real discovery. - Added `test_get_configs_default_glob_ignores_vendored_venv_scenarios`: with both a real `molecule/default` scenario and a vendored one under a `venv/lib/python3.12/site-packages/.../molecule/default/molecule.yml` path, default discovery returns only the project's own scenario. This pins the ansible#3992 behavior in place without the git filter. ## Local pipeline results Run with the repo's `tox`/`uv`/`prek` dev-env, python 3.14, host is rootless podman (no docker). - **lint** (`prek run` over the changed files: ruff, mypy, pydoclint, pylint, cspell, codespell): all pass. - **unit** (`tox -e py -- tests/unit`): 824 passed, 6 skipped. Coverage total 90%, unchanged from the `ab625b40` baseline (823 passed, 90%); `command/base.py` stays at 89% (removing the covered filter plus its tests, and adding one discovery test, is coverage-neutral). - **integration** (`tests/integration/test_command.py`, the modified file): 26 passed, 1 skipped. `test_podman` runs and passes on host rootless podman; the single skip is `test_docker`, gated off by design when docker is absent (community.docker#660). `tests/integration/test_workers.py`: 4 passed. Not run locally: the container-driver paths that specifically require Docker (`test_docker`) since only rootless podman is available here; CI exercises those. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Scenario discovery now includes valid scenarios located in git-ignored directories. * Default scenario discovery avoids traversing vendored virtual environments (for example, under `site-packages`). * Scenario configuration discovery remains restricted to appropriate project locations to reduce unexpected results. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: tanwigeetika1618 <84617407+tanwigeetika1618@users.noreply.github.com>
7 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes
filter_ignored_scenarios()and its call inget_configs(). Scenario discovery no longer consultsgit check-ignore, so it is decoupled from git entirely.Fixes #4659. Reverts the mechanism added in #3996 (for #3992).
Why the git filter is vestigial
The filter was added in #3996 to stop
molecule listfrom discovering scenarios that apip installhad dropped inside a project-local virtualenv (venv/lib/pythonX.Y/site-packages/.../molecule/*/molecule.yml). Becausevenv/is virtually always git-ignored, "is this git-ignored?" was used as a proxy for "this is installed, not part of my project."Discovery has since become anchored and collection-aware, and that alre 8000 ady keeps installed scenarios out before the git filter is ever consulted.
get_configs()finds scenarios through a single source,util.get_effective_molecule_glob(), and both patterns it returns are rooted where a project actually keeps its scenarios:molecule/*/molecule.ymlfor a normal project. The single*does not cross directory separators, so it only matches./molecule/<name>/molecule.ymlat the top of the project.extensions/molecule/**/molecule.ymlonce a collection is detected (Add collection-aware scenario discovery and creation #4499). This one is recursive but rooted underextensions/molecule/, where a virtualenv never lives.Neither pattern can descend into
venv/lib/.../site-packages/, so the installed scenarios #3992 was about are excluded by where discovery looks. The filter's only remaining effect on real projects is a bug: it drops a user's own scenarios when they keep theirmolecule/directory out of version control (#4659).molecule/dir, should be foundglob failed(#4659)The one behavior change, and its guardrail
Removing the filter changes behavior in exactly one, opt-in situation. If a user has both deliberately set a recursive custom
MOLECULE_GLOB(e.g.**/molecule.yml) and a project-local virtualenv with vendored scenarios in it, thenlistandmatrixwill now surface those vendored scenarios, because git-ignore is no longer there to prune them. That combination is an explicit opt-in to scanning everything under the project root; the default globs are rooted (see above) and are unaffected.The default path is pinned by test:
test_get_configs_default_glob_ignores_vendored_venv_scenariosplaces a realmolecule/defaultscenario alongside a vendoredvenv/lib/python3.12/site-packages/.../molecule/default/molecule.ymland asserts default discovery returns only the project's own scenario. So the #3992 case stays fixed without the git heuristic; only the recursive-custom-glob-plus-venv combination changes.Molecule already has a git-independent way to prune under a recursive glob: the
--excludeoption (_is_excluded()), which today applies torunbut not tolist/matrix. Extending that coverage is the natural git-independent replacement for this narrow case.The fix, reproduced
molecule 26.6.1.dev9, python 3.14. Baseupstream/mainatab625b40; fix commitcf502126.A project whose
molecule/directory is git-ignored:Before (on
ab625b40), the scenario is dropped and there is nothing left to discover:After (this branch), the same git-ignored scenario is discovered:
Tests
test_with_and_without_gitignore(the direct behavior test for the filter) and reworked it intotest_gitignored_scenario_is_discovered, which asserts the new contract: a scenario whose directory is git-ignored is discovered. Its docstring states this contract explicitly (discovery is now git-agnostic; the test does not consult git-ignore status). It reuses the pre-existing git-ignored.extensions/fixture path.filter_ignored_scenarios(intests/unit/conftest.pyandtests/unit/test_nested_scenarios.py) that existed only to defeat the filter on the git-ignored test cache path. Those tests now rely on real discovery.test_get_configs_default_glob_ignores_vendored_venv_scenarios: with both a realmolecule/defaultscenario and a vendored one under avenv/lib/python3.12/site-packages/.../molecule/default/molecule.ymlpath, default discovery returns only the project's own scenario. This pins themolecule listfinds scenarios in virtual envrionments #3992 behavior in place without the git filter.Local pipeline results
Run with the repo's
tox/uv/prekdev-env, python 3.14, host is rootless podman (no docker).prek runover the changed files: ruff, mypy, pydoclint, pylint, cspell, codespell): all pass.tox -e py -- tests/unit): 824 passed, 6 skipped. Coverage total 90%, unchanged from theab625b40baseline (823 passed, 90%);command/base.pystays at 89% (removing the covered filter plus its tests, and adding one discovery test, is coverage-neutral).tests/integration/test_command.py, the modified file): 26 passed, 1 skipped.test_podmanruns and passes on host rootless podman; the single skip istest_docker, gated off by design when docker is absent (community.docker#660).tests/integration/test_workers.py: 4 passed.Not run locally: the container-driver paths that specifically require Docker (
test_docker) since only rootless podman is available here; CI exercises those.Summary by CodeRabbit
site-packages).