From 728536cc1f710902fb9ca8fa59f3bfdc234a2b05 Mon Sep 17 00:00:00 2001 From: Bart Feenstra Date: Tue, 15 Oct 2024 19:17:10 +0100 Subject: [PATCH] Ensure that the CLI exits with code 2 for bad invocations (#2138) --- .../cli/commands/extension_new_translation.py | 7 +++- .../commands/extension_update_translations.py | 7 +++- betty/locale/translation.py | 9 +++-- .../test_extension_new_translation.py | 2 +- .../test_extension_update_translations.py | 2 +- betty/tests/locale/test_translation.py | 34 ++++++++++++++++--- 6 files changed, 49 insertions(+), 12 deletions(-) diff --git a/betty/cli/commands/extension_new_translation.py b/betty/cli/commands/extension_new_translation.py index d13258b2e..eb81be3e3 100644 --- a/betty/cli/commands/extension_new_translation.py +++ b/betty/cli/commands/extension_new_translation.py @@ -10,6 +10,7 @@ from betty.cli.commands import command, parameter_callback, Command from betty.locale import translation from betty.locale.localizable import _ +from betty.locale.translation import assert_extension_assets_directory_path from betty.plugin import ShorthandPluginBase from betty.project import extension @@ -51,7 +52,11 @@ async def click_command(self) -> click.Command: @click.argument( "extension", required=True, - callback=parameter_callback(extension_id_to_type_map.get), + callback=parameter_callback( + lambda extension_id: assert_extension_assets_directory_path( + extension_id_to_type_map.get(extension_id) + ) + ), ) @click.argument( "locale", required=True, callback=parameter_callback(assert_locale()) diff --git a/betty/cli/commands/extension_update_translations.py b/betty/cli/commands/extension_update_translations.py index 7e022afa1..8ce4e6aad 100644 --- a/betty/cli/commands/extension_update_translations.py +++ b/betty/cli/commands/extension_update_translations.py @@ -15,6 +15,7 @@ from betty.cli.commands import command, Command, parameter_callback from betty.locale import translation from betty.locale.localizable import _ +from betty.locale.translation import assert_extension_assets_directory_path from betty.plugin import ShorthandPluginBase from betty.project import extension @@ -57,7 +58,11 @@ async def click_command(self) -> click.Command: @click.argument( "extension", required=True, - callback=parameter_callback(extension_id_to_type_map.get), + callback=parameter_callback( + lambda extension_id: assert_extension_assets_directory_path( + extension_id_to_type_map.get(extension_id) + ) + ), ) @click.argument( "source", diff --git a/betty/locale/translation.py b/betty/locale/translation.py index 91819fa51..8296c0c04 100644 --- a/betty/locale/translation.py +++ b/betty/locale/translation.py @@ -25,7 +25,10 @@ from betty.project.extension import Extension -def _assert_extension_assets_directory_path(extension: type[Extension]) -> Path: +def assert_extension_assets_directory_path(extension: type[Extension]) -> Path: + """ + Check that the given extension has an assets directory, and return its path. + """ assets_directory_path = extension.assets_directory_path() if assets_directory_path is None: raise UserFacingError( @@ -40,7 +43,7 @@ async def new_extension_translation(locale: str, extension: type[Extension]) -> """ Create a new translation for the given extension. """ - await _new_translation(locale, _assert_extension_assets_directory_path(extension)) + await _new_translation(locale, assert_extension_assets_directory_path(extension)) async def new_project_translation(locale: str, project: Project) -> None: @@ -105,7 +108,7 @@ async def update_extension_translations( source_file_paths = set() await _update_translations( source_file_paths, - _assert_extension_assets_directory_path(extension), + assert_extension_assets_directory_path(extension), _output_assets_directory_path_override, ) diff --git a/betty/tests/cli/commands/test_extension_new_translation.py b/betty/tests/cli/commands/test_extension_new_translation.py index 62f6283d1..63bcc9c13 100644 --- a/betty/tests/cli/commands/test_extension_new_translation.py +++ b/betty/tests/cli/commands/test_extension_new_translation.py @@ -37,7 +37,7 @@ async def test_click_command_with_extension_without_assets_directory( "extension-new-translation", "without-assets", "nl-NL", - expected_exit_code=1, + expected_exit_code=2, ) async def test_click_command_with_invalid_locale( diff --git a/betty/tests/cli/commands/test_extension_update_translations.py b/betty/tests/cli/commands/test_extension_update_translations.py index 0db2fab54..ea0907ae7 100644 --- a/betty/tests/cli/commands/test_extension_update_translations.py +++ b/betty/tests/cli/commands/test_extension_update_translations.py @@ -70,7 +70,7 @@ async def test_click_command_with_extension_without_assets_directory( "extension-update-translations", "without-assets", str(source), - expected_exit_code=1, + expected_exit_code=2, ) async def test_click_command_with_invalid_source_directory( diff --git a/betty/tests/locale/test_translation.py b/betty/tests/locale/test_translation.py index 551c5a303..0240b2738 100644 --- a/betty/tests/locale/test_translation.py +++ b/betty/tests/locale/test_translation.py @@ -1,14 +1,38 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from pathlib import Path +import pytest +from typing_extensions import override + +from betty.error import UserFacingError from betty.fs import ASSETS_DIRECTORY_PATH -from betty.locale.translation import update_dev_translations +from betty.locale.translation import ( + update_dev_translations, + assert_extension_assets_directory_path, +) from betty.test_utils.locale import PotFileTestBase -from typing_extensions import override +from betty.test_utils.project.extension import DummyExtension + -if TYPE_CHECKING: - from pathlib import Path +class TestAssertExtensionAssetsDirectoryPath: + class _DummyExtensionWithAssetsDirectory(DummyExtension): + @override + @classmethod + def assets_directory_path(cls) -> Path | None: + return Path(__file__) + + def test_without_assets_directory(self) -> None: + with pytest.raises(UserFacingError): + assert_extension_assets_directory_path(DummyExtension) + + def test_with_assets_directory(self) -> None: + assert ( + assert_extension_assets_directory_path( + self._DummyExtensionWithAssetsDirectory + ) + == self._DummyExtensionWithAssetsDirectory.assets_directory_path() + ) class TestPotFile(PotFileTestBase):