src/snakemake/unit_tests/templates/ruletest.py.jinja2 (1)

10-10: Consider updating the snakemake invocation to also capture stderr.

For consistency with the changes in common.py.jinja2, this check_output call could also be updated to run(..., check=True, capture_output=True). This would ensure stderr is captured when the snakemake test run fails, aiding in debugging.

This is optional and could be addressed in a follow-up if desired.

♻️ Suggested change

-from subprocess import check_output
+from subprocess import run

         # Run the test job.
-        check_output(
+        run(
             [
                 "python",
                 "-m",
                 "snakemake",
                 ...
             ]
-            + conda_prefix
+            + conda_prefix,
+            check=True,
+            capture_output=True,
         )

Also applies to: 30-65

🤖 Prompt for AI Agents

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

In `@src/snakemake/unit_tests/templates/ruletest.py.jinja2` at line 10, The test
currently imports and uses subprocess.check_output which only captures stdout;
update the import and invocation to use subprocess.run and call run(...,
check=True, capture_output=True) instead of check_output so stderr is captured
on failure. Replace usages of check_output(...) in the ruletest template with
run(..., check=True, capture_output=True) and adjust any places reading the
output to use the CompletedProcess attributes (stdout/stderr) as needed; ensure
the import line in src/snakemake/unit_tests/templates/ruletest.py.jinja2 brings
in run (or import subprocess and prefix calls) so the new call resolves.

src/snakemake/unit_tests/templates/common.py.jinja2 (1)

7-7: ⚠️ Potential issue | 🟠 Major

Capturing the subprocess streams here still won't surface the failure details.

check_output() already stores stdout on CalledProcessError, and Line 59 now stores stderr too, but the default exception text stays generic. A failing cmp will therefore still bubble up without the captured diagnostics unless you catch CalledProcessError here and re-raise with the decoded stream contents.

🛠️ Proposed fix

-from subprocess import check_output, PIPE
+from subprocess import CalledProcessError, PIPE, check_output
...
     def compare_files(self, expected_file, generated_file, cmp_cmds):
-        check_output(
-            cmp_cmds.get(expected_file.suffix, ["cmp"])
-            + [expected_file, generated_file],
-            stderr=PIPE,
-        )
+        try:
+            check_output(
+                cmp_cmds.get(expected_file.suffix, ["cmp"])
+                + [expected_file, generated_file],
+                stderr=PIPE,
+            )
+        except CalledProcessError as e:
+            details = "\n".join(
+                part
+                for part in [
+                    e.output.decode(errors="replace").strip() if e.output else "",
+                    e.stderr.decode(errors="replace").strip() if e.stderr else "",
+                ]
+                if part
+            )
+            raise AssertionError(
+                f"File comparison failed for {generated_file} against {expected_file}"
+                + (f":\n{details}" if details else "")
+            ) from e

Run this read-only check to confirm that str(e) does not include the captured streams:

#!/bin/bash
python - <<'PY'
import subprocess, sys

try:
    subprocess.check_output(
        [
            sys.executable,
            "-c",
            "import sys; sys.stdout.write('stdout detail\\n'); sys.stderr.write('stderr detail\\n'); sys.exit(1)",
        ],
        stderr=subprocess.PIPE,
    )
except subprocess.CalledProcessError as e:
    print("str(e):", str(e))
    print("output:", e.output.decode().strip())
    print("stderr:", e.stderr.decode().strip())
PY

Expected: str(e) stays generic, while output and stderr contain the useful diagnostics.

Also applies to: 55-60

🤖 Prompt for AI Agents

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

In `@src/snakemake/unit_tests/templates/common.py.jinja2` at line 7, The call to
check_output() must be wrapped in a try/except to catch
subprocess.CalledProcessError and re-raise a clearer error including the decoded
e.output and e.stderr so failure diagnostics aren't lost; locate the
check_output invocation(s) (and the block around line ~59) and replace the bare
call with a try: ... except subprocess.CalledProcessError as e: raise
RuntimeError(f"cmp failed: stdout={e.output.decode().strip()},
stderr={e.stderr.decode().strip()}") from e (or similar message) ensuring you
reference e.output and e.stderr and decode them before including in the raised
exception.

src/snakemake/unit_tests/templates/common.py.jinja2 (1)

30-45: Drop the temporary DEBUG prints before merge.

Lines 30, 38, and 45 unconditionally dump full file sets on every successful test run, which will make CI logs noisy and can bury the comparison failure you actually want to inspect. Please gate these behind an opt-in verbose flag or remove them.

🤖 Prompt for AI Agents

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

In `@src/snakemake/unit_tests/templates/common.py.jinja2` around lines 30 - 45,
Remove the unconditional debug prints that dump input_files, workdir_files, and
expected_files; either delete the print(...) calls around those variables or
gate them behind an opt-in verbose/debug flag (e.g., check self.verbose or use a
logger.debug) so normal test runs don’t flood CI logs—update the code around the
variables input_files, workdir_files, and expected_files to only emit these
details when verbose/debug is enabled.