From 6104d1c9fc816ebe1db82fe9f84b3b65700db573 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 10 Nov 2022 14:52:05 +0100 Subject: [PATCH 1/6] don't write file when applying a patch during update --- nf_core/modules/update.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nf_core/modules/update.py b/nf_core/modules/update.py index 297e893401..fbc6c55949 100644 --- a/nf_core/modules/update.py +++ b/nf_core/modules/update.py @@ -173,7 +173,7 @@ def update(self, module=None): if patch_relpath is not None: patch_successful = self.try_apply_patch( - module, modules_repo.repo_path, patch_relpath, module_dir, module_install_dir + module, modules_repo.repo_path, patch_relpath, module_dir, module_install_dir, write_file=False ) if patch_successful: log.info(f"Module '{module_fullname}' patched successfully") @@ -630,7 +630,7 @@ def move_files_from_tmp_dir(self, module, install_folder, repo_path, new_version log.info(f"Updating '{repo_path}/{module}'") log.debug(f"Updating module '{module}' to {new_version} from {repo_path}") - def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_install_dir): + def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_install_dir, write_file=True): """ Try applying a patch file to the new module files @@ -698,7 +698,7 @@ def try_apply_patch(self, module, repo_path, patch_relpath, module_dir, module_i # Add the patch file to the modules.json file self.modules_json.add_patch_entry( - module, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=True + module, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=write_file ) return True From f0fed33eae441f2e4520914e78188641d7a8a987 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Thu, 10 Nov 2022 14:53:10 +0100 Subject: [PATCH 2/6] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4dd46decb..c01115a34b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ ### Modules - Update patch file paths if the modules directory has the old structure ([#1878](https://github.com/nf-core/tools/pull/1878)) +- Don't write to `modules.json` file when applying a patch file during `nf-core modules update` ### Subworkflows From 3b15910939d2db680dc19993800c0bedfce5b8c7 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 12:37:42 +0100 Subject: [PATCH 3/6] add tests for showing module diffs without updating --- tests/modules/update.py | 63 +++++++++++++++++++++++++++++++++++++++++ tests/test_modules.py | 2 ++ 2 files changed, 65 insertions(+) diff --git a/tests/modules/update.py b/tests/modules/update.py index 05f026fbbd..1aaf9f753a 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -2,13 +2,17 @@ import os import shutil import tempfile +from pathlib import Path +from unittest import mock +import questionary import yaml import nf_core.utils from nf_core.modules.install import ModuleInstall from nf_core.modules.modules_json import ModulesJson from nf_core.modules.modules_repo import NF_CORE_MODULES_NAME, NF_CORE_MODULES_REMOTE +from nf_core.modules.patch import ModulePatch from nf_core.modules.update import ModuleUpdate from ..utils import ( @@ -288,6 +292,65 @@ def test_update_different_branch_mix_modules_branch_test(self): assert modules_json.get_module_version("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA +# Mock questionary answer: do not update module, only show diffs +@mock.patch.object(questionary.Question, "unsafe_ask", return_value=False) +def test_update_only_show_differences(self, mock_prompt): + """Try updating all modules showing differences. + Don't update some of them. + Check that the sha in modules.json is not changed.""" + modules_json = ModulesJson(self.pipeline_dir) + update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=True) + + assert update_obj.update() is True + + mod_json = modules_json.get_modules_json() + # Loop through all modules and check that they are NOT updated (according to the modules.json file) + # Modules that can be updated but shouldn't are custom/dumpsoftwareversions and fastqc + # Module multiqc is already up to date so don't check + for mod in ["custom/dumpsoftwareversions", "fastqc"]: + correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] + current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + print(correct_git_sha, current_git_sha) + assert correct_git_sha != current_git_sha + + +# Mock questionary answer: do not update module, only show diffs +@mock.patch("questionary.confirm", side_effect=False) +def test_update_only_show_differences_when_patch(self): + """Try updating all modules showing differences when there's a patched module. + Don't update some of them. + Check that the sha in modules.json is not changed.""" + modules_json = ModulesJson(self.pipeline_dir) + update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=True) + + # Modify fastqc module, it will have a patch which will be applied during update + # We modify fastqc because it's one of the modules that can be updated and there's another one before it (custom/dumpsoftwareversions) + module_path = Path(self.pipeline_dir, "modules", "nf-core", "fastqc") + with open(module_path, "r") as fh: + lines = fh.readlines() + for line_index in range(len(lines)): + if lines[line_index] == " label 'process_medium'\n": + lines[line_index] = " label 'process_low'\n" + with open(module_path, "w") as fh: + fh.writelines(lines) + # Create a patch file + patch_obj = ModulePatch(self.pipeline_dir) + patch_obj.patch("fastqc") + patch_fn = "fastq.diff" + # Check that a patch file with the correct name has been created + assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", patch_fn} + + # Update all modules + assert update_obj.update() is True + + mod_json = modules_json.get_modules_json() + # Loop through all modules and check that they are NOT updated (according to the modules.json file) + for mod in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]: + correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] + current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + assert correct_git_sha != current_git_sha + + def cmp_module(dir1, dir2): """Compare two versions of the same module""" files = ["main.nf", "meta.yml"] diff --git a/tests/test_modules.py b/tests/test_modules.py index f14d256dc4..7d5288d781 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -200,6 +200,8 @@ def test_modulesrepo_class(self): test_update_different_branch_mix_modules_branch_test, test_update_different_branch_mixed_modules_main, test_update_different_branch_single_module, + test_update_only_show_differences, + test_update_only_show_differences_when_patch, test_update_with_config_dont_update, test_update_with_config_fix_all, test_update_with_config_fixed_version, From 963a0aacad2dd9d26dc1196b6a5803a9f48be25b Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Fri, 11 Nov 2022 12:53:34 +0100 Subject: [PATCH 4/6] fix patch --- tests/modules/update.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/modules/update.py b/tests/modules/update.py index 1aaf9f753a..a0ce20371c 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -310,13 +310,12 @@ def test_update_only_show_differences(self, mock_prompt): for mod in ["custom/dumpsoftwareversions", "fastqc"]: correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] - print(correct_git_sha, current_git_sha) assert correct_git_sha != current_git_sha # Mock questionary answer: do not update module, only show diffs -@mock.patch("questionary.confirm", side_effect=False) -def test_update_only_show_differences_when_patch(self): +@mock.patch.object(questionary.Question, "unsafe_ask", return_value=False) +def test_update_only_show_differences_when_patch(self, mock_prompt): """Try updating all modules showing differences when there's a patched module. Don't update some of them. Check that the sha in modules.json is not changed.""" @@ -326,28 +325,31 @@ def test_update_only_show_differences_when_patch(self): # Modify fastqc module, it will have a patch which will be applied during update # We modify fastqc because it's one of the modules that can be updated and there's another one before it (custom/dumpsoftwareversions) module_path = Path(self.pipeline_dir, "modules", "nf-core", "fastqc") - with open(module_path, "r") as fh: + main_path = Path(module_path, "main.nf") + with open(main_path, "r") as fh: lines = fh.readlines() for line_index in range(len(lines)): if lines[line_index] == " label 'process_medium'\n": lines[line_index] = " label 'process_low'\n" - with open(module_path, "w") as fh: + with open(main_path, "w") as fh: fh.writelines(lines) # Create a patch file patch_obj = ModulePatch(self.pipeline_dir) patch_obj.patch("fastqc") - patch_fn = "fastq.diff" # Check that a patch file with the correct name has been created - assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", patch_fn} + assert set(os.listdir(module_path)) == {"main.nf", "meta.yml", "fastqc.diff"} # Update all modules assert update_obj.update() is True mod_json = modules_json.get_modules_json() # Loop through all modules and check that they are NOT updated (according to the modules.json file) - for mod in mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME]: + # Modules that can be updated but shouldn't are custom/dumpsoftwareversions and fastqc + # Module multiqc is already up to date so don't check + for mod in ["custom/dumpsoftwareversions", "fastqc"]: correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] + print(correct_git_sha, current_git_sha) assert correct_git_sha != current_git_sha From 586883fbfb1ed924e8279dbff5cd2c174a6b9bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BAlia=20Mir=20Pedrol?= Date: Fri, 11 Nov 2022 15:41:21 +0100 Subject: [PATCH 5/6] Update tests/modules/update.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Matthias Hörtenhuber --- tests/modules/update.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/modules/update.py b/tests/modules/update.py index a0ce20371c..b4b311308d 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -331,6 +331,7 @@ def test_update_only_show_differences_when_patch(self, mock_prompt): for line_index in range(len(lines)): if lines[line_index] == " label 'process_medium'\n": lines[line_index] = " label 'process_low'\n" + break with open(main_path, "w") as fh: fh.writelines(lines) # Create a patch file From 9ce1c01f0304a79b7c96710b9d775c2e305999a1 Mon Sep 17 00:00:00 2001 From: mirpedrol Date: Mon, 14 Nov 2022 09:26:30 +0100 Subject: [PATCH 6/6] check that files are not updated --- tests/modules/update.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/modules/update.py b/tests/modules/update.py index a0ce20371c..5d4e219cbd 100644 --- a/tests/modules/update.py +++ b/tests/modules/update.py @@ -301,6 +301,10 @@ def test_update_only_show_differences(self, mock_prompt): modules_json = ModulesJson(self.pipeline_dir) update_obj = ModuleUpdate(self.pipeline_dir, update_all=True, show_diff=True) + tmpdir = tempfile.mkdtemp() + shutil.rmtree(tmpdir) + shutil.copytree(Path(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME), tmpdir) + assert update_obj.update() is True mod_json = modules_json.get_modules_json() @@ -311,6 +315,7 @@ def test_update_only_show_differences(self, mock_prompt): correct_git_sha = list(update_obj.modules_repo.get_component_git_log(mod, "modules", depth=1))[0]["git_sha"] current_git_sha = mod_json["repos"][NF_CORE_MODULES_REMOTE]["modules"][NF_CORE_MODULES_NAME][mod]["git_sha"] assert correct_git_sha != current_git_sha + assert cmp_module(Path(tmpdir, mod), Path(self.pipeline_dir, "modules", NF_CORE_MODULES_NAME, mod)) is True # Mock questionary answer: do not update module, only show diffs