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.

Sign up for GitHub

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

nf-test module/subworkflow lint #2494

Merged
merged 8 commits into from
Oct 31, 2023
Merged

nf-test module/subworkflow lint #2494

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5682b7b
fix linting for nf-test only modules
nvnieuwk Oct 25, 2023
ef18cbd
small fix for when pytest exists
nvnieuwk Oct 25, 2023
8fd8d76
improved the error messages a bit
nvnieuwk Oct 25, 2023
3a7315f
add the same logic to subwf lint
nvnieuwk Oct 25, 2023
c93e2e0
black
nvnieuwk Oct 25, 2023
f0afb7b
update changelog
nvnieuwk Oct 25, 2023
df78dfc
show warnings instead of failures for nf-test
nvnieuwk Oct 26, 2023
73831ae
fix typo
nvnieuwk Oct 26, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

- Added stub test creation to `create_test_yml` ([#2476](https://github.com/nf-core/tools/pull/2476))
- Replace ModulePatch by ComponentPatch ([#2482](https://github.com/nf-core/tools/pull/2482))
- Fixed `nf-core modules lint` to work with new module structure for nf-test ([#2494](https://github.com/nf-core/tools/pull/2494))

### Subworkflows

- Added stub test creation to `create_test_yml` ([#2476](https://github.com/nf-core/tools/pull/2476))
- Fixed `nf-core subworkflows lint` to work with new module structure for nf-test ([#2494](https://github.com/nf-core/tools/pull/2494))

### General

Expand Down
112 changes: 64 additions & 48 deletions nf_core/modules/lint/module_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,69 +19,85 @@ def module_tests(_, module):
file.

"""

nftest_testdir = os.path.join(module.component_dir, "tests")
if os.path.exists(nftest_testdir):
module.passed.append(("test_dir_exists", "nf-test test directory exists", nftest_testdir))
if os.path.exists(module.test_dir):
module.passed.append(("test_dir_exists", "Test directory exists", module.test_dir))
else:
module.failed.append(("test_dir_exists", "Test directory is missing", module.test_dir))
module.failed.append(("test_dir_exists", "nf-test directory is missing", nftest_testdir))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this throw a failure or a warning? I'm fine if we want to really push the hard shift, but I think just a warning for now might be the right direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will throw two failures right now, but yeah I agree maybe the nf-test failure should be a warning. I'll try to figure out how to do that :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allright fixed!

return

# Lint the test main.nf file
test_main_nf = os.path.join(module.test_dir, "main.nf")
if os.path.exists(test_main_nf):
pytest_main_nf = os.path.join(module.test_dir, "main.nf")
nftest_main_nf = os.path.join(module.component_dir, "tests", "main.nf.test")
if os.path.exists(nftest_main_nf):
module.passed.append(("test_main_exists", "test `main.nf.test` exists", nftest_main_nf))
elif os.path.exists(pytest_main_nf):
module.passed.append(("test_main_exists", "test `main.nf` exists", module.test_main_nf))
else:
module.failed.append(("test_main_exists", "test `main.nf` does not exist", module.test_main_nf))
module.failed.append(("test_main_exists", "test `main.nf.test` does not exist", nftest_main_nf))

# Check that entry in pytest_modules.yml exists
try:
pytest_yml_path = os.path.join(module.base_dir, "tests", "config", "pytest_modules.yml")
with open(pytest_yml_path, "r") as fh:
pytest_yml = yaml.safe_load(fh)
if module.component_name in pytest_yml.keys():
module.passed.append(("test_pytest_yml", "correct entry in pytest_modules.yml", pytest_yml_path))
else:
module.failed.append(("test_pytest_yml", "missing entry in pytest_modules.yml", pytest_yml_path))
except FileNotFoundError:
module.failed.append(("test_pytest_yml", "Could not open pytest_modules.yml file", pytest_yml_path))
if os.path.exists(pytest_main_nf):
# Check that entry in pytest_modules.yml exists
try:
pytest_yml_path = os.path.join(module.base_dir, "tests", "config", "pytest_modules.yml")
with open(pytest_yml_path, "r") as fh:
pytest_yml = yaml.safe_load(fh)
if module.component_name in pytest_yml.keys():
module.passed.append(("test_pytest_yml", "correct entry in pytest_modules.yml", pytest_yml_path))
elif os.path.exists(nftest_main_nf):
module.passed.append(
(
"test_pytest_yml",
"missing entry in pytest_modules.yml, but found nf-test test",
nftest_main_nf,
)
)
else:
module.failed.append(("test_pytest_yml", "missing entry in pytest_modules.yml", pytest_yml_path))
except FileNotFoundError:
module.failed.append(("test_pytest_yml", "Could not open pytest_modules.yml file", pytest_yml_path))

# Lint the test.yml file
try:
with open(module.test_yml, "r") as fh:
test_yml = yaml.safe_load(fh)
# Lint the test.yml file
try:
with open(module.test_yml, "r") as fh:
test_yml = yaml.safe_load(fh)

# Verify that tags are correct
all_tags_correct = True
for test in test_yml:
for tag in test["tags"]:
if not tag in [module.component_name, module.component_name.split("/")[0]]:
all_tags_correct = False
# Verify that tags are correct
all_tags_correct = True
for test in test_yml:
for tag in test["tags"]:
if not tag in [module.component_name, module.component_name.split("/")[0]]:
all_tags_correct = False

# Look for md5sums of empty files
for tfile in test.get("files", []):
if tfile.get("md5sum") == "d41d8cd98f00b204e9800998ecf8427e":
module.failed.append(
(
"test_yml_md5sum",
"md5sum for empty file found: d41d8cd98f00b204e9800998ecf8427e",
module.test_yml,
# Look for md5sums of empty files
for tfile in test.get("files", []):
if tfile.get("md5sum") == "d41d8cd98f00b204e9800998ecf8427e":
module.failed.append(
(
"test_yml_md5sum",
"md5sum for empty file found: d41d8cd98f00b204e9800998ecf8427e",
module.test_yml,
)
)
)
if tfile.get("md5sum") == "7029066c27ac6f5ef18d660d5741979a":
module.failed.append(
(
"test_yml_md5sum",
"md5sum for compressed empty file found: 7029066c27ac6f5ef18d660d5741979a",
module.test_yml,
if tfile.get("md5sum") == "7029066c27ac6f5ef18d660d5741979a":
module.failed.append(
(
"test_yml_md5sum",
"md5sum for compressed empty file found: 7029066c27ac6f5ef18d660d5741979a",
module.test_yml,
)
)
)

if all_tags_correct:
module.passed.append(("test_yml_tags", "tags adhere to guidelines", module.test_yml))
else:
module.failed.append(("test_yml_tags", "tags do not adhere to guidelines", module.test_yml))
if all_tags_correct:
module.passed.append(("test_yml_tags", "tags adhere to guidelines", module.test_yml))
else:
module.failed.append(("test_yml_tags", "tags do not adhere to guidelines", module.test_yml))

# Test that the file exists
module.passed.append(("test_yml_exists", "Test `test.yml` exists", module.test_yml))
except FileNotFoundError:
module.failed.append(("test_yml_exists", "Test `test.yml` does not exist", module.test_yml))
# Test that the file exists
module.passed.append(("test_yml_exists", "Test `test.yml` exists", module.test_yml))
except FileNotFoundError:
module.failed.append(("test_yml_exists", "Test `test.yml` does not exist", module.test_yml))
181 changes: 101 additions & 80 deletions nf_core/subworkflows/lint/subworkflow_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,103 +24,124 @@ def subworkflow_tests(_, subworkflow):
Additionally, hecks that all included components in test ``main.nf`` are specified in ``test.yml``
"""

if os.path.exists(subworkflow.test_dir):
nftest_testdir = os.path.join(subworkflow.component_dir, "tests")
if os.path.exists(nftest_testdir):
subworkflow.passed.append(("test_dir_exists", "nf-test test directory exists", nftest_testdir))
elif os.path.exists(subworkflow.test_dir):
subworkflow.passed.append(("test_dir_exists", "Test directory exists", subworkflow.test_dir))
else:
subworkflow.failed.append(("test_dir_exists", "Test directory is missing", subworkflow.test_dir))
subworkflow.failed.append(("test_dir_exists", "nf-test directory is missing", nftest_testdir))
return

# Lint the test main.nf file
test_main_nf = os.path.join(subworkflow.test_dir, "main.nf")
if os.path.exists(test_main_nf):
pytest_main_nf = os.path.join(subworkflow.test_dir, "main.nf")
nftest_main_nf = os.path.join(subworkflow.component_dir, "tests", "main.nf.test")
if os.path.exists(nftest_main_nf):
subworkflow.passed.append(("test_main_exists", "test `main.nf.test` exists", nftest_main_nf))
elif os.path.exists(pytest_main_nf):
subworkflow.passed.append(("test_main_exists", "test `main.nf` exists", subworkflow.test_main_nf))
else:
subworkflow.failed.append(("test_main_exists", "test `main.nf` does not exist", subworkflow.test_main_nf))
subworkflow.failed.append(("test_main_exists", "test `main.nf.test` does not exist", nftest_main_nf))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, warn for now, maybe toss a TODO in there.


# Check that entry in pytest_modules.yml exists
try:
pytest_yml_path = os.path.join(subworkflow.base_dir, "tests", "config", "pytest_modules.yml")
with open(pytest_yml_path, "r") as fh:
pytest_yml = yaml.safe_load(fh)
if "subworkflows/" + subworkflow.component_name in pytest_yml.keys():
subworkflow.passed.append(("test_pytest_yml", "correct entry in pytest_modules.yml", pytest_yml_path))
else:
subworkflow.failed.append(("test_pytest_yml", "missing entry in pytest_modules.yml", pytest_yml_path))
except FileNotFoundError:
subworkflow.failed.append(("test_pytest_yml", "Could not open pytest_modules.yml file", pytest_yml_path))
if os.path.exists(pytest_main_nf):
# Check that entry in pytest_modules.yml exists
try:
pytest_yml_path = os.path.join(subworkflow.base_dir, "tests", "config", "pytest_modules.yml")
with open(pytest_yml_path, "r") as fh:
pytest_yml = yaml.safe_load(fh)
if "subworkflows/" + subworkflow.component_name in pytest_yml.keys():
subworkflow.passed.append(
("test_pytest_yml", "correct entry in pytest_modules.yml", pytest_yml_path)
)
elif os.path.exists(nftest_main_nf):
subworkflow.passed.append(
(
"test_pytest_yml",
"missing entry in pytest_modules.yml, but found nf-test test",
nftest_main_nf,
)
)
else:
subworkflow.failed.append(
("test_pytest_yml", "missing entry in pytest_modules.yml", pytest_yml_path)
)
except FileNotFoundError:
subworkflow.failed.append(("test_pytest_yml", "Could not open pytest_modules.yml file", pytest_yml_path))

# Lint the test.yml file
try:
with open(subworkflow.test_yml, "r") as fh:
test_yml = yaml.safe_load(fh)
# Lint the test.yml file
try:
with open(subworkflow.test_yml, "r") as fh:
test_yml = yaml.safe_load(fh)

# Verify that tags are correct. All included components in test main.nf should be specified in test.yml so pytests run for all of them
included_components = nf_core.subworkflows.SubworkflowTestYmlBuilder.parse_module_tags(
subworkflow, subworkflow.component_dir
)
for test in test_yml:
for component in set(included_components):
if component in test["tags"]:
subworkflow.passed.append(
(
"test_yml_tags",
f"Included module/subworkflow `{component}` specified in `test.yml`",
subworkflow.test_yml,
# Verify that tags are correct. All included components in test main.nf should be specified in test.yml so pytests run for all of them
included_components = nf_core.subworkflows.SubworkflowTestYmlBuilder.parse_module_tags(
subworkflow, subworkflow.component_dir
)
for test in test_yml:
for component in set(included_components):
if component in test["tags"]:
subworkflow.passed.append(
(
"test_yml_tags",
f"Included module/subworkflow `{component}` specified in `test.yml`",
subworkflow.test_yml,
)
)
)
else:
subworkflow.failed.append(
(
"test_yml_tags",
f"Included module/subworkflow `{component}` missing in `test.yml`",
subworkflow.test_yml,
else:
subworkflow.failed.append(
(
"test_yml_tags",
f"Included module/subworkflow `{component}` missing in `test.yml`",
subworkflow.test_yml,
)
)
)
if component.startswith("subworkflows/"):
included_components += nf_core.subworkflows.SubworkflowTestYmlBuilder.parse_module_tags(
_,
Path(subworkflow.component_dir).parent.joinpath(component.replace("subworkflows/", "")),
)
included_components = list(set(included_components))
if component.startswith("subworkflows/"):
included_components += nf_core.subworkflows.SubworkflowTestYmlBuilder.parse_module_tags(
_,
Path(subworkflow.component_dir).parent.joinpath(component.replace("subworkflows/", "")),
)
included_components = list(set(included_components))

# Look for md5sums of empty files
for tfile in test.get("files", []):
if tfile.get("md5sum") == "d41d8cd98f00b204e9800998ecf8427e":
subworkflow.failed.append(
(
"test_yml_md5sum",
"md5sum for empty file found: d41d8cd98f00b204e9800998ecf8427e",
subworkflow.test_yml,
# Look for md5sums of empty files
for tfile in test.get("files", []):
if tfile.get("md5sum") == "d41d8cd98f00b204e9800998ecf8427e":
subworkflow.failed.append(
(
"test_yml_md5sum",
"md5sum for empty file found: d41d8cd98f00b204e9800998ecf8427e",
subworkflow.test_yml,
)
)
)
else:
subworkflow.passed.append(
(
"test_yml_md5sum",
"no md5sum for empty file found",
subworkflow.test_yml,
else:
subworkflow.passed.append(
(
"test_yml_md5sum",
"no md5sum for empty file found",
subworkflow.test_yml,
)
)
)
if tfile.get("md5sum") == "7029066c27ac6f5ef18d660d5741979a":
subworkflow.failed.append(
(
"test_yml_md5sum",
"md5sum for compressed empty file found: 7029066c27ac6f5ef18d660d5741979a",
subworkflow.test_yml,
if tfile.get("md5sum") == "7029066c27ac6f5ef18d660d5741979a":
subworkflow.failed.append(
(
"test_yml_md5sum",
"md5sum for compressed empty file found: 7029066c27ac6f5ef18d660d5741979a",
subworkflow.test_yml,
)
)
)
else:
subworkflow.passed.append(
(
"test_yml_md5sum",
"no md5sum for compressed empty file found",
subworkflow.test_yml,
else:
subworkflow.passed.append(
(
"test_yml_md5sum",
"no md5sum for compressed empty file found",
subworkflow.test_yml,
)
)
)

# Test that the file exists
subworkflow.passed.append(("test_yml_exists", "Test `test.yml` exists", subworkflow.test_yml))
except FileNotFoundError:
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
# Test that the file exists
subworkflow.passed.append(("test_yml_exists", "Test `test.yml` exists", subworkflow.test_yml))
except FileNotFoundError:
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
subworkflow.failed.append(("test_yml_exists", "Test `test.yml` does not exist", subworkflow.test_yml))
Loading