Lint for version captures in modules #3676

edmundmiller · 2025-07-11T19:28:21Z

~~Waiting on #3592 to get merged.~~

PR checklist

This comment contains a description of changes (with reason)
CHANGELOG.md is updated
If you've fixed a bug or added code that should be tested, add tests!
Documentation in docs is updated

mashehu

Will have a deeper look once the two other PRS it contains are cleared up.

CLAUDE.md

Add new lint check `test_snap_version_content` to ensure version information in test snapshots contains actual content instead of MD5/SHA hash values. This addresses the issue where version snapshots were storing hash values like "versions.yml:md5,949da9c6297b613b50e24c421576f3f1" instead of actual version content like {"ALE": {"ale": "20180904"}}. Changes: - Add version content validation in module_tests.py with regex patterns - Add comprehensive tests for both invalid (hash) and valid (content) cases - Add pytest issue marker support for linking tests to GitHub issues - Update pyproject.toml with new pytest marker configuration Fixes: nf-core/modules#6505 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…egex 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 <noreply@anthropic.com>

- Fix critical indentation issue where version checking ran outside the test loop - Remove redundant _check_version_content_format function for cleaner logic - Ensure version content validation runs for each test individually - Improve regex patterns with word boundaries for more precise hash detection - Add comprehensive test coverage for SHA hashes, mixed scenarios, and edge cases - All new tests now pass correctly after fixing the loop structure Related to: nf-core/modules#6505 Fixed in: #3676 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

codecov · 2025-08-26T19:39:53Z

Codecov Report

❌ Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.95%. Comparing base (2bc29fd) to head (1ddf9ce).
⚠️ Report is 401 commits behind head on dev.

Files with missing lines	Patch %	Lines
nf_core/modules/lint/module_tests.py	97.05%	1 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nf_core/modules/lint/module_tests.py

mirpedrol · 2025-09-02T08:14:54Z

nf_core/modules/lint/module_tests.py

+        r"\bversions\.yml:sha[0-9]*,[a-f0-9]+\b",  # SHA format with variable length
+    ]
+
+    # Convert to string only once and search efficiently


Suggested change

# Convert to string only once and search efficiently

mirpedrol · 2025-09-02T08:19:09Z

nf_core/modules/lint/module_tests.py

+                                    # 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
+                                    if _contains_version_hash(snap_content[test_name]):


From the previous if statement, looks like the version can be a key of snap_content, so we whould take this option into account too.

tests/modules/lint/test_module_tests.py

@mashehu

- Clean up AI-generated comments and verbose documentation - Improve version hash detection to handle both content and keys - Add test case for version hash in snapshot keys scenario - Update CHANGELOG.md with feature description - Preserve all existing pytest-workflow logic for separate PR Addresses feedback from @mashehu and @mirpedrol Related to: nf-core/modules#6505

mirpedrol · 2025-10-07T10:19:38Z

nf_core/modules/lint/module_tests.py

+    # 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


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?

mirpedrol · 2025-10-07T10:26:27Z

tests/modules/lint/test_module_tests.py

+        """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()


I think this is not needed

Suggested change

snap_file.read_text()

mirpedrol · 2025-10-07T10:27:14Z

tests/modules/lint/test_module_tests.py

+        """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()


not needed?

Suggested change

snap_file.read_text()

mirpedrol · 2025-10-07T10:28:18Z

tests/modules/lint/test_module_tests.py

+        """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()


Suggested change

snap_file.read_text()

mirpedrol · 2025-10-07T10:29:41Z

tests/modules/lint/test_module_tests.py

+        """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()


Suggested change

snap_file.read_text()

edmundmiller self-assigned this Jul 11, 2025

edmundmiller force-pushed the capture-versions branch from e8bcc7b to c809831 Compare July 11, 2025 19:29

edmundmiller requested review from mashehu and mirpedrol July 11, 2025 19:32

edmundmiller added this to the 3.4.0 milestone Jul 11, 2025

edmundmiller added this to nf-core infrastructure projects Jul 11, 2025

edmundmiller moved this to In Progress in nf-core infrastructure projects Jul 11, 2025

mashehu requested changes Jul 11, 2025

View reviewed changes

CLAUDE.md Outdated Show resolved Hide resolved

edmundmiller force-pushed the capture-versions branch from c809831 to 5b2a24f Compare July 14, 2025 14:09

mashehu mentioned this pull request Jul 14, 2025

Initialize Claude Code #3678

Closed

edmundmiller mentioned this pull request Jul 14, 2025

New RFC: AI generated Code nf-core/proposals#61

Open

1 task

edmundmiller requested a review from mashehu July 15, 2025 16:09

mashehu modified the milestones: 3.4.0, 3.5.0 Jul 21, 2025

edmundmiller mentioned this pull request Jul 29, 2025

Check for new version snapshotting nf-core/modules#7098

Closed

edmundmiller force-pushed the capture-versions branch from 5b2a24f to a35f9bc Compare August 26, 2025 16:01

edmundmiller force-pushed the capture-versions branch from 565e30d to 11bcdcb Compare August 26, 2025 19:31

edmundmiller marked this pull request as ready for review August 26, 2025 19:36

mashehu reviewed Sep 1, 2025

View reviewed changes

nf_core/modules/lint/module_tests.py Outdated Show resolved Hide resolved

This was referenced Sep 1, 2025

Create the topic channel RFC PR nf-core/proposals#76

Open

First pass at POC for topics update for bwamem2 and dependencies nf-core/modules#8978

Draft

mirpedrol reviewed Sep 2, 2025

View reviewed changes

edmundmiller force-pushed the capture-versions branch from 7e90309 to 1ddf9ce Compare September 27, 2025 23:56

edmundmiller requested a review from mashehu September 28, 2025 00:35

edmundmiller requested a review from mirpedrol September 28, 2025 00:35

mirpedrol mentioned this pull request Oct 1, 2025

feat: Add Claude Code hooks for automated testing and linting #3774

Draft

4 tasks

mirpedrol reviewed Oct 7, 2025

View reviewed changes

Lint for version captures in modules #3676

Are you sure you want to change the base?

Lint for version captures in modules #3676

Uh oh!

Conversation

edmundmiller commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

mashehu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

mirpedrol Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

mirpedrol Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mirpedrol Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

mirpedrol Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

mirpedrol Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

mirpedrol Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

mirpedrol Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

edmundmiller commented Jul 11, 2025 •

edited

Loading

codecov bot commented Aug 26, 2025 •

edited

Loading