From 7d24e7510d3e7e74e86dc9fdedc44ceec20b329c Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 12:18:59 +0100 Subject: [PATCH 01/25] added patch command for subworkflows --- nf_core/subworkflows/patch.py | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 nf_core/subworkflows/patch.py diff --git a/nf_core/subworkflows/patch.py b/nf_core/subworkflows/patch.py new file mode 100644 index 0000000000..3c8b3d5e4d --- /dev/null +++ b/nf_core/subworkflows/patch.py @@ -0,0 +1,10 @@ +import logging + +from nf_core.components.patch import ComponentPatch + +log = logging.getLogger(__name__) + + +class SubworkflowPatch(ComponentPatch): + def __init__(self, pipeline_dir, remote_url=None, branch=None, no_pull=False, installed_by=False): + super().__init__(pipeline_dir, "subworkflows", remote_url, branch, no_pull, installed_by) From 86e3e2f32dc235879340730fbb0f9bc17b6a4c72 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 12:29:37 +0100 Subject: [PATCH 02/25] forgot to import patch in init --- nf_core/subworkflows/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nf_core/subworkflows/__init__.py b/nf_core/subworkflows/__init__.py index 88e8a09388..8e3c85a271 100644 --- a/nf_core/subworkflows/__init__.py +++ b/nf_core/subworkflows/__init__.py @@ -3,5 +3,6 @@ from .install import SubworkflowInstall from .lint import SubworkflowLint from .list import SubworkflowList +from .patch import SubworkflowPatch from .remove import SubworkflowRemove from .update import SubworkflowUpdate From 26b39c2f336db7d31ccfa421c34f1a2137dd6640 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 12:58:13 +0100 Subject: [PATCH 03/25] added files for the tests --- nf_core/__main__.py | 35 +++++++++++++++++++++++++++++++++++ tests/test_subworkflows.py | 9 +++++++++ 2 files changed, 44 insertions(+) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index d6f6077be9..c372e88973 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -1684,6 +1684,41 @@ def subworkflows_install(ctx, subworkflow, dir, prompt, force, sha): log.error(e) sys.exit(1) +# nf-core subworkflows patch +@subworkflows.command("patch") +@click.pass_context +@click.argument("tool", type=str, required=False, metavar=" or ") +@click.option( + "-d", + "--dir", + type=click.Path(exists=True), + default=".", + help=r"Pipeline directory. [dim]\[default: current working directory][/]", +) +@click.option("-r", "--remove", is_flag=True, default=False) +def subworkflows_patch(ctx, tool, dir, remove): + """ + Create a patch file for minor changes in a subworkflow + + Checks if a subworkflow has been modified locally and creates a patch file + describing how the module has changed from the remote version + """ + from nf_core.subworkflows import SubworkflowPatch + + try: + subworkflow_patch = SubworkflowPatch( + dir, + ctx.obj["modules_repo_url"], + ctx.obj["modules_repo_branch"], + ctx.obj["modules_repo_no_pull"], + ) + if remove: + subworkflow_patch.remove(tool) + else: + subworkflow_patch.patch(tool) + except (UserWarning, LookupError) as e: + log.error(e) + sys.exit(1) # nf-core subworkflows remove @subworkflows.command("remove") diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 6163faa7a9..5a781cd871 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -138,6 +138,15 @@ def tearDown(self): test_subworkflows_list_remote, test_subworkflows_list_remote_gitlab, ) + from .subworkflows.patch import( # type: ignore[misc] + test_create_patch_change, + test_create_patch_no_change, + test_create_patch_try_apply_failed, + test_create_patch_try_apply_successful, + test_create_patch_update_fail, + test_create_patch_update_success, + test_remove_patch, + ) from .subworkflows.remove import ( # type: ignore[misc] test_subworkflows_remove_included_subworkflow, test_subworkflows_remove_one_of_two_subworkflow, From 015a61be7ae0d0a59c98615a251bd982c14e777a Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 13:37:46 +0100 Subject: [PATCH 04/25] created draft for test file --- tests/subworkflows/patch.py | 77 +++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/subworkflows/patch.py diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py new file mode 100644 index 0000000000..c2d7cd97f2 --- /dev/null +++ b/tests/subworkflows/patch.py @@ -0,0 +1,77 @@ +import os + +import pytest + +from nf_core.modules.modules_json import ModulesJson +from nf_core.subworkflows.install import SubworkflowInstall + +from ..utils import ( + GITLAB_BRANCH_TEST_BRANCH, + GITLAB_REPO, + GITLAB_SUBWORKFLOWS_BRANCH, + GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH, + GITLAB_URL, + with_temporary_folder, +) + +""" +Test the 'nf-core subworkflows patch' command +""" + +def setup_patch(self, pipeline_dir, modify_subworkflow): + # Install the subworkflow bam_sort_stats_samtools + subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + sub_subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") + samtools_index_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "index") + samtools_sort_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "sort") + samtools_stats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "stats") + samtools_idxstats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "idxstats") + samtools_flagstat_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "flagstat") + + + if modify_subworkflow: + # Modify the subworkflow + subworkflow_path = Path(pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + modify_subworkflow(subworkflow_path / "main.nf") + + +def modify_subworkflow(path): + """Modify a file to test patch creation""" + with open(path) as fh: + lines = fh.readlines() + # We want a patch file that looks something like: + # - ch_fasta // channel: [ val(meta), path(fasta) ] + for line_index in range(len(lines)): + if lines[line_index] == " ch_fasta // channel: [ val(meta), path(fasta) ]\n": + to_pop = line_index + lines.pop(to_pop) + with open(path, "w") as fh: + fh.writelines(lines) + +def test_create_patch_change(self): + """Test creating a patch when there is a change to the module""" + +def test_create_patch_no_change(self): + """Test creating a patch when there is no change to the subworkflow""" + # Try creating a patch file + # Check that no patch file has been added to the directory + +def test_create_patch_try_apply_failed(self): + """Test creating a patch file and applying it to a new version of the the files""" + +def test_create_patch_try_apply_successful(self): + """Test creating a patch file and applying it to a new version of the the files""" + +def test_create_patch_update_fail(self): + """Test creating a patch file and updating a subworkflow when there is a diff conflict""" + +def test_create_patch_update_success (self): + """ + Test creating a patch file and the updating the subworkflow + + Should have the same effect as 'test_create_patch_try_apply_successful' + but uses higher level api + """ + +def test_remove_patch(self): + """Test creating a patch when there is no change to the subworkflow""" From 563e2326fb063603508b67de6205998004fac9e5 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 13:41:28 +0100 Subject: [PATCH 05/25] ruff format --- nf_core/__main__.py | 2 ++ tests/subworkflows/patch.py | 11 +++++++++-- tests/test_subworkflows.py | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index c372e88973..3468f74e52 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -1684,6 +1684,7 @@ def subworkflows_install(ctx, subworkflow, dir, prompt, force, sha): log.error(e) sys.exit(1) + # nf-core subworkflows patch @subworkflows.command("patch") @click.pass_context @@ -1720,6 +1721,7 @@ def subworkflows_patch(ctx, tool, dir, remove): log.error(e) sys.exit(1) + # nf-core subworkflows remove @subworkflows.command("remove") @click.pass_context diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index c2d7cd97f2..b340d27fed 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -18,6 +18,7 @@ Test the 'nf-core subworkflows patch' command """ + def setup_patch(self, pipeline_dir, modify_subworkflow): # Install the subworkflow bam_sort_stats_samtools subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") @@ -28,7 +29,6 @@ def setup_patch(self, pipeline_dir, modify_subworkflow): samtools_idxstats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "idxstats") samtools_flagstat_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "flagstat") - if modify_subworkflow: # Modify the subworkflow subworkflow_path = Path(pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") @@ -48,24 +48,30 @@ def modify_subworkflow(path): with open(path, "w") as fh: fh.writelines(lines) + def test_create_patch_change(self): """Test creating a patch when there is a change to the module""" + def test_create_patch_no_change(self): """Test creating a patch when there is no change to the subworkflow""" # Try creating a patch file # Check that no patch file has been added to the directory + def test_create_patch_try_apply_failed(self): """Test creating a patch file and applying it to a new version of the the files""" + def test_create_patch_try_apply_successful(self): """Test creating a patch file and applying it to a new version of the the files""" + def test_create_patch_update_fail(self): """Test creating a patch file and updating a subworkflow when there is a diff conflict""" -def test_create_patch_update_success (self): + +def test_create_patch_update_success(self): """ Test creating a patch file and the updating the subworkflow @@ -73,5 +79,6 @@ def test_create_patch_update_success (self): but uses higher level api """ + def test_remove_patch(self): """Test creating a patch when there is no change to the subworkflow""" diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 5a781cd871..6a58473e0b 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -138,7 +138,7 @@ def tearDown(self): test_subworkflows_list_remote, test_subworkflows_list_remote_gitlab, ) - from .subworkflows.patch import( # type: ignore[misc] + from .subworkflows.patch import ( # type: ignore[misc] test_create_patch_change, test_create_patch_no_change, test_create_patch_try_apply_failed, From 31505ab1ae67254a81537950a00bf12b846049ad Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 14:16:11 +0100 Subject: [PATCH 06/25] cleaning up --- tests/subworkflows/patch.py | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index b340d27fed..c34711ba07 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -1,18 +1,5 @@ import os - -import pytest - -from nf_core.modules.modules_json import ModulesJson -from nf_core.subworkflows.install import SubworkflowInstall - -from ..utils import ( - GITLAB_BRANCH_TEST_BRANCH, - GITLAB_REPO, - GITLAB_SUBWORKFLOWS_BRANCH, - GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH, - GITLAB_URL, - with_temporary_folder, -) +from pathlib import Path """ Test the 'nf-core subworkflows patch' command @@ -22,12 +9,6 @@ def setup_patch(self, pipeline_dir, modify_subworkflow): # Install the subworkflow bam_sort_stats_samtools subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - sub_subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_stats_samtools") - samtools_index_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "index") - samtools_sort_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "sort") - samtools_stats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "stats") - samtools_idxstats_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "idxstats") - samtools_flagstat_path = os.path.join(self.subworkflow_install.dir, "modules", "nf-core", "samtools", "flagstat") if modify_subworkflow: # Modify the subworkflow From 4d0dc8532d6a5975531a9b9a47f06105762eeddc Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 14:59:23 +0100 Subject: [PATCH 07/25] added tests --- tests/subworkflows/patch.py | 149 ++++++++++++++++++++++++++++++++++-- 1 file changed, 142 insertions(+), 7 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index c34711ba07..cbb248e7e2 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -1,5 +1,28 @@ import os +import tempfile from pathlib import Path +from unittest import mock + + +import pytest + +from nf_core.modules.modules_json import ModulesJson +from nf_core.subworkflows.install import SubworkflowInstall +import nf_core.subworkflows +import nf_core.components.components_command + + +from ..utils import ( + GITLAB_BRANCH_TEST_BRANCH, + GITLAB_REPO, + GITLAB_SUBWORKFLOWS_BRANCH, + GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH, + GITLAB_URL, + with_temporary_folder, +) + +# TODO: #Change this for the correct SUCCEED_SHA +SUCCEED_SHA = "????" """ Test the 'nf-core subworkflows patch' command @@ -30,29 +53,123 @@ def modify_subworkflow(path): fh.writelines(lines) -def test_create_patch_change(self): +def test_create_patch_no_change(self): """Test creating a patch when there is a change to the module""" + setup_patch(self.pipeline_dir, False) + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + with pytest.raises(UserWarning): + patch_obj.patch("bam_sort_stats_samtools") -def test_create_patch_no_change(self): + subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + + # Check that no patch file has been added to the directory + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + +def test_create_patch_change(self): """Test creating a patch when there is no change to the subworkflow""" + setup_patch(self.pipeline_dir, True) + # Try creating a patch file - # Check that no patch file has been added to the directory + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") -def test_create_patch_try_apply_failed(self): - """Test creating a patch file and applying it to a new version of the the files""" + patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} + + # Check that the correct lines are in the patch file + with open(subworkflow_path / patch_fn) as fh: + patch_lines = fh.readlines() + subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) + assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" + assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines + assert "- ch_fasta // channel: [ val(meta), path(fasta) ]" in patch_lines def test_create_patch_try_apply_successful(self): """Test creating a patch file and applying it to a new version of the the files""" + setup_patch(self.pipeline_dir, True) + subworkflow_relpath = Path("subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} + + update_obj = nf_core.subworkflows.SubworkflowUpdate( + self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH + ) + + # Install the new files + install_dir = Path(tempfile.mkdtemp()) + update_obj.install_component_files("bam_sort_stats_samtools", SUCCEED_SHA, update_obj.modules_repo, install_dir) + + # Try applying the patch + subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" + patch_relpath = subworkflow_relpath / patch_fn + assert update_obj.try_apply_patch("bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir) is True + + # Move the files from the temporary directory + update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, "nf-core", SUCCEED_SHA) + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} + + # Check that the correct lines are in the patch file + with open(subworkflow_path / patch_fn) as fh: + patch_lines = fh.readlines() + subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) + assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" + assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines + assert "- ch_fasta // channel: [ val(meta), path(fasta) ]" in patch_lines + + # Check that 'main.nf' is updated correctly + with open(subworkflow_path / "main.nf") as fh: + main_nf_lines = fh.readlines() + # These lines should have been removed by the patch + assert " ch_fasta // channel: [ val(meta), path(fasta) ]\n" not in main_nf_lines -def test_create_patch_update_fail(self): - """Test creating a patch file and updating a subworkflow when there is a diff conflict""" +def test_create_patch_try_apply_failed(self): + """Test creating a patch file and applying it to a new version of the the files""" + setup_patch(self.pipeline_dir, True) + subworkflow_relpath = Path("subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} + + update_obj = nf_core.subworkflows.SubworkflowUpdate( + self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH + ) + # Install the new files + install_dir = Path(tempfile.mkdtemp()) + update_obj.install_component_files("bam_sort_stats_samtools", SUCCEED_SHA, update_obj.modules_repo, install_dir) + # Try applying the patch + subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" + patch_relpath = subworkflow_relpath / patch_fn + assert update_obj.try_apply_patch("bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir) is False + +# TODO: create those two missing tests def test_create_patch_update_success(self): + """Test creating a patch file and updating a subworkflow when there is a diff conflict""" + + +def test_create_patch_update_fail(self): """ Test creating a patch file and the updating the subworkflow @@ -63,3 +180,21 @@ def test_create_patch_update_success(self): def test_remove_patch(self): """Test creating a patch when there is no change to the subworkflow""" + setup_patch(self.pipeline_dir, True) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + + patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} + + with mock.patch.object(nf_core.create.questionary, "confirm") as mock_questionary: + mock_questionary.unsafe_ask.return_value = True + patch_obj.remove("bam_sort_stats_samtools") + # Check that the diff file has been removed + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + From 3030a76c5adfc867a876a34c0be405b3935c5f61 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:01:22 +0100 Subject: [PATCH 08/25] ruff --- tests/subworkflows/patch.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index cbb248e7e2..44c07d7d62 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -3,22 +3,14 @@ from pathlib import Path from unittest import mock - import pytest -from nf_core.modules.modules_json import ModulesJson -from nf_core.subworkflows.install import SubworkflowInstall -import nf_core.subworkflows import nf_core.components.components_command - +import nf_core.subworkflows from ..utils import ( GITLAB_BRANCH_TEST_BRANCH, - GITLAB_REPO, - GITLAB_SUBWORKFLOWS_BRANCH, - GITLAB_SUBWORKFLOWS_ORG_PATH_BRANCH, GITLAB_URL, - with_temporary_folder, ) # TODO: #Change this for the correct SUCCEED_SHA From 6b885ff43b3a031fd9f8da3af58b4eb72f5a6d3e Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:02:58 +0100 Subject: [PATCH 09/25] ruff format --- tests/subworkflows/patch.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index 44c07d7d62..6e26d4844f 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -59,6 +59,7 @@ def test_create_patch_no_change(self): # Check that no patch file has been added to the directory assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + def test_create_patch_change(self): """Test creating a patch when there is no change to the subworkflow""" setup_patch(self.pipeline_dir, True) @@ -107,7 +108,12 @@ def test_create_patch_try_apply_successful(self): # Try applying the patch subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" patch_relpath = subworkflow_relpath / patch_fn - assert update_obj.try_apply_patch("bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir) is True + assert ( + update_obj.try_apply_patch( + "bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir + ) + is True + ) # Move the files from the temporary directory update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, "nf-core", SUCCEED_SHA) @@ -129,6 +135,7 @@ def test_create_patch_try_apply_successful(self): # These lines should have been removed by the patch assert " ch_fasta // channel: [ val(meta), path(fasta) ]\n" not in main_nf_lines + def test_create_patch_try_apply_failed(self): """Test creating a patch file and applying it to a new version of the the files""" setup_patch(self.pipeline_dir, True) @@ -154,7 +161,13 @@ def test_create_patch_try_apply_failed(self): # Try applying the patch subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" patch_relpath = subworkflow_relpath / patch_fn - assert update_obj.try_apply_patch("bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir) is False + assert ( + update_obj.try_apply_patch( + "bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir + ) + is False + ) + # TODO: create those two missing tests def test_create_patch_update_success(self): @@ -189,4 +202,3 @@ def test_remove_patch(self): patch_obj.remove("bam_sort_stats_samtools") # Check that the diff file has been removed assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} - From dfcfb5b5111aa1d4d64ebcbd75a989d6b36ba0db Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:07:51 +0100 Subject: [PATCH 10/25] removed split --- tests/subworkflows/patch.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index 6e26d4844f..ebec1171fa 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -70,7 +70,7 @@ def test_create_patch_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -93,7 +93,7 @@ def test_create_patch_try_apply_successful(self): patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -146,7 +146,7 @@ def test_create_patch_try_apply_failed(self): patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -193,7 +193,7 @@ def test_remove_patch(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools".split('/'))}.diff" + patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} From 299193020c8c1e37b72633036d9e9a1b375cb899 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:09:59 +0100 Subject: [PATCH 11/25] mypy --- tests/subworkflows/patch.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index ebec1171fa..440c633e7e 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -70,7 +70,7 @@ def test_create_patch_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" + patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -93,7 +93,7 @@ def test_create_patch_try_apply_successful(self): patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" + patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -146,7 +146,7 @@ def test_create_patch_try_apply_failed(self): patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" + patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -193,7 +193,7 @@ def test_remove_patch(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - patch_fn = f"{'-'.join("bam_sort_stats_samtools")}.diff" + patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} From e909d3c19330f014c59dc3101d06e0fad24e70c4 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:22:22 +0100 Subject: [PATCH 12/25] setup_patch --- tests/subworkflows/patch.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index 440c633e7e..06bb5bcadd 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -15,15 +15,22 @@ # TODO: #Change this for the correct SUCCEED_SHA SUCCEED_SHA = "????" +ORG_SHA = "002623ccc88a3b0cb302c7d8f13792a95354d9f2" + """ Test the 'nf-core subworkflows patch' command """ -def setup_patch(self, pipeline_dir, modify_subworkflow): +def setup_patch(pipeline_dir, modify_subworkflow): # Install the subworkflow bam_sort_stats_samtools - subworkflow_path = os.path.join(self.subworkflow_install.dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + install_obj = nf_core.subworkflows.SubworkflowInstall( + pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, sha=ORG_SHA + ) + + # Install the module + install_obj.install("bam_sort_stats_samtools") if modify_subworkflow: # Modify the subworkflow From fd5b0d14193c16cf1fc716db851dd38df16277e2 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 15:33:05 +0100 Subject: [PATCH 13/25] called function correctly --- tests/subworkflows/patch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index 06bb5bcadd..19afc757f3 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -35,10 +35,10 @@ def setup_patch(pipeline_dir, modify_subworkflow): if modify_subworkflow: # Modify the subworkflow subworkflow_path = Path(pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") - modify_subworkflow(subworkflow_path / "main.nf") + modify_main_nf(subworkflow_path / "main.nf") -def modify_subworkflow(path): +def modify_main_nf(path): """Modify a file to test patch creation""" with open(path) as fh: lines = fh.readlines() From cdd9cfb67ac2e173440bcdc3eb112ddc41c795b7 Mon Sep 17 00:00:00 2001 From: ctuni Date: Mon, 18 Mar 2024 16:09:20 +0100 Subject: [PATCH 14/25] wraping up for the day --- tests/subworkflows/patch.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py index 19afc757f3..66065784cf 100644 --- a/tests/subworkflows/patch.py +++ b/tests/subworkflows/patch.py @@ -9,8 +9,9 @@ import nf_core.subworkflows from ..utils import ( - GITLAB_BRANCH_TEST_BRANCH, + GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, + GITLAB_REPO ) # TODO: #Change this for the correct SUCCEED_SHA @@ -26,7 +27,7 @@ def setup_patch(pipeline_dir, modify_subworkflow): # Install the subworkflow bam_sort_stats_samtools install_obj = nf_core.subworkflows.SubworkflowInstall( - pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH, sha=ORG_SHA + pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH, sha=ORG_SHA ) # Install the module @@ -34,7 +35,7 @@ def setup_patch(pipeline_dir, modify_subworkflow): if modify_subworkflow: # Modify the subworkflow - subworkflow_path = Path(pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") modify_main_nf(subworkflow_path / "main.nf") @@ -57,11 +58,11 @@ def test_create_patch_no_change(self): setup_patch(self.pipeline_dir, False) # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) with pytest.raises(UserWarning): patch_obj.patch("bam_sort_stats_samtools") - subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that no patch file has been added to the directory assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} @@ -72,10 +73,10 @@ def test_create_patch_change(self): setup_patch(self.pipeline_dir, True) # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created @@ -93,11 +94,11 @@ def test_create_patch_change(self): def test_create_patch_try_apply_successful(self): """Test creating a patch file and applying it to a new version of the the files""" setup_patch(self.pipeline_dir, True) - subworkflow_relpath = Path("subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) patch_obj.patch("bam_sort_stats_samtools") patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" @@ -105,7 +106,7 @@ def test_create_patch_try_apply_successful(self): assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} update_obj = nf_core.subworkflows.SubworkflowUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH + self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH ) # Install the new files @@ -117,13 +118,13 @@ def test_create_patch_try_apply_successful(self): patch_relpath = subworkflow_relpath / patch_fn assert ( update_obj.try_apply_patch( - "bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir + "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir ) is True ) # Move the files from the temporary directory - update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, "nf-core", SUCCEED_SHA) + update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, GITLAB_REPO, SUCCEED_SHA) # Check that a patch file with the correct name has been created assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} @@ -146,11 +147,11 @@ def test_create_patch_try_apply_successful(self): def test_create_patch_try_apply_failed(self): """Test creating a patch file and applying it to a new version of the the files""" setup_patch(self.pipeline_dir, True) - subworkflow_relpath = Path("subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) patch_obj.patch("bam_sort_stats_samtools") patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" @@ -158,7 +159,7 @@ def test_create_patch_try_apply_failed(self): assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} update_obj = nf_core.subworkflows.SubworkflowUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH + self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH ) # Install the new files @@ -170,7 +171,7 @@ def test_create_patch_try_apply_failed(self): patch_relpath = subworkflow_relpath / patch_fn assert ( update_obj.try_apply_patch( - "bam_sort_stats_samtools", "nf-core", patch_relpath, subworkflow_path, subworkflow_install_dir + "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir ) is False ) @@ -195,10 +196,10 @@ def test_remove_patch(self): setup_patch(self.pipeline_dir, True) # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_BRANCH_TEST_BRANCH) + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) patch_obj.patch("bam_sort_stats_samtools") - subworkflow_path = Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" # Check that a patch file with the correct name has been created From 7cd3dea18fed9af950c3a2249ab54797134e58e8 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 15 Nov 2024 16:42:14 +0100 Subject: [PATCH 15/25] update and fix swf patch tests --- nf_core/components/patch.py | 14 +- nf_core/components/remove.py | 2 +- nf_core/components/update.py | 11 +- nf_core/modules/lint/main_nf.py | 1 + nf_core/modules/lint/meta_yml.py | 2 + nf_core/modules/lint/module_changes.py | 1 + nf_core/modules/lint/module_patch.py | 1 + nf_core/modules/modules_differ.py | 20 ++- nf_core/modules/modules_json.py | 50 +++--- tests/modules/test_modules_json.py | 15 +- tests/subworkflows/patch.py | 212 ------------------------- tests/subworkflows/test_patch.py | 204 ++++++++++++++++++++++++ 12 files changed, 281 insertions(+), 252 deletions(-) delete mode 100644 tests/subworkflows/patch.py create mode 100644 tests/subworkflows/test_patch.py diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py index 41fccd8be2..77717877fc 100644 --- a/nf_core/components/patch.py +++ b/nf_core/components/patch.py @@ -65,7 +65,9 @@ def patch(self, component=None): component_fullname = str(Path(self.component_type, self.modules_repo.repo_path, component)) # Verify that the component has an entry in the modules.json file - if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir): + if not self.modules_json.component_present( + component, self.modules_repo.remote_url, component_dir, self.component_type + ): raise UserWarning( f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch" ) @@ -127,7 +129,9 @@ def patch(self, component=None): raise UserWarning(f"{self.component_type[:-1]} '{component_fullname}' is unchanged. No patch to compute") # Write changes to modules.json - self.modules_json.add_patch_entry(component, self.modules_repo.remote_url, component_dir, patch_relpath) + self.modules_json.add_patch_entry( + self.component_type, component, self.modules_repo.remote_url, component_dir, patch_relpath + ) log.debug(f"Wrote patch path for {self.component_type[:-1]} {component} to modules.json") # Show the changes made to the module @@ -166,7 +170,9 @@ def remove(self, component): component_fullname = str(Path(self.component_type, component_dir, component)) # Verify that the component has an entry in the modules.json file - if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir): + if not self.modules_json.component_present( + component, self.modules_repo.remote_url, component_dir, self.component_type + ): raise UserWarning( f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch" ) @@ -202,7 +208,7 @@ def remove(self, component): # Try to apply the patch in reverse and move resulting files to module dir temp_component_dir = self.modules_json.try_apply_patch_reverse( - component, self.modules_repo.repo_path, patch_relpath, component_path + self.component_type, component, self.modules_repo.repo_path, patch_relpath, component_path ) try: for file in Path(temp_component_dir).glob("*"): diff --git a/nf_core/components/remove.py b/nf_core/components/remove.py index c2c5843918..37208629c0 100644 --- a/nf_core/components/remove.py +++ b/nf_core/components/remove.py @@ -68,7 +68,7 @@ def remove(self, component, removed_by=None, removed_components=None, force=Fals if not component_dir.exists(): log.error(f"Installation directory '{component_dir}' does not exist.") - if modules_json.module_present(component, self.modules_repo.remote_url, repo_path): + if modules_json.component_present(component, self.modules_repo.remote_url, repo_path, self.component_type): log.error(f"Found entry for '{component}' in 'modules.json'. Removing...") modules_json.remove_entry(self.component_type, component, self.modules_repo.remote_url, repo_path) return False diff --git a/nf_core/components/update.py b/nf_core/components/update.py index bf176fb6d9..7edb0ffd06 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -810,7 +810,9 @@ def try_apply_patch( shutil.copytree(component_install_dir, temp_component_dir) try: - new_files = ModulesDiffer.try_apply_patch(component, repo_path, patch_path, temp_component_dir) + new_files = ModulesDiffer.try_apply_patch( + self.component_type, component, repo_path, patch_path, temp_component_dir + ) except LookupError: # Patch failed. Save the patch file by moving to the install dir shutil.move(patch_path, Path(component_install_dir, patch_path.relative_to(component_dir))) @@ -848,7 +850,12 @@ def try_apply_patch( # Add the patch file to the modules.json file self.modules_json.add_patch_entry( - component, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=write_file + self.component_type, + component, + self.modules_repo.remote_url, + repo_path, + patch_relpath, + write_file=write_file, ) return True diff --git a/nf_core/modules/lint/main_nf.py b/nf_core/modules/lint/main_nf.py index df5a48d5bf..2b7878ca0f 100644 --- a/nf_core/modules/lint/main_nf.py +++ b/nf_core/modules/lint/main_nf.py @@ -51,6 +51,7 @@ def main_nf( lines: List[str] = [] if module.is_patched: lines = ModulesDiffer.try_apply_patch( + module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, module.patch_path, diff --git a/nf_core/modules/lint/meta_yml.py b/nf_core/modules/lint/meta_yml.py index 4ad728d10b..59f0f01252 100644 --- a/nf_core/modules/lint/meta_yml.py +++ b/nf_core/modules/lint/meta_yml.py @@ -47,6 +47,7 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None meta_yaml = read_meta_yml(module_lint_object, module) if module.is_patched and module_lint_object.modules_repo.repo_path is not None: lines = ModulesDiffer.try_apply_patch( + module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, module.patch_path, @@ -208,6 +209,7 @@ def read_meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> # Check if we have a patch file, get original file in that case if module.is_patched: lines = ModulesDiffer.try_apply_patch( + module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, module.patch_path, diff --git a/nf_core/modules/lint/module_changes.py b/nf_core/modules/lint/module_changes.py index eb76f4b88b..708a2bad68 100644 --- a/nf_core/modules/lint/module_changes.py +++ b/nf_core/modules/lint/module_changes.py @@ -31,6 +31,7 @@ def module_changes(module_lint_object, module): shutil.copytree(module.component_dir, tempdir) try: new_lines = ModulesDiffer.try_apply_patch( + module.component_type, module.component_name, module.org, module.patch_path, diff --git a/nf_core/modules/lint/module_patch.py b/nf_core/modules/lint/module_patch.py index 29bf78a66b..19c6e76fec 100644 --- a/nf_core/modules/lint/module_patch.py +++ b/nf_core/modules/lint/module_patch.py @@ -162,6 +162,7 @@ def patch_reversible(module_lint_object, module, patch_path): """ try: ModulesDiffer.try_apply_patch( + module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, patch_path, diff --git a/nf_core/modules/modules_differ.py b/nf_core/modules/modules_differ.py index 6b0781bb89..c151fcce7c 100644 --- a/nf_core/modules/modules_differ.py +++ b/nf_core/modules/modules_differ.py @@ -466,16 +466,22 @@ def try_apply_single_patch(file_lines, patch, reverse=False): @staticmethod def try_apply_patch( - module: str, repo_path: Union[str, Path], patch_path: Union[str, Path], module_dir: Path, reverse: bool = False + component_type: str, + component: str, + repo_path: Union[str, Path], + patch_path: Union[str, Path], + component_dir: Path, + reverse: bool = False, ) -> Dict[str, List[str]]: """ - Try applying a full patch file to a module + Try applying a full patch file to a module or subworkflow Args: - module (str): Name of the module + component_type (str): The type of component (modules or subworkflows) + component (str): Name of the module or subworkflow repo_path (str): Name of the repository where the module resides patch_path (str): The absolute path to the patch file to be applied - module_dir (Path): The directory containing the module + component_dir (Path): The directory containing the component reverse (bool): Apply the patch in reverse Returns: @@ -485,13 +491,13 @@ def try_apply_patch( Raises: LookupError: If the patch application fails in a file """ - module_relpath = Path("modules", repo_path, module) + component_relpath = Path(component_type, repo_path, component) patches = ModulesDiffer.per_file_patch(patch_path) new_files = {} for file, patch in patches.items(): log.debug(f"Applying patch to {file}") - fn = Path(file).relative_to(module_relpath) - file_path = module_dir / fn + fn = Path(file).relative_to(component_relpath) + file_path = component_dir / fn try: with open(file_path) as fh: file_lines = fh.readlines() diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 05c64b6dee..5628c75742 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -308,7 +308,9 @@ def determine_branches_and_shas( # If the module/subworkflow is patched patch_file = component_path / f"{component}.diff" if patch_file.is_file(): - temp_module_dir = self.try_apply_patch_reverse(component, install_dir, patch_file, component_path) + temp_module_dir = self.try_apply_patch_reverse( + component_type, component, install_dir, patch_file, component_path + ) correct_commit_sha = self.find_correct_commit_sha( component_type, component, temp_module_dir, modules_repo ) @@ -805,7 +807,7 @@ def remove_entry(self, component_type, name, repo_url, install_dir, removed_by=N return False - def add_patch_entry(self, module_name, repo_url, install_dir, patch_filename, write_file=True): + def add_patch_entry(self, component_type, component_name, repo_url, install_dir, patch_filename, write_file=True): """ Adds (or replaces) the patch entry for a module """ @@ -815,9 +817,11 @@ def add_patch_entry(self, module_name, repo_url, install_dir, patch_filename, wr if repo_url not in self.modules_json["repos"]: raise LookupError(f"Repo '{repo_url}' not present in 'modules.json'") - if module_name not in self.modules_json["repos"][repo_url]["modules"][install_dir]: - raise LookupError(f"Module '{install_dir}/{module_name}' not present in 'modules.json'") - self.modules_json["repos"][repo_url]["modules"][install_dir][module_name]["patch"] = str(patch_filename) + if component_name not in self.modules_json["repos"][repo_url][component_type][install_dir]: + raise LookupError( + f"{component_type[:-1].title()} '{install_dir}/{component_name}' not present in 'modules.json'" + ) + self.modules_json["repos"][repo_url][component_type][install_dir][component_name]["patch"] = str(patch_filename) if write_file: self.dump() @@ -858,41 +862,46 @@ def get_patch_fn(self, module_name, repo_url, install_dir): ) return Path(path) if path is not None else None - def try_apply_patch_reverse(self, module, repo_name, patch_relpath, module_dir): + def try_apply_patch_reverse(self, component_type, component, repo_name, patch_relpath, component_dir): """ - Try reverse applying a patch file to the modified module files + Try reverse applying a patch file to the modified module or subworkflow files Args: - module (str): The name of the module - repo_name (str): The name of the repository where the module resides + component_type (str): The type of component [modules, subworkflows] + component (str): The name of the module or subworkflow + repo_name (str): The name of the repository where the component resides patch_relpath (Path | str): The path to patch file in the pipeline - module_dir (Path | str): The module directory in the pipeline + component_dir (Path | str): The component directory in the pipeline Returns: - (Path | str): The path of the folder where the module patched files are + (Path | str): The path of the folder where the component patched files are Raises: LookupError: If patch was not applied """ - module_fullname = str(Path(repo_name, module)) + component_fullname = str(Path(repo_name, component)) patch_path = Path(self.directory / patch_relpath) try: - new_files = ModulesDiffer.try_apply_patch(module, repo_name, patch_path, module_dir, reverse=True) + new_files = ModulesDiffer.try_apply_patch( + component_type, component, repo_name, patch_path, component_dir, reverse=True + ) except LookupError as e: - raise LookupError(f"Failed to apply patch in reverse for module '{module_fullname}' due to: {e}") + raise LookupError( + f"Failed to apply patch in reverse for {component_type[:-1]} '{component_fullname}' due to: {e}" + ) # Write the patched files to a temporary directory log.debug("Writing patched files to tmpdir") temp_dir = Path(tempfile.mkdtemp()) - temp_module_dir = temp_dir / module - temp_module_dir.mkdir(parents=True, exist_ok=True) + temp_component_dir = temp_dir / component + temp_component_dir.mkdir(parents=True, exist_ok=True) for file, new_content in new_files.items(): - fn = temp_module_dir / file + fn = temp_component_dir / file with open(fn, "w") as fh: fh.writelines(new_content) - return temp_module_dir + return temp_component_dir def repo_present(self, repo_name): """ @@ -908,20 +917,21 @@ def repo_present(self, repo_name): return repo_name in self.modules_json.get("repos", {}) - def module_present(self, module_name, repo_url, install_dir): + def component_present(self, module_name, repo_url, install_dir, component_type): """ Checks if a module is present in the modules.json file Args: module_name (str): Name of the module repo_url (str): URL of the repository install_dir (str): Name of the directory where modules are installed + component_type (str): Type of component [modules, subworkflows] Returns: (bool): Whether the module is present in the 'modules.json' file """ if self.modules_json is None: self.load() assert self.modules_json is not None # mypy - return module_name in self.modules_json.get("repos", {}).get(repo_url, {}).get("modules", {}).get( + return module_name in self.modules_json.get("repos", {}).get(repo_url, {}).get(component_type, {}).get( install_dir, {} ) diff --git a/tests/modules/test_modules_json.py b/tests/modules/test_modules_json.py index 0368c146c4..325a8073b7 100644 --- a/tests/modules/test_modules_json.py +++ b/tests/modules/test_modules_json.py @@ -175,14 +175,17 @@ def test_mod_json_repo_present(self): assert mod_json_obj.repo_present(NF_CORE_MODULES_REMOTE) is True assert mod_json_obj.repo_present("INVALID_REPO") is False - def test_mod_json_module_present(self): - """Tests the module_present function""" + def test_mod_json_component_present(self): + """Tests the component_present function""" mod_json_obj = ModulesJson(self.pipeline_dir) - assert mod_json_obj.module_present("fastqc", NF_CORE_MODULES_REMOTE, NF_CORE_MODULES_NAME) is True - assert mod_json_obj.module_present("INVALID_MODULE", NF_CORE_MODULES_REMOTE, NF_CORE_MODULES_NAME) is False - assert mod_json_obj.module_present("fastqc", "INVALID_REPO", "INVALID_DIR") is False - assert mod_json_obj.module_present("INVALID_MODULE", "INVALID_REPO", "INVALID_DIR") is False + assert mod_json_obj.component_present("fastqc", NF_CORE_MODULES_REMOTE, NF_CORE_MODULES_NAME, "modules") is True + assert ( + mod_json_obj.component_present("INVALID_MODULE", NF_CORE_MODULES_REMOTE, NF_CORE_MODULES_NAME, "modules") + is False + ) + assert mod_json_obj.component_present("fastqc", "INVALID_REPO", "INVALID_DIR", "modules") is False + assert mod_json_obj.component_present("INVALID_MODULE", "INVALID_REPO", "INVALID_DIR", "modules") is False def test_mod_json_get_module_version(self): """Test the get_module_version function""" diff --git a/tests/subworkflows/patch.py b/tests/subworkflows/patch.py deleted file mode 100644 index 66065784cf..0000000000 --- a/tests/subworkflows/patch.py +++ /dev/null @@ -1,212 +0,0 @@ -import os -import tempfile -from pathlib import Path -from unittest import mock - -import pytest - -import nf_core.components.components_command -import nf_core.subworkflows - -from ..utils import ( - GITLAB_SUBWORKFLOWS_BRANCH, - GITLAB_URL, - GITLAB_REPO -) - -# TODO: #Change this for the correct SUCCEED_SHA -SUCCEED_SHA = "????" -ORG_SHA = "002623ccc88a3b0cb302c7d8f13792a95354d9f2" - - -""" -Test the 'nf-core subworkflows patch' command -""" - - -def setup_patch(pipeline_dir, modify_subworkflow): - # Install the subworkflow bam_sort_stats_samtools - install_obj = nf_core.subworkflows.SubworkflowInstall( - pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH, sha=ORG_SHA - ) - - # Install the module - install_obj.install("bam_sort_stats_samtools") - - if modify_subworkflow: - # Modify the subworkflow - subworkflow_path = Path(pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - modify_main_nf(subworkflow_path / "main.nf") - - -def modify_main_nf(path): - """Modify a file to test patch creation""" - with open(path) as fh: - lines = fh.readlines() - # We want a patch file that looks something like: - # - ch_fasta // channel: [ val(meta), path(fasta) ] - for line_index in range(len(lines)): - if lines[line_index] == " ch_fasta // channel: [ val(meta), path(fasta) ]\n": - to_pop = line_index - lines.pop(to_pop) - with open(path, "w") as fh: - fh.writelines(lines) - - -def test_create_patch_no_change(self): - """Test creating a patch when there is a change to the module""" - setup_patch(self.pipeline_dir, False) - - # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) - with pytest.raises(UserWarning): - patch_obj.patch("bam_sort_stats_samtools") - - subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - - # Check that no patch file has been added to the directory - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} - - -def test_create_patch_change(self): - """Test creating a patch when there is no change to the subworkflow""" - setup_patch(self.pipeline_dir, True) - - # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) - patch_obj.patch("bam_sort_stats_samtools") - - subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - - patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" - # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} - - # Check that the correct lines are in the patch file - with open(subworkflow_path / patch_fn) as fh: - patch_lines = fh.readlines() - subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) - assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" - assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines - assert "- ch_fasta // channel: [ val(meta), path(fasta) ]" in patch_lines - - -def test_create_patch_try_apply_successful(self): - """Test creating a patch file and applying it to a new version of the the files""" - setup_patch(self.pipeline_dir, True) - subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) - - # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) - patch_obj.patch("bam_sort_stats_samtools") - - patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" - # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} - - update_obj = nf_core.subworkflows.SubworkflowUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH - ) - - # Install the new files - install_dir = Path(tempfile.mkdtemp()) - update_obj.install_component_files("bam_sort_stats_samtools", SUCCEED_SHA, update_obj.modules_repo, install_dir) - - # Try applying the patch - subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" - patch_relpath = subworkflow_relpath / patch_fn - assert ( - update_obj.try_apply_patch( - "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir - ) - is True - ) - - # Move the files from the temporary directory - update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, GITLAB_REPO, SUCCEED_SHA) - - # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} - - # Check that the correct lines are in the patch file - with open(subworkflow_path / patch_fn) as fh: - patch_lines = fh.readlines() - subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) - assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" - assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines - assert "- ch_fasta // channel: [ val(meta), path(fasta) ]" in patch_lines - - # Check that 'main.nf' is updated correctly - with open(subworkflow_path / "main.nf") as fh: - main_nf_lines = fh.readlines() - # These lines should have been removed by the patch - assert " ch_fasta // channel: [ val(meta), path(fasta) ]\n" not in main_nf_lines - - -def test_create_patch_try_apply_failed(self): - """Test creating a patch file and applying it to a new version of the the files""" - setup_patch(self.pipeline_dir, True) - subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) - - # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) - patch_obj.patch("bam_sort_stats_samtools") - - patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" - # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} - - update_obj = nf_core.subworkflows.SubworkflowUpdate( - self.pipeline_dir, sha=SUCCEED_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH - ) - - # Install the new files - install_dir = Path(tempfile.mkdtemp()) - update_obj.install_component_files("bam_sort_stats_samtools", SUCCEED_SHA, update_obj.modules_repo, install_dir) - - # Try applying the patch - subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" - patch_relpath = subworkflow_relpath / patch_fn - assert ( - update_obj.try_apply_patch( - "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir - ) - is False - ) - - -# TODO: create those two missing tests -def test_create_patch_update_success(self): - """Test creating a patch file and updating a subworkflow when there is a diff conflict""" - - -def test_create_patch_update_fail(self): - """ - Test creating a patch file and the updating the subworkflow - - Should have the same effect as 'test_create_patch_try_apply_successful' - but uses higher level api - """ - - -def test_remove_patch(self): - """Test creating a patch when there is no change to the subworkflow""" - setup_patch(self.pipeline_dir, True) - - # Try creating a patch file - patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) - patch_obj.patch("bam_sort_stats_samtools") - - subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") - - patch_fn = f"{'-'.join('bam_sort_stats_samtools')}.diff" - # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", patch_fn} - - with mock.patch.object(nf_core.create.questionary, "confirm") as mock_questionary: - mock_questionary.unsafe_ask.return_value = True - patch_obj.remove("bam_sort_stats_samtools") - # Check that the diff file has been removed - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} diff --git a/tests/subworkflows/test_patch.py b/tests/subworkflows/test_patch.py new file mode 100644 index 0000000000..ba452083fa --- /dev/null +++ b/tests/subworkflows/test_patch.py @@ -0,0 +1,204 @@ +import os +import tempfile +from pathlib import Path +from unittest import mock + +import pytest + +import nf_core.components.components_command +import nf_core.components.patch +import nf_core.subworkflows + +from ..test_subworkflows import TestSubworkflows +from ..utils import GITLAB_REPO, GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL + +OLD_SHA = "dbb12457e32d3da8eea7dc4ae096201fff4747c5" +SUCCEED_SHA = "0a33e6a0d730ad22a0ec9f7f9a7540af6e943221" +FAIL_SHA = "b6e5e8739de9a1a0c4f85267144e43dbaf8f1461" + + +class TestSubworkflowsPatch(TestSubworkflows): + """ + Test the 'nf-core subworkflows patch' command + """ + + def modify_main_nf(self, path): + """Modify a file to test patch creation""" + with open(path) as fh: + lines = fh.readlines() + # We want a patch file that looks something like: + # - ch_fasta // channel: [ fasta ] + for line_index in range(len(lines)): + if lines[line_index] == " ch_fasta // channel: [ fasta ]\n": + to_pop = line_index + lines.pop(to_pop) + with open(path, "w") as fh: + fh.writelines(lines) + + def setup_patch(self, pipeline_dir, modify_subworkflow): + # Install the subworkflow bam_sort_stats_samtools + install_obj = nf_core.subworkflows.SubworkflowInstall( + pipeline_dir, + prompt=False, + force=False, + remote_url=GITLAB_URL, + branch=GITLAB_SUBWORKFLOWS_BRANCH, + sha=OLD_SHA, + ) + + # Install the module + install_obj.install("bam_sort_stats_samtools") + + if modify_subworkflow: + # Modify the subworkflow + subworkflow_path = Path(pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + self.modify_main_nf(subworkflow_path / "main.nf") + + def test_create_patch_no_change(self): + """Test creating a patch when there is a change to the module""" + self.setup_patch(self.pipeline_dir, False) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + with pytest.raises(UserWarning): + patch_obj.patch("bam_sort_stats_samtools") + + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + + # Check that no patch file has been added to the directory + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + + def test_create_patch_change(self): + """Test creating a patch when there is no change to the subworkflow""" + self.setup_patch(self.pipeline_dir, True) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + + # Check that the correct lines are in the patch file + with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: + patch_lines = fh.readlines() + print(patch_lines) + subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) + assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" + assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines + assert "- ch_fasta // channel: [ fasta ]\n" in patch_lines + + def test_create_patch_try_apply_successful(self): + """Test creating a patch file and applying it to a new version of the the files""" + self.setup_patch(self.pipeline_dir, True) + subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + + update_obj = nf_core.subworkflows.SubworkflowUpdate( + self.pipeline_dir, sha=OLD_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH + ) + + # Install the new files + install_dir = Path(tempfile.mkdtemp()) + update_obj.install_component_files("bam_sort_stats_samtools", OLD_SHA, update_obj.modules_repo, install_dir) + + # Try applying the patch + subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" + patch_relpath = subworkflow_relpath / "bam_sort_stats_samtools.diff" + assert ( + update_obj.try_apply_patch( + "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir + ) + is True + ) + + # Move the files from the temporary directory + update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, GITLAB_REPO, OLD_SHA) + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + + # Check that the correct lines are in the patch file + with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: + patch_lines = fh.readlines() + subworkflow_relpath = subworkflow_path.relative_to(self.pipeline_dir) + assert f"--- {subworkflow_relpath / 'main.nf'}\n" in patch_lines, subworkflow_relpath / "main.nf" + assert f"+++ {subworkflow_relpath / 'main.nf'}\n" in patch_lines + assert "- ch_fasta // channel: [ fasta ]\n" in patch_lines + + # Check that 'main.nf' is updated correctly + with open(subworkflow_path / "main.nf") as fh: + main_nf_lines = fh.readlines() + # These lines should have been removed by the patch + assert "- ch_fasta // channel: [ fasta ]\n" not in main_nf_lines + + def test_create_patch_try_apply_failed(self): + """Test creating a patch file and applying it to a new version of the the files""" + self.setup_patch(self.pipeline_dir, True) + subworkflow_relpath = Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + subworkflow_path = Path(self.pipeline_dir, subworkflow_relpath) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + + update_obj = nf_core.subworkflows.SubworkflowUpdate( + self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH + ) + + # Install the new files + install_dir = Path(tempfile.mkdtemp()) + update_obj.install_component_files("bam_sort_stats_samtools", FAIL_SHA, update_obj.modules_repo, install_dir) + + # Try applying the patch + subworkflow_install_dir = install_dir / "bam_sort_stats_samtools" + patch_relpath = subworkflow_relpath / "bam_sort_stats_samtools.diff" + assert ( + update_obj.try_apply_patch( + "bam_sort_stats_samtools", GITLAB_REPO, patch_relpath, subworkflow_path, subworkflow_install_dir + ) + is False + ) + + # TODO: create those two missing tests + def test_create_patch_update_success(self): + """Test creating a patch file and updating a subworkflow when there is a diff conflict""" + + def test_create_patch_update_fail(self): + """ + Test creating a patch file and the updating the subworkflow + + Should have the same effect as 'test_create_patch_try_apply_successful' + but uses higher level api + """ + + def test_remove_patch(self): + """Test creating a patch when there is no change to the subworkflow""" + self.setup_patch(self.pipeline_dir, True) + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + + # Check that a patch file with the correct name has been created + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + + with mock.patch.object(nf_core.components.patch.questionary, "confirm") as mock_questionary: + mock_questionary.unsafe_ask.return_value = True + patch_obj.remove("bam_sort_stats_samtools") + # Check that the diff file has been removed + assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} From 25940f7e90fd7f94fa9eae596e7b7d0a9dc54a3c Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 22 Nov 2024 16:05:43 +0100 Subject: [PATCH 16/25] apply patch reverse when linting a patched subworkflow --- .../subworkflows/lint/subworkflow_changes.py | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/nf_core/subworkflows/lint/subworkflow_changes.py b/nf_core/subworkflows/lint/subworkflow_changes.py index a9c9616a21..e6af892125 100644 --- a/nf_core/subworkflows/lint/subworkflow_changes.py +++ b/nf_core/subworkflows/lint/subworkflow_changes.py @@ -2,9 +2,12 @@ Check whether the content of a subworkflow has changed compared to the original repository """ +import shutil +import tempfile from pathlib import Path import nf_core.modules.modules_repo +from nf_core.modules.modules_differ import ModulesDiffer def subworkflow_changes(subworkflow_lint_object, subworkflow): @@ -20,7 +23,29 @@ def subworkflow_changes(subworkflow_lint_object, subworkflow): Only runs when linting a pipeline, not the modules repository """ - tempdir = subworkflow.component_dir + if subworkflow.is_patched: + # If the subworkflow is patched, we need to apply + # the patch in reverse before comparing with the remote + tempdir_parent = Path(tempfile.mkdtemp()) + tempdir = tempdir_parent / "tmp_subworkflow_dir" + shutil.copytree(subworkflow.component_dir, tempdir) + try: + new_lines = ModulesDiffer.try_apply_patch( + subworkflow.component_type, + subworkflow.component_name, + subworkflow.org, + subworkflow.patch_path, + tempdir, + reverse=True, + ) + for file, lines in new_lines.items(): + with open(tempdir / file, "w") as fh: + fh.writelines(lines) + except LookupError: + # This error is already reported by subworkflow_patch, so just return + return + else: + tempdir = subworkflow.component_dir subworkflow.branch = subworkflow_lint_object.modules_json.get_component_branch( "subworkflows", subworkflow.component_name, subworkflow.repo_url, subworkflow.org ) From b2cfd0125ba3e0658a17d301291fc952ca1f8afc Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 22 Nov 2024 16:31:47 +0100 Subject: [PATCH 17/25] update get_patch_fn to work with subworkflows --- nf_core/components/update.py | 11 +++++++++-- nf_core/modules/modules_json.py | 16 ++++++++-------- tests/modules/test_patch.py | 22 +++++++++++----------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 7edb0ffd06..76c6b2b075 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -449,7 +449,9 @@ def get_single_component_info(self, component): self.modules_repo.setup_branch(current_branch) # If there is a patch file, get its filename - patch_fn = self.modules_json.get_patch_fn(component, self.modules_repo.remote_url, install_dir) + patch_fn = self.modules_json.get_patch_fn( + self.component_type, component, self.modules_repo.remote_url, install_dir + ) return (self.modules_repo, component, sha, patch_fn) @@ -695,7 +697,12 @@ def get_all_components_info(self, branch=None): # Add patch filenames to the components that have them components_info = [ - (repo, comp, sha, self.modules_json.get_patch_fn(comp, repo.remote_url, repo.repo_path)) + ( + repo, + comp, + sha, + self.modules_json.get_patch_fn(self.component_type, comp, repo.remote_url, repo.repo_path), + ) for repo, comp, sha in components_info ] diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 5628c75742..64aab54bff 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -837,17 +837,17 @@ def remove_patch_entry(self, module_name, repo_url, install_dir, write_file=True if write_file: self.dump() - def get_patch_fn(self, module_name, repo_url, install_dir): + def get_patch_fn(self, component_type, component_name, repo_url, install_dir): """ - Get the patch filename of a module + Get the patch filename of a component Args: - module_name (str): The name of the module - repo_url (str): The URL of the repository containing the module - install_dir (str): The name of the directory where modules are installed + component_name (str): The name of the component + repo_url (str): The URL of the repository containing the component + install_dir (str): The name of the directory where components are installed Returns: - (str): The patch filename for the module, None if not present + (str): The patch filename for the component, None if not present """ if self.modules_json is None: self.load() @@ -855,9 +855,9 @@ def get_patch_fn(self, module_name, repo_url, install_dir): path = ( self.modules_json["repos"] .get(repo_url, {}) - .get("modules") + .get(component_type) .get(install_dir) - .get(module_name, {}) + .get(component_name, {}) .get("patch") ) return Path(path) if path is not None else None diff --git a/tests/modules/test_patch.py b/tests/modules/test_patch.py index 2f60cd4a20..27c1e342e4 100644 --- a/tests/modules/test_patch.py +++ b/tests/modules/test_patch.py @@ -80,7 +80,7 @@ def test_create_patch_no_change(self): # Check the 'modules.json' contains no patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) is None + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) is None def test_create_patch_change(self): """Test creating a patch when there is a change to the module""" @@ -98,7 +98,7 @@ def test_create_patch_change(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -131,7 +131,7 @@ def test_create_patch_try_apply_successful(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -157,7 +157,7 @@ def test_create_patch_try_apply_successful(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -199,7 +199,7 @@ def test_create_patch_try_apply_failed(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -238,7 +238,7 @@ def test_create_patch_update_success(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, GITLAB_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, GITLAB_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -258,9 +258,9 @@ def test_create_patch_update_success(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, GITLAB_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, GITLAB_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn - ), modules_json_obj.get_patch_fn(BISMARK_ALIGN, GITLAB_URL, REPO_NAME) + ), modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, GITLAB_URL, REPO_NAME) # Check that the correct lines are in the patch file with open(module_path / patch_fn) as fh: @@ -299,7 +299,7 @@ def test_create_patch_update_fail(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -353,7 +353,7 @@ def test_remove_patch(self): # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) == Path( "modules", REPO_NAME, BISMARK_ALIGN, patch_fn ) @@ -365,4 +365,4 @@ def test_remove_patch(self): # Check that the 'modules.json' entry has been removed modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) - assert modules_json_obj.get_patch_fn(BISMARK_ALIGN, REPO_URL, REPO_NAME) is None + assert modules_json_obj.get_patch_fn("modules", BISMARK_ALIGN, REPO_URL, REPO_NAME) is None From 6ec2dcd763865dcfd3c3672e244b7e3a401ac037 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 22 Nov 2024 16:49:39 +0100 Subject: [PATCH 18/25] move modules_differ.py to components_differ.py --- .../components_differ.py} | 112 +++++++++--------- nf_core/components/patch.py | 6 +- nf_core/components/update.py | 12 +- nf_core/modules/lint/main_nf.py | 4 +- nf_core/modules/lint/meta_yml.py | 6 +- nf_core/modules/lint/module_changes.py | 4 +- nf_core/modules/lint/module_patch.py | 16 +-- nf_core/modules/modules_json.py | 4 +- .../subworkflows/lint/subworkflow_changes.py | 4 +- 9 files changed, 84 insertions(+), 84 deletions(-) rename nf_core/{modules/modules_differ.py => components/components_differ.py} (83%) diff --git a/nf_core/modules/modules_differ.py b/nf_core/components/components_differ.py similarity index 83% rename from nf_core/modules/modules_differ.py rename to nf_core/components/components_differ.py index c151fcce7c..db51c1910d 100644 --- a/nf_core/modules/modules_differ.py +++ b/nf_core/components/components_differ.py @@ -16,10 +16,10 @@ log = logging.getLogger(__name__) -class ModulesDiffer: +class ComponentsDiffer: """ Static class that provides functionality for computing diffs between - different instances of a module + different instances of a module or subworkflow """ class DiffEnum(enum.Enum): @@ -34,15 +34,15 @@ class DiffEnum(enum.Enum): REMOVED = enum.auto() @staticmethod - def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_dir=None): + def get_component_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_dir=None): """ - Compute the diff between the current module version + Compute the diff between the current component version and the new version. Args: - from_dir (strOrPath): The folder containing the old module files - to_dir (strOrPath): The folder containing the new module files - path_in_diff (strOrPath): The directory displayed containing the module + from_dir (strOrPath): The folder containing the old component files + to_dir (strOrPath): The folder containing the new component files + path_in_diff (strOrPath): The directory displayed containing the component file in the diff. Added so that temporary dirs are not shown for_git (bool): indicates whether the diff file is to be @@ -52,7 +52,7 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d dsp_to_dir (str | Path): The to directory to display in the diff Returns: - dict[str, (ModulesDiffer.DiffEnum, str)]: A dictionary containing + dict[str, (ComponentsDiffer.DiffEnum, str)]: A dictionary containing the diff type and the diff string (empty if no diff) """ if for_git: @@ -72,7 +72,7 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d ) files = list(files) - # Loop through all the module files and compute their diffs if needed + # Loop through all the component files and compute their diffs if needed for file in files: temp_path = Path(to_dir, file) curr_path = Path(from_dir, file) @@ -84,7 +84,7 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d if new_lines == old_lines: # The files are identical - diffs[file] = (ModulesDiffer.DiffEnum.UNCHANGED, ()) + diffs[file] = (ComponentsDiffer.DiffEnum.UNCHANGED, ()) else: # Compute the diff diff = difflib.unified_diff( @@ -93,7 +93,7 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d fromfile=str(Path(dsp_from_dir, file)), tofile=str(Path(dsp_to_dir, file)), ) - diffs[file] = (ModulesDiffer.DiffEnum.CHANGED, diff) + diffs[file] = (ComponentsDiffer.DiffEnum.CHANGED, diff) elif temp_path.exists(): with open(temp_path) as fh: @@ -106,7 +106,7 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d fromfile=str(Path("/dev", "null")), tofile=str(Path(dsp_to_dir, file)), ) - diffs[file] = (ModulesDiffer.DiffEnum.CREATED, diff) + diffs[file] = (ComponentsDiffer.DiffEnum.CREATED, diff) elif curr_path.exists(): # The file was removed @@ -119,14 +119,14 @@ def get_module_diffs(from_dir, to_dir, for_git=True, dsp_from_dir=None, dsp_to_d fromfile=str(Path(dsp_from_dir, file)), tofile=str(Path("/dev", "null")), ) - diffs[file] = (ModulesDiffer.DiffEnum.REMOVED, diff) + diffs[file] = (ComponentsDiffer.DiffEnum.REMOVED, diff) return diffs @staticmethod def write_diff_file( diff_path, - module, + component, repo_path, from_dir, to_dir, @@ -139,20 +139,19 @@ def write_diff_file( limit_output=False, ): """ - Writes the diffs of a module to the diff file. + Writes the diffs of a component to the diff file. Args: diff_path (str | Path): The path to the file that should be appended - module (str): The module name - repo_path (str): The name of the repo where the module resides - from_dir (str | Path): The directory containing the old module files - to_dir (str | Path): The directory containing the new module files - diffs (dict[str, (ModulesDiffer.DiffEnum, str)]): A dictionary containing + component (str): The component name + repo_path (str): The name of the repo where the component resides + from_dir (str | Path): The directory containing the old component files + to_dir (str | Path): The directory containing the new component files + diffs (dict[str, (ComponentsDiffer.DiffEnum, str)]): A dictionary containing the type of change and the diff (if any) - module_dir (str | Path): The path to the current installation of the module - current_version (str): The installed version of the module - new_version (str): The version of the module the diff is computed against + current_version (str): The installed version of the component + new_version (str): The version of the component the diff is computed against for_git (bool): indicates whether the diff file is to be compatible with `git apply`. If true it adds a/ and b/ prefixes to the file paths @@ -165,36 +164,36 @@ def write_diff_file( if dsp_to_dir is None: dsp_to_dir = to_dir - diffs = ModulesDiffer.get_module_diffs(from_dir, to_dir, for_git, dsp_from_dir, dsp_to_dir) - if all(diff_status == ModulesDiffer.DiffEnum.UNCHANGED for _, (diff_status, _) in diffs.items()): - raise UserWarning("Module is unchanged") - log.debug(f"Writing diff of '{module}' to '{diff_path}'") + diffs = ComponentsDiffer.get_component_diffs(from_dir, to_dir, for_git, dsp_from_dir, dsp_to_dir) + if all(diff_status == ComponentsDiffer.DiffEnum.UNCHANGED for _, (diff_status, _) in diffs.items()): + raise UserWarning("Component is unchanged") + log.debug(f"Writing diff of '{component}' to '{diff_path}'") with open(diff_path, file_action) as fh: if current_version is not None and new_version is not None: fh.write( - f"Changes in module '{Path(repo_path, module)}' between" + f"Changes in component '{Path(repo_path, component)}' between" f" ({current_version}) and" f" ({new_version})\n" ) else: - fh.write(f"Changes in module '{Path(repo_path, module)}'\n") + fh.write(f"Changes in component '{Path(repo_path, component)}'\n") for file, (diff_status, diff) in diffs.items(): - if diff_status == ModulesDiffer.DiffEnum.UNCHANGED: + if diff_status == ComponentsDiffer.DiffEnum.UNCHANGED: # The files are identical fh.write(f"'{Path(dsp_from_dir, file)}' is unchanged\n") - elif diff_status == ModulesDiffer.DiffEnum.CREATED: + elif diff_status == ComponentsDiffer.DiffEnum.CREATED: # The file was created between the commits fh.write(f"'{Path(dsp_from_dir, file)}' was created\n") - elif diff_status == ModulesDiffer.DiffEnum.REMOVED: + elif diff_status == ComponentsDiffer.DiffEnum.REMOVED: # The file was removed between the commits fh.write(f"'{Path(dsp_from_dir, file)}' was removed\n") elif limit_output and not file.suffix == ".nf": # Skip printing the diff for files other than main.nf - fh.write(f"Changes in '{Path(module, file)}' but not shown\n") + fh.write(f"Changes in '{Path(component, file)}' but not shown\n") else: # The file has changed write the diff lines to the file - fh.write(f"Changes in '{Path(module, file)}':\n") + fh.write(f"Changes in '{Path(component, file)}':\n") for line in diff: fh.write(line) fh.write("\n") @@ -237,7 +236,7 @@ def append_modules_json_diff(diff_path, old_modules_json, new_modules_json, modu @staticmethod def print_diff( - module, + component, repo_path, from_dir, to_dir, @@ -248,16 +247,15 @@ def print_diff( limit_output=False, ): """ - Prints the diffs between two module versions to the terminal + Prints the diffs between two component versions to the terminal Args: - module (str): The module name - repo_path (str): The name of the repo where the module resides - from_dir (str | Path): The directory containing the old module files - to_dir (str | Path): The directory containing the new module files - module_dir (str): The path to the current installation of the module - current_version (str): The installed version of the module - new_version (str): The version of the module the diff is computed against + component (str): The component name + repo_path (str): The name of the repo where the component resides + from_dir (str | Path): The directory containing the old component files + to_dir (str | Path): The directory containing the new component files + current_version (str): The installed version of the component + new_version (str): The version of the component the diff is computed against dsp_from_dir (str | Path): The 'from' directory displayed in the diff dsp_to_dir (str | Path): The 'to' directory displayed in the diff limit_output (bool): If true, don't print the diff for files other than main.nf @@ -267,41 +265,43 @@ def print_diff( if dsp_to_dir is None: dsp_to_dir = to_dir - diffs = ModulesDiffer.get_module_diffs( + diffs = ComponentsDiffer.get_component_diffs( from_dir, to_dir, for_git=False, dsp_from_dir=dsp_from_dir, dsp_to_dir=dsp_to_dir ) console = Console(force_terminal=nf_core.utils.rich_force_colors()) if current_version is not None and new_version is not None: log.info( - f"Changes in module '{Path(repo_path, module)}' between" f" ({current_version}) and" f" ({new_version})" + f"Changes in component '{Path(repo_path, component)}' between" + f" ({current_version}) and" + f" ({new_version})" ) else: - log.info(f"Changes in module '{Path(repo_path, module)}'") + log.info(f"Changes in component '{Path(repo_path, component)}'") panel_group: list[RenderableType] = [] for file, (diff_status, diff) in diffs.items(): - if diff_status == ModulesDiffer.DiffEnum.UNCHANGED: + if diff_status == ComponentsDiffer.DiffEnum.UNCHANGED: # The files are identical log.info(f"'{Path(dsp_from_dir, file)}' is unchanged") - elif diff_status == ModulesDiffer.DiffEnum.CREATED: + elif diff_status == ComponentsDiffer.DiffEnum.CREATED: # The file was created between the commits log.info(f"'{Path(dsp_from_dir, file)}' was created") - elif diff_status == ModulesDiffer.DiffEnum.REMOVED: + elif diff_status == ComponentsDiffer.DiffEnum.REMOVED: # The file was removed between the commits log.info(f"'{Path(dsp_from_dir, file)}' was removed") elif limit_output and not file.suffix == ".nf": # Skip printing the diff for files other than main.nf - log.info(f"Changes in '{Path(module, file)}' but not shown") + log.info(f"Changes in '{Path(component, file)}' but not shown") else: # The file has changed - log.info(f"Changes in '{Path(module, file)}':") + log.info(f"Changes in '{Path(component, file)}':") # Pretty print the diff using the pygments diff lexer syntax = Syntax("".join(diff), "diff", theme="ansi_dark", line_numbers=True) panel_group.append(Panel(syntax, title=str(file), title_align="left", padding=0)) console.print( Panel( Group(*panel_group), - title=f"[white]{str(module)}[/white]", + title=f"[white]{str(component)}[/white]", title_align="left", padding=0, border_style="blue", @@ -422,7 +422,7 @@ def try_apply_single_patch(file_lines, patch, reverse=False): LookupError: If it fails to find the old lines from the patch in the file. """ - org_lines, patch_lines = ModulesDiffer.get_new_and_old_lines(patch) + org_lines, patch_lines = ComponentsDiffer.get_new_and_old_lines(patch) if reverse: patch_lines, org_lines = org_lines, patch_lines @@ -479,7 +479,7 @@ def try_apply_patch( Args: component_type (str): The type of component (modules or subworkflows) component (str): Name of the module or subworkflow - repo_path (str): Name of the repository where the module resides + repo_path (str): Name of the repository where the component resides patch_path (str): The absolute path to the patch file to be applied component_dir (Path): The directory containing the component reverse (bool): Apply the patch in reverse @@ -492,7 +492,7 @@ def try_apply_patch( LookupError: If the patch application fails in a file """ component_relpath = Path(component_type, repo_path, component) - patches = ModulesDiffer.per_file_patch(patch_path) + patches = ComponentsDiffer.per_file_patch(patch_path) new_files = {} for file, patch in patches.items(): log.debug(f"Applying patch to {file}") @@ -504,6 +504,6 @@ def try_apply_patch( except FileNotFoundError: # The file was added with the patch file_lines = [""] - patched_new_lines = ModulesDiffer.try_apply_single_patch(file_lines, patch, reverse=reverse) + patched_new_lines = ComponentsDiffer.try_apply_single_patch(file_lines, patch, reverse=reverse) new_files[str(fn)] = patched_new_lines return new_files diff --git a/nf_core/components/patch.py b/nf_core/components/patch.py index 77717877fc..59ec7a381b 100644 --- a/nf_core/components/patch.py +++ b/nf_core/components/patch.py @@ -8,7 +8,7 @@ import nf_core.utils from nf_core.components.components_command import ComponentCommand -from nf_core.modules.modules_differ import ModulesDiffer +from nf_core.components.components_differ import ComponentsDiffer from nf_core.modules.modules_json import ModulesJson log = logging.getLogger(__name__) @@ -114,7 +114,7 @@ def patch(self, component=None): # Write the patch to a temporary location (otherwise it is printed to the screen later) patch_temp_path = tempfile.mktemp() try: - ModulesDiffer.write_diff_file( + ComponentsDiffer.write_diff_file( patch_temp_path, component, self.modules_repo.repo_path, @@ -135,7 +135,7 @@ def patch(self, component=None): log.debug(f"Wrote patch path for {self.component_type[:-1]} {component} to modules.json") # Show the changes made to the module - ModulesDiffer.print_diff( + ComponentsDiffer.print_diff( component, self.modules_repo.repo_path, component_install_dir, diff --git a/nf_core/components/update.py b/nf_core/components/update.py index 76c6b2b075..901a7f02fe 100644 --- a/nf_core/components/update.py +++ b/nf_core/components/update.py @@ -9,13 +9,13 @@ import nf_core.modules.modules_utils import nf_core.utils from nf_core.components.components_command import ComponentCommand +from nf_core.components.components_differ import ComponentsDiffer from nf_core.components.components_utils import ( get_components_to_install, prompt_component_version_sha, ) from nf_core.components.install import ComponentInstall from nf_core.components.remove import ComponentRemove -from nf_core.modules.modules_differ import ModulesDiffer from nf_core.modules.modules_json import ModulesJson from nf_core.modules.modules_repo import ModulesRepo from nf_core.utils import plural_es, plural_s, plural_y @@ -223,7 +223,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr f"Writing diff file for {self.component_type[:-1]} '{component_fullname}' to '{self.save_diff_fn}'" ) try: - ModulesDiffer.write_diff_file( + ComponentsDiffer.write_diff_file( self.save_diff_fn, component, modules_repo.repo_path, @@ -265,7 +265,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr self.manage_changes_in_linked_components(component, modules_to_update, subworkflows_to_update) elif self.show_diff: - ModulesDiffer.print_diff( + ComponentsDiffer.print_diff( component, modules_repo.repo_path, component_dir, @@ -313,7 +313,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr if self.save_diff_fn: # Write the modules.json diff to the file - ModulesDiffer.append_modules_json_diff( + ComponentsDiffer.append_modules_json_diff( self.save_diff_fn, old_modules_json, self.modules_json.get_modules_json(), @@ -817,7 +817,7 @@ def try_apply_patch( shutil.copytree(component_install_dir, temp_component_dir) try: - new_files = ModulesDiffer.try_apply_patch( + new_files = ComponentsDiffer.try_apply_patch( self.component_type, component, repo_path, patch_path, temp_component_dir ) except LookupError: @@ -837,7 +837,7 @@ def try_apply_patch( # Create the new patch file log.debug("Regenerating patch file") - ModulesDiffer.write_diff_file( + ComponentsDiffer.write_diff_file( Path(temp_component_dir, patch_path.relative_to(component_dir)), component, repo_path, diff --git a/nf_core/modules/lint/main_nf.py b/nf_core/modules/lint/main_nf.py index 2b7878ca0f..848e17130e 100644 --- a/nf_core/modules/lint/main_nf.py +++ b/nf_core/modules/lint/main_nf.py @@ -15,8 +15,8 @@ import nf_core import nf_core.modules.modules_utils +from nf_core.components.components_differ import ComponentsDiffer from nf_core.components.nfcore_component import NFCoreComponent -from nf_core.modules.modules_differ import ModulesDiffer log = logging.getLogger(__name__) @@ -50,7 +50,7 @@ def main_nf( # otherwise read the lines directly from the module lines: List[str] = [] if module.is_patched: - lines = ModulesDiffer.try_apply_patch( + lines = ComponentsDiffer.try_apply_patch( module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, diff --git a/nf_core/modules/lint/meta_yml.py b/nf_core/modules/lint/meta_yml.py index 59f0f01252..d0268a40cc 100644 --- a/nf_core/modules/lint/meta_yml.py +++ b/nf_core/modules/lint/meta_yml.py @@ -6,9 +6,9 @@ import ruamel.yaml from jsonschema import exceptions, validators +from nf_core.components.components_differ import ComponentsDiffer from nf_core.components.lint import ComponentLint, LintExceptionError from nf_core.components.nfcore_component import NFCoreComponent -from nf_core.modules.modules_differ import ModulesDiffer log = logging.getLogger(__name__) @@ -46,7 +46,7 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None # Check if we have a patch file, get original file in that case meta_yaml = read_meta_yml(module_lint_object, module) if module.is_patched and module_lint_object.modules_repo.repo_path is not None: - lines = ModulesDiffer.try_apply_patch( + lines = ComponentsDiffer.try_apply_patch( module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, @@ -208,7 +208,7 @@ def read_meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> yaml.preserve_quotes = True # Check if we have a patch file, get original file in that case if module.is_patched: - lines = ModulesDiffer.try_apply_patch( + lines = ComponentsDiffer.try_apply_patch( module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, diff --git a/nf_core/modules/lint/module_changes.py b/nf_core/modules/lint/module_changes.py index 708a2bad68..121de00c0a 100644 --- a/nf_core/modules/lint/module_changes.py +++ b/nf_core/modules/lint/module_changes.py @@ -7,7 +7,7 @@ from pathlib import Path import nf_core.modules.modules_repo -from nf_core.modules.modules_differ import ModulesDiffer +from nf_core.components.components_differ import ComponentsDiffer def module_changes(module_lint_object, module): @@ -30,7 +30,7 @@ def module_changes(module_lint_object, module): tempdir = tempdir_parent / "tmp_module_dir" shutil.copytree(module.component_dir, tempdir) try: - new_lines = ModulesDiffer.try_apply_patch( + new_lines = ComponentsDiffer.try_apply_patch( module.component_type, module.component_name, module.org, diff --git a/nf_core/modules/lint/module_patch.py b/nf_core/modules/lint/module_patch.py index 19c6e76fec..6347c5c553 100644 --- a/nf_core/modules/lint/module_patch.py +++ b/nf_core/modules/lint/module_patch.py @@ -1,7 +1,7 @@ from pathlib import Path +from ...components.components_differ import ComponentsDiffer from ...components.nfcore_component import NFCoreComponent -from ..modules_differ import ModulesDiffer def module_patch(module_lint_obj, module: NFCoreComponent): @@ -66,11 +66,11 @@ def check_patch_valid(module, patch_path): continue topath = Path(line.split(" ")[1].strip("\n")) if frompath == Path("/dev/null"): - paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.CREATED)) + paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.CREATED)) elif topath == Path("/dev/null"): - paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.REMOVED)) + paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.REMOVED)) elif frompath == topath: - paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.CHANGED)) + paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.CHANGED)) else: module.failed.append( ( @@ -105,7 +105,7 @@ def check_patch_valid(module, patch_path): # Warn about any created or removed files passed = True for path, diff_status in paths_in_patch: - if diff_status == ModulesDiffer.DiffEnum.CHANGED: + if diff_status == ComponentsDiffer.DiffEnum.CHANGED: if not Path(module.base_dir, path).exists(): module.failed.append( ( @@ -116,7 +116,7 @@ def check_patch_valid(module, patch_path): ) passed = False continue - elif diff_status == ModulesDiffer.DiffEnum.CREATED: + elif diff_status == ComponentsDiffer.DiffEnum.CREATED: if not Path(module.base_dir, path).exists(): module.failed.append( ( @@ -130,7 +130,7 @@ def check_patch_valid(module, patch_path): module.warned.append( ("patch", f"Patch file performs file creation of {path}. This is discouraged."), patch_path ) - elif diff_status == ModulesDiffer.DiffEnum.REMOVED: + elif diff_status == ComponentsDiffer.DiffEnum.REMOVED: if Path(module.base_dir, path).exists(): module.failed.append( ( @@ -161,7 +161,7 @@ def patch_reversible(module_lint_object, module, patch_path): (bool): False if any test failed, True otherwise """ try: - ModulesDiffer.try_apply_patch( + ComponentsDiffer.try_apply_patch( module.component_type, module.component_name, module_lint_object.modules_repo.repo_path, diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index 64aab54bff..9ae735b1c0 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -19,7 +19,7 @@ from nf_core.modules.modules_repo import ModulesRepo from nf_core.pipelines.lint_utils import dump_json_with_prettier -from .modules_differ import ModulesDiffer +from ..components.components_differ import ComponentsDiffer log = logging.getLogger(__name__) @@ -883,7 +883,7 @@ def try_apply_patch_reverse(self, component_type, component, repo_name, patch_re patch_path = Path(self.directory / patch_relpath) try: - new_files = ModulesDiffer.try_apply_patch( + new_files = ComponentsDiffer.try_apply_patch( component_type, component, repo_name, patch_path, component_dir, reverse=True ) except LookupError as e: diff --git a/nf_core/subworkflows/lint/subworkflow_changes.py b/nf_core/subworkflows/lint/subworkflow_changes.py index e6af892125..cf0fd7211c 100644 --- a/nf_core/subworkflows/lint/subworkflow_changes.py +++ b/nf_core/subworkflows/lint/subworkflow_changes.py @@ -7,7 +7,7 @@ from pathlib import Path import nf_core.modules.modules_repo -from nf_core.modules.modules_differ import ModulesDiffer +from nf_core.components.components_differ import ComponentsDiffer def subworkflow_changes(subworkflow_lint_object, subworkflow): @@ -30,7 +30,7 @@ def subworkflow_changes(subworkflow_lint_object, subworkflow): tempdir = tempdir_parent / "tmp_subworkflow_dir" shutil.copytree(subworkflow.component_dir, tempdir) try: - new_lines = ModulesDiffer.try_apply_patch( + new_lines = ComponentsDiffer.try_apply_patch( subworkflow.component_type, subworkflow.component_name, subworkflow.org, From 27582f94a1b9b8e2fe18b118e7138c367f46ea8b Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 11:06:07 +0100 Subject: [PATCH 19/25] add subworkflows patch missing tests --- tests/subworkflows/test_patch.py | 111 +++++++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 4 deletions(-) diff --git a/tests/subworkflows/test_patch.py b/tests/subworkflows/test_patch.py index ba452083fa..388c5adde5 100644 --- a/tests/subworkflows/test_patch.py +++ b/tests/subworkflows/test_patch.py @@ -172,17 +172,120 @@ def test_create_patch_try_apply_failed(self): is False ) - # TODO: create those two missing tests def test_create_patch_update_success(self): - """Test creating a patch file and updating a subworkflow when there is a diff conflict""" - - def test_create_patch_update_fail(self): """ Test creating a patch file and the updating the subworkflow Should have the same effect as 'test_create_patch_try_apply_successful' but uses higher level api """ + self.setup_patch(self.pipeline_dir, True) + swf_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + patch_fn = "bam_sort_stats_samtools.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + + # Check the 'modules.json' contains a patch file for the subworkflow + modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) + assert modules_json_obj.get_patch_fn( + "subworkflows", "bam_sort_stats_samtools", GITLAB_URL, GITLAB_REPO + ) == Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools", patch_fn) + + # Update the subworkflow + update_obj = nf_core.subworkflows.update.SubworkflowUpdate( + self.pipeline_dir, + sha=OLD_SHA, + show_diff=False, + update_deps=True, + remote_url=GITLAB_URL, + branch=GITLAB_SUBWORKFLOWS_BRANCH, + ) + assert update_obj.update("bam_sort_stats_samtools") + + # Check that a patch file with the correct name has been created + assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + + # Check the 'modules.json' contains a patch file for the subworkflow + modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) + assert modules_json_obj.get_patch_fn( + "subworkflows", "bam_sort_stats_samtools", GITLAB_URL, GITLAB_REPO + ) == Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools", patch_fn), modules_json_obj.get_patch_fn( + "subworkflows", "bam_sort_stats_samtools", GITLAB_URL, GITLAB_REPO + ) + + # Check that the correct lines are in the patch file + with open(swf_path / patch_fn) as fh: + patch_lines = fh.readlines() + swf_relpath = swf_path.relative_to(self.pipeline_dir) + assert f"--- {swf_relpath / 'main.nf'}\n" in patch_lines + assert f"+++ {swf_relpath / 'main.nf'}\n" in patch_lines + assert "- ch_fasta // channel: [ fasta ]\n" in patch_lines + + # Check that 'main.nf' is updated correctly + with open(swf_path / "main.nf") as fh: + main_nf_lines = fh.readlines() + # this line should have been removed by the patch + assert " ch_fasta // channel: [ fasta ]\n" not in main_nf_lines + + def test_create_patch_update_fail(self): + """ + Test creating a patch file and updating a subworkflow when there is a diff conflict + """ + self.setup_patch(self.pipeline_dir, True) + swf_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") + + # Try creating a patch file + patch_obj = nf_core.subworkflows.SubworkflowPatch(self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH) + patch_obj.patch("bam_sort_stats_samtools") + + patch_fn = "bam_sort_stats_samtools.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + + # Check the 'modules.json' contains a patch file for the subworkflow + modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) + assert modules_json_obj.get_patch_fn( + "subworkflows", "bam_sort_stats_samtools", GITLAB_URL, GITLAB_REPO + ) == Path("subworkflows", GITLAB_REPO, "bam_sort_stats_samtools", patch_fn) + + # Save the file contents for downstream comparison + with open(swf_path / patch_fn) as fh: + patch_contents = fh.read() + + update_obj = nf_core.subworkflows.update.SubworkflowUpdate( + self.pipeline_dir, + sha=FAIL_SHA, + show_diff=False, + update_deps=True, + remote_url=GITLAB_URL, + branch=GITLAB_SUBWORKFLOWS_BRANCH, + ) + update_obj.update("bam_sort_stats_samtools") + + # Check that the installed files have not been affected by the attempted patch + temp_dir = Path(tempfile.mkdtemp()) + nf_core.components.components_command.ComponentCommand( + "subworkflows", self.pipeline_dir, GITLAB_URL, GITLAB_SUBWORKFLOWS_BRANCH + ).install_component_files("bam_sort_stats_samtools", FAIL_SHA, update_obj.modules_repo, temp_dir) + + temp_module_dir = temp_dir / "bam_sort_stats_samtools" + for file in os.listdir(temp_module_dir): + assert file in os.listdir(swf_path) + with open(swf_path / file) as fh: + installed = fh.read() + with open(temp_module_dir / file) as fh: + shouldbe = fh.read() + assert installed == shouldbe + + # Check that the patch file is unaffected + with open(swf_path / patch_fn) as fh: + new_patch_contents = fh.read() + assert patch_contents == new_patch_contents def test_remove_patch(self): """Test creating a patch when there is no change to the subworkflow""" From 4f93d5759db5b23383a2bf95665de4c8172c9e03 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 11:56:51 +0100 Subject: [PATCH 20/25] fix subworkflows update test --- tests/subworkflows/test_update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/subworkflows/test_update.py b/tests/subworkflows/test_update.py index 153038cd1d..9f5d1939f3 100644 --- a/tests/subworkflows/test_update.py +++ b/tests/subworkflows/test_update.py @@ -98,7 +98,7 @@ def test_install_at_hash_and_update_and_save_diff_to_file(self): with open(patch_path) as fh: line = fh.readline() assert line.startswith( - "Changes in module 'nf-core/fastq_align_bowtie2' between (f3c078809a2513f1c95de14f6633fe1f03572fdb) and" + "Changes in component 'nf-core/fastq_align_bowtie2' between (f3c078809a2513f1c95de14f6633fe1f03572fdb) and" ) def test_install_at_hash_and_update_and_save_diff_limit_output(self): From 5ad45701e7d89736efef768ce712eddc5215f32e Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 15:24:04 +0100 Subject: [PATCH 21/25] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a16435057..fed991f850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ ### Subworkflows +- Add `nf-core subworkflows patch` command ([#2861](https://github.com/nf-core/tools/pull/2861)) + ### General - Include .nf-core.yml in `nf-core pipelines bump-version` ([#3220](https://github.com/nf-core/tools/pull/3220)) From 37ca244d7a0ebaa48b24b28d02c900114e780a01 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 15:24:17 +0100 Subject: [PATCH 22/25] add help text for --remove flag --- nf_core/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/__main__.py b/nf_core/__main__.py index 6780ad80c6..81d088e13d 100644 --- a/nf_core/__main__.py +++ b/nf_core/__main__.py @@ -1024,7 +1024,7 @@ def command_modules_update( default=".", help=r"Pipeline directory. [dim]\[default: current working directory][/]", ) -@click.option("-r", "--remove", is_flag=True, default=False) +@click.option("-r", "--remove", is_flag=True, default=False, help="Remove an existent patch file and regenerate it.") def command_modules_patch(ctx, tool, directory, remove): """ Create a patch file for minor changes in a module @@ -1578,7 +1578,7 @@ def command_subworkflows_install(ctx, subworkflow, directory, prompt, force, sha default=".", help=r"Pipeline directory. [dim]\[default: current working directory][/]", ) -@click.option("-r", "--remove", is_flag=True, default=False) +@click.option("-r", "--remove", is_flag=True, default=False, help="Remove an existent patch file and regenerate it.") def subworkflows_patch(ctx, tool, dir, remove): """ Create a patch file for minor changes in a subworkflow From 805ba91df58633eaf68df64677e714b4b7ca04f0 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 15:24:33 +0100 Subject: [PATCH 23/25] apply code review suggestions to patch tests --- tests/modules/test_patch.py | 20 ++++++++++---------- tests/subworkflows/test_patch.py | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/modules/test_patch.py b/tests/modules/test_patch.py index 27c1e342e4..1c23871ccc 100644 --- a/tests/modules/test_patch.py +++ b/tests/modules/test_patch.py @@ -76,7 +76,7 @@ def test_create_patch_no_change(self): module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) # Check that no patch file has been added to the directory - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml"} + assert "bismark-align.diff" in set(os.listdir(module_path)) # Check the 'modules.json' contains no patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -94,7 +94,7 @@ def test_create_patch_change(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -127,7 +127,7 @@ def test_create_patch_try_apply_successful(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -153,7 +153,7 @@ def test_create_patch_try_apply_successful(self): update_obj.move_files_from_tmp_dir(BISMARK_ALIGN, install_dir, REPO_NAME, SUCCEED_SHA) # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -195,7 +195,7 @@ def test_create_patch_try_apply_failed(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -234,7 +234,7 @@ def test_create_patch_update_success(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -254,7 +254,7 @@ def test_create_patch_update_success(self): assert update_obj.update(BISMARK_ALIGN) # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -295,7 +295,7 @@ def test_create_patch_update_fail(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -349,7 +349,7 @@ def test_remove_patch(self): # Check that a patch file with the correct name has been created patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml", patch_fn} + assert patch_fn in set(os.listdir(module_path)) # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -361,7 +361,7 @@ def test_remove_patch(self): mock_questionary.unsafe_ask.return_value = True patch_obj.remove(BISMARK_ALIGN) # Check that the diff file has been removed - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "environment.yml"} + assert patch_fn not in set(os.listdir(module_path)) # Check that the 'modules.json' entry has been removed modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) diff --git a/tests/subworkflows/test_patch.py b/tests/subworkflows/test_patch.py index 388c5adde5..3df575c3de 100644 --- a/tests/subworkflows/test_patch.py +++ b/tests/subworkflows/test_patch.py @@ -66,7 +66,7 @@ def test_create_patch_no_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that no patch file has been added to the directory - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + assert "bam_sort_stats_samtools.diff" not in set(os.listdir(subworkflow_path)) def test_create_patch_change(self): """Test creating a patch when there is no change to the subworkflow""" @@ -79,7 +79,7 @@ def test_create_patch_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) # Check that the correct lines are in the patch file with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: @@ -101,7 +101,7 @@ def test_create_patch_try_apply_successful(self): patch_obj.patch("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) update_obj = nf_core.subworkflows.SubworkflowUpdate( self.pipeline_dir, sha=OLD_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH @@ -125,7 +125,7 @@ def test_create_patch_try_apply_successful(self): update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, GITLAB_REPO, OLD_SHA) # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) # Check that the correct lines are in the patch file with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: @@ -152,7 +152,7 @@ def test_create_patch_try_apply_failed(self): patch_obj.patch("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) update_obj = nf_core.subworkflows.SubworkflowUpdate( self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH @@ -188,7 +188,7 @@ def test_create_patch_update_success(self): patch_fn = "bam_sort_stats_samtools.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + assert patch_fn in set(os.listdir(swf_path)) # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -208,7 +208,7 @@ def test_create_patch_update_success(self): assert update_obj.update("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + assert patch_fn in set(os.listdir(swf_path)) # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -245,7 +245,7 @@ def test_create_patch_update_fail(self): patch_fn = "bam_sort_stats_samtools.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(swf_path)) == {"main.nf", "meta.yml", patch_fn} + assert patch_fn in set(os.listdir(swf_path)) # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -298,10 +298,10 @@ def test_remove_patch(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml", "bam_sort_stats_samtools.diff"} + assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) with mock.patch.object(nf_core.components.patch.questionary, "confirm") as mock_questionary: mock_questionary.unsafe_ask.return_value = True patch_obj.remove("bam_sort_stats_samtools") # Check that the diff file has been removed - assert set(os.listdir(subworkflow_path)) == {"main.nf", "meta.yml"} + assert "bam_sort_stats_samtools.diff" not in set(os.listdir(subworkflow_path)) From 84cec26f4f8b350949f02d6a010b340a59d5cca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Mon, 25 Nov 2024 15:28:28 +0100 Subject: [PATCH 24/25] Update tests/modules/test_patch.py --- tests/modules/test_patch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modules/test_patch.py b/tests/modules/test_patch.py index 1c23871ccc..df24ce819b 100644 --- a/tests/modules/test_patch.py +++ b/tests/modules/test_patch.py @@ -76,7 +76,7 @@ def test_create_patch_no_change(self): module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) # Check that no patch file has been added to the directory - assert "bismark-align.diff" in set(os.listdir(module_path)) + assert "bismark-align.diff" not in set(os.listdir(module_path)) # Check the 'modules.json' contains no patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) From 8b2bec4933fe41645a3fe9a5bd7db32d030a5237 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 25 Nov 2024 15:39:55 +0100 Subject: [PATCH 25/25] apply suggestions by @mashehu --- tests/modules/test_patch.py | 20 ++++++++++---------- tests/subworkflows/test_patch.py | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/modules/test_patch.py b/tests/modules/test_patch.py index df24ce819b..f608278618 100644 --- a/tests/modules/test_patch.py +++ b/tests/modules/test_patch.py @@ -76,7 +76,7 @@ def test_create_patch_no_change(self): module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) # Check that no patch file has been added to the directory - assert "bismark-align.diff" not in set(os.listdir(module_path)) + assert not (module_path / "bismark-align.diff").exists() # Check the 'modules.json' contains no patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -94,7 +94,7 @@ def test_create_patch_change(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -127,7 +127,7 @@ def test_create_patch_try_apply_successful(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -153,7 +153,7 @@ def test_create_patch_try_apply_successful(self): update_obj.move_files_from_tmp_dir(BISMARK_ALIGN, install_dir, REPO_NAME, SUCCEED_SHA) # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -195,7 +195,7 @@ def test_create_patch_try_apply_failed(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -234,7 +234,7 @@ def test_create_patch_update_success(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -254,7 +254,7 @@ def test_create_patch_update_success(self): assert update_obj.update(BISMARK_ALIGN) # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -295,7 +295,7 @@ def test_create_patch_update_fail(self): patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -349,7 +349,7 @@ def test_remove_patch(self): # Check that a patch file with the correct name has been created patch_fn = f"{'-'.join(BISMARK_ALIGN.split('/'))}.diff" - assert patch_fn in set(os.listdir(module_path)) + assert (module_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the module modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -361,7 +361,7 @@ def test_remove_patch(self): mock_questionary.unsafe_ask.return_value = True patch_obj.remove(BISMARK_ALIGN) # Check that the diff file has been removed - assert patch_fn not in set(os.listdir(module_path)) + assert not (module_path / patch_fn).exists() # Check that the 'modules.json' entry has been removed modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) diff --git a/tests/subworkflows/test_patch.py b/tests/subworkflows/test_patch.py index 3df575c3de..5bb6a6798e 100644 --- a/tests/subworkflows/test_patch.py +++ b/tests/subworkflows/test_patch.py @@ -66,7 +66,7 @@ def test_create_patch_no_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that no patch file has been added to the directory - assert "bam_sort_stats_samtools.diff" not in set(os.listdir(subworkflow_path)) + assert not (subworkflow_path / "bam_sort_stats_samtools.diff").exists() def test_create_patch_change(self): """Test creating a patch when there is no change to the subworkflow""" @@ -79,7 +79,7 @@ def test_create_patch_change(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) + assert (subworkflow_path / "bam_sort_stats_samtools.diff").exists() # Check that the correct lines are in the patch file with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: @@ -101,7 +101,7 @@ def test_create_patch_try_apply_successful(self): patch_obj.patch("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) + assert (subworkflow_path / "bam_sort_stats_samtools.diff").exists() update_obj = nf_core.subworkflows.SubworkflowUpdate( self.pipeline_dir, sha=OLD_SHA, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH @@ -125,7 +125,7 @@ def test_create_patch_try_apply_successful(self): update_obj.move_files_from_tmp_dir("bam_sort_stats_samtools", install_dir, GITLAB_REPO, OLD_SHA) # Check that a patch file with the correct name has been created - assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) + assert (subworkflow_path / "bam_sort_stats_samtools.diff").exists() # Check that the correct lines are in the patch file with open(subworkflow_path / "bam_sort_stats_samtools.diff") as fh: @@ -152,7 +152,7 @@ def test_create_patch_try_apply_failed(self): patch_obj.patch("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) + assert (subworkflow_path / "bam_sort_stats_samtools.diff").exists() update_obj = nf_core.subworkflows.SubworkflowUpdate( self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_SUBWORKFLOWS_BRANCH @@ -188,7 +188,7 @@ def test_create_patch_update_success(self): patch_fn = "bam_sort_stats_samtools.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(swf_path)) + assert (swf_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -208,7 +208,7 @@ def test_create_patch_update_success(self): assert update_obj.update("bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(swf_path)) + assert (swf_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -245,7 +245,7 @@ def test_create_patch_update_fail(self): patch_fn = "bam_sort_stats_samtools.diff" # Check that a patch file with the correct name has been created - assert patch_fn in set(os.listdir(swf_path)) + assert (swf_path / patch_fn).exists() # Check the 'modules.json' contains a patch file for the subworkflow modules_json_obj = nf_core.modules.modules_json.ModulesJson(self.pipeline_dir) @@ -298,10 +298,10 @@ def test_remove_patch(self): subworkflow_path = Path(self.pipeline_dir, "subworkflows", GITLAB_REPO, "bam_sort_stats_samtools") # Check that a patch file with the correct name has been created - assert "bam_sort_stats_samtools.diff" in set(os.listdir(subworkflow_path)) + assert (subworkflow_path / "bam_sort_stats_samtools.diff").exists() with mock.patch.object(nf_core.components.patch.questionary, "confirm") as mock_questionary: mock_questionary.unsafe_ask.return_value = True patch_obj.remove("bam_sort_stats_samtools") # Check that the diff file has been removed - assert "bam_sort_stats_samtools.diff" not in set(os.listdir(subworkflow_path)) + assert not (subworkflow_path / "bam_sort_stats_samtools.diff").exists()