Skip to content
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

Linting checks test files for TODO statements as well as the main module code #1443

Merged
merged 2 commits into from
Mar 14, 2022
Merged
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 @@ -52,6 +52,7 @@
* Tweak handling of empty files when generating the test YAML ([#1376](https://github.com/nf-core/tools/issues/1376))
* Fail linting if a md5sum for an empty file is found (instead of a warning)
* Don't skip the md5 when generating a test file if an empty file is found (so that linting fails and can be manually checked)
* Linting checks test files for `TODO` statements as well as the main module code ([#1271](https://github.com/nf-core/tools/issues/1271))

## [v2.2 - Lead Liger](https://github.com/nf-core/tools/releases/tag/2.2) - [2021-12-14]

Expand Down
20 changes: 14 additions & 6 deletions nf_core/lint/pipeline_todos.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
log = logging.getLogger(__name__)


def pipeline_todos(self):
def pipeline_todos(self, root_dir=None):
"""Check for nf-core *TODO* lines.

The nf-core workflow template contains a number of comment lines to help developers
Expand All @@ -34,15 +34,19 @@ def pipeline_todos(self):
"""
passed = []
warned = []
failed = []
file_paths = []

# Pipelines don't provide a path, so use the workflow path.
# Modules run this function twice and provide a string path
if root_dir is None:
root_dir = self.wf_path

ignore = [".git"]
if os.path.isfile(os.path.join(self.wf_path, ".gitignore")):
with io.open(os.path.join(self.wf_path, ".gitignore"), "rt", encoding="latin1") as fh:
if os.path.isfile(os.path.join(root_dir, ".gitignore")):
with io.open(os.path.join(root_dir, ".gitignore"), "rt", encoding="latin1") as fh:
for l in fh:
ignore.append(os.path.basename(l.strip().rstrip("/")))
for root, dirs, files in os.walk(self.wf_path, topdown=True):
for root, dirs, files in os.walk(root_dir, topdown=True):
# Ignore files
for i_base in ignore:
i = os.path.join(root, i_base)
Expand All @@ -65,6 +69,10 @@ def pipeline_todos(self):
file_paths.append(os.path.join(root, fname))
except FileNotFoundError:
log.debug(f"Could not open file {fname} in pipeline_todos lint test")

if len(warned) == 0:
passed.append("No TODO strings found")

# HACK file paths are returned to allow usage of this function in modules/lint.py
# Needs to be refactored!
return {"passed": passed, "warned": warned, "failed": failed, "file_paths": file_paths}
return {"passed": passed, "warned": warned, "file_paths": file_paths}
18 changes: 14 additions & 4 deletions nf_core/modules/lint/module_todos.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,17 @@ def module_todos(module_lint_object, module):
Slight modification of the "nf_core.lint.pipeline_todos" function to make it work
for a single module
"""
module.wf_path = module.module_dir
results = pipeline_todos(module)
for i, warning in enumerate(results["warned"]):
module.warned.append(("module_todo", warning, results["file_paths"][i]))

# Main module directory
mod_results = pipeline_todos(None, root_dir=module.module_dir)
for i, warning in enumerate(mod_results["warned"]):
module.warned.append(("module_todo", warning, mod_results["file_paths"][i]))
for i, passed in enumerate(mod_results["passed"]):
module.passed.append(("module_todo", passed, module.module_dir))

# Module tests directory
test_results = pipeline_todos(None, root_dir=module.test_dir)
for i, warning in enumerate(test_results["warned"]):
module.warned.append(("module_todo", warning, test_results["file_paths"][i]))
for i, passed in enumerate(test_results["passed"]):
module.passed.append(("module_todo", passed, module.test_dir))