tests/tests.py (3)

2037-2040: ⚠️ Potential issue | 🔴 Critical

Use path.name for the TemporaryDirectory prefix.

dpath is the imported helper function, so dpath.name will raise AttributeError before this test reaches run(). This should use the resolved Path from Line 2037 instead.

🔧 Proposed fix

-    with tempfile.TemporaryDirectory(prefix=f"snakemake-{dpath.name}") as tmpdir:
+    with tempfile.TemporaryDirectory(prefix=f"snakemake-{path.name}-") as tmpdir:

#!/bin/bash
python - <<'PY'
from pathlib import Path

def dpath(arg):
    return Path(arg)

path = dpath("test_github_issue1158")
print("path.name:", path.name)
print("hasattr(dpath, 'name'):", hasattr(dpath, "name"))
PY

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 2037 - 2040, The TemporaryDirectory prefix is
incorrectly using the helper function name dpath (which has no .name) causing
AttributeError; change the prefix to use the resolved Path variable path (i.e.,
use path.name) in the TemporaryDirectory call so the prefix becomes
f"snakemake-{path.name}"; update the line that constructs TemporaryDirectory
(where TemporaryDirectory(prefix=f"snakemake-{dpath.name}") is used) and leave
prepare_tmpdir(path, dest_path=tmpdir) and run(path, ...) unchanged.

---

1785-1792: ⚠️ Potential issue | 🟠 Major

Put the post-run cleanup in finally.

If Line 1791 fails, Line 1792 never runs and this test still leaves litter behind.

🔧 Proposed fix

     tmpdir = run(
         dpath("test_github_issue640"),
         targets=["Output/FileWithRights"],
         executor="dryrun",
         cleanup=False,
     )
-    shell(f"chmod -R u+rwX {tmpdir}")
-    shutil.rmtree(tmpdir)
+    try:
+        shell(f"chmod -R u+rwX {tmpdir}")
+    finally:
+        shutil.rmtree(tmpdir)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 1785 - 1792, The test currently assigns tmpdir
from run(...), runs shell("chmod ...") and then calls shutil.rmtree(tmpdir) but
if the test fails before the rmtree call the temp directory is left behind; wrap
the run and shell calls in a try/finally and move the shutil.rmtree(tmpdir) into
the finally block so removal always runs; update the block surrounding the
tmpdir variable (the run(...) call and the subsequent shell(...) chmod) to use
try: ... finally: shutil.rmtree(tmpdir) and guard removal for a non-None tmpdir
where necessary.

---

2890-2898: ⚠️ Potential issue | 🟠 Major

Guarantee tmpdir cleanup with finally in both exception-path tests.

These tests still remove tmpdir only when the expected control flow succeeds. An unexpected exception, or a failing pytest.raises(...), will skip cleanup and leave the temporary directory behind.

🔧 Proposed fix

 def test_ambiguousruleexception():
     path = dpath("test_ambiguousruleexception")
     tmpdir = prepare_tmpdir(path)
     try:
-        run(path, tmpdir=tmpdir)
-    except AmbiguousRuleException:
-        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
-        return
-    shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
-    raise AssertionError("This is an ambiguous case! Should have raised an error...")
+        with pytest.raises(AmbiguousRuleException):
+            run(path, tmpdir=tmpdir)
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
 
 def test_wildcard_annotatedstrings():
     path = dpath("test_wildcard_annotatedstrings")
     tmpdir = prepare_tmpdir(path, path.name)
-    with pytest.raises(WorkflowError, match=r"unpack\(\) is not allowed with params"):
-        run(path, targets=["test.out"], tmpdir=tmpdir)
-    shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
+    try:
+        with pytest.raises(
+            WorkflowError, match=r"unpack\(\) is not allowed with params"
+        ):
+            run(path, targets=["test.out"], tmpdir=tmpdir)
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)

Also applies to: 2931-2935

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 2890 - 2898, The tmpdir cleanup currently only
happens inside the try/except and is skipped on unexpected failures; wrap the
run(...) call and its except branch in a try/finally so shutil.rmtree(tmpdir,
ignore_errors=ON_WINDOWS) is executed unconditionally—update the test block that
uses dpath("test_ambiguousruleexception"), prepare_tmpdir, run, and
AmbiguousRuleException to perform cleanup in a finally, and apply the same
change to the duplicate test block (the one around lines 2931-2935) to guarantee
tmpdir is always removed.

tests/tests.py (4)

2931-2936: ⚠️ Potential issue | 🟠 Major

Cleanup still needs a finally block here.

If pytest.raises(...) fails or run() raises a different exception, shutil.rmtree() is skipped and this test still leaves litter behind.

Suggested fix

 def test_wildcard_annotatedstrings():
     path = dpath("test_wildcard_annotatedstrings")
     tmpdir = prepare_tmpdir(path, path.name)
-    with pytest.raises(WorkflowError, match=r"unpack\(\) is not allowed with params"):
-        run(path, targets=["test.out"], tmpdir=tmpdir)
-    shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
+    try:
+        with pytest.raises(
+            WorkflowError, match=r"unpack\(\) is not allowed with params"
+        ):
+            run(path, targets=["test.out"], tmpdir=tmpdir)
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 2931 - 2936, The test
test_wildcard_annotatedstrings leaves the temporary directory behind if
pytest.raises or run() fails; wrap the run/assertion and the rmtree call in a
try/finally so shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS) always executes.
Locate the test_wildcard_annotatedstrings function (uses prepare_tmpdir,
pytest.raises and run) and move the run(...) call into a try block and perform
the cleanup in the finally block to guarantee removal of tmpdir.

---

1785-1792: ⚠️ Potential issue | 🟠 Major

Keep the chmod cleanup path in finally.

If shell(...) raises here, the rmtree() is skipped and this test still leaves litter behind. Let finally own cleanup, and tolerate deletion failures there because chmod is exactly what makes the tree removable.

Suggested fix

     tmpdir = run(
         dpath("test_github_issue640"),
         targets=["Output/FileWithRights"],
         executor="dryrun",
         cleanup=False,
     )
-    shell(f"chmod -R u+rwX {tmpdir}")
-    shutil.rmtree(tmpdir)
+    try:
+        shell(f"chmod -R u+rwX {tmpdir}")
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 1785 - 1792, The temporary-directory cleanup
currently calls shell("chmod -R u+rwX {tmpdir}") and then shutil.rmtree(tmpdir)
inline, which can skip removal if shell() raises; move both the chmod and
shutil.rmtree call into a finally block so cleanup always runs, applying chmod
before attempting rmtree; wrap shutil.rmtree(tmpdir) in a tolerant try/except
(catch Exception) to ignore removal failures (since chmod is what enables
deletion) and reference the existing tmpdir variable and the calls to run(),
shell(), and shutil.rmtree() when making the change.

---

2890-2899: ⚠️ Potential issue | 🔴 Critical

Don't delete the same tmpdir in both except and finally.

On the expected AmbiguousRuleException path, the first rmtree() removes the directory and the finally block immediately tries to remove it again. On non-Windows that second call raises FileNotFoundError, so the test fails for the wrong reason.

Suggested fix

 def test_ambiguousruleexception():
     path = dpath("test_ambiguousruleexception")
     tmpdir = prepare_tmpdir(path)
     try:
-        run(path, tmpdir=tmpdir)
-    except AmbiguousRuleException:
-        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
-        return
+        with pytest.raises(AmbiguousRuleException):
+            run(path, tmpdir=tmpdir)
     finally:
         shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
-    raise AssertionError("This is an ambiguous case! Should have raised an error...")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 2890 - 2899, The test currently calls
shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS) in both the except
AmbiguousRuleException handler and the finally block, which can cause a
FileNotFoundError on non-Windows; remove the redundant removal from the except
block so cleanup is performed only in the finally block (leave the finally's
shutil.rmtree as the single cleanup), keeping the try/run(path, tmpdir=tmpdir)
call and the except AmbiguousRuleException return behavior intact (references:
tmpdir, run(), AmbiguousRuleException, finally, shutil.rmtree, ON_WINDOWS).

---

2037-2040: ⚠️ Potential issue | 🔴 Critical

Use path.name here, not dpath.name.

dpath is the helper function, so dpath.name raises before the test body runs. The prefix needs to come from the resolved path object instead.

#!/bin/bash
sed -n '2037,2040p' tests/tests.py
rg -n -A4 '^def dpath' tests/common.py
python - <<'PY'
from pathlib import Path
def dpath(_): return Path("test_github_issue1158")
print("function_has_name_attr:", hasattr(dpath, "name"))
print("resolved_path_name:", dpath("x").name)
PY

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 2037 - 2040, The test constructs a
TemporaryDirectory prefix using the helper function name (dpath.name) which is
incorrect because dpath is a function; change the prefix to use the resolved
Path object's name (path.name) instead — update the TemporaryDirectory call in
the test around run(path, cluster="./qsub.py", tmpdir=tmpdir) so it uses
tempfile.TemporaryDirectory(prefix=f"snakemake-{path.name}") (referencing the
dpath() helper and the local variable path to locate the change).