Repository: LUMC/pytest-workflow (language: python)

Issue:
pytest-workflow crashes when files in git are removed
pytest-workflow crashes when it is run with `--git-aware` when files that are added in git are removed from the filesystem:

```
../../miniconda3/envs/freebayes-snakemake/lib/python3.10/site-packages/pytest_workflow/plugin.py:439: in collect
    workflow = self.queue_workflow()
../../miniconda3/envs/freebayes-snakemake/lib/python3.10/site-packages/pytest_workflow/plugin.py:397: in queue_workflow
    duplicate_tree(root_dir, tempdir,
../../miniconda3/envs/freebayes-snakemake/lib/python3.10/site-packages/pytest_workflow/util.py:161: in duplicate_tree
    copy(src_path, dest_path)
../../miniconda3/envs/freebayes-snakemake/lib/python3.10/shutil.py:434: in copy2
    copyfile(src, dst, follow_symlinks=follow_symlinks)
../../miniconda3/envs/freebayes-snakemake/lib/python3.10/shutil.py:254: in copyfile
    with open(src, 'rb') as fsrc:
E   FileNotFoundError: [Errno 2] No such file or directory: 'freebayes-snakemake/tests/data/sample1_R1.fastq.gz'
```

I think it would probably be fine to simply skip the copying of "git" files if they are missing, and let the user figure out what is going on. Most likely, they removed those files just moments before, so the fact that they are not copied should not surprise them.


Discussion / hints:
I don't like skipping silently. We should post a warning. Or even better, catch this exception and throw a better one.
"Files checked in to git were not present. Did you check in the removals, or use `git rm`"? Or something along those lines.
What I like about `--git-aware` is that it only copies checked in files, so it reduces the chances of "it worked on my system" errors. So an exception in that spirit would be good.
That also sounds good!

Fix patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index dfd74e98..31ca2a09 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -7,6 +7,11 @@ Changelog
 .. This document is user facing. Please word the changes in such a way
 .. that users understand how the changes affect the new version.
 
+version 1.7.0-dev
+---------------------------
++ Throw a more descriptive error when a file copied with the --git-aware flag
+  is not present on the filesystem anymore.
+
 version 1.6.0
 ---------------------------
 + Add a ``--git-aware`` or ``--ga`` option to only copy copy files listed by


Test patch:
diff --git a/src/pytest_workflow/util.py b/src/pytest_workflow/util.py
index 947eb71a..941fde88 100644
--- a/src/pytest_workflow/util.py
+++ b/src/pytest_workflow/util.py
@@ -72,16 +72,16 @@ def git_ls_files(path: Filepath) -> List[str]:
     return output.strip("\n").split("\n")
 
 
-def _duplicate_tree(src: Filepath, dest: Filepath
-                    ) -> Iterator[Tuple[str, str, bool]]:
+def _recurse_directory_tree(src: Filepath, dest: Filepath
+                            ) -> Iterator[Tuple[str, str, bool]]:
     """Traverses src and for each file or directory yields a path to it,
     its destination, and whether it is a directory."""
-    for entry in os.scandir(src):  # type: os.DirEntry
+    for entry in os.scandir(src):  # type: os.DirEntry  # type: ignore
         if entry.is_dir():
             dir_src = entry.path
             dir_dest = os.path.join(dest, entry.name)
             yield dir_src, dir_dest, True
-            yield from _duplicate_tree(dir_src, dir_dest)
+            yield from _recurse_directory_tree(dir_src, dir_dest)
         elif entry.is_file() or entry.is_symlink():
             yield entry.path, os.path.join(dest, entry.name), False
         else:
@@ -89,8 +89,8 @@ def _duplicate_tree(src: Filepath, dest: Filepath
                           f"Skipping {entry.path}")
 
 
-def _duplicate_git_tree(src: Filepath, dest: Filepath
-                        ) -> Iterator[Tuple[str, str, bool]]:
+def _recurse_git_repository_tree(src: Filepath, dest: Filepath
+                                 ) -> Iterator[Tuple[str, str, bool]]:
     """Traverses src, finds all files registered in git and for each file or
     directory yields a path to it, its destination and whether it is a
     directory"""
@@ -121,6 +121,14 @@ def _duplicate_git_tree(src: Filepath, dest: Filepath
 
         # Yield the actual file if the directory has already been yielded.
         src_path = os.path.join(src, path)
+        if not os.path.exists(src_path):
+            raise FileNotFoundError(
+                f"{path} from git repository {src} is checked in in git, "
+                f"but not present in the filesystem. If the file was removed, "
+                f"its removal can be recorded in git with "
+                f"\"git -C '{src}' rm '{path}'\". "
+                f"Removal can be reversed with "
+                f"\"git -C '{src}' checkout '{path}'\".")
         dest_path = os.path.join(dest, path)
         yield src_path, dest_path, False
 
@@ -144,9 +152,9 @@ def duplicate_tree(src: Filepath, dest: Filepath,
         raise NotADirectoryError(f"Not a directory: '{src}'")
 
     if git_aware:
-        path_iter = _duplicate_git_tree(src, dest)
+        path_iter = _recurse_git_repository_tree(src, dest)
     else:
-        path_iter = _duplicate_tree(src, dest)
+        path_iter = _recurse_directory_tree(src, dest)
     if symlink:
         copy: Callable[[Filepath, Filepath], None] = \
             functools.partial(os.symlink, target_is_directory=False)
diff --git a/tests/functional/simple_snakefile_test_cases.yml b/tests/functional/simple_snakefile_test_cases.yml
index 5b7f969b..0ca2f666 100644
--- a/tests/functional/simple_snakefile_test_cases.yml
+++ b/tests/functional/simple_snakefile_test_cases.yml
@@ -3,19 +3,19 @@
 - name: test-config-missing
   command: snakemake -n -r -p -s SimpleSnakefile
   exit_code: 1
-  stderr:
+  stdout:
     contains:
       - "You must set --config N_LINES_TO_READ=<a value>."
 - name: test-config-wrong-type
   command: snakemake -n -r -p -s SimpleSnakefile --config N_LINES_TO_READ=one
   exit_code: 1
-  stderr:
+  stdout:
     contains:
       - "N_LINES_TO_READ must be an integer."
 - name: test-config-invalid-value
   command: snakemake -n -r -p -s SimpleSnakefile --config N_LINES_TO_READ=-1
   exit_code: 1
-  stderr:
+  stdout:
     contains:
       - "N_LINES_TO_READ must at least be 1."
 - name: test-snakemake-run
diff --git a/tests/test_utils.py b/tests/test_utils.py
index aa74946e..95a02a73 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -117,6 +117,16 @@ def test_duplicate_git_tree(git_dir):
     assert (dest / "test" / "test.txt").exists()
 
 
+def test_duplicate_git_tree_file_removed_error(git_dir):
+    assert (git_dir / ".git").exists()
+    os.remove(git_dir / "test" / "test.txt")
+    dest = Path(tempfile.mkdtemp()) / "test"
+    with pytest.raises(FileNotFoundError) as e:
+        duplicate_tree(git_dir, dest, git_aware=True)
+    e.match("checked in")
+    e.match(f"\"git -C '{git_dir}' rm '{str(Path('test', 'test.txt'))}'\"")
+
+
 def test_duplicate(git_dir):
     assert (git_dir / ".git").exists()
     dest = Path(tempfile.mkdtemp()) / "test"