Lint for version captures in modules #3676
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
edmundmiller
force-pushed
the
capture-versions
branch
from
e8bcc7b to
c809831
Compare
mashehu
requested changes
Jul 11, 2025
edmundmiller
force-pushed
the
capture-versions
branch
from
c809831 to
5b2a24f
Compare
Closed
1 task
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>
edmundmiller
force-pushed
the
capture-versions
branch
from
5b2a24f to
a35f9bc
Compare
…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>
edmundmiller
added a commit
that referenced
this pull request
Aug 26, 2025
- 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>
- 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>
edmundmiller
force-pushed
the
capture-versions
branch
from
565e30d to
11bcdcb
Compare
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mashehu
reviewed
Sep 1, 2025
This was referenced Sep 1, 2025
mirpedrol
reviewed
Sep 2, 2025
nf_core/modules/lint/module_tests.py
Outdated
| r"\bversions\.yml:sha[0-9]*,[a-f0-9]+\b", # SHA format with variable length | ||
| ] | ||
|
|
||
| # Convert to string only once and search efficiently |
There was a problem hiding this comment.
Suggested change
| # Convert to string only once and search efficiently |
nf_core/modules/lint/module_tests.py
Outdated
| # 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]): |
There was a problem hiding this comment.
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.
- 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
edmundmiller
force-pushed
the
capture-versions
branch
from
7e90309 to
1ddf9ce
Compare
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Waiting on #3592 to get merged.PR checklist
CHANGELOG.mdis updateddocsis updated