diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bfd0400a4..b1f0d17153 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Linting - accept `process_low_memory` as a standard module label ([#4264](https://github.com/nf-core/tools/pull/4264)) +- improve linting for `modules.json` to add support for only installing subworkflows from a repository and provide more explicit error messages ([#4287](https://github.com/nf-core/tools/pull/4287)) ### Modules diff --git a/nf_core/pipelines/lint/modules_json.py b/nf_core/pipelines/lint/modules_json.py index f7f5a077e7..8fa09b379b 100644 --- a/nf_core/pipelines/lint/modules_json.py +++ b/nf_core/pipelines/lint/modules_json.py @@ -25,32 +25,32 @@ def modules_json(self) -> dict[str, list[str]]: all_modules_passed = True for repo in modules_json_dict["repos"]: - # Check if the modules.json has been updated to keep the - if "modules" not in modules_json_dict["repos"][repo] or not repo.startswith("http"): - failed.append( - "Your `modules.json` file is outdated. " - "It will be automatically generated by running any module command." - ) - continue - - for install_dir in modules_json_dict["repos"][repo]["modules"]: - for module, module_entry in modules_json_dict["repos"][repo]["modules"][install_dir].items(): - if not Path(modules_dir, install_dir, module).exists(): - failed.append( - f"Entry for `{Path(modules_dir, install_dir, module)}` found in `modules.json` but module is not installed in " - "pipeline." - ) - all_modules_passed = False - if module_entry.get("branch") is None: - failed.append( - f"Entry for `{Path(modules_dir, install_dir, module)}` is missing branch information." - ) - if module_entry.get("git_sha") is None: - failed.append( - f"Entry for `{Path(modules_dir, install_dir, module)}` is missing version information." - ) - if all_modules_passed: - passed.append("Only installed modules found in `modules.json`") + if not repo.startswith("http"): + failed.append(f'Repository link for {repo} doesn\'t start with "http" in `modules.json`.') + + if "modules" in modules_json_dict["repos"][repo]: + # Module linting + for install_dir in modules_json_dict["repos"][repo]["modules"]: + for module, module_entry in modules_json_dict["repos"][repo]["modules"][install_dir].items(): + if not Path(modules_dir, install_dir, module).exists(): + failed.append( + f"Entry for `{Path(modules_dir, install_dir, module)}` found in `modules.json` but module is not installed in " + "pipeline." + ) + all_modules_passed = False + if module_entry.get("branch") is None: + failed.append( + f"Entry for `{Path(modules_dir, install_dir, module)}` is missing branch information." + ) + if module_entry.get("git_sha") is None: + failed.append( + f"Entry for `{Path(modules_dir, install_dir, module)}` is missing version information." + ) + if all_modules_passed: + passed.append("Only installed modules found in `modules.json`") + + if not any(component in modules_json_dict["repos"][repo] for component in ["modules", "subworkflows"]): + failed.append(f"No modules or subworkflows installed for {repo} in `modules.json`.") else: warned.append("Could not open `modules.json` file.") diff --git a/tests/pipelines/lint/test_modules_json.py b/tests/pipelines/lint/test_modules_json.py index 0d8333d9a2..6e72930009 100644 --- a/tests/pipelines/lint/test_modules_json.py +++ b/tests/pipelines/lint/test_modules_json.py @@ -1,10 +1,74 @@ +import json + +from nf_core.modules.modules_json import ( + ModulesJson, + ModulesJsonType, +) + from ..test_lint import TestLint class TestLintModulesJson(TestLint): + def setUp(self) -> None: + super().setUp() + self._modules_json_object: ModulesJson = ModulesJson(self.pipeline_dir) + self._modules_json_object.load() + self.modules_json_path = self._modules_json_object.modules_json_path + assert self._modules_json_object.modules_json is not None + self.modules_json: ModulesJsonType = self._modules_json_object.modules_json + + def _update_modules_json(self, new_modules_json) -> None: + # Update modules_json so that it is concordant at all levels of the class + self.modules_json = new_modules_json + self._modules_json_object.modules_json = new_modules_json + with open(self.modules_json_path, "w") as modules_json_file: + json.dump(new_modules_json, modules_json_file) + def test_modules_json_pass(self): self.lint_obj._load() results = self.lint_obj.modules_json() assert len(results.get("warned", [])) == 0 assert len(results.get("failed", [])) == 0 assert len(results.get("passed", [])) > 0 + + def test_modules_json_with_repo_not_starting_with_http_fail(self): + # Change the link of the first repo in modules_json to not start with "http" + new_modules_json = self.modules_json + first_repo = next(iter(new_modules_json["repos"])) + new_modules_json["repos"]["incorrect_repo_link"] = new_modules_json["repos"].pop(first_repo) + + self._update_modules_json(new_modules_json) + + # Check linting results + results = self.lint_obj.modules_json() + assert len(results.get("warned", [])) == 0 + assert len(results.get("failed", [])) == 1 + assert len(results.get("passed", [])) == 1 # The repository still contains only installed modules + + def test_modules_json_with_only_subworkflow_entries(self): + # Keep only "subworkflows" in the first repo in modules_json + new_modules_json = self.modules_json + first_repo = next(iter(new_modules_json["repos"])) + new_modules_json["repos"][first_repo] = {"subworkflows": new_modules_json["repos"][first_repo]["subworkflows"]} + + self._update_modules_json(new_modules_json) + + # Check linting results + results = self.lint_obj.modules_json() + assert len(results.get("warned", [])) == 0 + assert len(results.get("failed", [])) == 0 + assert len(results.get("passed", [])) == 0 # There is no subworkflow linting so no "passed" is returned + + def test_modules_json_with_repo_with_no_modules_or_subworkflows_fail(self): + # Change the first repo in modules_json to have no modules or subworkflows + new_modules_json = self.modules_json + first_repo = next(iter(new_modules_json["repos"])) + new_modules_json["repos"][first_repo] = {} + + self._update_modules_json(new_modules_json) + + # Check linting results + results = self.lint_obj.modules_json() + assert len(results.get("warned", [])) == 0 + assert len(results.get("failed", [])) == 1 + assert len(results.get("passed", [])) == 0