Skip to content

Commit

Permalink
Merge pull request #1996 from mirpedrol/unittests
Browse files Browse the repository at this point in the history
add tests for subworkflows install command
  • Loading branch information
mirpedrol committed Nov 4, 2022
2 parents d325e69 + 36df3fb commit 71556eb
Show file tree
Hide file tree
Showing 13 changed files with 195 additions and 35 deletions.
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@
- Fix bug when updating modules from old version in old folder structure
- Don't remove local copy of modules repo, only update it with fetch ([#1881](https://github.com/nf-core/tools/pull/1881))
- Add subworkflow commands create-test-yml, create and install ([#1897](https://github.com/nf-core/tools/pull/1897))
- Update subworkflows install so it installs also imported modules and subworkflows ([#1904](https://github.com/nf-core/tools/pull/1904))
- Improve test coverage of `sync.py` and `__main__.py` ([#1936](https://github.com/nf-core/tools/pull/1936), [#1965](https://github.com/nf-core/tools/pull/1965))
- `check_up_to_date()` function from `modules_json` also checks for subworkflows.
- The default branch can now be specified when creating a new pipeline repo [#1959](https://github.com/nf-core/tools/pull/1959).
- Only warn when checking that the pipeline directory contains a `main.nf` and a `nextflow.config` file if the pipeline is not an nf-core pipeline [#1964](https://github.com/nf-core/tools/pull/1964)
- Add file `versions.yml` when generating `test.yml` with `nf-core modules create-test-yml` but don't check for md5sum [#1963](https://github.com/nf-core/tools/pull/1963)
Expand All @@ -32,6 +30,12 @@

- Update patch file paths if the modules directory has the old structure ([#1878](https://github.com/nf-core/tools/pull/1878))

### Subworkflows

- Add tests for subworkflows install command ([#1996](https://github.com/nf-core/tools/pull/1996))
- `check_up_to_date()` function from `modules_json` also checks for subworkflows.
- Update subworkflows install so it installs also imported modules and subworkflows ([#1904](https://github.com/nf-core/tools/pull/1904))

## [v2.6 - Tin Octopus](https://github.com/nf-core/tools/releases/tag/2.6) - [2022-10-04]

### Template
Expand Down
15 changes: 8 additions & 7 deletions nf_core/components/components_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,20 @@ def collect_and_verify_name(component_type, component, modules_repo):
return component


def check_component_installed(component_type, component, current_version, component_dir, modules_repo, force):
def check_component_installed(component_type, component, current_version, component_dir, modules_repo, force, prompt):
"""
Check that the module/subworkflow is not already installed
"""
if (current_version is not None and os.path.exists(component_dir)) and not force:
log.info(f"{component_type[:-1].title()} is already installed.")

message = "?" if component_type == "modules" else " of this subworkflow and all it's imported modules?"
force = questionary.confirm(
f"{component_type[:-1].title()} {component} is already installed. \nDo you want to force the reinstallation{message}",
style=nf_core.utils.nfcore_question_style,
default=False,
).unsafe_ask()
if prompt:
message = "?" if component_type == "modules" else " of this subworkflow and all it's imported modules?"
force = questionary.confirm(
f"{component_type[:-1].title()} {component} is already installed. \nDo you want to force the reinstallation{message}",
style=nf_core.utils.nfcore_question_style,
default=False,
).unsafe_ask()

if not force:
repo_flag = "" if modules_repo.repo_path == NF_CORE_MODULES_NAME else f"-g {modules_repo.remote_url} "
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def install(self, module, silent=False):

# Check that the module is not already installed
if not nf_core.components.components_install.check_component_installed(
self.component_type, module, current_version, module_dir, self.modules_repo, self.force
self.component_type, module, current_version, module_dir, self.modules_repo, self.force, self.prompt
):
return False

Expand Down
10 changes: 5 additions & 5 deletions nf_core/modules/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,9 +806,9 @@ def get_all_components(self, component_type):

return self.pipeline_components

def get_module_branch(self, module, repo_url, install_dir):
def get_component_branch(self, component_type, component, repo_url, install_dir):
"""
Gets the branch from which the module was installed
Gets the branch from which the module/subworkflow was installed
Returns:
(str): The branch name
Expand All @@ -820,14 +820,14 @@ def get_module_branch(self, module, repo_url, install_dir):
branch = (
self.modules_json["repos"]
.get(repo_url, {})
.get("modules", {})
.get(component_type, {})
.get(install_dir, {})
.get(module, {})
.get(component, {})
.get("branch")
)
if branch is None:
raise LookupError(
f"Could not find branch information for module '{Path(install_dir, module)}'."
f"Could not find branch information for component '{Path(install_dir, component)}'."
f"Please remove the 'modules.json' and rerun the command to recreate it"
)
return branch
Expand Down
4 changes: 3 additions & 1 deletion nf_core/modules/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ def patch(self, module=None):
f"The '{module_fullname}' module does not have a valid version in the 'modules.json' file. Cannot compute patch"
)
# Get the module branch and reset it in the ModulesRepo object
module_branch = self.modules_json.get_module_branch(module, self.modules_repo.remote_url, module_dir)
module_branch = self.modules_json.get_component_branch(
self.component_type, module, self.modules_repo.remote_url, module_dir
)
if module_branch != self.modules_repo.branch:
self.modules_repo.setup_branch(module_branch)

Expand Down
60 changes: 49 additions & 11 deletions nf_core/modules/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ def get_single_module_info(self, module):
log.info(f"Updating module to ({sha})")

# Check if the update branch is the same as the installation branch
current_branch = self.modules_json.get_module_branch(module, self.modules_repo.remote_url, install_dir)
current_branch = self.modules_json.get_component_branch(
self.component_type, module, self.modules_repo.remote_url, install_dir
)
new_branch = self.modules_repo.branch
if current_branch != new_branch:
log.warning(
Expand Down Expand Up @@ -361,11 +363,23 @@ def get_all_modules_info(self, branch=None):
for module_dir, module in modules:
try:
modules_info[repo_name][module_dir].append(
(module, self.sha, self.modules_json.get_module_branch(module, repo_name, module_dir))
(
module,
self.sha,
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
)
except KeyError:
modules_info[repo_name][module_dir] = [
(module, self.sha, self.modules_json.get_module_branch(module, repo_name, module_dir))
(
module,
self.sha,
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
]
elif isinstance(self.update_config[repo_name], dict):
# If it is a dict, then there are entries for individual modules or module directories
Expand All @@ -381,15 +395,19 @@ def get_all_modules_info(self, branch=None):
(
module,
custom_sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
)
except KeyError:
modules_info[repo_name][module_dir] = [
(
module,
custom_sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
]
if self.sha is not None:
Expand All @@ -409,15 +427,19 @@ def get_all_modules_info(self, branch=None):
(
module,
self.sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
)
except KeyError:
modules_info[repo_name][module_dir] = [
(
module,
self.sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
]
elif isinstance(dir_config[module], str):
Expand All @@ -428,15 +450,19 @@ def get_all_modules_info(self, branch=None):
(
module,
custom_sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
)
except KeyError:
modules_info[repo_name][module_dir] = [
(
module,
custom_sha,
self.modules_json.get_module_branch(module, repo_name, module_dir),
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
]
if self.sha is not None:
Expand All @@ -455,11 +481,23 @@ def get_all_modules_info(self, branch=None):
for module_dir, module in modules:
try:
modules_info[repo_name][module_dir].append(
(module, custom_sha, self.modules_json.get_module_branch(module, repo_name, module_dir))
(
module,
custom_sha,
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
)
except KeyError:
modules_info[repo_name][module_dir] = [
(module, custom_sha, self.modules_json.get_module_branch(module, repo_name, module_dir))
(
module,
custom_sha,
self.modules_json.get_component_branch(
self.component_type, module, repo_name, module_dir
),
)
]
if self.sha is not None:
overridden_repos.append(repo_name)
Expand Down
3 changes: 1 addition & 2 deletions nf_core/subworkflows/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import os

import yaml
from packaging.version import parse as parse_version

import nf_core
import nf_core.components.components_create
Expand Down Expand Up @@ -114,7 +113,7 @@ def create(self):
pytest_modules_yml = dict(sorted(pytest_modules_yml.items()))
with open(os.path.join(self.directory, "tests", "config", "pytest_modules.yml"), "w") as fh:
yaml.dump(pytest_modules_yml, fh, sort_keys=True, Dumper=nf_core.utils.custom_yaml_dumper())
except FileNotFoundError as e:
except FileNotFoundError:
raise UserWarning("Could not open 'tests/config/pytest_modules.yml' file!")

new_files = list(self.file_paths.values())
Expand Down
8 changes: 7 additions & 1 deletion nf_core/subworkflows/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,13 @@ def install(self, subworkflow, silent=False):

# Check that the subworkflow is not already installed
if not nf_core.components.components_install.check_component_installed(
self.component_type, subworkflow, current_version, subworkflow_dir, self.modules_repo, self.force
self.component_type,
subworkflow,
current_version,
subworkflow_dir,
self.modules_repo,
self.force,
self.prompt,
):
return False

Expand Down
5 changes: 4 additions & 1 deletion tests/modules/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@ def test_modules_install_different_branch_succeed(self):

# Verify that the branch entry was added correctly
modules_json = ModulesJson(self.pipeline_dir)
assert modules_json.get_module_branch("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_BRANCH
assert (
modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO)
== GITLAB_BRANCH_TEST_BRANCH
)
20 changes: 16 additions & 4 deletions tests/modules/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ def test_update_different_branch_single_module(self):

# Verify that the branch entry was updated correctly
modules_json = ModulesJson(self.pipeline_dir)
assert modules_json.get_module_branch("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_BRANCH
assert (
modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO)
== GITLAB_BRANCH_TEST_BRANCH
)
assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA


Expand All @@ -249,10 +252,16 @@ def test_update_different_branch_mixed_modules_main(self):

modules_json = ModulesJson(self.pipeline_dir)
# Verify that the branch entry was updated correctly
assert modules_json.get_module_branch("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_BRANCH
assert (
modules_json.get_component_branch(self.component_type, "fastp", GITLAB_URL, GITLAB_REPO)
== GITLAB_BRANCH_TEST_BRANCH
)
assert modules_json.get_module_version("fastp", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA
# MultiQC is present in both branches but should've been updated using the 'main' branch
assert modules_json.get_module_branch("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_DEFAULT_BRANCH
assert (
modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO)
== GITLAB_DEFAULT_BRANCH
)


def test_update_different_branch_mix_modules_branch_test(self):
Expand All @@ -272,7 +281,10 @@ def test_update_different_branch_mix_modules_branch_test(self):
)
assert update_obj.update()

assert modules_json.get_module_branch("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_BRANCH
assert (
modules_json.get_component_branch(self.component_type, "multiqc", GITLAB_URL, GITLAB_REPO)
== GITLAB_BRANCH_TEST_BRANCH
)
assert modules_json.get_module_version("multiqc", GITLAB_URL, GITLAB_REPO) == GITLAB_BRANCH_TEST_NEW_SHA


Expand Down
Loading

0 comments on commit 71556eb

Please sign in to comment.