Repository navigation
fix: reset shared ephemeral directories once - #4727
dajiaohuang wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Track directory removals src/molecule/command/base.py |
A helper skips ephemeral directories that were already removed. The command reset path uses the helper and tracks removals. |
Apply tracked cleanup in worker reset src/molecule/worker.py, tests/unit/command/test_reset.py |
The worker reset path uses a shared set and the helper instead of deleting directories directly. A test checks that shared and unique directories are each removed once. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers: cidrblock
Merge Risk: 
2939e
-
Reset with a shared ephemeral directory should now remove it once instead of failing on the second removal. No actionable merge-blocking risk remains; the PR notes that integration tests and CI were not run, which is normal pre-merge verification.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: removing shared ephemeral directories only once during reset. Linked Issues check Issue #4717requires serial reset of multiple scenarios with shared state to remove a shared ephemeral directory without a second-removal failure._run_scenariosnow creates one `removed_ephemeral_d…Out of Scope Changes check The changes are limited to shared ephemeral-directory removal during reset, the reusable helper in src/molecule/command/base.py, its use in the worker reset flow, and a focused regression test. The …Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. Full details: Linked Issues check
-
Explanation
Issue
#4717requires serial reset of multiple scenarios with shared state to remove a shared ephemeral directory without a second-removal failure._run_scenariosnow creates oneremoved_ephemeral_directoriesset and calls_remove_scenario_ephemeral_directoryfor each reset scenario. The helper skips paths already in the set, and the regression test verifies one removal for the shared path and one removal for a unique path.Scenario.ephemeral_directoryresolves to the shared path when shared state is enabled.
Full details: Out of Scope Changes check
-
Explanation
The changes are limited to shared ephemeral-directory removal during reset, the reusable helper in
src/molecule/command/base.py, its use in the worker reset flow, and a focused regression test. The worker change applies the same reset fix to the parallel flow and supports consistent behavior. No unrelated product behavior is established by the supplied summary or inspected files.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
Comment @coderabbitai help to get the list of available commands.
Fixes #4717.
When multiple scenarios share an ephemeral directory, reset attempts to remove the same path once per scenario; the second removal fails. Track directories removed during a reset and remove each unique path only once. Use the same helper in serial and worker reset flows.
Validation:
test_reset_removes_shared_ephemeral_directory_once: 1 passed in a task-local Python 3.13 environment. A fail-fastfcntlshim was used only to allow collection on Windows; the test does not exercise locking.py_compilepassed for the changed Python files.INP001ignored because this sparse checkout lacks package marker files.fcntl, and this sparse test fixture tree lacksconfig_instance; with the fail-fast shim, the module reported 7 passed, 2 failures in existingcaplogassertions, and 1 fixture error. No integration tests or CI were run.Summary by CodeRabbit