From faf3a8750277c7382e52e31dcdfdc8f16bf5f698 Mon Sep 17 00:00:00 2001
From: Arthur Gymer <24782660+awgymer@users.noreply.github.com>
Date: Wed, 20 Mar 2024 15:48:51 +0000
Subject: [PATCH 01/10] fix: assertions for called_with/called_once_with were
 silently returning mocks before 3.12. Broken in 3.12.

---
 tests/test_cli.py  | 4 ++--
 tests/test_sync.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 54e420f5e4..618ef0d290 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -364,7 +364,7 @@ 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")
+        mock_get_schema_path.assert_called_with("nextflow_schema.json")
         assert "nextflow_schema.json" in result.output
 
     @mock.patch("nf_core.schema.PipelineSchema.get_schema_path")
@@ -372,7 +372,7 @@ 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")
+        mock_get_schema_path.assert_called_with("some_other_filename")
         assert "some_other_filename" in result.output
         assert "nextflow_schema.json" not in result.output
 
diff --git a/tests/test_sync.py b/tests/test_sync.py
index 6f0e502e8b..13f8559045 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -374,7 +374,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_to_update_pr")
 
     @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 +397,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("bad_url_to_update_pr")
 
     def test_reset_target_dir(self):
         """Try resetting target pipeline directory"""

From a124c1479c020ad950b50e79706313b0de549ae6 Mon Sep 17 00:00:00 2001
From: nf-core-bot <core@nf-co.re>
Date: Wed, 20 Mar 2024 16:00:39 +0000
Subject: [PATCH 02/10] [automated] Update CHANGELOG.md

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94cc5cb407..ee67744a69 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))
 
 ## [v2.13.1 - Tin Puppy Patch](https://github.com/nf-core/tools/releases/tag/2.13) - [2024-02-29]
 

From d29f2ba52d505a89abc2e8f93ba4496d649ce9ae Mon Sep 17 00:00:00 2001
From: Arthur Gymer <24782660+awgymer@users.noreply.github.com>
Date: Wed, 20 Mar 2024 16:29:41 +0000
Subject: [PATCH 03/10] Update expected assertion for mock patch calls

---
 tests/test_sync.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/test_sync.py b/tests/test_sync.py
index 13f8559045..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)
-        mock_patch.assert_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)
-        mock_patch.assert_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"""

From fb2ded4bbf57de8c5c3e3f9e97c34fc7e639ed6b Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 13:06:27 +0100
Subject: [PATCH 04/10] fix type hints

---
 nf_core/lint/__init__.py | 296 +++++++++++++++++++--------------------
 nf_core/schema.py        |   6 +-
 2 files changed, 152 insertions(+), 150 deletions(-)

diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py
index be9ac183a6..c19d107610 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.
 
@@ -409,7 +268,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",
@@ -421,7 +280,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",
@@ -433,7 +292,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",
@@ -445,7 +304,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",
@@ -457,7 +316,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",
@@ -638,3 +497,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, 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/schema.py b/nf_core/schema.py
index 4096c80f93..eabef74833 100644
--- a/nf_core/schema.py
+++ b/nf_core/schema.py
@@ -62,8 +62,10 @@ 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")
+            if self.pipeline_dir is not None:
+                self.schema_filename = Path(self.pipeline_dir, "nextflow_schema.json")
+            else:
+                self.schema_filename = None
 
         # Only looking for local paths, overwrite with None to be safe
         else:

From 4fa15142ac03aebab4f9cb0323a2fdc3411346de Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 13:59:08 +0100
Subject: [PATCH 05/10] fix tests

---
 tests/test_cli.py | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 618ef0d290..1f7368af12 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,23 @@ 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)
+        Path("nextflow_schema.json").touch()
+        self.invoke_cli(cmd)
         mock_get_schema_path.assert_called_with("nextflow_schema.json")
-        assert "nextflow_schema.json" in result.output
+
+        # clean up
+        Path("nextflow_schema.json").unlink()
 
     @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"""
+        Path("some_other_filename").touch()
         cmd = ["schema", "lint", "some_other_filename"]
-        result = self.invoke_cli(cmd)
+        self.invoke_cli(cmd)
         mock_get_schema_path.assert_called_with("some_other_filename")
-        assert "some_other_filename" in result.output
-        assert "nextflow_schema.json" not in result.output
+
+        # clean up
+        Path("some_other_filename").unlink()
 
     @mock.patch("nf_core.create_logo.create_logo")
     def test_create_logo(self, mock_create_logo):

From 016db6d9f3e84fb079258d7f1600e020e07d592b Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 15:21:15 +0100
Subject: [PATCH 06/10] try to fix the remote schema functionality

---
 nf_core/components/components_command.py |  2 +-
 nf_core/modules/modules_json.py          | 22 ++++++++--------
 nf_core/schema.py                        | 32 +++++++++++++++++++-----
 tests/test_schema.py                     |  7 +++---
 4 files changed, 42 insertions(+), 21 deletions(-)

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/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 eabef74833..a4b635c819 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():
@@ -62,17 +66,20 @@ 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:
-            if self.pipeline_dir is not None:
-                self.schema_filename = Path(self.pipeline_dir, "nextflow_schema.json")
+            self.schema_filename = path / "nextflow_schema.json"
+            if revision is not None:
+                self.load_remote_schema(
+                    f"https://raw.githubusercontent.com/nf-core/{path}/{revision}/nextflow_schema.json"
+                )
             else:
-                self.schema_filename = None
+                self.load_remote_schema(f"https://raw.githubusercontent.com/nf-core/{path}/master/nextflow_schema.json")
 
         # 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)
@@ -105,7 +112,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:
@@ -114,6 +121,19 @@ def load_schema(self):
         self.schema_params = {}
         log.debug(f"JSON file loaded: {self.schema_filename}")
 
+    def load_remote_schema(self, url):
+        """Load a pipeline schema from a remote URL"""
+        import requests
+
+        response = requests.get(url)
+        if response.status_code != 200:
+            raise AssertionError(f"Could not load schema from {url}")
+        self.schema = response.json()
+        self.schema_filename = url
+        self.schema_defaults = {}
+        self.schema_params = {}
+        log.debug(f"JSON file loaded: {self.schema_filename}")
+
     def sanitise_param_default(self, param):
         """
         Given a param, ensure that the default value is the correct variable type
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 29f4921985..5c28ee92bf 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
@@ -46,7 +47,7 @@ def test_load_lint_schema(self):
 
     def test_load_lint_schema_nofile(self):
         """Check that linting raises properly if a non-existant file is given"""
-        with pytest.raises(RuntimeError):
+        with pytest.raises(AssertionError):
             self.schema_obj.get_schema_path("fake_file")
 
     def test_load_lint_schema_notjson(self):
@@ -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)
 

From b69f328c2d441e64aa5ded97a8bd850741e1720d Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 15:24:43 +0100
Subject: [PATCH 07/10] add isolated filesystem to cli tests

---
 tests/test_cli.py | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 1f7368af12..913a4aac1d 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -363,23 +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"]
-        Path("nextflow_schema.json").touch()
-        self.invoke_cli(cmd)
-        mock_get_schema_path.assert_called_with("nextflow_schema.json")
-
-        # clean up
-        Path("nextflow_schema.json").unlink()
+        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"""
-        Path("some_other_filename").touch()
         cmd = ["schema", "lint", "some_other_filename"]
-        self.invoke_cli(cmd)
-        mock_get_schema_path.assert_called_with("some_other_filename")
-
-        # clean up
-        Path("some_other_filename").unlink()
+        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):

From 738e4e0db6a07b9c59ed6a1f0853af6e4f326eb7 Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 15:31:40 +0100
Subject: [PATCH 08/10] use correct types

---
 nf_core/lint/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py
index 1dda6a2078..2a8576d5fb 100644
--- a/nf_core/lint/__init__.py
+++ b/nf_core/lint/__init__.py
@@ -513,7 +513,7 @@ def run_linting(
     md_fn=None,
     json_fn=None,
     hide_progress: bool = False,
-) -> Tuple[PipelineLint, ComponentLint, ComponentLint | None]:
+) -> 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.

From a7e5593a62ad3e93f28ddc9adfc0a84e65cebf51 Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 18:50:34 +0100
Subject: [PATCH 09/10] revert remote changes

---
 nf_core/schema.py | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/nf_core/schema.py b/nf_core/schema.py
index a4b635c819..08c8403d85 100644
--- a/nf_core/schema.py
+++ b/nf_core/schema.py
@@ -66,13 +66,8 @@ def get_schema_path(
 
         # Path does not exist - assume a name of a remote workflow
         elif not local_only:
-            self.schema_filename = path / "nextflow_schema.json"
-            if revision is not None:
-                self.load_remote_schema(
-                    f"https://raw.githubusercontent.com/nf-core/{path}/{revision}/nextflow_schema.json"
-                )
-            else:
-                self.load_remote_schema(f"https://raw.githubusercontent.com/nf-core/{path}/master/nextflow_schema.json")
+            self.pipeline_dir = nf_core.list.get_local_wf(path, revision=revision)
+            self.schema_filename = Path(self.pipeline_dir or "", "nextflow_schema.json")
 
         # Only looking for local paths, overwrite with None to be safe
         else:
@@ -121,19 +116,6 @@ def load_schema(self):
         self.schema_params = {}
         log.debug(f"JSON file loaded: {self.schema_filename}")
 
-    def load_remote_schema(self, url):
-        """Load a pipeline schema from a remote URL"""
-        import requests
-
-        response = requests.get(url)
-        if response.status_code != 200:
-            raise AssertionError(f"Could not load schema from {url}")
-        self.schema = response.json()
-        self.schema_filename = url
-        self.schema_defaults = {}
-        self.schema_params = {}
-        log.debug(f"JSON file loaded: {self.schema_filename}")
-
     def sanitise_param_default(self, param):
         """
         Given a param, ensure that the default value is the correct variable type

From 3fa8131a30cb60d727d4dd6a9b56454fe10d1e13 Mon Sep 17 00:00:00 2001
From: mashehu <mashehu3@gmail.com>
Date: Tue, 26 Mar 2024 19:02:08 +0100
Subject: [PATCH 10/10] fix tests

---
 nf_core/schema.py    | 4 +++-
 tests/test_schema.py | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/nf_core/schema.py b/nf_core/schema.py
index 08c8403d85..a4e8ad7cdc 100644
--- a/nf_core/schema.py
+++ b/nf_core/schema.py
@@ -68,7 +68,9 @@ def get_schema_path(
         elif not local_only:
             self.pipeline_dir = nf_core.list.get_local_wf(path, revision=revision)
             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
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 5c28ee92bf..b9cb108fae 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -47,7 +47,7 @@ def test_load_lint_schema(self):
 
     def test_load_lint_schema_nofile(self):
         """Check that linting raises properly if a non-existant file is given"""
-        with pytest.raises(AssertionError):
+        with pytest.raises(RuntimeError):
             self.schema_obj.get_schema_path("fake_file")
 
     def test_load_lint_schema_notjson(self):