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

add tests for subworkflows install command #1996

Merged
merged 6 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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