Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

- Support modules with `exec:` blocks ([#3633](https://github.com/nf-core/tools/pull/3633))
- feat: nf-core modules bump-version supports specifying the toolkit ([#3608](https://github.com/nf-core/tools/pull/3608))
- Lint for version captures in modules - detect hash format instead of actual YAML content ([#3676](https://github.com/nf-core/tools/pull/3676))

### Subworkflows

Expand Down
77 changes: 63 additions & 14 deletions nf_core/modules/lint/module_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,38 @@
log = logging.getLogger(__name__)


def _contains_version_hash(test_content):
"""Check if test content contains version information in hash format rather than actual YAML content."""
version_hash_patterns = [
r"\bversions\.yml:md5,[a-f0-9]{32}\b",
r"\bversions\.yml:sha[0-9]*,[a-f0-9]+\b",
]

content_str = str(test_content)

for pattern in version_hash_patterns:
if re.search(pattern, content_str):
return True

return False


def _check_snapshot_for_version_hash(snap_content, test_name):
"""Check both snapshot content and keys for version hash patterns."""
# Check test content for version hashes
if _contains_version_hash(snap_content[test_name]):
return True

# Check specific test's keys for version hashes
test_data = snap_content.get(test_name, {})
if isinstance(test_data, dict):
for key in test_data.keys():
if _contains_version_hash(str(key)):
return True
Comment on lines +40 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this block ever be run if we check the whole test content in the previous lines?
Also, I am not sure why we are checking keys, if hashes should be inside lists. Or is this applied to a different use case?


return False


def module_tests(_, module: NFCoreComponent, allow_missing: bool = False):
"""
Lint the tests of a module in ``nf-core/modules``
Expand Down Expand Up @@ -162,22 +194,39 @@ def module_tests(_, module: NFCoreComponent, allow_missing: bool = False):
snap_file,
)
)
if "versions" in str(snap_content[test_name]) or "versions" in str(snap_content.keys()):
module.passed.append(
(
"test_snap_versions",
"versions found in snapshot file",
snap_file,
if "versions" in str(snap_content[test_name]) or "versions" in str(snap_content.keys()):
module.passed.append(
(
"test_snap_versions",
"versions found in snapshot file",
snap_file,
)
)
)
else:
module.failed.append(
(
"test_snap_versions",
"versions not found in snapshot file",
snap_file,
# Check if version content contains hash instead of actual YAML content
if _check_snapshot_for_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.passed.append(
(
"test_snap_version_content",
"version information contains actual content instead of hash",
snap_file,
)
)
else:
module.failed.append(
(
"test_snap_versions",
"versions not found in snapshot file",
snap_file,
)
)
)
except json.decoder.JSONDecodeError as e:
module.failed.append(
(
Expand Down
6 changes: 5 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ build-backend = "setuptools.build_meta"
requires = ["setuptools>=40.6.0", "wheel"]

[tool.pytest.ini_options]
markers = ["datafiles: load datafiles", "integration"]
markers = [
"datafiles: load datafiles",
"integration",
"issue: mark test with related issue URL"
]
testpaths = ["tests"]
python_files = ["test_*.py"]
asyncio_mode = "auto"
Expand Down
152 changes: 140 additions & 12 deletions tests/modules/lint/test_module_tests.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
from pathlib import Path

import pytest
from git.repo import Repo

import nf_core.modules.lint
Expand Down Expand Up @@ -134,12 +135,20 @@ def test_nftest_failing_linting(self):
module_lint = nf_core.modules.lint.ModuleLint(directory=self.nfcore_modules)
module_lint.lint(print_results=False, module="kallisto/quant")

assert len(module_lint.failed) == 2, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
assert len(module_lint.failed) == 4, 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"
assert module_lint.failed[1].lint_test == "test_main_tags"
assert "kallisto/index" in module_lint.failed[1].message

# Check for expected failure types
failed_tests = [x.lint_test for x in module_lint.failed]
assert "meta_yml_valid" in failed_tests
assert "test_main_tags" in failed_tests
assert failed_tests.count("test_snap_version_content") == 2 # Should appear twice for the two version entries

# Check test_main_tags failure contains the expected message
main_tags_failures = [x for x in module_lint.failed if x.lint_test == "test_main_tags"]
assert len(main_tags_failures) == 1
assert "kallisto/index" in main_tags_failures[0].message

def test_modules_absent_version(self):
"""Test linting a nf-test module if the versions is absent in the snapshot file `"""
Expand All @@ -162,7 +171,7 @@ def test_modules_empty_file_in_snapshot(self):
"""Test linting a nf-test module with an empty file sha sum in the test snapshot, which should make it fail (if it is not a stub)"""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
content = snap_file.read_text()
snap_file.read_text()
snap["my test"]["content"][0]["0"] = "test:md5,d41d8cd98f00b204e9800998ecf8427e"

with open(snap_file, "w") as fh:
Expand All @@ -175,15 +184,11 @@ def test_modules_empty_file_in_snapshot(self):
assert len(module_lint.warned) >= 0
assert module_lint.failed[0].lint_test == "test_snap_md5sum"

# reset the file
with open(snap_file, "w") as fh:
fh.write(content)

def test_modules_empty_file_in_stub_snapshot(self):
"""Test linting a nf-test module with an empty file sha sum in the stub test snapshot, which should make it not fail"""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
content = snap_file.read_text()
snap_file.read_text()
snap["my_test_stub"] = {"content": [{"0": "test:md5,d41d8cd98f00b204e9800998ecf8427e", "versions": {}}]}

with open(snap_file, "w") as fh:
Expand All @@ -210,6 +215,129 @@ def test_modules_empty_file_in_stub_snapshot(self):

assert found_test, "test_snap_md5sum not found in passed tests"

# reset the file
@pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
def test_modules_version_snapshot_content_md5_hash(self):
"""Test linting a nf-test module with version information as MD5 hash instead of actual content, which should fail."""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
snap_file.read_text()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not needed

Suggested change
snap_file.read_text()


# Add a version entry with MD5 hash format (the old way that should be flagged)
snap["my test"]["content"][0]["versions"] = "versions.yml:md5,949da9c6297b613b50e24c421576f3f1"

with open(snap_file, "w") as fh:
fh.write(content)
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 MD5 hash instead of actual content
# Filter for only our specific 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, (
f"Expected 1 test_snap_version_content failure, got {len(version_content_failures)}"
)

@pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
def test_modules_version_snapshot_content_valid(self):
"""Test linting a nf-test module with version information as actual content, which should pass."""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
snap_file.read_text()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed?

Suggested change
snap_file.read_text()


# Add a version entry with actual content (the new way that should pass)
snap["my test"]["content"][0]["versions"] = {"ALE": {"ale": "20180904"}}

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 pass because version contains actual content
# Filter for only our specific test
version_content_failures = [x for x in module_lint.failed if x.lint_test == "test_snap_version_content"]
assert len(version_content_failures) == 0, (
f"Expected 0 test_snap_version_content failures, got {len(version_content_failures)}"
)

# Check for test_snap_version_content in passed tests
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) > 0, "test_snap_version_content not found in passed tests"

@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."""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
snap_file.read_text()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)}"
)

@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."""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
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"

# Should have test_snap_versions failure since no versions are present
version_failures = [x for x in module_lint.failed if x.lint_test == "test_snap_versions"]
assert len(version_failures) == 1, "Expected test_snap_versions failure when no versions present"

@pytest.mark.issue("https://github.com/nf-core/modules/issues/6505")
def test_modules_version_snapshot_hash_in_keys(self):
"""Test linting when version hash appears in snapshot keys rather than content."""
snap_file = self.bpipe_test_module_path / "tests" / "main.nf.test.snap"
snap = json.load(snap_file.open())
snap_file.read_text()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
snap_file.read_text()


# Create a test where version hash appears in the test keys
snap["test_with_hash_key"] = {
"content": [{"versions.yml:md5,949da9c6297b613b50e24c421576f3f1": "some_value"}],
"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 fail because version hash is in the keys
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 failure for hash in keys"
Loading