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

Restructure pipeline tests and move pipeline linting into subfolder #3070

Merged
merged 92 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
e238691
move pipelines tests into `pipelines` subdirectory to mirror command …
mashehu Jul 15, 2024
891d08a
move lint tests into pipeline folder
mashehu Jul 16, 2024
11130e6
make pipeline tests a subclass of TestPipeline
mashehu Jul 16, 2024
20f0ba6
move pytest.ini into pyproject.toml
mashehu Jul 16, 2024
36050fd
migrate more tests to pathilb, use commonly shared TEST_DATA_DIR var
mashehu Jul 16, 2024
fb27410
use testpipeline class in test_lint
mashehu Jul 16, 2024
1d2076b
fix tests
mashehu Jul 16, 2024
65084dd
[automated] Update CHANGELOG.md
nf-core-bot Jul 16, 2024
2a9205e
Merge branch 'dev' of github.com:nf-core/tools into move-pipeline-lin…
mashehu Jul 16, 2024
0491cf5
Merge branch 'move-pipeline-linting' of github.com:mashehu/tools into…
mashehu Jul 16, 2024
45fdfb9
more fine-grained setup steps
mashehu Jul 17, 2024
189015f
don't run list_files on every init, only needed for one linting step
mashehu Jul 17, 2024
2ddc860
create git repo with testpipeline
mashehu Jul 17, 2024
c313fed
update textual snapshots to new location
mashehu Jul 17, 2024
d656065
fix tests
mashehu Jul 17, 2024
21cdbbd
remove asyncio mode (and see what happens)
mashehu Jul 17, 2024
e0a82d5
fix refgenie tests
mashehu Jul 17, 2024
1fde6ca
add cleanup step to some download tests
mashehu Jul 17, 2024
a0f788d
start converting linting tests to new subclass structure
mashehu Jul 17, 2024
13be56f
update prettier version
mashehu Jul 17, 2024
da155ab
import linting tests correctly to avoid "module is not callable" warn…
mashehu Jul 17, 2024
4c77621
convert rest of the lint test to new subclass structure
mashehu Jul 17, 2024
fcc43fd
add more tests to CI
mashehu Jul 17, 2024
a779d12
simplify sed command
mashehu Jul 17, 2024
ca0769b
fix ci tests
mashehu Jul 17, 2024
768cfc3
find ALL test files in CI
mashehu Jul 17, 2024
3aee9cb
add pytest-asyncio
mashehu Jul 17, 2024
09b0623
convert subworkflow tests to new structure
mashehu Jul 17, 2024
f7be79d
migrate modules tests to new structure
mashehu Jul 18, 2024
d95f7f8
avoid naming collision with variable name `dir`
mashehu Jul 18, 2024
60973a1
fix ALL the mypy errors
mashehu Jul 19, 2024
61898dc
fix rich imports, remove boolean type for `installed_by`, fix more oc…
mashehu Jul 22, 2024
47229ff
fix types and tests
mashehu Jul 22, 2024
ff1a495
avoid circular import due to modules_json being importetd
mashehu Jul 24, 2024
0cf5f8b
remove broken _repr_ (breaks because not all parameters are intilaized)
mashehu Jul 25, 2024
879e808
add types
mashehu Jul 25, 2024
178146f
fix export of pydantic models
mashehu Jul 25, 2024
d0a968c
fix tests
mashehu Jul 25, 2024
33f461a
migrate to pathlib
mashehu Jul 25, 2024
1473611
change import strategy for lint tests
mashehu Jul 25, 2024
9dc61d6
migrate to pathlib
mashehu Jul 25, 2024
0a3cb5f
fix mypy linting
mashehu Jul 25, 2024
b7b45df
fix circular import
mashehu Jul 25, 2024
c6227a7
fix module command imports
mashehu Jul 25, 2024
847c683
fix: cannot import name 'NotRequired'
mashehu Jul 25, 2024
e7ac781
more dir-> directory conversions
mashehu Jul 25, 2024
5a8ae12
fix pydantic warnings
mashehu Jul 25, 2024
ac1ea23
fix notrequired not found
mashehu Jul 25, 2024
79727c0
fix pydantic type
mashehu Jul 25, 2024
ee67c5f
fix before model creation
mashehu Jul 25, 2024
dae348e
fix modules repo import error
mashehu Jul 25, 2024
b53a4b8
fix incorrect type name
mashehu Jul 26, 2024
bdbdd17
fix types
mashehu Jul 26, 2024
98c3fb0
fix type definition for older python version
mashehu Jul 26, 2024
7b883eb
fix incorrect types and missing cli option names
mashehu Jul 26, 2024
0fe3910
fix create-logo cli command
mashehu Jul 26, 2024
ce32536
add types, include review comments, add pydantic mypy plugin
mashehu Jul 26, 2024
8eeaf28
fix mypy error
mashehu Jul 26, 2024
95de96b
allow dashes in pipeilne short name (why didn't this fail before?)
mashehu Jul 26, 2024
1b6f231
Revert "allow dashes in pipeilne short name (why didn't this fail bef…
mashehu Jul 26, 2024
5186ac7
use path.walk correctly
mashehu Jul 26, 2024
8733987
Revert "use path.walk correctly"
mashehu Jul 26, 2024
1679594
switch back to os.walk()
mashehu Jul 26, 2024
3c0433e
fix sync
mashehu Jul 26, 2024
4d6930b
fix types
mashehu Jul 26, 2024
26b1cd1
set force true in sync to create a template file
mashehu Jul 29, 2024
59a3eb3
fix missing initialization in download.py
mashehu Jul 29, 2024
fec536f
Apply suggestions from code review
mashehu Jul 29, 2024
7f03f9e
fix mypy warnings
mashehu Jul 29, 2024
d2020f5
set default value for self.directory and more dir->directory switches
mashehu Jul 29, 2024
ebdb398
remove unnecessary checks
mashehu Jul 29, 2024
15a9071
remove unnecessary Path conversions
mashehu Jul 29, 2024
fa00b1b
fix types for 3.8
mashehu Jul 29, 2024
e7f06be
limit `component` to type string
mashehu Jul 29, 2024
ae7d912
handle more type warnings
mashehu Jul 29, 2024
d7587dc
fix import errors, handle outdir as string
mashehu Jul 29, 2024
5979b9d
add error message and resolve circular import
mashehu Jul 29, 2024
24a3ece
actually test installation with a correct subworkflow
mashehu Jul 29, 2024
05def59
fix list and info command
mashehu Jul 29, 2024
8de3871
exclude click from rich traceback
mashehu Jul 30, 2024
8fe4be6
give nicer message on install failure, fix install test
mashehu Jul 30, 2024
6bc7361
use self.registry
mashehu Jul 30, 2024
e4717cc
move subworkflows main_nf linting closer to the modules version
mashehu Jul 30, 2024
b022862
fix handling of missing dir in list command
mashehu Jul 30, 2024
e9bf654
remove unnecessary string conversion
mashehu Jul 30, 2024
d4c06cb
fix tests
mashehu Jul 30, 2024
d57c815
add type hints to utils functions
mashehu Jul 30, 2024
495e05a
fix refgenie tests
mashehu Jul 30, 2024
4c07bdf
fix tests
mashehu Jul 30, 2024
4f75538
update ruff
mashehu Aug 5, 2024
b83da1f
Merge branch 'dev' of github.com:nf-core/tools into move-pipeline-lin…
mashehu Aug 12, 2024
d4d2314
reset lintconfigtype
mashehu Aug 12, 2024
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: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ indent_style = unset
[**/Makefile]
indent_style = unset

[tests/__snapshots__/*]
[tests/pipelines/__snapshots__/*]
charset = unset
end_of_line = unset
insert_final_newline = unset
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
- name: List tests
id: list_tests
run: |
echo "tests=$(find tests/test_* | tac | sed 's/tests\///g' | jq -R -s -c '{test: (split("\n")[:-1])}')" >> $GITHUB_OUTPUT
echo "tests=$(find tests/**/test_* | tac | sed 's/tests\///g' | jq -R -s -c '{test: (split("\n")[:-1])}')" >> $GITHUB_OUTPUT
outputs:
tests: ${{ steps.list_tests.outputs.tests }}

Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ repos:
hooks:
- id: prettier
additional_dependencies:
- prettier@3.2.5
- prettier@3.3.3

- repo: https://github.com/editorconfig-checker/editorconfig-checker.python
rev: "2.7.3"
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

- Fix linting fail on nfcore_external_java_deps if nf_schema is used ([#2976](https://github.com/nf-core/tools/pull/2976))
- Conda module linting: Include package name in log file ([#3014](https://github.com/nf-core/tools/pull/3014))
- Rrestructure pipeline tests and move pipeline linting into subfolder ([#3070](https://github.com/nf-core/tools/pull/3070))
mashehu marked this conversation as resolved.
Show resolved Hide resolved

### Download

Expand Down
2 changes: 1 addition & 1 deletion nf_core/components/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def __init__(
)
for comp in self.get_local_components()
]
self.config = nf_core.utils.fetch_wf_config(self.dir, cache_config=True)
self.config = nf_core.utils.fetch_wf_config(Path(self.dir), cache_config=True)
else:
component_dir = Path(
self.dir,
Expand Down
8 changes: 4 additions & 4 deletions nf_core/modules/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ def __init__(self, pipeline_dir: str):
Args:
pipeline_dir (str): The pipeline directory
"""
self.dir = pipeline_dir
self.modules_dir = Path(self.dir, "modules")
self.subworkflows_dir = Path(self.dir, "subworkflows")
self.modules_json_path = Path(self.dir, "modules.json")
self.dir = Path(pipeline_dir)
self.modules_dir = self.dir / "modules"
self.subworkflows_dir = self.dir / "subworkflows"
self.modules_json_path = self.dir / "modules.json"
self.modules_json = None
self.pipeline_modules = None
self.pipeline_subworkflows = None
Expand Down
13 changes: 9 additions & 4 deletions nf_core/pipelines/create/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def __init__(
version: str = "1.0.0dev",
no_git: bool = False,
force: bool = False,
outdir: Optional[str] = None,
outdir: Optional[Union[Path, str]] = None,
template_config: Optional[Union[str, CreateConfig, Path]] = None,
organisation: str = "nf-core",
from_config_file: bool = False,
Expand Down Expand Up @@ -355,7 +355,6 @@ def render_template(self):
# Remove all unused parameters in the nextflow schema
if not self.jinja_params["igenomes"] or not self.jinja_params["nf_core_configs"]:
self.update_nextflow_schema()

if self.config.is_nfcore:
# Make a logo and save it, if it is a nf-core pipeline
self.make_pipeline_logo()
Expand All @@ -372,6 +371,8 @@ def render_template(self):
config_fn, config_yml = nf_core.utils.load_tools_config(self.outdir)
with open(config_fn, "w") as fh:
config_yml.update(template=self.config.model_dump())
# convert posix path to string for yaml dump
config_yml["template"]["outdir"] = str(config_yml["template"]["outdir"])
yaml.safe_dump(config_yml, fh)
log.debug(f"Dumping pipeline template yml to pipeline config file '{config_fn.name}'")
run_prettier_on_file(self.outdir / config_fn)
Expand Down Expand Up @@ -511,11 +512,15 @@ def fix_linting(self):
def make_pipeline_logo(self):
"""Fetch a logo for the new pipeline from the nf-core website"""
email_logo_path = Path(self.outdir) / "assets"
create_logo(text=self.jinja_params["short_name"], dir=email_logo_path, theme="light", force=self.force)
create_logo(text=self.jinja_params["short_name"], dir=email_logo_path, theme="light", force=bool(self.force))
for theme in ["dark", "light"]:
readme_logo_path = Path(self.outdir) / "docs" / "images"
create_logo(
text=self.jinja_params["short_name"], dir=readme_logo_path, width=600, theme=theme, force=self.force
text=self.jinja_params["short_name"],
dir=readme_logo_path,
width=600,
theme=theme,
force=bool(self.force),
)

def git_init_pipeline(self):
Expand Down
12 changes: 7 additions & 5 deletions nf_core/pipelines/create_logo.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ def create_logo(
force: bool = False,
) -> Path:
"""Create a logo for a pipeline."""

if not text:
raise UserWarning("Please provide the name of the text to put on the logo.")
dir = Path(dir)
Expand Down Expand Up @@ -91,11 +90,14 @@ def create_logo(
color = theme == "dark" and (250, 250, 250) or (5, 5, 5)
draw.text((110, 465), text, color, font=font)

# Crop to max width
img = img.crop((0, 0, max_width, height))
if img is not None:
# Crop to max width
img = img.crop((0, 0, max_width, height))

# Resize
img = img.resize((width, int((width / max_width) * height)))
# Resize
img = img.resize((width, int((width / max_width) * height)))
else:
log.error("Failed to create logo, no image object created.")

# Save to cache
Path(cache_path.parent).mkdir(parents=True, exist_ok=True)
Expand Down
9 changes: 5 additions & 4 deletions nf_core/pipelines/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import tarfile
import textwrap
from datetime import datetime
from pathlib import Path
from typing import List, Optional, Tuple
from zipfile import ZipFile

Expand Down Expand Up @@ -230,7 +231,7 @@ def download_workflow(self):
summary_log.append(f"Enabled for Seqera Platform: '{self.platform}'")

# Check that the outdir doesn't already exist
if os.path.exists(self.outdir):
if self.outdir is not None and os.path.exists(self.outdir):
if not self.force:
raise DownloadError(
f"Output directory '{self.outdir}' already exists (use [red]--force[/] to overwrite)"
Expand Down Expand Up @@ -697,7 +698,7 @@ def wf_use_local_configs(self, revision_dirname):
with open(nfconfig_fn, "w") as nfconfig_fh:
nfconfig_fh.write(nfconfig)

def find_container_images(self, workflow_directory):
def find_container_images(self, workflow_directory: str) -> None:
"""Find container image names for workflow.

Starts by using `nextflow config` to pull out any process.container
Expand All @@ -716,7 +717,7 @@ def find_container_images(self, workflow_directory):
module_findings = []

# Use linting code to parse the pipeline nextflow config
self.nf_config = nf_core.utils.fetch_wf_config(workflow_directory)
self.nf_config = nf_core.utils.fetch_wf_config(Path(workflow_directory))

# Find any config variables that look like a container
for k, v in self.nf_config.items():
Expand Down Expand Up @@ -1007,7 +1008,7 @@ def gather_registries(self, workflow_directory: str) -> None:

# should exist, because find_container_images() is always called before
if not self.nf_config:
self.nf_config = nf_core.utils.fetch_wf_config(workflow_directory)
self.nf_config = nf_core.utils.fetch_wf_config(Path(workflow_directory))

# Select registries defined in pipeline config
configured_registries = [
Expand Down
5 changes: 3 additions & 2 deletions nf_core/pipelines/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import re
import subprocess
import webbrowser
from pathlib import Path

import questionary
from rich.console import Console
Expand Down Expand Up @@ -46,7 +47,7 @@ def __init__(
self.schema_obj = None
self.use_params_file = False if command_only else True
self.params_in = params_in
self.params_out = params_out if params_out else os.path.join(os.getcwd(), "nf-params.json")
self.params_out = params_out if params_out else Path.cwd() / "nf-params.json"
self.save_all = save_all
self.show_hidden = show_hidden
self.web_schema_launch_url = url if url else "https://nf-co.re/launch"
Expand Down Expand Up @@ -697,7 +698,7 @@ def build_command(self):
# Write the user selection to a file and run nextflow with that
if self.use_params_file:
dump_json_with_prettier(self.params_out, self.schema_obj.input_params)
self.nextflow_cmd += f' -params-file "{os.path.relpath(self.params_out)}"'
self.nextflow_cmd += f' -params-file "{Path(self.params_out)}"'

# Call nextflow with a list of command line flags
else:
Expand Down
76 changes: 48 additions & 28 deletions nf_core/pipelines/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,29 @@
from nf_core.utils import plural_s as _s
from nf_core.utils import strip_ansi_codes

from .actions_awsfulltest import actions_awsfulltest
from .actions_awstest import actions_awstest
from .actions_ci import actions_ci
from .actions_schema_validation import actions_schema_validation
from .configs import base_config, modules_config
from .files_exist import files_exist
from .files_unchanged import files_unchanged
from .merge_markers import merge_markers
from .modules_json import modules_json
from .modules_structure import modules_structure
from .multiqc_config import multiqc_config
from .nextflow_config import nextflow_config
from .nfcore_yml import nfcore_yml
from .pipeline_name_conventions import pipeline_name_conventions
from .pipeline_todos import pipeline_todos
from .readme import readme
from .schema_description import schema_description
from .schema_lint import schema_lint
from .schema_params import schema_params
from .system_exit import system_exit
from .template_strings import template_strings
from .version_consistency import version_consistency

log = logging.getLogger(__name__)


Expand All @@ -52,32 +75,30 @@ class PipelineLint(nf_core.utils.Pipeline):
warned (list): A list of tuples of the form: ``(<warned no>, <reason>)``
"""

from .actions_awsfulltest import actions_awsfulltest # type: ignore[misc]
from .actions_awstest import actions_awstest # type: ignore[misc]
from .actions_ci import actions_ci # type: ignore[misc]
from .actions_schema_validation import ( # type: ignore[misc]
actions_schema_validation,
)
from .configs import base_config, modules_config # type: ignore[misc]
from .files_exist import files_exist # type: ignore[misc]
from .files_unchanged import files_unchanged # type: ignore[misc]
from .merge_markers import merge_markers # type: ignore[misc]
from .modules_json import modules_json # type: ignore[misc]
from .modules_structure import modules_structure # type: ignore[misc]
from .multiqc_config import multiqc_config # type: ignore[misc]
from .nextflow_config import nextflow_config # type: ignore[misc]
from .nfcore_yml import nfcore_yml # type: ignore[misc]
from .pipeline_name_conventions import ( # type: ignore[misc]
pipeline_name_conventions,
)
from .pipeline_todos import pipeline_todos # type: ignore[misc]
from .readme import readme # type: ignore[misc]
from .schema_description import schema_description # type: ignore[misc]
from .schema_lint import schema_lint # type: ignore[misc]
from .schema_params import schema_params # type: ignore[misc]
from .system_exit import system_exit # type: ignore[misc]
from .template_strings import template_strings # type: ignore[misc]
from .version_consistency import version_consistency # type: ignore[misc]
# Import all linting tests as methods for this class
actions_awsfulltest = actions_awsfulltest
mashehu marked this conversation as resolved.
Show resolved Hide resolved
actions_awstest = actions_awstest
actions_ci = actions_ci
actions_schema_validation = actions_schema_validation
base_config = base_config
modules_config = modules_config
files_exist = files_exist
files_unchanged = files_unchanged
merge_markers = merge_markers
modules_json = modules_json
modules_structure = modules_structure
multiqc_config = multiqc_config
nextflow_config = nextflow_config
nfcore_yml = nfcore_yml
pipeline_name_conventions = pipeline_name_conventions
pipeline_todos = pipeline_todos
readme = readme
schema_description = schema_description
schema_lint = schema_lint
schema_params = schema_params
system_exit = system_exit
template_strings = template_strings
version_consistency = version_consistency

def __init__(
self, wf_path, release_mode=False, fix=(), key=None, fail_ignored=False, fail_warned=False, hide_progress=False
Expand Down Expand Up @@ -559,8 +580,7 @@ def run_linting(

# Load the various pipeline configs
lint_obj._load_lint_config()
lint_obj._load_pipeline_config()
lint_obj._list_files()
lint_obj.load_pipeline_config()

# Create the modules lint object
module_lint_obj = nf_core.modules.lint.ModuleLint(pipeline_dir, hide_progress=hide_progress)
Expand Down
2 changes: 1 addition & 1 deletion nf_core/pipelines/lint/files_exist.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
log = logging.getLogger(__name__)


def files_exist(self) -> Dict[str, Union[List[str], bool]]:
def files_exist(self) -> Dict[str, List[str]]:
"""Checks a given pipeline directory for required files.

Iterates through the pipeline's directory content and checks that specified
Expand Down
3 changes: 2 additions & 1 deletion nf_core/pipelines/lint/template_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,11 @@ def template_strings(self):
ignored = []
# Files that should be ignored according to the linting config
ignore_files = self.lint_config.get("template_strings", [])
files = self.list_files()

# Loop through files, searching for string
num_matches = 0
for fn in self.files:
for fn in files:
if str(fn.relative_to(self.wf_path)) in ignore_files:
ignored.append(f"Ignoring Jinja template strings in file `{fn}`")
continue
Expand Down
2 changes: 1 addition & 1 deletion nf_core/pipelines/refgenie.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def update_config(rgc):

# Save the updated genome config
try:
with open(refgenie_genomes_config_file, "w+") as fh:
with open(str(refgenie_genomes_config_file), "w+") as fh:
fh.write(refgenie_genomes)
log.info(f"Updated nf-core genomes config: {refgenie_genomes_config_file}")
except FileNotFoundError:
Expand Down
5 changes: 3 additions & 2 deletions nf_core/pipelines/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os
import re
import shutil
from pathlib import Path

import git
import questionary
Expand Down Expand Up @@ -69,7 +70,7 @@ def __init__(
):
"""Initialise syncing object"""

self.pipeline_dir = os.path.abspath(pipeline_dir)
self.pipeline_dir = Path(pipeline_dir).resolve()
self.from_branch = from_branch
self.original_branch = None
self.original_merge_branch = f"nf-core-template-merge-{nf_core.__version__}"
Expand Down Expand Up @@ -209,7 +210,7 @@ def get_wf_config(self):

# Fetch workflow variables
log.debug("Fetching workflow config variables")
self.wf_config = nf_core.utils.fetch_wf_config(self.pipeline_dir)
self.wf_config = nf_core.utils.fetch_wf_config(Path(self.pipeline_dir))

# Check that we have the required variables
for rvar in self.required_config_vars:
Expand Down
Loading
Loading