Merge pull request #3592 from nf-core/refactor-module-lint-test

Refactor module lint tests

Author: edmundmiller

.gitignore

@@ -117,3 +117,6 @@ pip-wheel-metadata
 
 # Textual
 snapshot_report.html
+
+# AI
+CLAUDE.md

.vscode/settings.json

@@ -5,5 +5,12 @@
     "python.testing.pytestArgs": ["tests", "-v", "--tb=short"],
     "python.testing.autoTestDiscoverOnSaveEnabled": true,
     "python.terminal.activateEnvInCurrentTerminal": true,
-    "python.terminal.shellIntegration.enabled": true
+    "python.terminal.shellIntegration.enabled": true,
+    "[python]": {
+        "editor.defaultFormatter": "charliermarsh.ruff",
+        "editor.codeActionsOnSave": {
+            "source.fixAll": "explicit",
+            "source.organizeImports": "explicit"
+        }
+    }
 }

nf_core/utils.py

@@ -313,12 +313,18 @@ def fetch_wf_config(wf_path: Path, cache_config: bool = True) -> dict:
         cache_path = Path(cache_basedir, cache_fn)
         if cache_path.is_file() and cache_config is True:
             log.debug(f"Found a config cache, loading: {cache_path}")
-            with open(cache_path) as fh:
-                try:
+            try:
+                with open(cache_path) as fh:
                     config = json.load(fh)
-                except json.JSONDecodeError as e:
-                    raise UserWarning(f"Unable to load JSON file '{cache_path}' due to error {e}")
-            return config
+                return config
+            except json.JSONDecodeError as e:
+                # Log warning but don't raise - just regenerate the cache
+                log.warning(f"Unable to load cached JSON file '{cache_path}' due to error: {e}")
+                log.debug("Removing corrupted cache file and regenerating...")
+                try:
+                    cache_path.unlink()
+                except OSError:
+                    pass  # If we can't delete it, just continue
     log.debug("No config cache found")
 
     # Call `nextflow config`

tests/modules/lint/test_lint_utils.py

@@ -0,0 +1,35 @@
+import nf_core.modules.lint
+
+from ...test_modules import TestModules
+
+
+# A skeleton object with the passed/warned/failed list attrs
+# Use this in place of a ModuleLint object to test behaviour of
+# linting methods which don't need the full setup
+class MockModuleLint:
+    def __init__(self):
+        self.passed = []
+        self.warned = []
+        self.failed = []
+
+        self.main_nf = "main_nf"
+
+
+class TestModulesLint(TestModules):
+    """Core ModuleLint functionality tests"""
+
+    def test_modules_lint_init(self):
+        """Test ModuleLint initialization"""
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir)
+        assert module_lint.directory == self.pipeline_dir
+        assert hasattr(module_lint, "passed")
+        assert hasattr(module_lint, "warned")
+        assert hasattr(module_lint, "failed")
+
+    def test_mock_module_lint(self):
+        """Test MockModuleLint utility class"""
+        mock_lint = MockModuleLint()
+        assert isinstance(mock_lint.passed, list)
+        assert isinstance(mock_lint.warned, list)
+        assert isinstance(mock_lint.failed, list)
+        assert mock_lint.main_nf == "main_nf"

tests/modules/lint/test_main_nf.py

@@ -0,0 +1,124 @@
+import pytest
+
+import nf_core.modules.lint
+import nf_core.modules.patch
+from nf_core.modules.lint.main_nf import check_container_link_line, check_process_labels
+
+from ...test_modules import TestModules
+from .test_lint_utils import MockModuleLint
+
+
+@pytest.mark.parametrize(
+    "content,passed,warned,failed",
+    [
+        # Valid process label
+        ("label 'process_high'\ncpus 12", 1, 0, 0),
+        # Non-alphanumeric characters in label
+        ("label 'a:label:with:colons'\ncpus 12", 0, 2, 0),
+        # Conflicting labels
+        ("label 'process_high'\nlabel 'process_low'\ncpus 12", 0, 1, 0),
+        # Duplicate labels
+        ("label 'process_high'\nlabel 'process_high'\ncpus 12", 0, 2, 0),
+        # Valid and non-standard labels
+        ("label 'process_high'\nlabel 'process_extra_label'\ncpus 12", 1, 1, 0),
+        # Non-standard label only
+        ("label 'process_extra_label'\ncpus 12", 0, 2, 0),
+        # Non-standard duplicates without quotes
+        ("label process_extra_label\nlabel process_extra_label\ncpus 12", 0, 3, 0),
+        # No label found
+        ("cpus 12", 0, 1, 0),
+    ],
+)
+def test_process_labels(content, passed, warned, failed):
+    """Test process label validation"""
+    mock_lint = MockModuleLint()
+    check_process_labels(mock_lint, content.splitlines())
+
+    assert len(mock_lint.passed) == passed
+    assert len(mock_lint.warned) == warned
+    assert len(mock_lint.failed) == failed
+
+
+@pytest.mark.parametrize(
+    "content,passed,warned,failed",
+    [
+        # Single-line container definition should pass
+        ('container "quay.io/nf-core/gatk:4.4.0.0" //Biocontainers is missing a package', 2, 0, 0),
+        # Multi-line container definition should pass
+        (
+            '''container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
+                'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0':
+                'biocontainers/gatk4:4.4.0.0--py36hdfd78af_0' }"''',
+            6,
+            0,
+            0,
+        ),
+        # Space in container URL should fail
+        (
+            '''container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
+                'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0 ':
+                'biocontainers/gatk4:4.4.0.0--py36hdfd78af_0' }"''',
+            5,
+            0,
+            1,
+        ),
+        # Incorrect quoting of container string should fail
+        (
+            '''container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
+                'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0 ':
+                "biocontainers/gatk4:4.4.0.0--py36hdfd78af_0" }"''',
+            4,
+            0,
+            1,
+        ),
+    ],
+)
+def test_container_links(content, passed, warned, failed):
+    """Test container link validation"""
+    mock_lint = MockModuleLint()
+
+    for line in content.splitlines():
+        if line.strip():
+            check_container_link_line(mock_lint, line, registry="quay.io")
+
+    assert len(mock_lint.passed) == passed
+    assert len(mock_lint.warned) == warned
+    assert len(mock_lint.failed) == failed
+
+
+class TestMainNfLinting(TestModules):
+    """
+    Test main.nf linting functionality.
+
+    This class tests various aspects of main.nf file linting including:
+    - Process label validation and standards compliance
+    - Container definition syntax and URL validation
+    - Integration testing with alternative registries
+    - General module linting workflow
+    """
+
+    def setUp(self):
+        """Set up test fixtures by installing required modules"""
+        super().setUp()
+        # Install samtools/sort module for all tests in this class
+        if not self.mods_install.install("samtools/sort"):
+            self.skipTest("Could not install samtools/sort module")
+
+    def test_main_nf_lint_with_alternative_registry(self):
+        """Test main.nf linting with alternative container registry"""
+        # Test with alternative registry - should warn/fail when containers don't match the registry
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir, registry="public.ecr.aws")
+        module_lint.lint(print_results=False, module="samtools/sort")
+
+        # Alternative registry should produce warnings or failures for container mismatches
+        # since samtools/sort module likely uses biocontainers/quay.io, not public.ecr.aws
+        total_issues = len(module_lint.failed) + len(module_lint.warned)
+        assert total_issues > 0, (
+            "Expected warnings/failures when using alternative registry that doesn't match module containers"
+        )
+
+        # Test with default registry - should pass cleanly
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir)
+        module_lint.lint(print_results=False, module="samtools/sort")
+        assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) > 0

tests/modules/lint/test_meta_yml.py

@@ -0,0 +1,70 @@
+from pathlib import Path
+
+import yaml
+
+import nf_core.modules.lint
+
+from ...test_modules import TestModules
+
+
+class TestMetaYml(TestModules):
+    """Test meta.yml functionality"""
+
+    def test_modules_lint_update_meta_yml(self):
+        """update the meta.yml of a module"""
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules, fix=True)
+        module_lint.lint(print_results=False, module="bpipe/test")
+        assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) > 0
+        assert len(module_lint.warned) >= 0
+
+    def test_modules_meta_yml_incorrect_licence_field(self):
+        """Test linting a module with an incorrect Licence field in meta.yml"""
+        with open(self.bpipe_test_module_path / "meta.yml") as fh:
+            meta_yml = yaml.safe_load(fh)
+        meta_yml["tools"][0]["bpipe"]["licence"] = "[MIT]"
+        with open(
+            self.bpipe_test_module_path / "meta.yml",
+            "w",
+        ) as fh:
+            fh.write(yaml.dump(meta_yml))
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
+        assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) >= 0
+        assert len(module_lint.warned) >= 0
+        assert module_lint.failed[0].lint_test == "meta_yml_valid"
+
+    def test_modules_meta_yml_output_mismatch(self):
+        """Test linting a module with an extra entry in output fields in meta.yml compared to module.output"""
+        with open(Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "main.nf")) as fh:
+            main_nf = fh.read()
+        main_nf_new = main_nf.replace("emit: sequence_report", "emit: bai")
+        with open(Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "main.nf"), "w") as fh:
+            fh.write(main_nf_new)
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+        with open(Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "main.nf"), "w") as fh:
+            fh.write(main_nf)
+        assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) >= 0
+        assert "Module `meta.yml` does not match `main.nf`" in module_lint.failed[0].message
+
+    def test_modules_meta_yml_incorrect_name(self):
+        """Test linting a module with an incorrect name in meta.yml"""
+        with open(Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "meta.yml")) as fh:
+            meta_yml = yaml.safe_load(fh)
+        meta_yml["name"] = "bpipe/test"
+        with open(
+            Path(self.nfcore_modules, "modules", "nf-core", "bpipe", "test", "meta.yml"),
+            "w",
+        ) as fh:
+            fh.write(yaml.dump(meta_yml))
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
+        assert len(module_lint.failed) == 1, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+        assert len(module_lint.passed) >= 0
+        assert len(module_lint.warned) >= 0
+        assert module_lint.failed[0].lint_test == "meta_name"

tests/modules/lint/test_module_changes.py

@@ -0,0 +1,72 @@
+import pytest
+
+import nf_core.modules.lint
+
+from ...test_modules import TestModules
+
+
+class TestModuleChanges(TestModules):
+    """Test module_changes.py functionality"""
+
+    def setUp(self):
+        """Set up test fixtures by installing required modules"""
+        super().setUp()
+        # Install samtools/sort module for all tests in this class
+        assert self.mods_install.install("samtools/sort")
+
+    def test_module_changes_unchanged(self):
+        """Test module changes when module is unchanged"""
+        # Run lint on the unchanged module
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir)
+        module_lint.lint(print_results=False, module="samtools/sort", key=["module_changes"])
+
+        # Check that module_changes test passed (no changes detected)
+        assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
+
+        # Should have passed entries for files being up to date
+        passed_test_names = [test.lint_test for test in module_lint.passed]
+        assert "check_local_copy" in passed_test_names
+
+    def test_module_changes_modified_main_nf(self):
+        """Test module changes when main.nf is modified"""
+        # Modify the main.nf file
+        main_nf_path = self.pipeline_dir / "modules" / "nf-core" / "samtools" / "sort" / "main.nf"
+        with open(main_nf_path, "a") as fh:
+            fh.write("\n// This is a test modification\n")
+
+        # Run lint on the modified module
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir)
+        module_lint.lint(print_results=False, module="samtools/sort", key=["module_changes"])
+
+        # Check that module_changes test failed (changes detected)
+        assert len(module_lint.failed) > 0, "Expected linting to fail due to modified file"
+
+        # Should have failed entry for local copy not matching remote
+        failed_test_names = [test.lint_test for test in module_lint.failed]
+        assert "check_local_copy" in failed_test_names
+
+    def test_module_changes_modified_meta_yml(self):
+        """Test module changes when meta.yml is modified"""
+        # Modify the meta.yml file
+        meta_yml_path = self.pipeline_dir / "modules" / "nf-core" / "samtools" / "sort" / "meta.yml"
+        with open(meta_yml_path, "a") as fh:
+            fh.write("\n# This is a test comment\n")
+
+        # Run lint on the modified module
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.pipeline_dir)
+        module_lint.lint(print_results=False, module="samtools/sort", key=["module_changes"])
+
+        # Check that module_changes test failed (changes detected)
+        assert len(module_lint.failed) > 0, "Expected linting to fail due to modified file"
+
+        # Should have failed entry for local copy not matching remote
+        failed_test_names = [test.lint_test for test in module_lint.failed]
+        assert "check_local_copy" in failed_test_names
+
+    @pytest.mark.skip(reason="Patch testing requires complex setup - test framework needs improvement")
+    def test_module_changes_patched_module(self):
+        """Test module changes when module is patched"""
+        # This test would require creating a patched module which is complex
+        # in the current test framework. Skip for now until patch test infrastructure
+        # is improved.
+        pass