Skip to content

Commit

Permalink
Merge pull request #2686 from mirpedrol/lint-ignore-files
Browse files Browse the repository at this point in the history
Linting: allow ignoring specific files when linting template_strings
  • Loading branch information
mirpedrol authored Jan 29, 2024
2 parents b6eb508 + 263b2e0 commit 93b96d0
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- Fix linting of a pipeline with patched custom module ([#2669](https://github.com/nf-core/tools/pull/2669))
- linting a pipeline also lints the installed subworkflows ([#2677](https://github.com/nf-core/tools/pull/2677))
- environment.yml name must be lowercase ([#2676](https://github.com/nf-core/tools/pull/2676))
- allow ignoring specific files when template_strings ([#2686](https://github.com/nf-core/tools/pull/2686))
- lint `nextflow.config` default values match the ones specified in `nextflow_schema.json` ([#2684](https://github.com/nf-core/tools/pull/2684))

### Modules
Expand Down
11 changes: 11 additions & 0 deletions nf_core/lint/merge_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ def merge_markers(self):
This test looks for remaining merge markers in the code, e.g.:
>>>>>>> or <<<<<<<
.. tip:: You can choose to ignore this lint tests by editing the file called
``.nf-core.yml`` in the root of your pipeline and setting the test to false:
.. code-block:: yaml
lint:
merge_markers: False
To disable this test only for specific files, you can specify a list of file paths to ignore.
For example, to ignore a pdf you added to the docs:
.. code-block:: yaml
lint:
merge_markers:
- docs/my_pdf.pdf
"""
passed = []
failed = []
Expand Down
25 changes: 24 additions & 1 deletion nf_core/lint/template_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,36 @@ def template_strings(self):
This test ignores any double-brackets prefixed with a dollar sign, such as
``${{ secrets.AWS_ACCESS_KEY_ID }}`` as these placeholders are used in GitHub Actions workflows.
.. tip:: You can choose to ignore lint test tests by editing the file called
``.nf-core.yml`` in the root of your pipeline and setting the test to false:
.. code-block:: yaml
lint:
template_strings: False
To disable this test only for specific files, you can specify a list of file paths to ignore.
For example, to ignore a pdf you added to the docs:
.. code-block:: yaml
lint:
template_strings:
- docs/my_pdf.pdf
"""
passed = []
failed = []
ignored = []
# Files that should be ignored according to the linting config
ignore_files = self.lint_config.get("template_strings", [])

# Loop through files, searching for string
num_matches = 0
for fn in self.files:
if str(fn.relative_to(self.wf_path)) in ignore_files:
ignored.append(f"Ignoring Jinja template strings in file `{fn}`")
continue
# Skip binary files
binary_ftypes = ["image", "application/java-archive"]
(ftype, encoding) = mimetypes.guess_type(fn)
Expand All @@ -41,4 +64,4 @@ def template_strings(self):
if num_matches == 0:
passed.append(f"Did not find any Jinja template strings ({len(self.files)} files)")

return {"passed": passed, "failed": failed}
return {"passed": passed, "failed": failed, "ignored": ignored}
6 changes: 3 additions & 3 deletions nf_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ def _list_files(self):
git_ls_files = subprocess.check_output(["git", "ls-files"], cwd=self.wf_path).splitlines()
self.files = []
for fn in git_ls_files:
full_fn = os.path.join(self.wf_path, fn.decode("utf-8"))
if os.path.isfile(full_fn):
full_fn = Path(self.wf_path) / fn.decode("utf-8")
if full_fn.is_file():
self.files.append(full_fn)
else:
log.debug(f"`git ls-files` returned '{full_fn}' but could not open it!")
Expand All @@ -170,7 +170,7 @@ def _list_files(self):
self.files = []
for subdir, _, files in os.walk(self.wf_path):
for fn in files:
self.files.append(os.path.join(subdir, fn))
self.files.append(Path(subdir) / fn)

def _load_pipeline_config(self):
"""Get the nextflow config for this pipeline
Expand Down
54 changes: 54 additions & 0 deletions tests/lint/template_strings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import subprocess
from pathlib import Path

import nf_core.create
import nf_core.lint


def test_template_strings(self):
"""Tests finding a template string in a file fails linting."""
new_pipeline = self._make_pipeline_copy()
# Add template string to a file
txt_file = Path(new_pipeline) / "docs" / "test.txt"
with open(txt_file, "w") as f:
f.write("my {{ template_string }}")
subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline)
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
result = lint_obj.template_strings()
print(result["failed"])
assert len(result["failed"]) == 1
assert len(result["ignored"]) == 0


def test_template_strings_ignored(self):
"""Tests ignoring template_strings"""
new_pipeline = self._make_pipeline_copy()
# Ignore template_strings test
nf_core_yml = Path(new_pipeline) / ".nf-core.yml"
with open(nf_core_yml, "w") as f:
f.write("repository_type: pipeline\nlint:\n template_strings: False")
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
lint_obj._lint_pipeline()
assert len(lint_obj.failed) == 0
assert len(lint_obj.ignored) == 1


def test_template_strings_ignore_file(self):
"""Tests ignoring template_strings file"""
new_pipeline = self._make_pipeline_copy()
# Add template string to a file
txt_file = Path(new_pipeline) / "docs" / "test.txt"
with open(txt_file, "w") as f:
f.write("my {{ template_string }}")
subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline)
# Ignore template_strings test
nf_core_yml = Path(new_pipeline) / ".nf-core.yml"
with open(nf_core_yml, "w") as f:
f.write("repository_type: pipeline\nlint:\n template_strings:\n - docs/test.txt")
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
result = lint_obj.template_strings()
assert len(result["failed"]) == 0
assert len(result["ignored"]) == 1
5 changes: 5 additions & 0 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ def test_sphinx_md_files(self):
test_nextflow_config_example_pass,
test_nextflow_config_missing_test_profile_failed,
)
from .lint.template_strings import ( # type: ignore[misc]
test_template_strings,
test_template_strings_ignore_file,
test_template_strings_ignored,
)
from .lint.version_consistency import test_version_consistency # type: ignore[misc]


Expand Down
6 changes: 3 additions & 3 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ def test_load_pipeline_config(self):
def test_list_files_git(self):
"""Test listing pipeline files using `git ls`"""
self.pipeline_obj._list_files()
assert os.path.join(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files
assert Path(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files

@with_temporary_folder
def test_list_files_no_git(self, tmpdir):
"""Test listing pipeline files without `git-ls`"""
# Create a test file in a temporary directory
tmp_fn = os.path.join(tmpdir, "testfile")
Path(tmp_fn).touch()
tmp_fn = Path(tmpdir, "testfile")
tmp_fn.touch()
pipeline_obj = nf_core.utils.Pipeline(tmpdir)
pipeline_obj._list_files()
assert tmp_fn in pipeline_obj.files
Expand Down

0 comments on commit 93b96d0

Please sign in to comment.