-
Notifications
You must be signed in to change notification settings - Fork 214
Testing out Reftrace for regex replacement in modules #3745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ningyuxin1999
wants to merge
13
commits into
nf-core:dev
Choose a base branch
from
ningyuxin1999:reftrace
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0d8bfae
start with process label checks
ningyuxin1999 bbdfb1b
[automated] Update CHANGELOG.md
nf-core-bot 6b976de
Merge branch 'dev' into reftrace
ningyuxin1999 5cb0d4f
feat: replace regex with Reftrace for process label parsing in module…
edmundmiller 21c9eb7
fix test
ningyuxin1999 774af6b
Merge branch 'dev' into reftrace
ningyuxin1999 c67f526
Merge remote-tracking branch 'origin/reftrace' into reftrace
ningyuxin1999 9733aec
Merge branch 'dev' into reftrace
ningyuxin1999 e714260
Merge branch 'dev' into reftrace
ningyuxin1999 b3ed924
Merge branch 'dev' into reftrace
ningyuxin1999 a9a0b5e
update requirement
ningyuxin1999 d88d913
tempfile
ningyuxin1999 4de14dc
debug
ningyuxin1999 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,3 +24,4 @@ textual==5.1.1 | |
trogon | ||
pdiff | ||
ruamel.yaml | ||
reftrace |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
import os | ||
import tempfile | ||
|
||
import nf_core.modules.lint | ||
|
||
from ...test_modules import TestModules | ||
|
@@ -12,7 +15,32 @@ def __init__(self): | |
self.warned = [] | ||
self.failed = [] | ||
|
||
self.main_nf = "main_nf" | ||
# Create a temporary file with basic Nextflow process structure | ||
# that Reftrace can parse | ||
self._temp_file = tempfile.NamedTemporaryFile(mode="w", suffix=".nf", delete=False) | ||
basic_process = """process TEST_PROCESS { | ||
label 'process_high' | ||
|
||
input: | ||
path input_file | ||
|
||
output: | ||
path "output.txt" | ||
|
||
script: | ||
''' | ||
echo "test" > output.txt | ||
''' | ||
} | ||
Comment on lines
+21
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
""" | ||
self._temp_file.write(basic_process) | ||
self._temp_file.close() | ||
self.main_nf = self._temp_file.name | ||
|
||
def cleanup(self): | ||
"""Clean up the temporary file""" | ||
if hasattr(self, "_temp_file") and os.path.exists(self._temp_file.name): | ||
os.unlink(self._temp_file.name) | ||
|
||
|
||
class TestModulesLint(TestModules): | ||
|
@@ -32,4 +60,4 @@ def test_mock_module_lint(self): | |
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" | ||
assert mock_lint.main_nf == mock_lint._temp_file.name |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import pytest | ||
from reftrace import Module, ParseError | ||
|
||
import nf_core.modules.lint | ||
import nf_core.modules.patch | ||
|
@@ -7,36 +8,104 @@ | |
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), | ||
# ], | ||
# ) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"content,passed,warned,failed", | ||
"label_content,passed,warned,failed", | ||
[ | ||
# Valid process label | ||
("label 'process_high'\ncpus 12", 1, 0, 0), | ||
("label 'process_high'", 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), | ||
("label 'a:label:with:colons'", 0, 2, 0), | ||
# Conflicting labels (multiple label lines) | ||
("label 'process_low'\nlabel 'process_high'", 0, 1, 0), | ||
# Duplicate labels | ||
("label 'process_high'\nlabel 'process_high'\ncpus 12", 0, 2, 0), | ||
("label 'process_high'\nlabel 'process_high'", 0, 2, 0), | ||
# Valid and non-standard labels | ||
("label 'process_high'\nlabel 'process_extra_label'\ncpus 12", 1, 1, 0), | ||
("label 'process_high'\nlabel 'process_extra_label'", 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), | ||
("label 'process_extra_label'", 0, 2, 0), | ||
# Duplicate non-standard labels | ||
("label 'process_extra_label'\nlabel 'process_extra_label'", 0, 3, 0), | ||
# No label found | ||
("cpus 12", 0, 1, 0), | ||
("cpus 2", 0, 1, 0), | ||
], | ||
) | ||
def test_process_labels(content, passed, warned, failed): | ||
def test_process_labels(label_content, passed, warned, failed): | ||
"""Test process label validation""" | ||
mock_lint = MockModuleLint() | ||
check_process_labels(mock_lint, content.splitlines()) | ||
# Create a temporary file with the specific label content | ||
import os | ||
import tempfile | ||
|
||
# Create proper Nextflow content with the label | ||
process_content = f"""process TEST_PROCESS {{ | ||
{label_content} | ||
input: | ||
path input_file | ||
output: | ||
path "output.txt" | ||
script: | ||
''' | ||
echo "test" > output.txt | ||
''' | ||
}} | ||
""" | ||
|
||
temp_file = tempfile.NamedTemporaryFile(mode="w", suffix=".nf", delete=False) | ||
temp_file.write(process_content) | ||
temp_file.close() | ||
|
||
try: | ||
# Create MockModuleLint but override with our specific test file | ||
mock_lint = MockModuleLint() | ||
mock_lint.cleanup() # Clean up the default temp file | ||
mock_lint.main_nf = temp_file.name | ||
|
||
# Parse with Reftrace | ||
module = Module.from_file(temp_file.name) | ||
assert not isinstance(module, ParseError), f"Failed to parse test file: {module}" | ||
# Run the check_process_labels function | ||
check_process_labels(mock_lint, module) | ||
|
||
# Verify results | ||
assert len(mock_lint.passed) == passed, ( | ||
f"Expected {passed} passed tests, got {len(mock_lint.passed)}: {mock_lint.passed}" | ||
) | ||
assert len(mock_lint.warned) == warned, ( | ||
f"Expected {warned} warned tests, got {len(mock_lint.warned)}: {mock_lint.warned}" | ||
) | ||
assert len(mock_lint.failed) == failed, ( | ||
f"Expected {failed} failed tests, got {len(mock_lint.failed)}: {mock_lint.failed}" | ||
) | ||
|
||
assert len(mock_lint.passed) == passed | ||
assert len(mock_lint.warned) == warned | ||
assert len(mock_lint.failed) == failed | ||
finally: | ||
# Clean up the temporary file | ||
if os.path.exists(temp_file.name): | ||
os.unlink(temp_file.name) | ||
Comment on lines
+107
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use pathlib please |
||
|
||
|
||
@pytest.mark.parametrize( | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add type hints to the function?