From daecc3d6265a7bbf497177f301b0bd13eb82ffae Mon Sep 17 00:00:00 2001 From: kevinmenden <kevin.menden@t-online.de> Date: Thu, 18 Feb 2021 15:51:50 +0100 Subject: [PATCH 1/3] added lint test for merge_markers --- CHANGELOG.md | 1 + docs/api/_src/lint_tests/merge_markers.rst | 4 ++ nf_core/lint/__init__.py | 2 + nf_core/lint/merge_markers.py | 43 ++++++++++++++++++++++ tests/lint/merge_markers.py | 23 ++++++++++++ tests/test_lint.py | 2 + 6 files changed, 75 insertions(+) create mode 100644 docs/api/_src/lint_tests/merge_markers.rst create mode 100644 nf_core/lint/merge_markers.py create mode 100644 tests/lint/merge_markers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f85d565059..2c2a42d22f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ ### Linting +* Added lint check for merge markers [[#321]](https://github.com/nf-core/tools/issues/321) * Added new option `--fix` to automatically correct some problems detected by linting * Added validation of default params to `nf-core schema lint` [[#823](https://github.com/nf-core/tools/issues/823)] * Added schema validation of GitHub action workflows to lint function [[#795](https://github.com/nf-core/tools/issues/795)] diff --git a/docs/api/_src/lint_tests/merge_markers.rst b/docs/api/_src/lint_tests/merge_markers.rst new file mode 100644 index 0000000000..ea5e3be84b --- /dev/null +++ b/docs/api/_src/lint_tests/merge_markers.rst @@ -0,0 +1,4 @@ +merge_markers +============== + +.. automethod:: nf_core.lint.PipelineLint.merge_markers diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index 264e115233..1fc0f9fab9 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -113,6 +113,7 @@ class PipelineLint(nf_core.utils.Pipeline): from .schema_lint import schema_lint from .schema_params import schema_params from .actions_schema_validation import actions_schema_validation + from .merge_markers import merge_markers def __init__(self, wf_path, release_mode=False, fix=()): """ Initialise linting object """ @@ -144,6 +145,7 @@ def __init__(self, wf_path, release_mode=False, fix=()): "schema_lint", "schema_params", "actions_schema_validation", + "merge_markers", ] if self.release_mode: self.lint_tests.extend(["version_consistency"]) diff --git a/nf_core/lint/merge_markers.py b/nf_core/lint/merge_markers.py new file mode 100644 index 0000000000..f8ccd8be72 --- /dev/null +++ b/nf_core/lint/merge_markers.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python + +import logging +import os +import io +import fnmatch + +log = logging.getLogger(__name__) + + +def merge_markers(self): + """Check for remaining merge markers. + + This test looks for remaining merge markers in the code, e.g.: + >>>>>>> or <<<<<<< + + + """ + passed = [] + warned = [] + failed = [] + + 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: + for l in fh: + ignore.append(os.path.basename(l.strip().rstrip("/"))) + for root, dirs, files in os.walk(self.wf_path): + # Ignore files + for i in ignore: + dirs = [d for d in dirs if not fnmatch.fnmatch(os.path.join(root, d), i)] + files = [f for f in files if not fnmatch.fnmatch(os.path.join(root, f), i)] + for fname in files: + try: + with io.open(os.path.join(root, fname), "rt", encoding="latin1") as fh: + for l in fh: + if ">>>>>>>" in l: + warned.append(f"Merge marker in `{fname}`: {l}") + if "<<<<<<<" in l: + warned.append(f"Merge marker in `{fname}`: {l}") + except FileNotFoundError: + log.debug(f"Could not open file {fname} in pipeline_todos lint test") + return {"passed": passed, "warned": warned, "failed": failed} diff --git a/tests/lint/merge_markers.py b/tests/lint/merge_markers.py new file mode 100644 index 0000000000..9bb3d9b1d1 --- /dev/null +++ b/tests/lint/merge_markers.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python + +import os +import yaml +import nf_core.lint + + +def test_merge_markers_found(self): + """Missing 'jobs' field should result in failure""" + new_pipeline = self._make_pipeline_copy() + + with open(os.path.join(new_pipeline, "main.nf"), "r") as fh: + main_nf_content = fh.read() + main_nf_content = ">>>>>>>\n" + main_nf_content + with open(os.path.join(new_pipeline, "main.nf"), "w") as fh: + fh.write(main_nf_content) + + lint_obj = nf_core.lint.PipelineLint(new_pipeline) + lint_obj._load() + + results = lint_obj.merge_markers() + assert len(results["warned"]) > 0 + assert "Merge marker in `main.nf`: >>>>>>>\n" == results["warned"][0] diff --git a/tests/test_lint.py b/tests/test_lint.py index 9c3015a9b3..6a2aadea87 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -200,6 +200,8 @@ def test_sphinx_rst_files(self): test_actions_schema_validation_missing_on, ) + from lint.merge_markers import test_merge_markers_found + # def test_critical_missingfiles_example(self): # """Tests for missing nextflow config and main.nf files""" From 4f5dfe58fe92fec55d3ea18f29eecd47001a327d Mon Sep 17 00:00:00 2001 From: kevinmenden <kevin.menden@t-online.de> Date: Fri, 19 Feb 2021 10:21:28 +0100 Subject: [PATCH 2/3] typo --- nf_core/lint/merge_markers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nf_core/lint/merge_markers.py b/nf_core/lint/merge_markers.py index f8ccd8be72..5bb43d7cf6 100644 --- a/nf_core/lint/merge_markers.py +++ b/nf_core/lint/merge_markers.py @@ -39,5 +39,5 @@ def merge_markers(self): if "<<<<<<<" in l: warned.append(f"Merge marker in `{fname}`: {l}") except FileNotFoundError: - log.debug(f"Could not open file {fname} in pipeline_todos lint test") + log.debug(f"Could not open file {fname} in merge_markers lint test") return {"passed": passed, "warned": warned, "failed": failed} From 3982b112264ac01f51c7755105d60bdb5d649ec0 Mon Sep 17 00:00:00 2001 From: Kevin Menden <kevin.menden@t-online.de> Date: Thu, 11 Mar 2021 14:47:00 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se> --- nf_core/lint/merge_markers.py | 9 +++++---- tests/lint/merge_markers.py | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/nf_core/lint/merge_markers.py b/nf_core/lint/merge_markers.py index 5bb43d7cf6..f15e76130b 100644 --- a/nf_core/lint/merge_markers.py +++ b/nf_core/lint/merge_markers.py @@ -17,7 +17,6 @@ def merge_markers(self): """ passed = [] - warned = [] failed = [] ignore = [".git"] @@ -35,9 +34,11 @@ def merge_markers(self): with io.open(os.path.join(root, fname), "rt", encoding="latin1") as fh: for l in fh: if ">>>>>>>" in l: - warned.append(f"Merge marker in `{fname}`: {l}") + failed.append(f"Merge marker in `{fname}`: {l}") if "<<<<<<<" in l: - warned.append(f"Merge marker in `{fname}`: {l}") + failed.append(f"Merge marker in `{fname}`: {l}") except FileNotFoundError: log.debug(f"Could not open file {fname} in merge_markers lint test") - return {"passed": passed, "warned": warned, "failed": failed} + if len(failed) == 0: + passed.append("No merge markers found in pipeline files") + return {"passed": passed, "failed": failed} diff --git a/tests/lint/merge_markers.py b/tests/lint/merge_markers.py index 9bb3d9b1d1..b235979485 100644 --- a/tests/lint/merge_markers.py +++ b/tests/lint/merge_markers.py @@ -19,5 +19,6 @@ def test_merge_markers_found(self): lint_obj._load() results = lint_obj.merge_markers() - assert len(results["warned"]) > 0 - assert "Merge marker in `main.nf`: >>>>>>>\n" == results["warned"][0] + assert len(results["failed"]) > 0 + assert len(results["passed"]) == 0 + assert "Merge marker in `main.nf`: >>>>>>>\n" == results["failed"][0]