diff --git a/CHANGELOG.md b/CHANGELOG.md index ff85badfbc..f0a91902d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ - Fix issue with config resolution that was causing nested configs to behave unexpectedly ([#2862](https://github.com/nf-core/tools/pull/2862)) - Fix schema docs console output truncating ([#2880](https://github.com/nf-core/tools/pull/2880)) - fix: ensure path object converted to string before stripping quotes ([#2878](https://github.com/nf-core/tools/pull/2878)) +- Fix incorrect assertions for called_with on mocks ([#2891](https://github.com/nf-core/tools/pull/2891)) - Make cli-provided module/subworkflow names case insensitive ([#2869](https://github.com/nf-core/tools/pull/2869)) - Update gitpod/workspace-base Docker digest to 168d78b ([#2899](https://github.com/nf-core/tools/pull/2899)) - Update pre-commit hook astral-sh/ruff-pre-commit to v0.3.4 ([#2894](https://github.com/nf-core/tools/pull/2894)) diff --git a/nf_core/components/components_command.py b/nf_core/components/components_command.py index 8332429835..4df67639e2 100644 --- a/nf_core/components/components_command.py +++ b/nf_core/components/components_command.py @@ -245,7 +245,7 @@ def check_patch_paths(self, patch_path: Path, module_name: str) -> None: # Update path in modules.json if the file is in the correct format modules_json = ModulesJson(self.dir) modules_json.load() - if modules_json.has_git_url_and_modules(): + if modules_json.has_git_url_and_modules() and modules_json.modules_json is not None: modules_json.modules_json["repos"][self.modules_repo.remote_url]["modules"][ self.modules_repo.repo_path ][module_name]["patch"] = str(patch_path.relative_to(Path(self.dir).resolve())) diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index a14ac15691..2a8576d5fb 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -9,7 +9,7 @@ import logging import os from pathlib import Path -from typing import List, Union +from typing import List, Tuple, Union import git import rich @@ -25,6 +25,7 @@ import nf_core.subworkflows.lint import nf_core.utils from nf_core import __version__ +from nf_core.components.lint import ComponentLint from nf_core.lint_utils import console from nf_core.utils import plural_s as _s from nf_core.utils import strip_ansi_codes @@ -32,148 +33,6 @@ log = logging.getLogger(__name__) -def run_linting( - pipeline_dir, - release_mode=False, - fix=(), - key=(), - show_passed=False, - fail_ignored=False, - fail_warned=False, - sort_by="test", - md_fn=None, - json_fn=None, - hide_progress=False, -): - """Runs all nf-core linting checks on a given Nextflow pipeline project - in either `release` mode or `normal` mode (default). Returns an object - of type :class:`PipelineLint` after finished. - - Args: - pipeline_dir (str): The path to the Nextflow pipeline root directory - release_mode (bool): Set this to `True`, if the linting should be run in the `release` mode. - See :class:`PipelineLint` for more information. - - Returns: - An object of type :class:`PipelineLint` that contains all the linting results. - An object of type :class:`ComponentLint` that contains all the linting results for the modules. - An object of type :class:`ComponentLint` that contains all the linting results for the subworkflows. - """ - - # Verify that the requested tests exist - if key: - all_tests = set(PipelineLint._get_all_lint_tests(release_mode)).union( - set(nf_core.modules.lint.ModuleLint.get_all_module_lint_tests(is_pipeline=True)) - ) - bad_keys = [k for k in key if k not in all_tests] - if len(bad_keys) > 0: - raise AssertionError( - "Test name{} not recognised: '{}'".format( - _s(bad_keys), - "', '".join(bad_keys), - ) - ) - log.info("Only running tests: '{}'".format("', '".join(key))) - - # Check if we were given any keys, and if they match any pipeline tests - if key: - pipeline_keys = list(set(key).intersection(set(PipelineLint._get_all_lint_tests(release_mode)))) - else: - # If no key is supplied, run all tests - pipeline_keys = None - - # Create the lint object - lint_obj = PipelineLint(pipeline_dir, release_mode, fix, pipeline_keys, fail_ignored, fail_warned, hide_progress) - - # Load the various pipeline configs - lint_obj._load_lint_config() - lint_obj._load_pipeline_config() - lint_obj._list_files() - - # Create the modules lint object - module_lint_obj = nf_core.modules.lint.ModuleLint(pipeline_dir, hide_progress=hide_progress) - # Create the subworkflows lint object - try: - subworkflow_lint_obj = nf_core.subworkflows.lint.SubworkflowLint(pipeline_dir, hide_progress=hide_progress) - except LookupError: - subworkflow_lint_obj = None - - # Verify that the pipeline is correctly configured and has a modules.json file - module_lint_obj.has_valid_directory() - module_lint_obj.has_modules_file() - - # Run only the tests we want - if key: - # Select only the module lint tests - module_lint_tests = list( - set(key).intersection(set(nf_core.modules.lint.ModuleLint.get_all_module_lint_tests(is_pipeline=True))) - ) - # Select only the subworkflow lint tests - subworkflow_lint_tests = list( - set(key).intersection( - set(nf_core.subworkflows.lint.SubworkflowLint.get_all_subworkflow_lint_tests(is_pipeline=True)) - ) - ) - else: - # If no key is supplied, run the default modules tests - module_lint_tests = ("module_changes", "module_version") - subworkflow_lint_tests = ("subworkflow_changes", "subworkflow_version") - module_lint_obj.filter_tests_by_key(module_lint_tests) - if subworkflow_lint_obj is not None: - subworkflow_lint_obj.filter_tests_by_key(subworkflow_lint_tests) - - # Set up files for component linting test - module_lint_obj.set_up_pipeline_files() - if subworkflow_lint_obj is not None: - subworkflow_lint_obj.set_up_pipeline_files() - - # Run the pipeline linting tests - try: - lint_obj._lint_pipeline() - except AssertionError as e: - log.critical(f"Critical error: {e}") - log.info("Stopping tests...") - return lint_obj, module_lint_obj - - # Run the module lint tests - if len(module_lint_obj.all_local_components) > 0: - module_lint_obj.lint_modules(module_lint_obj.all_local_components, local=True) - if len(module_lint_obj.all_remote_components) > 0: - module_lint_obj.lint_modules(module_lint_obj.all_remote_components, local=False) - # Run the subworkflows lint tests - if subworkflow_lint_obj is not None: - if len(subworkflow_lint_obj.all_local_components) > 0: - subworkflow_lint_obj.lint_subworkflows(subworkflow_lint_obj.all_local_components, local=True) - if len(subworkflow_lint_obj.all_remote_components) > 0: - subworkflow_lint_obj.lint_subworkflows(subworkflow_lint_obj.all_remote_components, local=False) - - # Print the results - lint_obj._print_results(show_passed) - module_lint_obj._print_results(show_passed, sort_by=sort_by) - if subworkflow_lint_obj is not None: - subworkflow_lint_obj._print_results(show_passed, sort_by=sort_by) - nf_core.lint_utils.print_joint_summary(lint_obj, module_lint_obj, subworkflow_lint_obj) - nf_core.lint_utils.print_fixes(lint_obj) - - # Save results to Markdown file - if md_fn is not None: - log.info(f"Writing lint results to {md_fn}") - markdown = lint_obj._get_results_md() - with open(md_fn, "w") as fh: - fh.write(markdown) - - # Save results to JSON file - if json_fn is not None: - lint_obj._save_json_results(json_fn) - - # Reminder about --release mode flag if we had failures - if len(lint_obj.failed) > 0: - if release_mode: - log.info("Reminder: Lint tests were run in --release mode.") - - return lint_obj, module_lint_obj, subworkflow_lint_obj - - class PipelineLint(nf_core.utils.Pipeline): """Object to hold linting information and results. @@ -411,7 +270,7 @@ def format_result(test_results): # Table of passed tests if len(self.passed) > 0 and show_passed: console.print( - rich.panel.Panel( + Panel( format_result(self.passed), title=rf"[bold][✔] {len(self.passed)} Pipeline Test{_s(self.passed)} Passed", title_align="left", @@ -423,7 +282,7 @@ def format_result(test_results): # Table of fixed tests if len(self.fixed) > 0: console.print( - rich.panel.Panel( + Panel( format_result(self.fixed), title=rf"[bold][?] {len(self.fixed)} Pipeline Test{_s(self.fixed)} Fixed", title_align="left", @@ -435,7 +294,7 @@ def format_result(test_results): # Table of ignored tests if len(self.ignored) > 0: console.print( - rich.panel.Panel( + Panel( format_result(self.ignored), title=rf"[bold][?] {len(self.ignored)} Pipeline Test{_s(self.ignored)} Ignored", title_align="left", @@ -447,7 +306,7 @@ def format_result(test_results): # Table of warning tests if len(self.warned) > 0: console.print( - rich.panel.Panel( + Panel( format_result(self.warned), title=rf"[bold][!] {len(self.warned)} Pipeline Test Warning{_s(self.warned)}", title_align="left", @@ -459,7 +318,7 @@ def format_result(test_results): # Table of failing tests if len(self.failed) > 0: console.print( - rich.panel.Panel( + Panel( format_result(self.failed), title=rf"[bold][✗] {len(self.failed)} Pipeline Test{_s(self.failed)} Failed", title_align="left", @@ -640,3 +499,144 @@ def _wrap_quotes(self, files: Union[List[str], List[Path], Path]) -> str: files = [files] bfiles = [f"`{str(f)}`" for f in files] return " or ".join(bfiles) + + +def run_linting( + pipeline_dir, + release_mode: bool = False, + fix=(), + key=(), + show_passed: bool = False, + fail_ignored: bool = False, + fail_warned: bool = False, + sort_by: str = "test", + md_fn=None, + json_fn=None, + hide_progress: bool = False, +) -> Tuple[PipelineLint, ComponentLint, Union[ComponentLint, None]]: + """Runs all nf-core linting checks on a given Nextflow pipeline project + in either `release` mode or `normal` mode (default). Returns an object + of type :class:`PipelineLint` after finished. + + Args: + pipeline_dir (str): The path to the Nextflow pipeline root directory + release_mode (bool): Set this to `True`, if the linting should be run in the `release` mode. + See :class:`PipelineLint` for more information. + + Returns: + An object of type :class:`PipelineLint` that contains all the linting results. + An object of type :class:`ComponentLint` that contains all the linting results for the modules. + An object of type :class:`ComponentLint` that contains all the linting results for the subworkflows. + """ + + # Verify that the requested tests exist + if key: + all_tests = set(PipelineLint._get_all_lint_tests(release_mode)).union( + set(nf_core.modules.lint.ModuleLint.get_all_module_lint_tests(is_pipeline=True)) + ) + bad_keys = [k for k in key if k not in all_tests] + if len(bad_keys) > 0: + raise AssertionError( + "Test name{} not recognised: '{}'".format( + _s(bad_keys), + "', '".join(bad_keys), + ) + ) + log.info("Only running tests: '{}'".format("', '".join(key))) + + # Check if we were given any keys, and if they match any pipeline tests + if key: + pipeline_keys = list(set(key).intersection(set(PipelineLint._get_all_lint_tests(release_mode)))) + else: + # If no key is supplied, run all tests + pipeline_keys = None + + # Create the lint object + lint_obj = PipelineLint(pipeline_dir, release_mode, fix, pipeline_keys, fail_ignored, fail_warned, hide_progress) + + # Load the various pipeline configs + lint_obj._load_lint_config() + lint_obj._load_pipeline_config() + lint_obj._list_files() + + # Create the modules lint object + module_lint_obj = nf_core.modules.lint.ModuleLint(pipeline_dir, hide_progress=hide_progress) + # Create the subworkflows lint object + try: + subworkflow_lint_obj = nf_core.subworkflows.lint.SubworkflowLint(pipeline_dir, hide_progress=hide_progress) + except LookupError: + subworkflow_lint_obj = None + + # Verify that the pipeline is correctly configured and has a modules.json file + module_lint_obj.has_valid_directory() + module_lint_obj.has_modules_file() + # Run only the tests we want + if key: + # Select only the module lint tests + module_lint_tests = list( + set(key).intersection(set(nf_core.modules.lint.ModuleLint.get_all_module_lint_tests(is_pipeline=True))) + ) + # Select only the subworkflow lint tests + subworkflow_lint_tests = list( + set(key).intersection( + set(nf_core.subworkflows.lint.SubworkflowLint.get_all_subworkflow_lint_tests(is_pipeline=True)) + ) + ) + else: + # If no key is supplied, run the default modules tests + module_lint_tests = list(("module_changes", "module_version")) + subworkflow_lint_tests = list(("subworkflow_changes", "subworkflow_version")) + module_lint_obj.filter_tests_by_key(module_lint_tests) + if subworkflow_lint_obj is not None: + subworkflow_lint_obj.filter_tests_by_key(subworkflow_lint_tests) + + # Set up files for component linting test + module_lint_obj.set_up_pipeline_files() + if subworkflow_lint_obj is not None: + subworkflow_lint_obj.set_up_pipeline_files() + + # Run the pipeline linting tests + try: + lint_obj._lint_pipeline() + except AssertionError as e: + log.critical(f"Critical error: {e}") + log.info("Stopping tests...") + return lint_obj, module_lint_obj, subworkflow_lint_obj + + # Run the module lint tests + if len(module_lint_obj.all_local_components) > 0: + module_lint_obj.lint_modules(module_lint_obj.all_local_components, local=True) + if len(module_lint_obj.all_remote_components) > 0: + module_lint_obj.lint_modules(module_lint_obj.all_remote_components, local=False) + # Run the subworkflows lint tests + if subworkflow_lint_obj is not None: + if len(subworkflow_lint_obj.all_local_components) > 0: + subworkflow_lint_obj.lint_subworkflows(subworkflow_lint_obj.all_local_components, local=True) + if len(subworkflow_lint_obj.all_remote_components) > 0: + subworkflow_lint_obj.lint_subworkflows(subworkflow_lint_obj.all_remote_components, local=False) + + # Print the results + lint_obj._print_results(show_passed) + module_lint_obj._print_results(show_passed, sort_by=sort_by) + if subworkflow_lint_obj is not None: + subworkflow_lint_obj._print_results(show_passed, sort_by=sort_by) + nf_core.lint_utils.print_joint_summary(lint_obj, module_lint_obj, subworkflow_lint_obj) + nf_core.lint_utils.print_fixes(lint_obj) + + # Save results to Markdown file + if md_fn is not None: + log.info(f"Writing lint results to {md_fn}") + markdown = lint_obj._get_results_md() + with open(md_fn, "w") as fh: + fh.write(markdown) + + # Save results to JSON file + if json_fn is not None: + lint_obj._save_json_results(json_fn) + + # Reminder about --release mode flag if we had failures + if len(lint_obj.failed) > 0: + if release_mode: + log.info("Reminder: Lint tests were run in --release mode.") + + return lint_obj, module_lint_obj, subworkflow_lint_obj diff --git a/nf_core/modules/modules_json.py b/nf_core/modules/modules_json.py index f68c27b2d8..7d78268e92 100644 --- a/nf_core/modules/modules_json.py +++ b/nf_core/modules/modules_json.py @@ -6,7 +6,6 @@ import shutil import tempfile from pathlib import Path -from typing import Union import git import questionary @@ -32,7 +31,7 @@ class ModulesJson: An object for handling a 'modules.json' file in a pipeline """ - def __init__(self, pipeline_dir): + def __init__(self, pipeline_dir: str): """ Initialise the object. @@ -43,7 +42,7 @@ def __init__(self, pipeline_dir): self.modules_dir = Path(self.dir, "modules") self.subworkflows_dir = Path(self.dir, "subworkflows") self.modules_json_path = Path(self.dir, "modules.json") - self.modules_json: Union(dict, None) = None + self.modules_json = None self.pipeline_modules = None self.pipeline_subworkflows = None self.pipeline_components = None @@ -1051,17 +1050,18 @@ def get_component_branch(self, component_type, component, repo_url, install_dir) ) return branch - def dump(self, run_prettier: bool = False): + def dump(self, run_prettier: bool = False) -> None: """ Sort the modules.json, and write it to file """ - # Sort the modules.json - self.modules_json["repos"] = nf_core.utils.sort_dictionary(self.modules_json["repos"]) - if run_prettier: - dump_json_with_prettier(self.modules_json_path, self.modules_json) - else: - with open(self.modules_json_path, "w") as fh: - json.dump(self.modules_json, fh, indent=4) + if self.modules_json is not None: + # Sort the modules.json + self.modules_json["repos"] = nf_core.utils.sort_dictionary(self.modules_json["repos"]) + if run_prettier: + dump_json_with_prettier(self.modules_json_path, self.modules_json) + else: + with open(self.modules_json_path, "w") as fh: + json.dump(self.modules_json, fh, indent=4) def resolve_missing_installation(self, missing_installation, component_type): missing_but_in_mod_json = [ diff --git a/nf_core/schema.py b/nf_core/schema.py index 4096c80f93..a4e8ad7cdc 100644 --- a/nf_core/schema.py +++ b/nf_core/schema.py @@ -6,6 +6,7 @@ import tempfile import webbrowser from pathlib import Path +from typing import Union import jinja2 import jsonschema @@ -46,11 +47,14 @@ def __init__(self): self.web_schema_build_web_url = None self.web_schema_build_api_url = None - def get_schema_path(self, path, local_only=False, revision=None): + def get_schema_path( + self, path: Union[str, Path], local_only: bool = False, revision: Union[str, None] = None + ) -> None: """Given a pipeline name, directory, or path, set self.schema_filename""" path = Path(path) # Supplied path exists - assume a local pipeline directory or schema if path.exists(): + log.debug(f"Path exists: {path}. Assuming local pipeline directory or schema") if revision is not None: log.warning(f"Local workflow supplied, ignoring revision '{revision}'") if path.is_dir(): @@ -63,14 +67,16 @@ def get_schema_path(self, path, local_only=False, revision=None): # Path does not exist - assume a name of a remote workflow elif not local_only: self.pipeline_dir = nf_core.list.get_local_wf(path, revision=revision) - self.schema_filename = Path(self.pipeline_dir, "nextflow_schema.json") - + self.schema_filename = Path(self.pipeline_dir or "", "nextflow_schema.json") + # check if the schema file exists + if not self.schema_filename.exists(): + self.schema_filename = None # Only looking for local paths, overwrite with None to be safe else: self.schema_filename = None # Check that the schema file exists - if self.schema_filename is None or not Path(self.schema_filename).exists(): + if self.schema_filename is None or not Path(self.schema_filename).exists() and local_only: error = f"Could not find pipeline schema for '{path}': {self.schema_filename}" log.error(error) raise AssertionError(error) @@ -103,7 +109,7 @@ def load_lint_schema(self): def load_schema(self): """Load a pipeline schema from a file""" - if self.schema_filename is None: + if self.schema_filename is None or not Path(self.schema_filename).exists(): raise AssertionError("Pipeline schema filename could not be found.") with open(self.schema_filename) as fh: diff --git a/tests/test_cli.py b/tests/test_cli.py index 54e420f5e4..913a4aac1d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -31,13 +31,13 @@ def test_header_outdated(mock_check_outdated, mock_nf_core_cli, capsys): class TestCli(unittest.TestCase): - """Class for testing the commandline interface""" + """Class for testing the command line interface""" def setUp(self): self.runner = CliRunner() def assemble_params(self, params): - """Assemble a dictionnary of parameters into a list of arguments for the cli + """Assemble a dictionary of parameters into a list of arguments for the cli Note: if the value of a parameter is None, it will be considered a flag. @@ -363,18 +363,21 @@ def test_lint_log_user_warning(self, mock_lint, mock_is_pipeline): def test_schema_lint(self, mock_get_schema_path): """Test nf-core schema lint defaults to nextflow_schema.json""" cmd = ["schema", "lint"] - result = self.invoke_cli(cmd) - assert mock_get_schema_path.called_with("nextflow_schema.json") - assert "nextflow_schema.json" in result.output + with self.runner.isolated_filesystem(): + with open("nextflow_schema.json", "w") as f: + f.write("{}") + self.invoke_cli(cmd) + mock_get_schema_path.assert_called_with("nextflow_schema.json") @mock.patch("nf_core.schema.PipelineSchema.get_schema_path") def test_schema_lint_filename(self, mock_get_schema_path): """Test nf-core schema lint accepts a filename""" cmd = ["schema", "lint", "some_other_filename"] - result = self.invoke_cli(cmd) - assert mock_get_schema_path.called_with("some_other_filename") - assert "some_other_filename" in result.output - assert "nextflow_schema.json" not in result.output + with self.runner.isolated_filesystem(): + with open("some_other_filename", "w") as f: + f.write("{}") + self.invoke_cli(cmd) + mock_get_schema_path.assert_called_with("some_other_filename") @mock.patch("nf_core.create_logo.create_logo") def test_create_logo(self, mock_create_logo): diff --git a/tests/test_schema.py b/tests/test_schema.py index 29f4921985..b9cb108fae 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -5,6 +5,7 @@ import shutil import tempfile import unittest +from pathlib import Path from unittest import mock import pytest @@ -314,9 +315,9 @@ def test_build_schema_from_scratch(self, tmp_dir): Pretty much a copy of test_launch.py test_make_pipeline_schema """ - test_pipeline_dir = os.path.join(tmp_dir, "wf") + test_pipeline_dir = Path(tmp_dir, "wf") shutil.copytree(self.template_dir, test_pipeline_dir) - os.remove(os.path.join(test_pipeline_dir, "nextflow_schema.json")) + Path(test_pipeline_dir, "nextflow_schema.json").unlink() self.schema_obj.build_schema(test_pipeline_dir, True, False, None) diff --git a/tests/test_sync.py b/tests/test_sync.py index 6f0e502e8b..b94968cd4c 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -26,7 +26,12 @@ def setUp(self): self.pipeline_dir = os.path.join(self.tmp_dir, "testpipeline") default_branch = "master" self.create_obj = nf_core.create.PipelineCreate( - "testing", "test pipeline", "tester", outdir=self.pipeline_dir, plain=True, default_branch=default_branch + "testing", + "test pipeline", + "tester", + outdir=self.pipeline_dir, + plain=True, + default_branch=default_branch, ) self.create_obj.init_pipeline() self.remote_path = os.path.join(self.tmp_dir, "remote_repo") @@ -374,7 +379,7 @@ def test_close_open_pr(self, mock_patch, mock_post): } assert psync.close_open_pr(pr) - assert mock_patch.called_once_with("url_to_update_pr") + mock_patch.assert_called_once_with(url="url_to_update_pr", data='{"state": "closed"}') @mock.patch("nf_core.utils.gh_api.post", side_effect=mocked_requests_post) @mock.patch("nf_core.utils.gh_api.patch", side_effect=mocked_requests_patch) @@ -397,7 +402,7 @@ def test_close_open_pr_fail(self, mock_patch, mock_post): } assert not psync.close_open_pr(pr) - assert mock_patch.called_once_with("bad_url_to_update_pr") + mock_patch.assert_called_once_with(url="bad_url_to_update_pr", data='{"state": "closed"}') def test_reset_target_dir(self): """Try resetting target pipeline directory"""