From 203380750f229cfc96c6201a197d913fca3b6b1f Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 29 Mar 2023 15:49:43 -0400 Subject: [PATCH 01/18] Add check for multiple remotes with the same org --- nf_core/components/install.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 850cc9d60f..fc88f36ce2 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -58,6 +58,9 @@ def install(self, component, silent=False): if not silent: modules_json.check_up_to_date() + # Verify that the remote repo's org_path does not match the org_path of any alternate repo among the installed modules + self.check_alternate_remotes(modules_json) + # Verify SHA if not self.modules_repo.verify_sha(self.prompt, self.sha): return False @@ -264,3 +267,24 @@ def clean_modules_json(self, component, modules_repo, modules_json): self.component_type, component, repo_to_remove, modules_repo.repo_path ) return component_values["installed_by"] + + def check_alternate_remotes(self, modules_json): + """ + Check whether there are previously installed components with the same org_path but different remote urls + Log warning if multiple remotes exist. + + Return: + True: if problematic components are found + False: if problematic components are not found + """ + alternate_remotes = False + for repo_url, repo_content in modules_json.modules_json["repos"].items(): # ex: https, [] + for dir in repo_content[self.component_type].keys(): # ex: nf-core + if dir == self.org and repo_url != self.modules_repo.remote_url: + alternate_remotes = True + if alternate_remotes: + warn_msg = f"Multiple module remotes are used with the same org_path '{self.org}': {', '.join(alternate_remotes)}. This may result in reinstalled modules from the wrong remote." + log.warning(warn_msg) + return True + else: + return False From cf39ce123554b2efdb592da813a0970bf01ed31c Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 29 Mar 2023 18:27:40 -0400 Subject: [PATCH 02/18] bugfix --- nf_core/components/install.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index fc88f36ce2..5b1557447b 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -278,6 +278,7 @@ def check_alternate_remotes(self, modules_json): False: if problematic components are not found """ alternate_remotes = False + modules_json.load() for repo_url, repo_content in modules_json.modules_json["repos"].items(): # ex: https, [] for dir in repo_content[self.component_type].keys(): # ex: nf-core if dir == self.org and repo_url != self.modules_repo.remote_url: From 2be86d67f7e227d836299d02b4fa4fa0fa82fb2b Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 29 Mar 2023 18:42:48 -0400 Subject: [PATCH 03/18] updated `CHANGELOG.md` with new PR --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d62f1345..605e41c9d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Add the Nextflow version to Gitpod container matching the minimal Nextflow version for nf-core (according to `nextflow.config`) ([#2196](https://github.com/nf-core/tools/pull/2196)) - Use `nfcore/gitpod:dev` container in the dev branch ([#2196](https://github.com/nf-core/tools/pull/2196)) - Replace requests_mock with responses in test mocks ([#2165](https://github.com/nf-core/tools/pull/2165)). +- Add warning when installing a module from an `org_path` that exists in multiple remotes in `modules.json` ([#2228](https://github.com/nf-core/tools/pull/2228)). ## [v2.7.2 - Mercury Eagle Patch](https://github.com/nf-core/tools/releases/tag/2.7.2) - [2022-12-19] From 96aea751c24ea50a674700f6c5cbe4b79397df4c Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 29 Mar 2023 21:11:52 -0400 Subject: [PATCH 04/18] bugfix --- nf_core/components/install.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 5b1557447b..429c4ec00c 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -279,8 +279,8 @@ def check_alternate_remotes(self, modules_json): """ alternate_remotes = False modules_json.load() - for repo_url, repo_content in modules_json.modules_json["repos"].items(): # ex: https, [] - for dir in repo_content[self.component_type].keys(): # ex: nf-core + for repo_url, repo_content in modules_json.modules_json.get("repos",dict()).items(): + for dir in repo_content.get(self.component_type,dict()).keys(): if dir == self.org and repo_url != self.modules_repo.remote_url: alternate_remotes = True if alternate_remotes: From b2455b410f2ff68f866483402e46c271a3795992 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 29 Mar 2023 21:52:46 -0400 Subject: [PATCH 05/18] fix linting with black --- nf_core/components/install.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 429c4ec00c..c5dcb7d8f5 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -279,8 +279,8 @@ def check_alternate_remotes(self, modules_json): """ alternate_remotes = False modules_json.load() - for repo_url, repo_content in modules_json.modules_json.get("repos",dict()).items(): - for dir in repo_content.get(self.component_type,dict()).keys(): + for repo_url, repo_content in modules_json.modules_json.get("repos", dict()).items(): + for dir in repo_content.get(self.component_type, dict()).keys(): if dir == self.org and repo_url != self.modules_repo.remote_url: alternate_remotes = True if alternate_remotes: From 729d2ee7d152936e18054bd76d12099353d48c49 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 30 Mar 2023 12:45:36 -0400 Subject: [PATCH 06/18] Change logged warning to logged error with clearer message --- nf_core/components/install.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index c5dcb7d8f5..8c85f5a52b 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -59,7 +59,9 @@ def install(self, component, silent=False): modules_json.check_up_to_date() # Verify that the remote repo's org_path does not match the org_path of any alternate repo among the installed modules - self.check_alternate_remotes(modules_json) + if self.check_alternate_remotes(modules_json): + err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.org}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name." + log.err(err_msg) # Verify SHA if not self.modules_repo.verify_sha(self.prompt, self.sha): @@ -271,7 +273,7 @@ def clean_modules_json(self, component, modules_repo, modules_json): def check_alternate_remotes(self, modules_json): """ Check whether there are previously installed components with the same org_path but different remote urls - Log warning if multiple remotes exist. + Log error if multiple remotes exist. Return: True: if problematic components are found @@ -284,8 +286,6 @@ def check_alternate_remotes(self, modules_json): if dir == self.org and repo_url != self.modules_repo.remote_url: alternate_remotes = True if alternate_remotes: - warn_msg = f"Multiple module remotes are used with the same org_path '{self.org}': {', '.join(alternate_remotes)}. This may result in reinstalled modules from the wrong remote." - log.warning(warn_msg) return True else: return False From f9ea217544cf6942fa39d8b15b702b1c061e0f7f Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 30 Mar 2023 12:57:17 -0400 Subject: [PATCH 07/18] Simplified logic for returning True/False in check --- nf_core/components/install.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 8c85f5a52b..3d38712af8 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -279,13 +279,9 @@ def check_alternate_remotes(self, modules_json): True: if problematic components are found False: if problematic components are not found """ - alternate_remotes = False modules_json.load() for repo_url, repo_content in modules_json.modules_json.get("repos", dict()).items(): for dir in repo_content.get(self.component_type, dict()).keys(): if dir == self.org and repo_url != self.modules_repo.remote_url: - alternate_remotes = True - if alternate_remotes: - return True - else: - return False + return True + return False From 1b3cb23b8d17e5676cc7d61a238660856e00e470 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Fri, 31 Mar 2023 11:12:00 -0400 Subject: [PATCH 08/18] Update nf_core/components/install.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- nf_core/components/install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 3d38712af8..c37103b694 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -60,7 +60,7 @@ def install(self, component, silent=False): # Verify that the remote repo's org_path does not match the org_path of any alternate repo among the installed modules if self.check_alternate_remotes(modules_json): - err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.org}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name." + err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.modules_repo.repo_path}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name." log.err(err_msg) # Verify SHA From bf043bba52beaa10f5ce260f6da84dc23b357dad Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Fri, 31 Mar 2023 11:12:08 -0400 Subject: [PATCH 09/18] Update nf_core/components/install.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- nf_core/components/install.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index c37103b694..a492b59642 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -62,6 +62,7 @@ def install(self, component, silent=False): if self.check_alternate_remotes(modules_json): err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.modules_repo.repo_path}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name." log.err(err_msg) + return False # Verify SHA if not self.modules_repo.verify_sha(self.prompt, self.sha): From 473ca496b5b1bfbfc3cfa0e3d40af07c552f9fc2 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Fri, 31 Mar 2023 11:12:18 -0400 Subject: [PATCH 10/18] Update nf_core/components/install.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Júlia Mir Pedrol --- nf_core/components/install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index a492b59642..ad43f2ade7 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -283,6 +283,6 @@ def check_alternate_remotes(self, modules_json): modules_json.load() for repo_url, repo_content in modules_json.modules_json.get("repos", dict()).items(): for dir in repo_content.get(self.component_type, dict()).keys(): - if dir == self.org and repo_url != self.modules_repo.remote_url: + if dir == self.modules_repo.repo_path and repo_url != self.modules_repo.remote_url: return True return False From 9934081ae666d35c5479fb5b76c2195ddb001976 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Mon, 3 Apr 2023 12:16:08 -0400 Subject: [PATCH 11/18] change log.err to log.error --- nf_core/components/install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index ad43f2ade7..3e440424d6 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -61,7 +61,7 @@ def install(self, component, silent=False): # Verify that the remote repo's org_path does not match the org_path of any alternate repo among the installed modules if self.check_alternate_remotes(modules_json): err_msg = f"You are trying to install {self.component_type} from different repositories with the same organization name '{self.modules_repo.repo_path}' (set in the `.nf-core.yml` file in the `org_path` field).\nThis is not supported, and will likely cause problems. org_path should be set to the github account/organization name." - log.err(err_msg) + log.error(err_msg) return False # Verify SHA From fc991988f75ddb7927e76c0a3d56f4f62d6c625f Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 5 Apr 2023 20:02:56 -0400 Subject: [PATCH 12/18] Changed functionality of check so that conflict is not limited to component type --- nf_core/components/install.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nf_core/components/install.py b/nf_core/components/install.py index 3e440424d6..f7a5fe6680 100644 --- a/nf_core/components/install.py +++ b/nf_core/components/install.py @@ -282,7 +282,8 @@ def check_alternate_remotes(self, modules_json): """ modules_json.load() for repo_url, repo_content in modules_json.modules_json.get("repos", dict()).items(): - for dir in repo_content.get(self.component_type, dict()).keys(): - if dir == self.modules_repo.repo_path and repo_url != self.modules_repo.remote_url: - return True + for component_type in repo_content: + for dir in repo_content.get(component_type, dict()).keys(): + if dir == self.modules_repo.repo_path and repo_url != self.modules_repo.remote_url: + return True return False From ccd506fd02c68cf0646b1fadd017e9bfdae17e26 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 5 Apr 2023 20:03:54 -0400 Subject: [PATCH 13/18] fixed pytests and added tests for alternate remotes when installing modules or subworkflows --- tests/modules/install.py | 9 ++++++++ tests/modules/lint.py | 8 +++++-- tests/modules/patch.py | 41 +++++++++++++++++++++++++++++------ tests/modules/remove.py | 6 ++++- tests/modules/update.py | 11 +++++++++- tests/subworkflows/install.py | 8 +++++++ tests/subworkflows/list.py | 2 ++ tests/test_modules.py | 6 +++-- tests/test_subworkflows.py | 6 +++-- tests/utils.py | 19 ++++++++++++++++ 10 files changed, 101 insertions(+), 15 deletions(-) diff --git a/tests/modules/install.py b/tests/modules/install.py index d01459f142..985f916998 100644 --- a/tests/modules/install.py +++ b/tests/modules/install.py @@ -12,6 +12,7 @@ with_temporary_folder, ) +from ..utils import remove_template_modules def test_modules_install_nopipeline(self): """Test installing a module - no pipeline given""" @@ -49,6 +50,7 @@ def test_modules_install_trimgalore_twice(self): def test_modules_install_from_gitlab(self): """Test installing a module from GitLab""" + remove_template_modules(self) assert self.mods_install_gitlab.install("fastqc") is True @@ -61,6 +63,7 @@ def test_modules_install_different_branch_fail(self): def test_modules_install_different_branch_succeed(self): """Test installing a module from a different branch""" + remove_template_modules(self) install_obj = ModuleInstall(self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH) # The fastp module does exists in the branch-test branch assert install_obj.install("fastp") is True @@ -83,3 +86,9 @@ def test_modules_install_tracking(self): assert mod_json["repos"]["https://github.com/nf-core/modules.git"]["modules"]["nf-core"]["trimgalore"][ "installed_by" ] == ["modules"] + +def test_modules_install_alternate_remote(self): + """Test installing a module from a different remote with the same organization path""" + install_obj = ModuleInstall(self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH) + # The fastp module does exists in the branch-test branch + assert install_obj.install("fastp") is False diff --git a/tests/modules/lint.py b/tests/modules/lint.py index 18c0dc4dab..21e9fdd16e 100644 --- a/tests/modules/lint.py +++ b/tests/modules/lint.py @@ -6,6 +6,7 @@ import nf_core.modules from ..utils import GITLAB_URL, set_wd +from ..utils import remove_template_modules from .patch import BISMARK_ALIGN, CORRECT_SHA, PATCH_BRANCH, REPO_NAME, modify_main_nf @@ -62,6 +63,7 @@ def test_modules_lint_no_gitlab(self): def test_modules_lint_gitlab_modules(self): """Lint modules from a different remote""" + remove_template_modules(self) self.mods_install_gitlab.install("fastqc") self.mods_install_gitlab.install("multiqc") module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) @@ -70,10 +72,11 @@ def test_modules_lint_gitlab_modules(self): assert len(module_lint.passed) > 0 assert len(module_lint.warned) >= 0 - +#should we remove this? We want to test multiple remotes but not for linting! def test_modules_lint_multiple_remotes(self): """Lint modules from a different remote""" - self.mods_install.install("fastqc") + remove_template_modules(self) + #self.mods_install.install("fastqc") self.mods_install_gitlab.install("multiqc") module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) module_lint.lint(print_results=False, all_modules=True) @@ -86,6 +89,7 @@ def test_modules_lint_patched_modules(self): """ Test creating a patch file and applying it to a new version of the the files """ + remove_template_modules(self) setup_patch(self.pipeline_dir, True) # Create a patch file diff --git a/tests/modules/patch.py b/tests/modules/patch.py index 09b892e2c8..370dd88d82 100644 --- a/tests/modules/patch.py +++ b/tests/modules/patch.py @@ -9,6 +9,8 @@ from ..utils import GITLAB_URL +from ..utils import remove_template_modules + """ Test the 'nf-core modules patch' command @@ -26,7 +28,7 @@ REPO_URL = "https://gitlab.com/nf-core/modules-test.git" -def setup_patch(pipeline_dir, modify_module): +def setup_patch(pipeline_dir, modify_module, pipeline_name): install_obj = nf_core.modules.ModuleInstall( pipeline_dir, prompt=False, force=False, remote_url=GITLAB_URL, branch=PATCH_BRANCH, sha=ORG_SHA ) @@ -39,6 +41,13 @@ def setup_patch(pipeline_dir, modify_module): module_path = Path(pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) modify_main_nf(module_path / "main.nf") +def modify_workflow_nf(path): + with open(path, "r") as fh: + lines = fh.readlines() + with open(path, "w") as fh: + for line in lines: + if not line.startswith("include {"): + fh.write(line) def modify_main_nf(path): """Modify a file to test patch creation""" @@ -60,7 +69,10 @@ def modify_main_nf(path): def test_create_patch_no_change(self): """Test creating a patch when there is no change to the module""" - setup_patch(self.pipeline_dir, False) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, False, self.pipeline_name) # Try creating a patch file patch_obj = nf_core.modules.ModulePatch(self.pipeline_dir, GITLAB_URL, PATCH_BRANCH) @@ -79,7 +91,10 @@ def test_create_patch_no_change(self): def test_create_patch_change(self): """Test creating a patch when there is a change to the module""" - setup_patch(self.pipeline_dir, True) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, True, self.pipeline_name) # Try creating a patch file patch_obj = nf_core.modules.ModulePatch(self.pipeline_dir, GITLAB_URL, PATCH_BRANCH) @@ -112,7 +127,10 @@ 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) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, True, self.pipeline_name) module_relpath = Path("modules", REPO_NAME, BISMARK_ALIGN) module_path = Path(self.pipeline_dir, module_relpath) @@ -178,7 +196,10 @@ 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) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, True, self.pipeline_name) module_relpath = Path("modules", REPO_NAME, BISMARK_ALIGN) module_path = Path(self.pipeline_dir, module_relpath) @@ -216,7 +237,10 @@ def test_create_patch_update_success(self): Should have the same effect as 'test_create_patch_try_apply_successful' but uses higher level api """ - setup_patch(self.pipeline_dir, True) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, True, self.pipeline_name) module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) # Try creating a patch file @@ -277,7 +301,10 @@ def test_create_patch_update_fail(self): """ Test creating a patch file and updating a module when there is a diff conflict """ - setup_patch(self.pipeline_dir, True) + # Remove modules that may cause org_path conflict + remove_template_modules(self) + + setup_patch(self.pipeline_dir, True, self.pipeline_name) module_path = Path(self.pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) # Try creating a patch file diff --git a/tests/modules/remove.py b/tests/modules/remove.py index b343a02da7..b2af1f10bd 100644 --- a/tests/modules/remove.py +++ b/tests/modules/remove.py @@ -1,5 +1,6 @@ import os +from ..utils import remove_template_modules def test_modules_remove_trimgalore(self): """Test removing TrimGalore! module after installing it""" @@ -13,9 +14,12 @@ def test_modules_remove_trimgalore_uninstalled(self): """Test removing TrimGalore! module without installing it""" assert self.mods_remove.remove("trimgalore") is False - +# Should this check be removed? def test_modules_remove_multiqc_from_gitlab(self): """Test removing multiqc module after installing it from an alternative source""" + # Remove modules that may cause org_path conflict + remove_template_modules(self) + self.mods_install_gitlab.install("multiqc") module_path = os.path.join(self.mods_install_gitlab.dir, "modules", "nf-core", "multiqc") assert self.mods_remove_gitlab.remove("multiqc", force=True) diff --git a/tests/modules/update.py b/tests/modules/update.py index fcfd92fc39..ea94488b5d 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -26,6 +26,7 @@ OLD_TRIMGALORE_SHA, ) +from ..utils import remove_template_modules def test_install_and_update(self): """Installs a module in the pipeline and updates it (no change)""" @@ -41,9 +42,9 @@ def test_install_and_update(self): assert update_obj.update("trimgalore") is True assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True - def test_install_at_hash_and_update(self): """Installs an old version of a module in the pipeline and updates it""" + remove_template_modules(self) assert self.mods_install_old.install("trimgalore") update_obj = ModuleUpdate( self.pipeline_dir, show_diff=False, update_deps=True, remote_url=GITLAB_URL, branch=OLD_TRIMGALORE_BRANCH @@ -69,6 +70,7 @@ def test_install_at_hash_and_update(self): def test_install_at_hash_and_update_and_save_diff_to_file(self): """Installs an old version of a module in the pipeline and updates it""" + remove_template_modules(self) self.mods_install_old.install("trimgalore") patch_path = os.path.join(self.pipeline_dir, "trimgalore.patch") update_obj = ModuleUpdate( @@ -109,6 +111,7 @@ def test_update_all(self): def test_update_with_config_fixed_version(self): """Try updating when there are entries in the .nf-core.yml""" + remove_template_modules(self) # Install trimgalore at the latest version assert self.mods_install_trimgalore.install("trimgalore") @@ -134,6 +137,7 @@ def test_update_with_config_fixed_version(self): def test_update_with_config_dont_update(self): """Try updating when module is to be ignored""" + remove_template_modules(self) # Install an old version of trimgalore self.mods_install_old.install("trimgalore") @@ -164,6 +168,7 @@ def test_update_with_config_dont_update(self): def test_update_with_config_fix_all(self): """Fix the version of all nf-core modules""" + remove_template_modules(self) self.mods_install_trimgalore.install("trimgalore") # Fix the version of all nf-core modules in the .nf-core.yml to an old version @@ -187,6 +192,7 @@ def test_update_with_config_fix_all(self): def test_update_with_config_no_updates(self): """Don't update any nf-core modules""" + remove_template_modules(self) self.mods_install_old.install("trimgalore") old_mod_json = ModulesJson(self.pipeline_dir).get_modules_json() @@ -220,6 +226,7 @@ def test_update_with_config_no_updates(self): def test_update_different_branch_single_module(self): """Try updating a module in a specific branch""" + remove_template_modules(self) install_obj = ModuleInstall( self.pipeline_dir, prompt=False, @@ -246,6 +253,7 @@ def test_update_different_branch_single_module(self): def test_update_different_branch_mixed_modules_main(self): """Try updating all modules where MultiQC is installed from main branch""" + remove_template_modules(self) # Install fastp assert self.mods_install_gitlab_old.install("fastp") @@ -272,6 +280,7 @@ def test_update_different_branch_mixed_modules_main(self): def test_update_different_branch_mix_modules_branch_test(self): """Try updating all modules where MultiQC is installed from branch-test branch""" + remove_template_modules(self) # Install multiqc from the branch-test branch assert self.mods_install_gitlab_old.install( "multiqc" diff --git a/tests/subworkflows/install.py b/tests/subworkflows/install.py index 6c04c9ad22..bf41694e1a 100644 --- a/tests/subworkflows/install.py +++ b/tests/subworkflows/install.py @@ -13,6 +13,8 @@ with_temporary_folder, ) +from ..utils import remove_template_modules + def test_subworkflow_install_nopipeline(self): """Test installing a subworkflow - no pipeline given""" @@ -62,6 +64,7 @@ def test_subworkflows_install_bam_sort_stats_samtools_twice(self): def test_subworkflows_install_from_gitlab(self): """Test installing a subworkflow from GitLab""" + remove_template_modules(self) assert self.subworkflow_install_gitlab.install("bam_stats_samtools") is True # Verify that the branch entry was added correctly modules_json = ModulesJson(self.pipeline_dir) @@ -140,3 +143,8 @@ def test_subworkflows_install_tracking_added_super_subworkflow(self): "installed_by" ] ) == sorted(["subworkflows", "bam_sort_stats_samtools"]) + +def test_subworkflows_install_alternate_remote(self): + """Test installing a subworkflow from a different remote with the same organization path""" + self.subworkflow_install.install("bam_sort_stats_samtools") + assert self.subworkflow_install_gitlab.install("bam_stats_samtools") is False diff --git a/tests/subworkflows/list.py b/tests/subworkflows/list.py index c65999d42c..2d44112859 100644 --- a/tests/subworkflows/list.py +++ b/tests/subworkflows/list.py @@ -4,6 +4,7 @@ from ..utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL +from ..utils import remove_template_modules def test_subworkflows_list_remote(self): """Test listing available subworkflows""" @@ -40,6 +41,7 @@ def test_subworkflows_install_and_list_subworkflows(self): def test_subworkflows_install_gitlab_and_list_subworkflows(self): """Test listing locally installed subworkflows""" + remove_template_modules(self) self.subworkflow_install_gitlab.install("bam_sort_stats_samtools") subworkflows_list = nf_core.subworkflows.SubworkflowList(self.pipeline_dir, remote=False) listed_subworkflows = subworkflows_list.list_components() diff --git a/tests/test_modules.py b/tests/test_modules.py index 21c003112e..e3fdd810ea 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -59,9 +59,10 @@ def setUp(self): # Set up the schema root_repo_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) self.template_dir = os.path.join(root_repo_dir, "nf_core", "pipeline-template") - self.pipeline_dir = os.path.join(self.tmp_dir, "mypipeline") + self.pipeline_name = "mypipeline" + self.pipeline_dir = os.path.join(self.tmp_dir, self.pipeline_name) nf_core.create.PipelineCreate( - "mypipeline", "it is mine", "me", no_git=True, outdir=self.pipeline_dir, plain=True + self.pipeline_name, "it is mine", "me", no_git=True, outdir=self.pipeline_dir, plain=True ).init_pipeline() # Set up install objects self.mods_install = nf_core.modules.ModuleInstall(self.pipeline_dir, prompt=False, force=True) @@ -159,6 +160,7 @@ def test_modulesrepo_class(self): test_modules_install_tracking, test_modules_install_trimgalore, test_modules_install_trimgalore_twice, + test_modules_install_alternate_remote, ) from .modules.lint import ( test_modules_lint_empty, diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 19a090f7f0..336e736da1 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -48,9 +48,10 @@ def setUp(self): # Set up the pipeline structure root_repo_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) self.template_dir = os.path.join(root_repo_dir, "nf_core", "pipeline-template") - self.pipeline_dir = os.path.join(self.tmp_dir, "mypipeline") + self.pipeline_name = "mypipeline" + self.pipeline_dir = os.path.join(self.tmp_dir, self.pipeline_name) nf_core.create.PipelineCreate( - "mypipeline", "it is mine", "me", no_git=True, outdir=self.pipeline_dir, plain=True + self.pipeline_name, "it is mine", "me", no_git=True, outdir=self.pipeline_dir, plain=True ).init_pipeline() # Set up the nf-core/modules repo dummy @@ -117,6 +118,7 @@ def tearDown(self): test_subworkflows_install_tracking, test_subworkflows_install_tracking_added_already_installed, test_subworkflows_install_tracking_added_super_subworkflow, + test_subworkflows_install_alternate_remote, ) from .subworkflows.list import ( test_subworkflows_install_and_list_subworkflows, diff --git a/tests/utils.py b/tests/utils.py index 0dd60dd051..eafe9bdaac 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,6 +10,8 @@ import responses +import nf_core.modules + OLD_TRIMGALORE_SHA = "06348dffce2a732fc9e656bdc5c64c3e02d302cb" OLD_TRIMGALORE_BRANCH = "mimic-old-trimgalore" GITLAB_URL = "https://gitlab.com/nf-core/modules-test.git" @@ -104,3 +106,20 @@ def mock_biocontainers_api_calls(rsps: responses.RequestsMock, module, version): ], } rsps.get(biocontainers_api_url, json=biocontainers_mock, status=200) + +def remove_template_modules(self): + # Remove modules that may cause org_path conflict + workflow_path = Path(self.pipeline_dir, "workflows", self.pipeline_name + ".nf") + with open(workflow_path, "r") as fh: + lines = fh.readlines() + with open(workflow_path, "w") as fh: + for line in lines: + if not line.startswith("include {"): + fh.write(line) + + remove_obj = nf_core.modules.ModuleRemove( + self.pipeline_dir, no_pull=False + ) + + for i in ['multiqc','fastqc','custom/dumpsoftwareversions']: + remove_obj.remove(i) From 4f130bdd60c0d890f404b6a1f942cc255235298c Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Wed, 5 Apr 2023 20:08:32 -0400 Subject: [PATCH 14/18] fix black and isort linting errors --- tests/modules/install.py | 3 ++- tests/modules/lint.py | 8 ++++---- tests/modules/patch.py | 6 +++--- tests/modules/remove.py | 2 ++ tests/modules/update.py | 3 ++- tests/subworkflows/install.py | 4 ++-- tests/subworkflows/list.py | 3 +-- tests/test_modules.py | 2 +- tests/test_subworkflows.py | 2 +- tests/utils.py | 7 +++---- 10 files changed, 21 insertions(+), 19 deletions(-) diff --git a/tests/modules/install.py b/tests/modules/install.py index 985f916998..fa588daf00 100644 --- a/tests/modules/install.py +++ b/tests/modules/install.py @@ -9,10 +9,10 @@ GITLAB_BRANCH_TEST_BRANCH, GITLAB_REPO, GITLAB_URL, + remove_template_modules, with_temporary_folder, ) -from ..utils import remove_template_modules def test_modules_install_nopipeline(self): """Test installing a module - no pipeline given""" @@ -87,6 +87,7 @@ def test_modules_install_tracking(self): "installed_by" ] == ["modules"] + def test_modules_install_alternate_remote(self): """Test installing a module from a different remote with the same organization path""" install_obj = ModuleInstall(self.pipeline_dir, remote_url=GITLAB_URL, branch=GITLAB_BRANCH_TEST_BRANCH) diff --git a/tests/modules/lint.py b/tests/modules/lint.py index 21e9fdd16e..9780c6a3aa 100644 --- a/tests/modules/lint.py +++ b/tests/modules/lint.py @@ -5,8 +5,7 @@ import nf_core.modules -from ..utils import GITLAB_URL, set_wd -from ..utils import remove_template_modules +from ..utils import GITLAB_URL, remove_template_modules, set_wd from .patch import BISMARK_ALIGN, CORRECT_SHA, PATCH_BRANCH, REPO_NAME, modify_main_nf @@ -72,11 +71,12 @@ def test_modules_lint_gitlab_modules(self): assert len(module_lint.passed) > 0 assert len(module_lint.warned) >= 0 -#should we remove this? We want to test multiple remotes but not for linting! + +# should we remove this? We want to test multiple remotes but not for linting! def test_modules_lint_multiple_remotes(self): """Lint modules from a different remote""" remove_template_modules(self) - #self.mods_install.install("fastqc") + # self.mods_install.install("fastqc") self.mods_install_gitlab.install("multiqc") module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) module_lint.lint(print_results=False, all_modules=True) diff --git a/tests/modules/patch.py b/tests/modules/patch.py index 370dd88d82..26ca5805a5 100644 --- a/tests/modules/patch.py +++ b/tests/modules/patch.py @@ -7,9 +7,7 @@ import nf_core.components.components_command import nf_core.modules -from ..utils import GITLAB_URL - -from ..utils import remove_template_modules +from ..utils import GITLAB_URL, remove_template_modules """ Test the 'nf-core modules patch' command @@ -41,6 +39,7 @@ def setup_patch(pipeline_dir, modify_module, pipeline_name): module_path = Path(pipeline_dir, "modules", REPO_NAME, BISMARK_ALIGN) modify_main_nf(module_path / "main.nf") + def modify_workflow_nf(path): with open(path, "r") as fh: lines = fh.readlines() @@ -49,6 +48,7 @@ def modify_workflow_nf(path): if not line.startswith("include {"): fh.write(line) + def modify_main_nf(path): """Modify a file to test patch creation""" with open(path, "r") as fh: diff --git a/tests/modules/remove.py b/tests/modules/remove.py index b2af1f10bd..267c757e56 100644 --- a/tests/modules/remove.py +++ b/tests/modules/remove.py @@ -2,6 +2,7 @@ from ..utils import remove_template_modules + def test_modules_remove_trimgalore(self): """Test removing TrimGalore! module after installing it""" self.mods_install.install("trimgalore") @@ -14,6 +15,7 @@ def test_modules_remove_trimgalore_uninstalled(self): """Test removing TrimGalore! module without installing it""" assert self.mods_remove.remove("trimgalore") is False + # Should this check be removed? def test_modules_remove_multiqc_from_gitlab(self): """Test removing multiqc module after installing it from an alternative source""" diff --git a/tests/modules/update.py b/tests/modules/update.py index ea94488b5d..e28f058760 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -24,9 +24,9 @@ GITLAB_URL, OLD_TRIMGALORE_BRANCH, OLD_TRIMGALORE_SHA, + remove_template_modules, ) -from ..utils import remove_template_modules def test_install_and_update(self): """Installs a module in the pipeline and updates it (no change)""" @@ -42,6 +42,7 @@ def test_install_and_update(self): assert update_obj.update("trimgalore") is True assert cmp_module(trimgalore_tmpdir, trimgalore_path) is True + def test_install_at_hash_and_update(self): """Installs an old version of a module in the pipeline and updates it""" remove_template_modules(self) diff --git a/tests/subworkflows/install.py b/tests/subworkflows/install.py index bf41694e1a..1c451d8964 100644 --- a/tests/subworkflows/install.py +++ b/tests/subworkflows/install.py @@ -10,11 +10,10 @@ GITLAB_REPO, GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, + remove_template_modules, with_temporary_folder, ) -from ..utils import remove_template_modules - def test_subworkflow_install_nopipeline(self): """Test installing a subworkflow - no pipeline given""" @@ -144,6 +143,7 @@ def test_subworkflows_install_tracking_added_super_subworkflow(self): ] ) == sorted(["subworkflows", "bam_sort_stats_samtools"]) + def test_subworkflows_install_alternate_remote(self): """Test installing a subworkflow from a different remote with the same organization path""" self.subworkflow_install.install("bam_sort_stats_samtools") diff --git a/tests/subworkflows/list.py b/tests/subworkflows/list.py index 2d44112859..cde6808bb8 100644 --- a/tests/subworkflows/list.py +++ b/tests/subworkflows/list.py @@ -2,9 +2,8 @@ import nf_core.subworkflows -from ..utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL +from ..utils import GITLAB_SUBWORKFLOWS_BRANCH, GITLAB_URL, remove_template_modules -from ..utils import remove_template_modules def test_subworkflows_list_remote(self): """Test listing available subworkflows""" diff --git a/tests/test_modules.py b/tests/test_modules.py index e3fdd810ea..057e77737c 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -151,6 +151,7 @@ def test_modulesrepo_class(self): test_modules_info_remote_gitlab, ) from .modules.install import ( + test_modules_install_alternate_remote, test_modules_install_different_branch_fail, test_modules_install_different_branch_succeed, test_modules_install_emptypipeline, @@ -160,7 +161,6 @@ def test_modulesrepo_class(self): test_modules_install_tracking, test_modules_install_trimgalore, test_modules_install_trimgalore_twice, - test_modules_install_alternate_remote, ) from .modules.lint import ( test_modules_lint_empty, diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 336e736da1..27373d2478 100644 --- a/tests/test_subworkflows.py +++ b/tests/test_subworkflows.py @@ -109,6 +109,7 @@ def tearDown(self): ) from .subworkflows.install import ( test_subworkflow_install_nopipeline, + test_subworkflows_install_alternate_remote, test_subworkflows_install_bam_sort_stats_samtools, test_subworkflows_install_bam_sort_stats_samtools_twice, test_subworkflows_install_different_branch_fail, @@ -118,7 +119,6 @@ def tearDown(self): test_subworkflows_install_tracking, test_subworkflows_install_tracking_added_already_installed, test_subworkflows_install_tracking_added_super_subworkflow, - test_subworkflows_install_alternate_remote, ) from .subworkflows.list import ( test_subworkflows_install_and_list_subworkflows, diff --git a/tests/utils.py b/tests/utils.py index eafe9bdaac..f33af2a7af 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -107,6 +107,7 @@ def mock_biocontainers_api_calls(rsps: responses.RequestsMock, module, version): } rsps.get(biocontainers_api_url, json=biocontainers_mock, status=200) + def remove_template_modules(self): # Remove modules that may cause org_path conflict workflow_path = Path(self.pipeline_dir, "workflows", self.pipeline_name + ".nf") @@ -117,9 +118,7 @@ def remove_template_modules(self): if not line.startswith("include {"): fh.write(line) - remove_obj = nf_core.modules.ModuleRemove( - self.pipeline_dir, no_pull=False - ) + remove_obj = nf_core.modules.ModuleRemove(self.pipeline_dir, no_pull=False) - for i in ['multiqc','fastqc','custom/dumpsoftwareversions']: + for i in ["multiqc", "fastqc", "custom/dumpsoftwareversions"]: remove_obj.remove(i) From 70ce71bf80f9f5dffc53e2c0af9ce70574fde9a7 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 6 Apr 2023 12:15:10 -0400 Subject: [PATCH 15/18] fix MakeTestWorkflow CI tests --- .github/workflows/create-lint-wf.yml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index d09bdfd822..aa9af45b64 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -107,15 +107,24 @@ jobs: - name: nf-core modules install run: nf-core --log-file log.txt modules install fastqc --dir nf-core-testpipeline/ --force - - name: nf-core modules install gitlab - run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main install fastqc --force --dir nf-core-testpipeline/ - - name: nf-core modules list local run: nf-core --log-file log.txt modules list local --dir nf-core-testpipeline/ - name: nf-core modules list remote run: nf-core --log-file log.txt modules list remote + - name: nf-core modules remove + run: | + nf-core --log-file log.txt modules remove fastqc + nf-core --log-file log.txt modules remove multiqc + nf-core --log-file log.txt modules remove custom/dumpsoftwareversions + + - name: nf-core modules install gitlab + run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main install fastqc --force --dir nf-core-testpipeline/ + + - name: nf-core modules list local gitlab + run: nf-core --log-file log.txt modules list local --dir nf-core-testpipeline/ + - name: nf-core modules list remote gitlab run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git list remote From 3593c3c72b681778b4824e19abb5194e2f986eca Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 6 Apr 2023 12:47:54 -0400 Subject: [PATCH 16/18] fix MakeTestWorkflow CI tests again --- .github/workflows/create-lint-wf.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index aa9af45b64..3fb520189d 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -115,9 +115,9 @@ jobs: - name: nf-core modules remove run: | - nf-core --log-file log.txt modules remove fastqc - nf-core --log-file log.txt modules remove multiqc - nf-core --log-file log.txt modules remove custom/dumpsoftwareversions + nf-core --log-file log.txt modules remove fastqc --dir nf-core-testpipeline/ + nf-core --log-file log.txt modules remove multiqc --dir nf-core-testpipeline/ + nf-core --log-file log.txt modules remove custom/dumpsoftwareversions --dir nf-core-testpipeline/ - name: nf-core modules install gitlab run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main install fastqc --force --dir nf-core-testpipeline/ From 195ee31d02027879769082307548e8b20e3c3e66 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 6 Apr 2023 12:48:09 -0400 Subject: [PATCH 17/18] cleanup --- tests/modules/lint.py | 2 -- tests/modules/remove.py | 1 - 2 files changed, 3 deletions(-) diff --git a/tests/modules/lint.py b/tests/modules/lint.py index 9780c6a3aa..cc38e0ac34 100644 --- a/tests/modules/lint.py +++ b/tests/modules/lint.py @@ -72,11 +72,9 @@ def test_modules_lint_gitlab_modules(self): assert len(module_lint.warned) >= 0 -# should we remove this? We want to test multiple remotes but not for linting! def test_modules_lint_multiple_remotes(self): """Lint modules from a different remote""" remove_template_modules(self) - # self.mods_install.install("fastqc") self.mods_install_gitlab.install("multiqc") module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir, remote_url=GITLAB_URL) module_lint.lint(print_results=False, all_modules=True) diff --git a/tests/modules/remove.py b/tests/modules/remove.py index 267c757e56..9a8953a7ca 100644 --- a/tests/modules/remove.py +++ b/tests/modules/remove.py @@ -16,7 +16,6 @@ def test_modules_remove_trimgalore_uninstalled(self): assert self.mods_remove.remove("trimgalore") is False -# Should this check be removed? def test_modules_remove_multiqc_from_gitlab(self): """Test removing multiqc module after installing it from an alternative source""" # Remove modules that may cause org_path conflict From 3f44849d8fa79b1ee9edd4959a7ba764b702d662 Mon Sep 17 00:00:00 2001 From: Anne Marie Noronha Date: Thu, 6 Apr 2023 13:42:11 -0400 Subject: [PATCH 18/18] fix MakeTestWorkflow CI tests again --- .github/workflows/create-lint-wf.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/create-lint-wf.yml b/.github/workflows/create-lint-wf.yml index 3fb520189d..a5b69650bf 100644 --- a/.github/workflows/create-lint-wf.yml +++ b/.github/workflows/create-lint-wf.yml @@ -115,6 +115,7 @@ jobs: - name: nf-core modules remove run: | + find nf-core-testpipeline/workflows/ -type f -exec sed -i '/^include /d' {} \; nf-core --log-file log.txt modules remove fastqc --dir nf-core-testpipeline/ nf-core --log-file log.txt modules remove multiqc --dir nf-core-testpipeline/ nf-core --log-file log.txt modules remove custom/dumpsoftwareversions --dir nf-core-testpipeline/