From 499044d7bcbc4bdb0c5185abebaae2e48ffe13dd Mon Sep 17 00:00:00 2001 From: fabianegli Date: Wed, 1 Jun 2022 20:03:02 +0200 Subject: [PATCH 1/4] raise if container retrieval fails --- nf_core/download.py | 18 +++++++++++++----- tests/test_download.py | 29 ++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/nf_core/download.py b/nf_core/download.py index 12817e959e..b0ad1ca62b 100644 --- a/nf_core/download.py +++ b/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: diff --git a/tests/test_download.py b/tests/test_download.py index 95b3cca759..f2b0d47b02 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -165,14 +165,33 @@ 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) + + # + # Tests for 'singularity_pull_image' + # + # 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 if 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' From b4fff9163e0cb500c1bd45c598af745121bb8caa Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Wed, 1 Jun 2022 20:11:43 +0200 Subject: [PATCH 2/4] remove unnecessary comments --- tests/test_download.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index f2b0d47b02..b9b5d68df8 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -178,9 +178,6 @@ def test_singularity_pull_image_singularity_installed(self, tmp_dir, mock_rich_p with pytest.raises(FileNotFoundError): download_obj.singularity_pull_image("a-container", tmp_dir, None, mock_rich_progress) - # - # Tests for 'singularity_pull_image' - # # 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, From c174a1c12e434da5cd8ebf77422eb2d3126a65af Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Tue, 14 Jun 2022 02:13:07 +0200 Subject: [PATCH 3/4] Update CHANGELOG for #1622 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ebed5628f..af5795ccfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ ### General +- Fix and improve broken test for Singularity container download [#1622](https://github.com/nf-core/tools/pull/1622). + ### Modules - Fix a bug in the regex extracting the version from biocontainers URLs [#1598](https://github.com/nf-core/tools/pull/1598) From 8b751efedda532feeeffafe0ef8a59283f392667 Mon Sep 17 00:00:00 2001 From: Fabian Egli Date: Tue, 14 Jun 2022 12:04:29 +0200 Subject: [PATCH 4/4] better wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- tests/test_download.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_download.py b/tests/test_download.py index cc8bc85d67..0b06c0d1a8 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -181,7 +181,7 @@ def test_singularity_pull_image_singularity_installed(self, tmp_dir, mock_rich_p # 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 if sungularity is not installed if it is.", + 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")