From 88d35de004149b864bfb1305ba71c2358e7ff8e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arne=20S=C3=B8rli?= <81353974+arneso-ssb@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:01:07 +0100 Subject: [PATCH] Change to use annotated syntax for typer optional parameters - Solves wrong type for optional parameters https://github.com/tiangolo/typer/issues/106 - Add testcases for --no-kernel option --- docs/contributing.md | 2 +- docs/index.md | 2 +- src/ssb_project_cli/ssb_project/app.py | 64 ++++++++++++++------------ tests/unit/build_test/test_build.py | 23 +++++---- tests/unit/create_test/test_create.py | 55 ++++++++++++++++++++++ 5 files changed, 106 insertions(+), 40 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index 44fcc634..5c10c35f 120000 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1 +1 @@ -../CONTRIBUTING.md \ No newline at end of file +../CONTRIBUTING.md diff --git a/docs/index.md b/docs/index.md index 32d46ee8..94389aee 120000 --- a/docs/index.md +++ b/docs/index.md @@ -1 +1 @@ -../README.md \ No newline at end of file +../README.md diff --git a/src/ssb_project_cli/ssb_project/app.py b/src/ssb_project_cli/ssb_project/app.py index 936298a7..87c1e63f 100644 --- a/src/ssb_project_cli/ssb_project/app.py +++ b/src/ssb_project_cli/ssb_project/app.py @@ -4,6 +4,7 @@ import typer from rich.console import Console +from typing_extensions import Annotated from ssb_project_cli.ssb_project.util import set_debug_logging @@ -48,7 +49,7 @@ @app.command() -def create( # noqa: C901 +def create( # noqa: C901, S107 project_name: str = typer.Argument(..., help="Project name"), # noqa: B008 description: str = typer.Argument( # noqa: B008 "", help="A short description of your project" @@ -56,35 +57,38 @@ def create( # noqa: C901 repo_privacy: RepoPrivacy = typer.Argument( # noqa: B008 RepoPrivacy.internal, help="Visibility of the Github repo" ), - add_github: bool = typer.Option( # noqa: B008 - False, - "--github", - help="Create the repo on Github as well", - ), - github_token: str = typer.Option( # noqa: B008 - "", - help="Your Github Personal Access Token, follow these instructions to create one: https://manual.dapla.ssb.no/git-github.html#personal-access-token-pat", - ), - verify_config: bool = typer.Option( # noqa: B008 - True, - "--no-verify", - help="Verify git configuration files. Use --no-verify to disable verification (defaults to True).", - show_default=True, - ), - template_git_url: str = typer.Option( # noqa: B008 - STAT_TEMPLATE_REPO_URL, - help="The Cookiecutter template URI.", - ), - checkout: t.Optional[str] = typer.Option( # noqa: B008 - None, - help="The git reference to check against. Supports branches, tags and commit hashes.", - ), - no_kernel: bool = typer.Option( # noqa: B008 - False, - "--no-kernel", - help="Do not create a kernel after the project is created (defaults to False).", - show_default=True, - ), + add_github: Annotated[ + bool, typer.Option("--github", help="Create the repo on Github as well") + ] = False, + github_token: Annotated[ + str, + typer.Option( + help="Your Github Personal Access Token, follow these instructions to create one: https://manual.dapla.ssb.no/git-github.html#personal-access-token-pat" + ), + ] = "", + verify_config: Annotated[ + bool, + typer.Option( + "--no-verify", + help="Verify git configuration files. Use --no-verify to disable verification (defaults to True).", + ), + ] = True, + template_git_url: Annotated[ + str, typer.Option(help="The Cookiecutter template URI.") + ] = STAT_TEMPLATE_REPO_URL, + checkout: Annotated[ + t.Optional[str], + typer.Option( + help="The git reference to check against. Supports branches, tags and commit hashes.", + ), + ] = None, + no_kernel: Annotated[ + bool, + typer.Option( + "--no-kernel", + help="Do not create a kernel after the project is created (defaults to False).", + ), + ] = False, ) -> None: """:sparkles: Create a project locally, and optionally on GitHub with the flag --github. The project will follow SSB's best practice for development.""" if not checkout and template_git_url is STAT_TEMPLATE_REPO_URL: diff --git a/tests/unit/build_test/test_build.py b/tests/unit/build_test/test_build.py index 8404c9a7..e0d6902f 100644 --- a/tests/unit/build_test/test_build.py +++ b/tests/unit/build_test/test_build.py @@ -30,12 +30,13 @@ @patch("ssb_project_cli.ssb_project.build.environment.verify_local_config") @patch(f"{BUILD}.get_project_name_and_root_path") @pytest.mark.parametrize( - "running_onprem_return,poetry_source_includes_source_name_return,calls_to_poetry_source_includes_source_name,calls_to_poetry_source_add,calls_to_poetry_source_remove", + "running_onprem_return,poetry_source_includes_source_name_return,calls_to_poetry_source_includes_source_name,calls_to_poetry_source_add,calls_to_poetry_source_remove,no_kernel", [ - (False, False, 1, 0, 0), - (True, False, 1, 1, 0), - (True, True, 1, 1, 1), - (False, True, 1, 0, 1), + (False, False, 1, 0, 0, False), + (True, False, 1, 1, 0, False), + (True, True, 1, 1, 1, False), + (False, True, 1, 0, 1, False), + (False, False, 1, 0, 0, True), ], ) def test_build( @@ -55,6 +56,7 @@ def test_build( calls_to_poetry_source_includes_source_name: int, calls_to_poetry_source_add: int, calls_to_poetry_source_remove: int, + no_kernel: bool, tmp_path: Path, ) -> None: """Check that build calls poetry_install, install_ipykernel and poetry_source_includes_source_name.""" @@ -67,14 +69,19 @@ def test_build( poetry_source_includes_source_name_return ) build_project( - tmp_path, tmp_path, STAT_TEMPLATE_REPO_URL, STAT_TEMPLATE_DEFAULT_REFERENCE + tmp_path, + tmp_path, + STAT_TEMPLATE_REPO_URL, + STAT_TEMPLATE_DEFAULT_REFERENCE, + True, + no_kernel, ) assert mock_kvakk.called assert mock_verify_local_config assert mock_confirm.call_count == 1 assert mock_poetry_install.call_count == 1 - assert mock_install_ipykernel.call_count == 1 - assert mock_ipykernel_attach_bashrc.call_count == 1 + assert mock_install_ipykernel.call_count == int(not no_kernel) + assert mock_ipykernel_attach_bashrc.call_count == int(not no_kernel) assert mock_running_onprem.call_count == 1 assert ( mock_poetry_source_includes_source_name.call_count diff --git a/tests/unit/create_test/test_create.py b/tests/unit/create_test/test_create.py index a0c3099b..c568e986 100644 --- a/tests/unit/create_test/test_create.py +++ b/tests/unit/create_test/test_create.py @@ -203,3 +203,58 @@ def test_is_valid_project_name() -> None: assert is_valid_project_name("Test") == False # noqa: E712 assert is_valid_project_name("Should-fail") == False # noqa: E712 assert is_valid_project_name("123randomletteRs") == False # noqa: E712 + + +@patch("ssb_project_cli.ssb_project.app.create_project", return_value=None) +def test_default_options_and_types(mock_create_project: Mock) -> None: + """Check default options andt types retuned by the create typer CLI command.""" + # Check when all optional parameters are given + create( + "test_project", + "description", + RepoPrivacy.internal, + False, + "github_token", + False, + "", + None, + False, + ) + assert mock_create_project.call_count == 1 + args, _ = mock_create_project.call_args + assert len(args) == 12 + no_kernel = args[11] + assert isinstance(no_kernel, bool) and not no_kernel + + # Only mandatory parameters given, check default values and types of optional parameters. + create("test_project", "description", RepoPrivacy.internal) + assert mock_create_project.call_count == 2 + args, _ = mock_create_project.call_args + assert len(args) == 12 + + add_github = args[3] + github_token = args[4] + verify_config = args[10] + template_repo_url = args[8] + checkout = args[9] + no_kernel = args[11] + print(f"\ncheckout has type {type(checkout)} with content {checkout}") + assert ( + isinstance(add_github, bool) and not add_github + ), "add_github: Wrong type or value" + assert ( + isinstance(github_token, str) and github_token == "" # noqa: S105 + ), "github_token: Wrong type or value" + assert ( + isinstance(verify_config, bool) and verify_config + ), "verify_config: Wrong type or value" + assert ( + isinstance(template_repo_url, str) + and template_repo_url == STAT_TEMPLATE_REPO_URL + ), "template_repo_url: Wrong type or value" + assert ( + isinstance(checkout, str) and checkout == STAT_TEMPLATE_DEFAULT_REFERENCE + ), "checkout: Wrong type or value" + assert ( + isinstance(no_kernel, bool) and not no_kernel + ), "no_kernel: Wrong type or value"