Skip to content

Commit

Permalink
Merge pull request #2 from KevinMenden/lint-refactoring
Browse files Browse the repository at this point in the history
Some additions
  • Loading branch information
ewels authored Dec 8, 2020
2 parents e459973 + 7228e3d commit 2986826
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 4 deletions.
2 changes: 1 addition & 1 deletion nf_core/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def _lint_pipeline(self):
"Running lint checks", total=len(self.lint_tests), func_name=self.lint_tests[0]
)
for fun_name in self.lint_tests:
if self.lint_config.get(fun_name) is False:
if self.lint_config.get(fun_name, {}) is False:
log.debug("Skipping lint test '{}'".format(fun_name))
self.ignored.append((fun_name, fun_name))
continue
Expand Down
18 changes: 17 additions & 1 deletion nf_core/lint/files_exist.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def files_exist(self):
passed = []
warned = []
failed = []
ignored = []

# NB: Should all be files, not directories
# List of lists. Passes if any of the files in the sublist are found.
Expand Down Expand Up @@ -85,6 +86,9 @@ def files_exist(self):
]
files_warn_ifexists = [".travis.yml"]

# Remove files that should be ignored according to the linting config
ignore_files = self.lint_config.get('files_exist', [])

def pf(file_path):
return os.path.join(self.wf_path, file_path)

Expand All @@ -95,30 +99,42 @@ def pf(file_path):

# Files that cause an error if they don't exist
for files in files_fail:
if any([f in ignore_files for f in files]):
continue
if any([os.path.isfile(pf(f)) for f in files]):
passed.append("File found: {}".format(self._wrap_quotes(files)))
else:
failed.append("File not found: {}".format(self._wrap_quotes(files)))

# Files that cause a warning if they don't exist
for files in files_warn:
if any([f in ignore_files for f in files]):
continue
if any([os.path.isfile(pf(f)) for f in files]):
passed.append("File found: {}".format(self._wrap_quotes(files)))
else:
warned.append("File not found: {}".format(self._wrap_quotes(files)))

# Files that cause an error if they exist
for file in files_fail_ifexists:
if file in ignore_files:
continue
if os.path.isfile(pf(file)):
failed.append("File must be removed: {}".format(self._wrap_quotes(file)))
else:
passed.append("File not found check: {}".format(self._wrap_quotes(file)))

# Files that cause a warning if they exist
for file in files_warn_ifexists:
if file in ignore_files:
continue
if os.path.isfile(pf(file)):
warned.append("File should be removed: {}".format(self._wrap_quotes(file)))
else:
passed.append("File not found check: {}".format(self._wrap_quotes(file)))

return {"passed": passed, "warned": warned, "failed": failed}
# Files that are ignoed
for file in ignore_files:
ignored.append("File is ignored: {}".format(self._wrap_quotes(file)))

return {"passed": passed, "warned": warned, "failed": failed, "ignored": ignored}
16 changes: 15 additions & 1 deletion nf_core/lint/nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def nextflow_config(self):
passed = []
warned = []
failed = []
ignored = []

# Fail tests if these are missing
config_fail = [
Expand Down Expand Up @@ -52,21 +53,31 @@ def nextflow_config(self):
"params.igenomesIgnore",
]

# Remove field that should be ignored according to the linting config
ignore_configs = self.lint_config.get('nextflow_config', [])


for cfs in config_fail:
for cf in cfs:
if cf in ignore_configs:
continue
if cf in self.nf_config.keys():
passed.append("Config variable found: {}".format(self._wrap_quotes(cf)))
break
else:
failed.append("Config variable not found: {}".format(self._wrap_quotes(cfs)))
for cfs in config_warn:
for cf in cfs:
if cf in ignore_configs:
continue
if cf in self.nf_config.keys():
passed.append("Config variable found: {}".format(self._wrap_quotes(cf)))
break
else:
warned.append("Config variable not found: {}".format(self._wrap_quotes(cfs)))
for cf in config_fail_ifdefined:
if cf in ignore_configs:
continue
if cf not in self.nf_config.keys():
passed.append("Config variable (correctly) not found: {}".format(self._wrap_quotes(cf)))
else:
Expand Down Expand Up @@ -183,4 +194,7 @@ def nextflow_config(self):
self.nf_config["manifest.version"]
)
)
return {"passed": passed, "warned": warned, "failed": failed}

for config in ignore_configs:
ignored.append("Config ignored: {}".format(self._wrap_quotes(config)))
return {"passed": passed, "warned": warned, "failed": failed, "ignored":ignored}
1 change: 0 additions & 1 deletion tests/lint/actions_branch_protection.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env python

import nf_core.lint
import os
import yaml
Expand Down
53 changes: 53 additions & 0 deletions tests/lint/files_exist.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#!/usr/bin/env python

import os
import yaml
import nf_core.lint

def test_files_exist_missing_config(self):
"""Lint test: critical files missing FAIL"""
new_pipeline = self._make_pipeline_copy()

os.remove(os.path.join(new_pipeline, "nextflow.config"))

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.files_exist()
assert results["failed"] == ["File not found: `nextflow.config`"]

def test_files_exist_missing_main(self):
"""Check if missing main issues warning"""
new_pipeline = self._make_pipeline_copy()

os.remove(os.path.join(new_pipeline, "main.nf"))

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.files_exist()
assert results["warned"] == ["File not found: `main.nf`"]

def test_files_exist_depreciated_file(self):
"""Check whether depreciated file issues warning"""
new_pipeline = self._make_pipeline_copy()

nf = os.path.join(new_pipeline, "parameters.settings.json")
os.system("touch {}".format(nf))

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.files_exist()
assert results["failed"] == ["File must be removed: `parameters.settings.json`"]

def test_files_exist_pass(self):
"""Lint check should pass if all files are there"""

new_pipeline = self._make_pipeline_copy()
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.files_exist()
assert results["failed"] == []

27 changes: 27 additions & 0 deletions tests/lint/licence.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env python

import os
import yaml
import nf_core.lint

def test_licence_pass(self):
"""Lint test: check a valid MIT licence"""
new_pipeline = self._make_pipeline_copy()
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.licence()
assert results["passed"] == ["Licence check passed"]

def test_licence_fail(self):
"""Lint test: invalid MIT licence"""
new_pipeline = self._make_pipeline_copy()
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

fh = open(os.path.join(new_pipeline, "LICENSE"), "a")
fh.write("[year]")
fh.close()

results = lint_obj.licence()
assert results["failed"] == ["Licence file contains placeholders: {}".format(os.path.join(new_pipeline, "LICENSE"))]
10 changes: 10 additions & 0 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ def test_sphinx_rst_files(self):
test_actions_awsfulltest_fail,
)
from lint.actions_awstest import test_actions_awstest_pass, test_actions_awstest_fail
from lint.files_exist import (
test_files_exist_missing_config,
test_files_exist_missing_main,
test_files_exist_depreciated_file,
test_files_exist_pass
)
from lint.licence import (
test_licence_pass,
test_licence_fail
)
from lint.actions_branch_protection import (
test_actions_branch_protection_pass,
test_actions_branch_protection_fail,
Expand Down

0 comments on commit 2986826

Please sign in to comment.