diff --git a/commodore/cli/component.py b/commodore/cli/component.py index 6c816021..abfa1530 100644 --- a/commodore/cli/component.py +++ b/commodore/cli/component.py @@ -291,12 +291,7 @@ def component_group(config: Config, verbose): show_default=True, help="The URL of the component cookiecutter template.", ) -@click.option( - "--template-version", - default="main", - show_default=True, - help="The component template version (Git tree-ish) to use.", -) +@options.template_version("main") @new_update_options(new_cmd=True) @options.verbosity @options.pass_config @@ -430,6 +425,7 @@ def component_new( default=True, help="Whether to commit the rendered template changes.", ) +@options.template_version(None) @options.verbosity @options.pass_config def component_update( @@ -437,6 +433,7 @@ def component_update( verbose: int, component_path: str, copyright_holder: str, + template_version: Optional[str], golden_tests: Optional[bool], matrix_tests: Optional[bool], lib: Optional[bool], @@ -492,6 +489,8 @@ def component_update( t.automerge_patch_v0 = automerge_patch_v0 if autorelease is not None: t.autorelease = autorelease + if template_version is not None: + t.template_version = template_version test_cases = t.test_cases test_cases.extend(additional_test_case) @@ -625,6 +624,7 @@ def component_compile( @options.pr_batch_size @options.github_pause @options.dependency_filter +@options.template_version(None) def component_sync( config: Config, verbose: int, @@ -636,6 +636,7 @@ def component_sync( pr_batch_size: int, github_pause: int, filter: str, + template_version: Optional[str], ): """This command processes all components listed in the provided `COMPONENT_LIST` YAML file. @@ -672,4 +673,5 @@ def component_sync( pr_batch_size, timedelta(seconds=github_pause), filter, + template_version, ) diff --git a/commodore/cli/options.py b/commodore/cli/options.py index cd139c70..8ab77ba7 100644 --- a/commodore/cli/options.py +++ b/commodore/cli/options.py @@ -1,5 +1,7 @@ """Click options which are reused for multiple commands""" +from typing import Optional + import click from commodore.config import Config @@ -125,6 +127,22 @@ ) +def template_version(default: Optional[str]): + help_str = "The component template version (Git tree-ish) to use." + if default is None: + help_str = ( + help_str + + " If not provided, the currently active template version will be used." + ) + + return click.option( + "--template-version", + default=default, + show_default=default is not None, + help=help_str, + ) + + def local(help: str): return click.option( "--local", diff --git a/commodore/cli/package.py b/commodore/cli/package.py index 61f3958c..8edfcc2c 100644 --- a/commodore/cli/package.py +++ b/commodore/cli/package.py @@ -61,12 +61,7 @@ def package_group(config: Config, verbose: int): show_default=True, help="The URL of the package cookiecutter template.", ) -@click.option( - "--template-version", - default="main", - show_default=True, - help="The package template version (Git tree-ish) to use.", -) +@options.template_version("main") @click.option( "--output-dir", default="", @@ -167,6 +162,7 @@ def package_new( default=True, help="Whether to commit the rendered template changes.", ) +@options.template_version(None) @options.verbosity @options.pass_config # pylint: disable=too-many-arguments @@ -180,6 +176,7 @@ def package_update( additional_test_case: Iterable[str], remove_test_case: Iterable[str], commit: bool, + template_version: Optional[str], ): """This command updates the package at PACKAGE_PATH to the latest version of the template which was originally used to create it, if the template version is given as @@ -201,6 +198,9 @@ def package_update( t.golden_tests = golden_tests if update_copyright_year: t.copyright_year = None + if template_version is not None: + t.template_version = template_version + test_cases = t.test_cases test_cases.extend(additional_test_case) t.test_cases = [tc for tc in test_cases if tc not in remove_test_case] @@ -278,6 +278,7 @@ def package_compile( @options.pr_batch_size @options.github_pause @options.dependency_filter +@options.template_version(None) def package_sync( config: Config, verbose: int, @@ -289,6 +290,7 @@ def package_sync( pr_batch_size: int, github_pause: int, filter: str, + template_version: Optional[str], ): """This command processes all packages listed in the provided `PACKAGE_LIST` YAML file. @@ -324,4 +326,5 @@ def package_sync( pr_batch_size, timedelta(seconds=github_pause), filter, + template_version, ) diff --git a/commodore/dependency_syncer.py b/commodore/dependency_syncer.py index 0a200a00..d85b6ddf 100644 --- a/commodore/dependency_syncer.py +++ b/commodore/dependency_syncer.py @@ -6,7 +6,7 @@ from collections.abc import Iterable from datetime import timedelta from pathlib import Path -from typing import Union, Type +from typing import Optional, Union, Type import click import git @@ -36,11 +36,17 @@ def sync_dependencies( pr_batch_size: int = 10, pause: timedelta = timedelta(seconds=120), depfilter: str = "", + template_version: Optional[str] = None, ) -> None: if not config.github_token: raise click.ClickException("Can't continue, missing GitHub API token.") - deptype_str = deptype.__name__.lower() + if template_version is not None and not dry_run: + click.secho( + " > Custom template version provided for sync, but dry run not active. Forcing dry run", + fg="yellow", + ) + dry_run = True deps = read_dependency_list(dependency_list, depfilter) dep_count = len(deps) @@ -49,38 +55,17 @@ def sync_dependencies( # Keep track of how many PRs we've created to better avoid running into rate limits update_count = 0 for i, dn in enumerate(deps, start=1): - click.secho(f"Synchronizing {dn}", bold=True) - _, dreponame = dn.split("/") - dname = dreponame.replace(f"{deptype_str}-", "", 1) - - # Clone dependency - try: - gr = gh.get_repo(dn) - except github.UnknownObjectException: - click.secho(f" > Repository {dn} doesn't exist, skipping...", fg="yellow") - continue - - if gr.archived: - click.secho(f" > Repository {dn} is archived, skipping...", fg="yellow") - continue - - d = deptype.clone(config, gr.clone_url, dname, version=gr.default_branch) - - if not (d.target_dir / ".cruft.json").is_file(): - click.echo(f" > Skipping repo {dn} which doesn't have `.cruft.json`") - continue - - # Update the dependency - t = templater.from_existing(config, d.target_dir) - changed = t.update( - print_completion_message=False, - commit=not dry_run, - ignore_template_commit=True, + changed = sync_dependency( + config, + gh, + dn, + deptype, + templater, + template_version, + dry_run, + pr_branch, + pr_label, ) - - # Create or update PR if there were updates - comment = render_pr_comment(d) - create_or_update_pr(d, dn, gr, changed, pr_branch, pr_label, dry_run, comment) if changed: update_count += 1 if not dry_run and i < dep_count: @@ -90,6 +75,56 @@ def sync_dependencies( _maybe_pause(update_count, pr_batch_size, pause) +def sync_dependency( + config: Config, + gh: github.Github, + depname: str, + deptype: Type[Union[Component, Package]], + templater, + template_version: Optional[str], + dry_run: bool, + pr_branch: str, + pr_label: Iterable[str], +): + deptype_str = deptype.__name__.lower() + + click.secho(f"Synchronizing {depname}", bold=True) + _, dreponame = depname.split("/") + dname = dreponame.replace(f"{deptype_str}-", "", 1) + + # Clone dependency + try: + gr = gh.get_repo(depname) + except github.UnknownObjectException: + click.secho(f" > Repository {depname} doesn't exist, skipping...", fg="yellow") + return + + if gr.archived: + click.secho(f" > Repository {depname} is archived, skipping...", fg="yellow") + return + + d = deptype.clone(config, gr.clone_url, dname, version=gr.default_branch) + + if not (d.target_dir / ".cruft.json").is_file(): + click.echo(f" > Skipping repo {depname} which doesn't have `.cruft.json`") + return + + # Update the dependency + t = templater.from_existing(config, d.target_dir) + if template_version is not None: + t.template_version = template_version + changed = t.update( + print_completion_message=False, + commit=not dry_run, + ignore_template_commit=True, + ) + + # Create or update PR if there were updates + comment = render_pr_comment(d) + create_or_update_pr(d, depname, gr, changed, pr_branch, pr_label, dry_run, comment) + return changed + + def read_dependency_list(dependency_list: Path, depfilter: str) -> list[str]: try: deps = yaml_load(dependency_list) diff --git a/commodore/dependency_templater.py b/commodore/dependency_templater.py index 4a06f7d9..2f571e4f 100644 --- a/commodore/dependency_templater.py +++ b/commodore/dependency_templater.py @@ -49,12 +49,20 @@ def _ignore_cruft_json_commit_id( n=0, ) )[3:] - # If the context-less diff has exactly 2 lines and both of them start with - # '[-+] "commit":', we omit the diff if ( + # If the context-less diff has exactly 2 lines and both of them start with + # '[-+] "commit":', we omit the diff len(minimal_diff) == 2 and minimal_diff[0].startswith('- "commit":') and minimal_diff[1].startswith('+ "commit":') + ) or ( + # If the context-less diff has exactly 4 lines and the two pairs start with + # '[-+] "commit":' and '[-+] "checkout":', we omit the diff + len(minimal_diff) == 4 + and minimal_diff[0].startswith('- "commit":') + and minimal_diff[1].startswith('- "checkout":') + and minimal_diff[2].startswith('+ "commit":') + and minimal_diff[3].startswith('+ "checkout":') ): omit = True # never suppress diffs in default difffunc diff --git a/docs/modules/ROOT/pages/reference/cli.adoc b/docs/modules/ROOT/pages/reference/cli.adoc index 0af3ad2a..6008bafd 100644 --- a/docs/modules/ROOT/pages/reference/cli.adoc +++ b/docs/modules/ROOT/pages/reference/cli.adoc @@ -308,6 +308,10 @@ NOTE: If autorelease is enabled, new releases will be generated for automerged d *--commit / --no-commit*:: Whether to commit the rendered template changes. +*--template-version* TEXT:: + The component template version (Git tree-ish) to use. + If not provided, the currently active template version will be used. + *--automerge-patch / --no-automerge-patch*:: Enable automerging of patch-level dependency PRs. @@ -440,6 +444,10 @@ However, labels added by previous runs can't be removed since we've got no easy Regex to select which dependencies to sync. If the option isn't given, all dependencies listed in the provided YAML are synced. +*--template-version* TEXT:: + The component template version (Git tree-ish) to use. + If not provided, the currently active template version will be used. + == Inventory Components / Packages / Show *-f, --values*:: @@ -549,6 +557,10 @@ However, labels added by previous runs can't be removed since we've got no easy *--commit / --no-commit*:: Whether to commit the rendered template changes. +*--template-version* TEXT:: + The component template version (Git tree-ish) to use. + If not provided, the currently active template version will be used. + == Package Compile *-f, --values* FILE:: @@ -638,3 +650,7 @@ However, labels added by previous runs can't be removed since we've got no easy *--filter* REGEX:: Regex to select which dependencies to sync. If the option isn't given, all dependencies listed in the provided YAML are synced. + +*--template-version* TEXT:: + The component template version (Git tree-ish) to use. + If not provided, the currently active template version will be used. diff --git a/tests/conftest.py b/tests/conftest.py index e84fe95d..3b1e4645 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -141,3 +141,24 @@ def bare_repo(self) -> GitRepo: @pytest.fixture def mockdep(tmp_path): return MockMultiDependency(Repo.init(tmp_path / "repo.git")) + + +class MockTemplater: + def __init__(self): + self.template_version = None + self.test_cases = [] + + def update(self, *args, **kwargs): + pass + + +def make_mock_templater(mock_templater, expected_path): + mt = MockTemplater() + + def mock_from_existing(_config: Config, path: Path): + assert path == expected_path + return mt + + mock_templater.from_existing = mock_from_existing + + return mt diff --git a/tests/test_cli_component.py b/tests/test_cli_component.py index 9c4104ce..4fed1591 100644 --- a/tests/test_cli_component.py +++ b/tests/test_cli_component.py @@ -5,7 +5,7 @@ from collections.abc import Iterable from datetime import timedelta from pathlib import Path -from typing import Type +from typing import Optional, Type from unittest import mock import pytest @@ -16,7 +16,7 @@ import commodore.cli.component as component -from conftest import RunnerFunc +from conftest import RunnerFunc, make_mock_templater @pytest.mark.parametrize("repo_dir", [False, True]) @@ -50,10 +50,41 @@ def _compile(cfg, path, alias, values, search_paths, output, name): ) +@pytest.mark.parametrize("template_version", [None, "main^"]) +@mock.patch.object(component, "ComponentTemplater") +def test_update_component_cli(mock_templater, tmp_path, cli_runner, template_version): + cpath = tmp_path / "test-component" + cpath.mkdir() + + mt = make_mock_templater(mock_templater, cpath) + + template_arg = ( + [f"--template-version={template_version}"] + if template_version is not None + else [] + ) + + result = cli_runner(["component", "update", str(cpath)] + template_arg) + + assert result.exit_code == 0 + assert mt.template_version == template_version + + @mock.patch.object(component, "sync_dependencies") -@pytest.mark.parametrize("ghtoken", [None, "ghp_fake-token"]) +@pytest.mark.parametrize( + "ghtoken,template_version", + [ + (None, None), + ("ghp_fake-token", None), + ("ghp_fake-token", "custom-template-version"), + ], +) def test_component_sync_cli( - mock_sync_dependencies, ghtoken, tmp_path: Path, cli_runner: RunnerFunc + mock_sync_dependencies, + ghtoken, + template_version, + tmp_path: Path, + cli_runner: RunnerFunc, ): os.chdir(tmp_path) if ghtoken is not None: @@ -74,6 +105,7 @@ def sync_deps( pr_batch_size: int, github_pause: int, filter: str, + tmpl_version: Optional[str], ): assert config.github_token == ghtoken assert deplist.absolute() == dep_list.absolute() @@ -85,7 +117,13 @@ def sync_deps( assert pr_batch_size == 10 assert github_pause == timedelta(seconds=120) assert filter == "" + assert tmpl_version == template_version mock_sync_dependencies.side_effect = sync_deps - result = cli_runner(["component", "sync", "deps.yaml"]) + template_version_flag = ( + [f"--template-version={template_version}"] + if template_version is not None + else [] + ) + result = cli_runner(["component", "sync", "deps.yaml"] + template_version_flag) assert result.exit_code == (1 if ghtoken is None else 0) diff --git a/tests/test_cli_package.py b/tests/test_cli_package.py index 4857d7b6..d682d2ae 100644 --- a/tests/test_cli_package.py +++ b/tests/test_cli_package.py @@ -5,7 +5,7 @@ from collections.abc import Iterable from datetime import timedelta from pathlib import Path -from typing import Type +from typing import Optional, Type from unittest import mock import pytest @@ -15,13 +15,44 @@ from commodore.package import Package from commodore.package.template import PackageTemplater -from conftest import RunnerFunc +from conftest import RunnerFunc, make_mock_templater + + +@pytest.mark.parametrize("template_version", [None, "main^"]) +@mock.patch.object(package, "PackageTemplater") +def test_update_package_cli(mock_templater, tmp_path, cli_runner, template_version): + ppath = tmp_path / "test-package" + ppath.mkdir() + + mt = make_mock_templater(mock_templater, ppath) + + template_arg = ( + [f"--template-version={template_version}"] + if template_version is not None + else [] + ) + + result = cli_runner(["package", "update", str(ppath)] + template_arg) + + assert result.exit_code == 0 + assert mt.template_version == template_version @mock.patch.object(package, "sync_dependencies") -@pytest.mark.parametrize("ghtoken", [None, "ghp_fake-token"]) +@pytest.mark.parametrize( + "ghtoken,template_version", + [ + (None, None), + ("ghp_fake-token", None), + ("ghp_fake-token", "custom-template-version"), + ], +) def test_package_sync_cli( - mock_sync_packages, ghtoken, tmp_path: Path, cli_runner: RunnerFunc + mock_sync_packages, + ghtoken, + template_version, + tmp_path: Path, + cli_runner: RunnerFunc, ): os.chdir(tmp_path) if ghtoken is not None: @@ -42,6 +73,7 @@ def sync_pkgs( pr_batch_size: int, github_pause: int, filter: str, + tmpl_version: Optional[str], ): assert config.github_token == ghtoken assert pkglist.absolute() == pkg_list.absolute() @@ -53,8 +85,14 @@ def sync_pkgs( assert pr_batch_size == 10 assert github_pause == timedelta(seconds=120) assert filter == "" + assert tmpl_version == template_version mock_sync_packages.side_effect = sync_pkgs - result = cli_runner(["package", "sync", "pkgs.yaml"]) + template_version_flag = ( + [f"--template-version={template_version}"] + if template_version is not None + else [] + ) + result = cli_runner(["package", "sync", "pkgs.yaml"] + template_version_flag) print(result.stdout) assert result.exit_code == (1 if ghtoken is None else 0) diff --git a/tests/test_dependency_sync.py b/tests/test_dependency_sync.py index 37f90292..50485fdb 100644 --- a/tests/test_dependency_sync.py +++ b/tests/test_dependency_sync.py @@ -5,7 +5,7 @@ import json from pathlib import Path -from typing import Union +from typing import Optional, Union from unittest.mock import patch, MagicMock import click @@ -377,18 +377,21 @@ def test_sync_packages_package_list_parsing( @pytest.mark.parametrize( - "dry_run,second_pkg,needs_update", + "dry_run,second_pkg,needs_update,template_version", [ # no dry-run, no 2nd package, update required - (False, False, True), + (False, False, True, None), # no dry-run, no 2nd package, no update required - (False, False, False), + (False, False, False, None), # no dry-run, 2nd package, update required - (False, True, True), + (False, True, True, None), # dry-run, no 2nd package, no update required - (True, False, False), + (True, False, False, None), # dry-run, no 2nd package, update required - (True, False, True), + (True, False, True, None), + # no dry-run, no 2nd package, don't force update, custom version -- should + # require update but will force dry-run + (False, False, False, "main"), ], ) @responses.activate @@ -401,6 +404,7 @@ def test_sync_packages( dry_run: bool, second_pkg: bool, needs_update: bool, + template_version: Optional[str], ): config.github_token = "ghp_fake-token" responses.add_passthru("https://github.com") @@ -471,6 +475,7 @@ def _maybe_pause(updated: int, pr_batch_size: int, pause: datetime.timedelta): PackageTemplater, 1, datetime.timedelta(seconds=10), + template_version=template_version, ) if needs_update and not dry_run and second_pkg: