Skip to content

Commit

Permalink
Extracted container link check, fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@fa2k
fa2k committed Oct 24, 2023
1 parent 3f3fc9f commit 9e37803
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 70 deletions.
117 changes: 63 additions & 54 deletions nf_core/modules/lint/main_nf.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,61 +315,8 @@ def check_process_section(self, lines, registry, fix_version, progress_bar):
url = urlparse(l.split("'")[0])

if l.startswith("container") or _container_type(l) == "docker" or _container_type(l) == "singularity":
# lint double quotes
if l.count('"') > 2:
self.failed.append(
(
"container_links",
f"Too many double quotes found when specifying container: {l.lstrip('container ')}",
self.main_nf,
)
)
else:
self.passed.append(
(
"container_links",
f"Correct number of double quotes found when specifying container: {l.lstrip('container ')}",
self.main_nf,
)
)
check_container_link_line(self, raw_line, registry)

# Check for spaces in url
single_quoted_items = raw_line.split("'")
double_quoted_items = raw_line.split('"')
# Look for container link as single item surrounded by quotes
# (if there are multiple links, this will be warned in the next check)
container_link = None
if len(single_quoted_items) == 3:
container_link = single_quoted_items[1]
elif len(double_quoted_items) == 3:
container_link = double_quoted_items[1]
if container_link:
if " " in container_link:
self.failed.append(
(
"container_links",
f"Space character found in container: {container_link}",
self.main_nf,
)
)
else:
self.passed.append(
(
"container_links",
f"No space characters found in container: {container_link}",
self.main_nf,
)
)

# lint more than one container in the same line
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or l.startswith(registry)):
self.warned.append(
(
"container_links",
"Docker and Singularity containers specified in the same line. Only first one checked.",
self.main_nf,
)
)
# Try to connect to container URLs
if url is None:
continue
Expand Down Expand Up @@ -512,6 +459,68 @@ def check_process_labels(self, lines):
self.warned.append(("process_standard_label", "Process label not specified", self.main_nf))


def check_container_link_line(self, raw_line, registry):
"""Look for common problems in the container name / URL, for docker and singularity."""

l = raw_line.strip(" \n'\"}:")

# lint double quotes
if l.count('"') > 2:
self.failed.append(
(
"container_links",
f"Too many double quotes found when specifying container: {l.lstrip('container ')}",
self.main_nf,
)
)
else:
self.passed.append(
(
"container_links",
f"Correct number of double quotes found when specifying container: {l.lstrip('container ')}",
self.main_nf,
)
)

# Check for spaces in url
single_quoted_items = raw_line.split("'")
double_quoted_items = raw_line.split('"')
# Look for container link as single item surrounded by quotes
# (if there are multiple links, this will be warned in the next check)
container_link = None
if len(single_quoted_items) == 3:
container_link = single_quoted_items[1]
elif len(double_quoted_items) == 3:
container_link = double_quoted_items[1]
if container_link:
if " " in container_link:
self.failed.append(
(
"container_links",
f"Space character found in container: '{container_link}'",
self.main_nf,
)
)
else:
self.passed.append(
(
"container_links",
f"No space characters found in container: '{container_link}'",
self.main_nf,
)
)

# lint more than one container in the same line
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or l.startswith(registry)):
self.warned.append(
(
"container_links",
"Docker and Singularity containers specified in the same line. Only first one checked.",
self.main_nf,
)
)


def _parse_input(self, line_raw):
"""
Return list of input channel names from an input line.
Expand Down
39 changes: 23 additions & 16 deletions tests/modules/lint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,49 +236,49 @@ def test_modules_lint_check_process_labels(self):
# Test cases for linting the container definitions

CONTAINER_SINGLE_GOOD = (
"Single-line container definition should pass",
"""
//Conda is not supported at the moment: https://github.com/broadinstitute/gatk/issues/7811
container "quay.io/nf-core/gatk:4.4.0.0" //Biocontainers is missing a package
""",
1, # passed
2, # passed
0, # warned
0, # failed
)

CONTAINER_TWO_LINKS_GOOD = (
"Multi-line container definition should pass",
"""
conda "bioconda::gatk4=4.4.0.0"
container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0':
'biocontainers/gatk4:4.4.0.0--py36hdfd78af_0' }"
""",
1,
6,
0,
0,
)

CONTAINER_WITH_SPACE_BAD = (
"Space in container URL should fail",
"""
conda "bioconda::gatk4=4.4.0.0"
container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0 ':
'biocontainers/gatk4:4.4.0.0--py36hdfd78af_0' }"
""",
5,
0,
1,
0,
)

CONTAINER_MULTIPLE_DBLQUOTES_BAD = (
"Incorrect quoting of container string should fail",
"""
conda "bioconda::gatk4=4.4.0.0"
container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
'https://depot.galaxyproject.org/singularity/gatk4:4.4.0.0--py36hdfd78af_0 ':
"biocontainers/gatk4:4.4.0.0--py36hdfd78af_0" }"
""",
4,
0,
1,
0,
)

CONTAINER_TEST_CASES = [
Expand All @@ -289,13 +289,20 @@ def test_modules_lint_check_process_labels(self):
]


def test_modules_lint_check_process_labels(self):
def test_modules_lint_check_url_spaces(self):
for test_case in CONTAINER_TEST_CASES:
process, passed, warned, failed = test_case
test, process, passed, warned, failed = test_case
mocked_ModuleLint = MockModuleLint()
main_nf.check_process_section(
mocked_ModuleLint, process.splitlines(), registry="quay.io", fix_version=False, progress_bar=False
)
assert len(mocked_ModuleLint.passed) == passed
assert len(mocked_ModuleLint.warned) == warned
assert len(mocked_ModuleLint.failed) == failed
for line in process.splitlines():
if line.strip():
main_nf.check_container_link_line(mocked_ModuleLint, line, registry="quay.io")

assert (
len(mocked_ModuleLint.passed) == passed
), f"{test}: Expected {passed} PASS, got {len(mocked_ModuleLint.passed)}."
assert (
len(mocked_ModuleLint.warned) == warned
), f"{test}: Expected {warned} WARN, got {len(mocked_ModuleLint.warned)}."
assert (
len(mocked_ModuleLint.failed) == failed
), f"{test}: Expected {failed} FAIL, got {len(mocked_ModuleLint.failed)}."
1 change: 1 addition & 0 deletions tests/test_modules.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ def test_modulesrepo_class(self):
)
from .modules.lint import (
test_modules_lint_check_process_labels,
test_modules_lint_check_url_spaces,
test_modules_lint_empty,
test_modules_lint_gitlab_modules,
test_modules_lint_multiple_remotes,
Expand Down

0 comments on commit 9e37803

Please sign in to comment.