Skip to content

Commit

Permalink
fix: avoid erroneous too early deletion of parent directories in case…
Browse files Browse the repository at this point in the history
… of failed jobs (thanks to @SichongP). (#1601)

* fix: avoid erroneous too early deletion of parent directories in case of failed jobs.

* fmt

* fix test handling

* skip win test

* dbg: try setting nodes to None
  • Loading branch information
johanneskoester committed May 3, 2022
1 parent cc7e0e3 commit b0917e6
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 41 deletions.
2 changes: 1 addition & 1 deletion docs/snakefiles/reporting.rst
Expand Up @@ -5,7 +5,7 @@ Reports
-------

From Snakemake 5.1 on, it is possible to automatically generate detailed self-contained HTML reports that encompass runtime statistics, provenance information, workflow topology and results.
**A realistic example report from a real workflow can be found** `here <https://koesterlab.github.io/resources/report.html>`_.
**As an example, the report of the Snakemake rolling paper can be found** `here <https://snakemake.github.io/resources/report.html>`_.

For including results into the report, the Snakefile has to be annotated with additional information.
Each output file that shall be part of the report has to be marked with the ``report`` flag, which optionally points to a caption in `restructured text format <https://docutils.sourceforge.io/docs/user/rst/quickstart.html>`_ and allows to define a ``category`` for grouping purposes.
Expand Down
2 changes: 1 addition & 1 deletion snakemake/workflow.py
Expand Up @@ -1109,7 +1109,7 @@ def files(items):

success = self.scheduler.schedule()

if not immediate_submit and not dryrun:
if not immediate_submit and not dryrun and self.mode == Mode.default:
dag.cleanup_workdir()

if success:
Expand Down
85 changes: 46 additions & 39 deletions tests/common.py
Expand Up @@ -99,9 +99,9 @@ def run(
subpath=None,
no_tmpdir=False,
check_md5=True,
check_results=True,
check_results=None,
cores=3,
nodes=1,
nodes=None,
set_pythonpath=True,
cleanup=True,
conda_frontend="mamba",
Expand All @@ -119,6 +119,12 @@ def run(
directory to the calling test for inspection, and the test should
clean it up.
"""
if check_results is None:
if not shouldfail:
check_results = True
else:
check_results = False

if set_pythonpath:
# Enforce current workdir (the snakemake source dir) to also be in PYTHONPATH
# when subprocesses are invoked in the tempdir defined below.
Expand Down Expand Up @@ -204,43 +210,44 @@ def run(
assert not success, "expected error on execution"
else:
assert success, "expected successful execution"
if check_results:
for resultfile in get_expected_files(results_dir):
if resultfile in [".gitignore", ".gitkeep"] or not os.path.isfile(
os.path.join(results_dir, resultfile)
):
# this means tests cannot use directories as output files
continue
targetfile = join(tmpdir, resultfile)
expectedfile = join(results_dir, resultfile)

if ON_WINDOWS:
if os.path.exists(join(results_dir, resultfile + "_WIN")):
continue # Skip test if a Windows specific file exists
if resultfile.endswith("_WIN"):
targetfile = join(tmpdir, resultfile[:-4])
elif resultfile.endswith("_WIN"):
# Skip win specific result files on Posix platforms
continue

assert os.path.exists(
targetfile
), 'expected file "{}" not produced'.format(resultfile)
if check_md5:
md5expected = md5sum(expectedfile, ignore_newlines=ON_WINDOWS)
md5target = md5sum(targetfile, ignore_newlines=ON_WINDOWS)
if md5target != md5expected:
with open(expectedfile) as expected:
expected_content = expected.read()
with open(targetfile) as target:
content = target.read()
assert (
False
), "wrong result produced for file '{resultfile}':\n------found------\n{content}\n-----expected-----\n{expected_content}\n-----------------".format(
resultfile=resultfile,
content=content,
expected_content=expected_content,
)

if check_results:
for resultfile in get_expected_files(results_dir):
if resultfile in [".gitignore", ".gitkeep"] or not os.path.isfile(
os.path.join(results_dir, resultfile)
):
# this means tests cannot use directories as output files
continue
targetfile = join(tmpdir, resultfile)
expectedfile = join(results_dir, resultfile)

if ON_WINDOWS:
if os.path.exists(join(results_dir, resultfile + "_WIN")):
continue # Skip test if a Windows specific file exists
if resultfile.endswith("_WIN"):
targetfile = join(tmpdir, resultfile[:-4])
elif resultfile.endswith("_WIN"):
# Skip win specific result files on Posix platforms
continue

assert os.path.exists(targetfile), 'expected file "{}" not produced'.format(
resultfile
)
if check_md5:
md5expected = md5sum(expectedfile, ignore_newlines=ON_WINDOWS)
md5target = md5sum(targetfile, ignore_newlines=ON_WINDOWS)
if md5target != md5expected:
with open(expectedfile) as expected:
expected_content = expected.read()
with open(targetfile) as target:
content = target.read()
assert (
False
), "wrong result produced for file '{resultfile}':\n------found------\n{content}\n-----expected-----\n{expected_content}\n-----------------".format(
resultfile=resultfile,
content=content,
expected_content=expected_content,
)

if not cleanup:
return tmpdir
Expand Down
20 changes: 20 additions & 0 deletions tests/test_github_issue1261/Snakefile
@@ -0,0 +1,20 @@
import time

rule all:
input:
['out/A.txt', 'out/B.txt'],

rule A:
output:
out= 'out/A.txt',
run:
time.sleep(10)
with open(output.out, 'w') as fout:
fout.write('done')

rule B:
output:
'out/B.txt',
run:
time.sleep(5)
raise Exception("Artificially failing rule B, A should go on and finalize properly.")
1 change: 1 addition & 0 deletions tests/test_github_issue1261/expected-results/out/A.txt
@@ -0,0 +1 @@
done
5 changes: 5 additions & 0 deletions tests/tests.py
Expand Up @@ -1616,6 +1616,11 @@ def test_github_issue1389():
run(dpath("test_github_issue1389"), resources={"foo": 4}, shouldfail=True)


@skip_on_windows
def test_github_issue1261():
run(dpath("test_github_issue1261"), shouldfail=True, check_results=True)


def test_rule_inheritance_globals():
run(
dpath("test_rule_inheritance_globals"),
Expand Down

0 comments on commit b0917e6

Please sign in to comment.