Merge pull request #1622 from fabianegli/raise-for-singularity-contai…

…ner-retrieval-fail

raise if Singularity container retrieval fails

CHANGELOG.md

@@ -13,6 +13,7 @@
 
 ### General
 
+- Fix and improve broken test for Singularity container download [#1622](https://github.com/nf-core/tools/pull/1622).
 - Updated the package requirements to prevent defunct installations of nf-core [#1620](https://github.com/nf-core/tools/pull/1620)
 - Add `--fail-warned` flag to `nf-core lint` to make warnings fail [#1593](https://github.com/nf-core/tools/pull/1593)
 - Add `--fail-warned` flag to pipeline linting workflow [#1593](https://github.com/nf-core/tools/pull/1593)

nf_core/download.py

@@ -742,16 +742,24 @@ def singularity_pull_image(self, container, out_path, cache_path, progress):
         task = progress.add_task(container, start=False, total=False, progress_type="singularity_pull", current_log="")
 
         # Run the singularity pull command
-        proc = subprocess.Popen(
+        with subprocess.Popen(
             singularity_command,
             stdout=subprocess.PIPE,
             stderr=subprocess.STDOUT,
             universal_newlines=True,
             bufsize=1,
-        )
-        for line in proc.stdout:
-            log.debug(line.strip())
-            progress.update(task, current_log=line.strip())
+        ) as proc:
+            lines = []
+            for line in proc.stdout:
+                lines.append(line)
+                progress.update(task, current_log=line.strip())
+
+        if lines:
+            # something went wrong with the container retrieval
+            if any("FATAL: " in line for line in lines):
+                log.info("Singularity container retrieval fialed with the following error:")
+                log.info("".join(lines))
+                raise FileNotFoundError(f'The container "{container}" is unavailable.\n{"".join(lines)}')
 
         # Copy cached download if we are using the cache
         if cache_path:

tests/test_download.py

@@ -165,14 +165,30 @@ def test_mismatching_md5sums(self, tmpfile):
     #
     # Tests for 'singularity_pull_image'
     #
-    # If Singularity is not installed, will log an error and exit
-    # If Singularity is installed, should raise an OSError due to non-existant image
+    # If Singularity is installed, but the container can't be accessed because it does not exist or there are aceess
+    # restrictions, a FileNotFoundError is raised due to the unavailability of the image.
+    @pytest.mark.skipif(
+        shutil.which("singularity") is None,
+        reason="Can't test what Singularity does if it's not installed.",
+    )
     @with_temporary_folder
-    @pytest.mark.xfail(raises=OSError)
     @mock.patch("rich.progress.Progress.add_task")
-    def test_singularity_pull_image(self, tmp_dir, mock_rich_progress):
+    def test_singularity_pull_image_singularity_installed(self, tmp_dir, mock_rich_progress):
         download_obj = DownloadWorkflow(pipeline="dummy", outdir=tmp_dir)
-        download_obj.singularity_pull_image("a-container", tmp_dir, None, mock_rich_progress)
+        with pytest.raises(FileNotFoundError):
+            download_obj.singularity_pull_image("a-container", tmp_dir, None, mock_rich_progress)
+
+    # If Singularity is not installed, it raises a FileNotFoundError because the singularity command can't be found.
+    @pytest.mark.skipif(
+        shutil.which("singularity") is not None,
+        reason="Can't test how the code behaves when sungularity is not installed if it is.",
+    )
+    @with_temporary_folder
+    @mock.patch("rich.progress.Progress.add_task")
+    def test_singularity_pull_image_singularity_not_installed(self, tmp_dir, mock_rich_progress):
+        download_obj = DownloadWorkflow(pipeline="dummy", outdir=tmp_dir)
+        with pytest.raises(FileNotFoundError):
+            download_obj.singularity_pull_image("a-container", tmp_dir, None, mock_rich_progress)
 
     #
     # Tests for the main entry method 'download_workflow'