From 85fccba4a540970d0991918653390e450bcf1e1f Mon Sep 17 00:00:00 2001 From: qdelamea Date: Thu, 7 Nov 2024 22:30:43 +0100 Subject: [PATCH 1/4] chore: refactor project structure --- src/armonik_cli/commands/common.py | 100 ----------------- src/armonik_cli/commands/sessions.py | 7 +- src/armonik_cli/core/__init__.py | 6 ++ src/armonik_cli/{ => core}/console.py | 2 +- .../{errors.py => core/decorators.py} | 20 +--- src/armonik_cli/core/params.py | 101 ++++++++++++++++++ .../{utils.py => core/serialize.py} | 0 src/armonik_cli/exceptions.py | 16 +++ tests/test_errors.py | 3 +- 9 files changed, 131 insertions(+), 124 deletions(-) create mode 100644 src/armonik_cli/core/__init__.py rename src/armonik_cli/{ => core}/console.py (97%) rename src/armonik_cli/{errors.py => core/decorators.py} (68%) create mode 100644 src/armonik_cli/core/params.py rename src/armonik_cli/{utils.py => core/serialize.py} (100%) create mode 100644 src/armonik_cli/exceptions.py diff --git a/src/armonik_cli/commands/common.py b/src/armonik_cli/commands/common.py index 3171871..70c1a32 100644 --- a/src/armonik_cli/commands/common.py +++ b/src/armonik_cli/commands/common.py @@ -1,10 +1,5 @@ -import re - import rich_click as click -from datetime import timedelta -from typing import cast, Tuple, Union - endpoint_option = click.option( "-e", @@ -26,98 +21,3 @@ debug_option = click.option( "--debug", is_flag=True, default=False, help="Print debug logs and internal errors." ) - - -class KeyValuePairParam(click.ParamType): - """ - A custom Click parameter type that parses a key-value pair in the format "key=value". - - Attributes: - name: The name of the parameter type, used by Click. - """ - - name = "key_value_pair" - - def convert( - self, value: str, param: Union[click.Parameter, None], ctx: Union[click.Context, None] - ) -> Tuple[str, str]: - """ - Converts the input value into a tuple of (key, value) if it matches the required format. - - Args: - value: The input value to be converted. - param: The parameter object passed by Click. - ctx: The context in which the parameter is being used. - - Returns: - A tuple (key, value) if the input matches the format "key=value". - - Raises: - click.BadParameter: If the input does not match the expected format. - """ - pattern = r"^([a-zA-Z0-9_-]+)=([a-zA-Z0-9_-]+)$" - match_result = re.match(pattern, value) - if match_result: - return cast(Tuple[str, str], match_result.groups()) - self.fail( - f"{value} is not a valid key value pair. Use key=value where both key and value contain only alphanumeric characters, dashes (-), and underscores (_).", - param, - ctx, - ) - - -class TimeDeltaParam(click.ParamType): - """ - A custom Click parameter type that parses a time duration string in the format "HH:MM:SS.MS". - - Attributes: - name: The name of the parameter type, used by Click. - """ - - name = "timedelta" - - def convert( - self, value: str, param: Union[click.Parameter, None], ctx: Union[click.Context, None] - ) -> timedelta: - """ - Converts the input value into a timedelta object if it matches the required time format. - - Args: - value: The input value to be converted. - param: The parameter object passed by Click. - ctx: The context in which the parameter is being used. - - Returns: - A timedelta object representing the parsed time duration. - - Raises: - click.BadParameter: If the input does not match the expected time format. - """ - try: - return self._parse_time_delta(value) - except ValueError: - self.fail(f"{value} is not a valid time delta. Use HH:MM:SS.MS.", param, ctx) - - @staticmethod - def _parse_time_delta(time_str: str) -> timedelta: - """ - Parses a time string in the format "HH:MM:SS.MS" into a datetime.timedelta object. - - Args: - time_str (str): A string representing a time duration in hours, minutes, - seconds, and milliseconds (e.g., "12:34:56.789"). - - Returns: - timedelta: A datetime.timedelta object representing the parsed time duration. - - Raises: - ValueError: If the input string is not in the correct format. - """ - hours, minutes, seconds = time_str.split(":") - sec, microseconds = (seconds.split(".") + ["0"])[:2] # Handle missing milliseconds - return timedelta( - hours=int(hours), - minutes=int(minutes), - seconds=int(sec), - milliseconds=int(microseconds.ljust(3, "0")), # Ensure 3 digits for milliseconds - ) diff --git a/src/armonik_cli/commands/sessions.py b/src/armonik_cli/commands/sessions.py index daabecc..03937ec 100644 --- a/src/armonik_cli/commands/sessions.py +++ b/src/armonik_cli/commands/sessions.py @@ -7,14 +7,11 @@ from armonik.client.sessions import ArmoniKSessions from armonik.common import SessionStatus, Session, TaskOptions -from armonik_cli.console import console -from armonik_cli.errors import error_handler +from armonik_cli.core import console, error_handler, KeyValuePairParam, TimeDeltaParam from armonik_cli.commands.common import ( endpoint_option, output_option, debug_option, - KeyValuePairParam, - TimeDeltaParam, ) @@ -22,7 +19,7 @@ session_argument = click.argument("session-id", required=True, type=str, metavar="SESSION_ID") -@click.group(name="sessions") +@click.group(name="session") def sessions() -> None: """Manage cluster sessions.""" pass diff --git a/src/armonik_cli/core/__init__.py b/src/armonik_cli/core/__init__.py new file mode 100644 index 0000000..4a5529d --- /dev/null +++ b/src/armonik_cli/core/__init__.py @@ -0,0 +1,6 @@ +from armonik_cli.core.console import console +from armonik_cli.core.decorators import error_handler +from armonik_cli.core.params import KeyValuePairParam, TimeDeltaParam + + +__all__ = ["error_handler", "KeyValuePairParam", "TimeDeltaParam", "console"] diff --git a/src/armonik_cli/console.py b/src/armonik_cli/core/console.py similarity index 97% rename from src/armonik_cli/console.py rename to src/armonik_cli/core/console.py index 3a38270..66f8043 100644 --- a/src/armonik_cli/console.py +++ b/src/armonik_cli/core/console.py @@ -6,7 +6,7 @@ from rich.console import Console from rich.table import Table -from armonik_cli.utils import CLIJSONEncoder +from armonik_cli.core.serialize import CLIJSONEncoder class ArmoniKCLIConsole(Console): diff --git a/src/armonik_cli/errors.py b/src/armonik_cli/core/decorators.py similarity index 68% rename from src/armonik_cli/errors.py rename to src/armonik_cli/core/decorators.py index 3463ecb..2ea73d8 100644 --- a/src/armonik_cli/errors.py +++ b/src/armonik_cli/core/decorators.py @@ -3,25 +3,11 @@ import grpc import rich_click as click -from armonik_cli.console import console +from armonik_cli.core.console import console +from armonik_cli.exceptions import NotFoundError, InternalError -class ArmoniKCLIError(click.ClickException): - """Base exception for ArmoniK CLI errors.""" - - def __init__(self, message: str) -> None: - super().__init__(message) - - -class InternalError(ArmoniKCLIError): - """Error raised when an unknown internal error occured.""" - - -class NotFoundError(ArmoniKCLIError): - """Error raised when a given object of the API is not found.""" - - -def error_handler(func): +def error_handler(func=None): """A decorator to manage the correct display of errors..""" # Allow to call the decorator with parenthesis. if not func: diff --git a/src/armonik_cli/core/params.py b/src/armonik_cli/core/params.py new file mode 100644 index 0000000..2633342 --- /dev/null +++ b/src/armonik_cli/core/params.py @@ -0,0 +1,101 @@ +import re + +import rich_click as click + +from datetime import timedelta +from typing import cast, Tuple, Union + + +class KeyValuePairParam(click.ParamType): + """ + A custom Click parameter type that parses a key-value pair in the format "key=value". + + Attributes: + name: The name of the parameter type, used by Click. + """ + + name = "key_value_pair" + + def convert( + self, value: str, param: Union[click.Parameter, None], ctx: Union[click.Context, None] + ) -> Tuple[str, str]: + """ + Converts the input value into a tuple of (key, value) if it matches the required format. + + Args: + value: The input value to be converted. + param: The parameter object passed by Click. + ctx: The context in which the parameter is being used. + + Returns: + A tuple (key, value) if the input matches the format "key=value". + + Raises: + click.BadParameter: If the input does not match the expected format. + """ + pattern = r"^([a-zA-Z0-9_-]+)=([a-zA-Z0-9_-]+)$" + match_result = re.match(pattern, value) + if match_result: + return cast(Tuple[str, str], match_result.groups()) + self.fail( + f"{value} is not a valid key value pair. Use key=value where both key and value contain only alphanumeric characters, dashes (-), and underscores (_).", + param, + ctx, + ) + + +class TimeDeltaParam(click.ParamType): + """ + A custom Click parameter type that parses a time duration string in the format "HH:MM:SS.MS". + + Attributes: + name: The name of the parameter type, used by Click. + """ + + name = "timedelta" + + def convert( + self, value: str, param: Union[click.Parameter, None], ctx: Union[click.Context, None] + ) -> timedelta: + """ + Converts the input value into a timedelta object if it matches the required time format. + + Args: + value: The input value to be converted. + param: The parameter object passed by Click. + ctx: The context in which the parameter is being used. + + Returns: + A timedelta object representing the parsed time duration. + + Raises: + click.BadParameter: If the input does not match the expected time format. + """ + try: + return self._parse_time_delta(value) + except ValueError: + self.fail(f"{value} is not a valid time delta. Use HH:MM:SS.MS.", param, ctx) + + @staticmethod + def _parse_time_delta(time_str: str) -> timedelta: + """ + Parses a time string in the format "HH:MM:SS.MS" into a datetime.timedelta object. + + Args: + time_str (str): A string representing a time duration in hours, minutes, + seconds, and milliseconds (e.g., "12:34:56.789"). + + Returns: + timedelta: A datetime.timedelta object representing the parsed time duration. + + Raises: + ValueError: If the input string is not in the correct format. + """ + hours, minutes, seconds = time_str.split(":") + sec, microseconds = (seconds.split(".") + ["0"])[:2] # Handle missing milliseconds + return timedelta( + hours=int(hours), + minutes=int(minutes), + seconds=int(sec), + milliseconds=int(microseconds.ljust(3, "0")), # Ensure 3 digits for milliseconds + ) diff --git a/src/armonik_cli/utils.py b/src/armonik_cli/core/serialize.py similarity index 100% rename from src/armonik_cli/utils.py rename to src/armonik_cli/core/serialize.py diff --git a/src/armonik_cli/exceptions.py b/src/armonik_cli/exceptions.py new file mode 100644 index 0000000..54c406e --- /dev/null +++ b/src/armonik_cli/exceptions.py @@ -0,0 +1,16 @@ +import rich_click as click + + +class ArmoniKCLIError(click.ClickException): + """Base exception for ArmoniK CLI errors.""" + + def __init__(self, message: str) -> None: + super().__init__(message) + + +class InternalError(ArmoniKCLIError): + """Error raised when an unknown internal error occured.""" + + +class NotFoundError(ArmoniKCLIError): + """Error raised when a given object of the API is not found.""" diff --git a/tests/test_errors.py b/tests/test_errors.py index 708075d..f43c716 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -2,7 +2,8 @@ from grpc import RpcError, StatusCode -from armonik_cli.errors import error_handler, NotFoundError, InternalError +from armonik_cli.core import error_handler +from armonik_cli.exceptions import NotFoundError, InternalError class DummyRpcError(RpcError): From 69eda789c77ec0d9b7ffb87c42d28dcbefd9680c Mon Sep 17 00:00:00 2001 From: qdelamea Date: Thu, 7 Nov 2024 23:09:17 +0100 Subject: [PATCH 2/4] feat: add base_command decorator --- src/armonik_cli/commands/common.py | 23 ---------- src/armonik_cli/commands/sessions.py | 59 +++++------------------- src/armonik_cli/core/__init__.py | 4 +- src/armonik_cli/core/decorators.py | 68 +++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 73 deletions(-) delete mode 100644 src/armonik_cli/commands/common.py diff --git a/src/armonik_cli/commands/common.py b/src/armonik_cli/commands/common.py deleted file mode 100644 index 70c1a32..0000000 --- a/src/armonik_cli/commands/common.py +++ /dev/null @@ -1,23 +0,0 @@ -import rich_click as click - - -endpoint_option = click.option( - "-e", - "--endpoint", - type=str, - required=True, - help="Endpoint of the cluster to connect to.", - metavar="ENDPOINT", -) -output_option = click.option( - "-o", - "--output", - type=click.Choice(["yaml", "json", "table"], case_sensitive=False), - default="json", - show_default=True, - help="Commands output format.", - metavar="FORMAT", -) -debug_option = click.option( - "--debug", is_flag=True, default=False, help="Print debug logs and internal errors." -) diff --git a/src/armonik_cli/commands/sessions.py b/src/armonik_cli/commands/sessions.py index 03937ec..558223e 100644 --- a/src/armonik_cli/commands/sessions.py +++ b/src/armonik_cli/commands/sessions.py @@ -7,12 +7,7 @@ from armonik.client.sessions import ArmoniKSessions from armonik.common import SessionStatus, Session, TaskOptions -from armonik_cli.core import console, error_handler, KeyValuePairParam, TimeDeltaParam -from armonik_cli.commands.common import ( - endpoint_option, - output_option, - debug_option, -) +from armonik_cli.core import console, base_command, KeyValuePairParam, TimeDeltaParam SESSION_TABLE_COLS = [("ID", "SessionId"), ("Status", "Status"), ("CreatedAt", "CreatedAt")] @@ -26,10 +21,7 @@ def sessions() -> None: @sessions.command() -@endpoint_option -@output_option -@debug_option -@error_handler +@base_command def list(endpoint: str, output: str, debug: bool) -> None: """List the sessions of an ArmoniK cluster.""" with grpc.insecure_channel(endpoint) as channel: @@ -44,11 +36,8 @@ def list(endpoint: str, output: str, debug: bool) -> None: @sessions.command() -@endpoint_option -@output_option -@debug_option @session_argument -@error_handler +@base_command def get(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Get details of a given session.""" with grpc.insecure_channel(endpoint) as channel: @@ -59,7 +48,6 @@ def get(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option @click.option( "--max-retries", type=int, @@ -127,9 +115,7 @@ def get(endpoint: str, output: str, session_id: str, debug: bool) -> None: help="Additional default options.", metavar="KEY=VALUE", ) -@output_option -@debug_option -@error_handler +@base_command def create( endpoint: str, max_retries: int, @@ -170,12 +156,9 @@ def create( @sessions.command() -@endpoint_option @click.confirmation_option("--confirm", prompt="Are you sure you want to cancel this session?") -@output_option -@debug_option @session_argument -@error_handler +@base_command def cancel(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Cancel a session.""" with grpc.insecure_channel(endpoint) as channel: @@ -186,11 +169,8 @@ def cancel(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option -@output_option -@debug_option @session_argument -@error_handler +@base_command def pause(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Pause a session.""" with grpc.insecure_channel(endpoint) as channel: @@ -201,11 +181,8 @@ def pause(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option -@output_option -@debug_option @session_argument -@error_handler +@base_command def resume(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Resume a session.""" with grpc.insecure_channel(endpoint) as channel: @@ -216,12 +193,9 @@ def resume(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option @click.confirmation_option("--confirm", prompt="Are you sure you want to close this session?") -@output_option -@debug_option @session_argument -@error_handler +@base_command def close(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Close a session.""" with grpc.insecure_channel(endpoint) as channel: @@ -232,12 +206,9 @@ def close(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option @click.confirmation_option("--confirm", prompt="Are you sure you want to purge this session?") -@output_option -@debug_option @session_argument -@error_handler +@base_command def purge(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Purge a session.""" with grpc.insecure_channel(endpoint) as channel: @@ -248,12 +219,9 @@ def purge(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option @click.confirmation_option("--confirm", prompt="Are you sure you want to delete this session?") -@output_option -@debug_option @session_argument -@error_handler +@base_command def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None: """Delete a session and associated data from the cluster.""" with grpc.insecure_channel(endpoint) as channel: @@ -264,8 +232,6 @@ def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None: @sessions.command() -@endpoint_option -@session_argument @click.option( "--clients-only", is_flag=True, @@ -278,9 +244,8 @@ def delete(endpoint: str, output: str, session_id: str, debug: bool) -> None: default=False, help="Prevent only workers from submitting new tasks in the session.", ) -@output_option -@debug_option -@error_handler +@session_argument +@base_command def stop_submission( endpoint: str, session_id: str, clients_only: bool, workers_only: bool, output: str, debug: bool ) -> None: diff --git a/src/armonik_cli/core/__init__.py b/src/armonik_cli/core/__init__.py index 4a5529d..ed1f7af 100644 --- a/src/armonik_cli/core/__init__.py +++ b/src/armonik_cli/core/__init__.py @@ -1,6 +1,6 @@ from armonik_cli.core.console import console -from armonik_cli.core.decorators import error_handler +from armonik_cli.core.decorators import base_command from armonik_cli.core.params import KeyValuePairParam, TimeDeltaParam -__all__ = ["error_handler", "KeyValuePairParam", "TimeDeltaParam", "console"] +__all__ = ["base_command", "KeyValuePairParam", "TimeDeltaParam", "console"] diff --git a/src/armonik_cli/core/decorators.py b/src/armonik_cli/core/decorators.py index 2ea73d8..8b0818a 100644 --- a/src/armonik_cli/core/decorators.py +++ b/src/armonik_cli/core/decorators.py @@ -8,7 +8,15 @@ def error_handler(func=None): - """A decorator to manage the correct display of errors..""" + """Decorator to ensure correct display of errors. + + Args: + func: The command function to be decorated. If None, a partial function is returned, + allowing the decorator to be used with parentheses. + + Returns: + The wrapped function with added CLI options. + """ # Allow to call the decorator with parenthesis. if not func: return partial(error_handler) @@ -34,3 +42,61 @@ def wrapper(*args, **kwargs): raise InternalError("An internal fatal error occured.") return wrapper + + +def base_command(func=None): + """Decorator to add common CLI options to a Click command function, including + 'endpoint', 'output', and 'debug'. These options are automatically passed + as arguments to the decorated function. + + The following options are added to the command: + - `--endpoint` (required): Specifies the cluster endpoint. + - `--output`: Sets the output format, with options 'yaml', 'json', or 'table' (default is 'json'). + - `--debug`: Enables debug mode, printing additional logs if set. + + Warning: + If the decorated function has parameters with the same names as the options added by + this decorator, this can lead to conflicts and unpredictable behavior. + + Args: + func: The command function to be decorated. If None, a partial function is returned, + allowing the decorator to be used with parentheses. + + Returns: + The wrapped function with added CLI options. + """ + + # Allow to call the decorator with parenthesis. + if not func: + return partial(base_command) + + # Define the wrapper function with added Click options + @click.option( + "-e", + "--endpoint", + type=str, + required=True, + help="Endpoint of the cluster to connect to.", + metavar="ENDPOINT", + ) + @click.option( + "-o", + "--output", + type=click.Choice(["yaml", "json", "table"], case_sensitive=False), + default="json", + show_default=True, + help="Commands output format.", + metavar="FORMAT", + ) + @click.option( + "--debug", is_flag=True, default=False, help="Print debug logs and internal errors." + ) + @error_handler + @wraps(func) + def wrapper(endpoint: str, output: str, debug: bool, *args, **kwargs): + kwargs["endpoint"] = endpoint + kwargs["output"] = output + kwargs["debug"] = debug + return func(*args, **kwargs) + + return wrapper From 6e2aa5908f9c6023bd7d93f875e044c9796e8e7d Mon Sep 17 00:00:00 2001 From: qdelamea Date: Sat, 9 Nov 2024 16:29:38 +0100 Subject: [PATCH 3/4] test: restructure and complete the test suite --- .github/workflows/ci.yaml | 6 +- .github/workflows/publish.yaml | 3 +- src/armonik_cli/commands/sessions.py | 7 +- src/armonik_cli/core/decorators.py | 6 +- tests/commands/test_sessions.py | 219 ++++++++++++++++-- tests/conftest.py | 39 +++- .../test_decorators.py} | 25 +- tests/core/test_params.py | 41 ++++ tests/core/test_serialize.py | 74 ++++++ tests/outputs/sessions_list.txt | 18 -- tests/outputs/sessions_list_empty.txt | 2 - tests/test_cli.py | 12 +- 12 files changed, 383 insertions(+), 69 deletions(-) rename tests/{test_errors.py => core/test_decorators.py} (54%) create mode 100644 tests/core/test_params.py create mode 100644 tests/core/test_serialize.py delete mode 100644 tests/outputs/sessions_list.txt delete mode 100644 tests/outputs/sessions_list_empty.txt diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ea6ca43..cd07c1e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -75,17 +75,17 @@ jobs: - name: Get Report uses: orgoro/coverage@3f13a558c5af7376496aa4848bf0224aead366ac with: - coverageFile: packages/python/coverage.xml + coverageFile: coverage.xml token: ${{ secrets.GITHUB_TOKEN }} - name: Archive code coverage results html uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a with: name: code-coverage-report-html - path: packages/python/coverage_report + path: coverage_report - name: Archive code coverage results xml uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a with: name: code-coverage-report-xml - path: packages/python/coverage.xml + path: coverage.xml diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 62a5314..7cec4c0 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -1,7 +1,6 @@ name: Python Package on: - push: pull_request: branches: - main @@ -46,7 +45,7 @@ jobs: packages-dir: dist/ repository-url: https://test.pypi.org/legacy/ - - name: Publish to PyPiTest + - name: Publish to PyPi if: github.event_name == 'release' # Publish on releases uses: pypa/gh-action-pypi-publish@release/v1 with: diff --git a/src/armonik_cli/commands/sessions.py b/src/armonik_cli/commands/sessions.py index 558223e..24feb6e 100644 --- a/src/armonik_cli/commands/sessions.py +++ b/src/armonik_cli/commands/sessions.py @@ -32,7 +32,8 @@ def list(endpoint: str, output: str, debug: bool) -> None: sessions = [_clean_up_status(s) for s in sessions] console.formatted_print(sessions, format=output, table_cols=SESSION_TABLE_COLS) - console.print(f"\n{total} sessions found.") + # TODO: Use logger to display this information + # console.print(f"\n{total} sessions found.") @sessions.command() @@ -255,7 +256,9 @@ def stop_submission( session = sessions_client.stop_submission_session( session_id=session_id, client=clients_only, worker=workers_only ) - console.formatted_print(session, format=output, table_cols=SESSION_TABLE_COLS) + console.formatted_print( + _clean_up_status(session), format=output, table_cols=SESSION_TABLE_COLS + ) def _clean_up_status(session: Session) -> Session: diff --git a/src/armonik_cli/core/decorators.py b/src/armonik_cli/core/decorators.py index 8b0818a..dc5b596 100644 --- a/src/armonik_cli/core/decorators.py +++ b/src/armonik_cli/core/decorators.py @@ -9,7 +9,7 @@ def error_handler(func=None): """Decorator to ensure correct display of errors. - + Args: func: The command function to be decorated. If None, a partial function is returned, allowing the decorator to be used with parentheses. @@ -46,7 +46,7 @@ def wrapper(*args, **kwargs): def base_command(func=None): """Decorator to add common CLI options to a Click command function, including - 'endpoint', 'output', and 'debug'. These options are automatically passed + 'endpoint', 'output', and 'debug'. These options are automatically passed as arguments to the decorated function. The following options are added to the command: @@ -55,7 +55,7 @@ def base_command(func=None): - `--debug`: Enables debug mode, printing additional logs if set. Warning: - If the decorated function has parameters with the same names as the options added by + If the decorated function has parameters with the same names as the options added by this decorator, this can lead to conflicts and unpredictable behavior. Args: diff --git a/tests/commands/test_sessions.py b/tests/commands/test_sessions.py index d6ae6e9..9c4482d 100644 --- a/tests/commands/test_sessions.py +++ b/tests/commands/test_sessions.py @@ -1,26 +1,213 @@ import pytest +from datetime import datetime, timedelta +from copy import deepcopy + from armonik.client import ArmoniKSessions -from armonik.common import Session -from click.testing import CliRunner +from armonik.common import Session, TaskOptions, SessionStatus +from conftest import run_cmd_and_assert_exit_code, reformat_cmd_output + +ENDPOINT = "172.17.119.85:5001" + +raw_session = Session( + session_id="id", + status=SessionStatus.RUNNING, + client_submission=True, + worker_submission=True, + partition_ids=["default"], + options=TaskOptions( + max_duration=timedelta(hours=1), + priority=1, + max_retries=2, + partition_id="default", + application_name="", + application_version="", + application_namespace="", + application_service="", + engine_type="", + options={}, + ), + created_at=datetime(year=2024, month=11, day=11), + cancelled_at=None, + closed_at=None, + purged_at=None, + deleted_at=None, + duration=timedelta(hours=0), +) +serialized_session = { + "SessionId": "id", + "Status": "Running", + "ClientSubmission": True, + "WorkerSubmission": True, + "PartitionIds": ["default"], + "Options": { + "MaxDuration": "1:00:00", + "Priority": 1, + "MaxRetries": 2, + "PartitionId": "default", + "ApplicationName": "", + "ApplicationVersion": "", + "ApplicationNamespace": "", + "ApplicationService": "", + "EngineType": "", + "Options": {}, + }, + "CreatedAt": "2024-11-11 00:00:00", + "CancelledAt": None, + "ClosedAt": None, + "PurgedAt": None, + "DeletedAt": None, + "Duration": "0:00:00", +} + + +@pytest.mark.parametrize( + "cmd", + [ + f"session list --endpoint {ENDPOINT}", + ], +) +def test_session_list(mocker, cmd): + mocker.patch.object(ArmoniKSessions, "list_sessions", return_value=(1, [deepcopy(raw_session)])) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == [serialized_session] + + +@pytest.mark.parametrize( + "cmd", + [ + f"session get --endpoint {ENDPOINT} id", + ], +) +def test_session_get(mocker, cmd): + mocker.patch.object(ArmoniKSessions, "get_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == serialized_session + + +@pytest.mark.parametrize( + "cmd", + [ + f"session create --priority 1 --max-duration 01:00:0 --max-retries 2 --endpoint {ENDPOINT}", + f"session create --priority 1 --max-duration 01:00:0 --max-retries 2 --endpoint {ENDPOINT} " + "--default-partition bench --partition bench --partition htcmock --option op1=val1 --option opt2=val2 " + "--application-name app --application-version v1 --application-namespace ns --application-service svc --engine-type eng", + ], +) +def test_session_create(mocker, cmd): + mocker.patch.object(ArmoniKSessions, "create_session", return_value="id") + mocker.patch.object(ArmoniKSessions, "get_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == serialized_session + + +@pytest.mark.parametrize( + ("cmd", "prompt"), + [ + (f"session cancel --confirm --endpoint {ENDPOINT} id", None), + (f"session cancel --endpoint {ENDPOINT} id", "y"), + ], +) +def test_session_cancel(mocker, cmd, prompt): + mocker.patch.object(ArmoniKSessions, "cancel_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd, input=prompt) + assert ( + reformat_cmd_output( + result.output, deserialize=True, first_line_out=True if prompt else False + ) + == serialized_session + ) + -from armonik_cli.commands.sessions import list +@pytest.mark.parametrize( + "cmd", + [ + f"session pause --endpoint {ENDPOINT} id", + ], +) +def test_session_pause(mocker, cmd): + mocker.patch.object(ArmoniKSessions, "pause_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == serialized_session + + +@pytest.mark.parametrize( + "cmd", + [ + f"session resume --endpoint {ENDPOINT} id", + ], +) +def test_session_resume(mocker, cmd): + mocker.patch.object(ArmoniKSessions, "resume_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == serialized_session + + +@pytest.mark.parametrize( + ("cmd", "prompt"), + [ + (f"session close --confirm --endpoint {ENDPOINT} id", None), + (f"session close --endpoint {ENDPOINT} id", "y"), + ], +) +def test_session_close(mocker, cmd, prompt): + mocker.patch.object(ArmoniKSessions, "close_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd, input=prompt) + assert ( + reformat_cmd_output( + result.output, deserialize=True, first_line_out=True if prompt else False + ) + == serialized_session + ) + + +@pytest.mark.parametrize( + ("cmd", "prompt"), + [ + (f"session purge --confirm --endpoint {ENDPOINT} id", None), + (f"session purge --endpoint {ENDPOINT} id", "y"), + ], +) +def test_session_purge(mocker, cmd, prompt): + mocker.patch.object(ArmoniKSessions, "purge_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd, input=prompt) + assert ( + reformat_cmd_output( + result.output, deserialize=True, first_line_out=True if prompt else False + ) + == serialized_session + ) + + +@pytest.mark.parametrize( + ("cmd", "prompt"), + [ + (f"session delete --confirm --endpoint {ENDPOINT} id", None), + (f"session delete --endpoint {ENDPOINT} id", "y"), + ], +) +def test_session_delete(mocker, cmd, prompt): + mocker.patch.object(ArmoniKSessions, "delete_session", return_value=deepcopy(raw_session)) + result = run_cmd_and_assert_exit_code(cmd, input=prompt) + assert ( + reformat_cmd_output( + result.output, deserialize=True, first_line_out=True if prompt else False + ) + == serialized_session + ) @pytest.mark.parametrize( - ("args", "mock_return", "output_id"), + "cmd", [ - (["--endpoint", "endpoint"], (0, []), "sessions_list_empty"), - ( - ["--endpoint", "endpoint", "-o", "json"], - (1, [Session(session_id="id")]), - "sessions_list", - ), + f"session stop-submission --endpoint {ENDPOINT} id", + f"session stop-submission --clients-only --endpoint {ENDPOINT} id", + f"session stop-submission --workers-only --endpoint {ENDPOINT} id", ], ) -def test_armonik_sessions_list(mocker, cmd_outputs, args, mock_return, output_id): - mocker.patch.object(ArmoniKSessions, "list_sessions", return_value=mock_return) - runner = CliRunner() - result = runner.invoke(list, args) - assert result.exit_code == 0 - assert result.output == cmd_outputs[output_id] +def test_session_stop_submission(mocker, cmd): + mocker.patch.object( + ArmoniKSessions, "stop_submission_session", return_value=deepcopy(raw_session) + ) + result = run_cmd_and_assert_exit_code(cmd) + assert reformat_cmd_output(result.output, deserialize=True) == serialized_session diff --git a/tests/conftest.py b/tests/conftest.py index d941f8f..93b595b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,15 +1,30 @@ -import pytest +import json -from pathlib import Path +from typing import Dict, Optional +from click.testing import CliRunner, Result -@pytest.fixture -def cmd_outputs(): - """Read command output files located in tests/outputs and return a dictionnary which keys are - file names and values file contents.""" - output_files = [ - d - for d in (Path(__file__).parent / "outputs").iterdir() - if d.is_file() and d.suffix == ".txt" - ] - return {f.name.removesuffix(".txt"): f.open("r").read() for f in output_files} +from armonik_cli.cli import cli + + +def run_cmd_and_assert_exit_code( + cmd: str, exit_code: int = 0, input: Optional[str] = None, env: Optional[Dict[str, str]] = None +) -> Result: + cmd = cmd.split() + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke(cli, cmd, input=input, env=env) + assert result.exit_code == exit_code + return result + + +def reformat_cmd_output( + output: str, deserialize: bool = False, first_line_out: bool = False +) -> str: + if first_line_out: + output = "\n".join(output.split("\n")[1:]) + output = output.replace("\n", "") + output = " ".join(output.split()) + if deserialize: + return json.loads(output) + return output diff --git a/tests/test_errors.py b/tests/core/test_decorators.py similarity index 54% rename from tests/test_errors.py rename to tests/core/test_decorators.py index f43c716..4008858 100644 --- a/tests/test_errors.py +++ b/tests/core/test_decorators.py @@ -2,7 +2,7 @@ from grpc import RpcError, StatusCode -from armonik_cli.core import error_handler +from armonik_cli.core.decorators import error_handler, base_command from armonik_cli.exceptions import NotFoundError, InternalError @@ -31,8 +31,9 @@ def raise_error(code, details): raise_error(code, "") -def test_error_handler_other_no_debug(): - @error_handler +@pytest.mark.parametrize("decorator", [error_handler, error_handler()]) +def test_error_handler_other_no_debug(decorator): + @decorator def raise_error(): raise ValueError() @@ -40,9 +41,23 @@ def raise_error(): raise_error() -def test_error_handler_other_debug(): - @error_handler +@pytest.mark.parametrize("decorator", [error_handler, error_handler()]) +def test_error_handler_other_debug(decorator): + @decorator def raise_error(debug=None): raise ValueError() raise_error(debug=True) + + +@pytest.mark.parametrize("decorator", [base_command, base_command()]) +def test_base_command(decorator): + @decorator + def test_func(): + pass + + assert test_func.__name__ == "test_func" + assert len(test_func.__click_params__) == 3 + assert test_func.__click_params__[0].name == "debug" + assert test_func.__click_params__[1].name == "output" + assert test_func.__click_params__[2].name == "endpoint" diff --git a/tests/core/test_params.py b/tests/core/test_params.py new file mode 100644 index 0000000..b2e751e --- /dev/null +++ b/tests/core/test_params.py @@ -0,0 +1,41 @@ +import click +import pytest + +from datetime import timedelta + +from armonik_cli.core import KeyValuePairParam, TimeDeltaParam + + +@pytest.mark.parametrize( + ("input", "output"), + [ + ("key=value", ("key", "value")), + ("ke_y=valu_e", ("ke_y", "valu_e")), + ], +) +def test_key_value_pair_param(input, output): + assert KeyValuePairParam().convert(input, None, None) == output + + +@pytest.mark.parametrize("input", ["key value", "ke?y=value"]) +def test_key_value_pair_param_fail(input): + with pytest.raises(click.BadParameter): + KeyValuePairParam().convert(input, None, None) + + +@pytest.mark.parametrize( + ("input", "output"), + [ + ("12:11:10.987", timedelta(hours=12, minutes=11, seconds=10, milliseconds=987)), + ("12:11:10", timedelta(hours=12, minutes=11, seconds=10)), + ("0:10:0", timedelta(minutes=10)), + ], +) +def test_timedelta_parm_success(input, output): + assert TimeDeltaParam().convert(input, None, None) == output + + +@pytest.mark.parametrize("input", ["1.0", "10", "00:10"]) +def test_timedelta_parm_fail(input): + with pytest.raises(click.BadParameter): + assert TimeDeltaParam().convert(input, None, None) diff --git a/tests/core/test_serialize.py b/tests/core/test_serialize.py new file mode 100644 index 0000000..a4070c9 --- /dev/null +++ b/tests/core/test_serialize.py @@ -0,0 +1,74 @@ +import json + +import pytest + +from datetime import timedelta, datetime + +from armonik.common import Session, TaskOptions, SessionStatus + +from armonik_cli.core.serialize import CLIJSONEncoder + + +@pytest.mark.parametrize( + ("obj", "obj_dict"), + [ + ( + TaskOptions( + max_duration=timedelta(minutes=5), + priority=1, + max_retries=2, + partition_id="default", + application_name="app", + application_namespace="ns", + application_service="svc", + application_version="v1", + engine_type="eng", + options={"k1": "v1", "k2": "v2"}, + ), + { + "MaxDuration": "0:05:00", + "Priority": 1, + "MaxRetries": 2, + "PartitionId": "default", + "ApplicationName": "app", + "ApplicationNamespace": "ns", + "ApplicationService": "svc", + "ApplicationVersion": "v1", + "EngineType": "eng", + "Options": {"k1": "v1", "k2": "v2"}, + }, + ), + ( + Session( + session_id="id", + status=SessionStatus.RUNNING, + client_submission=True, + worker_submission=False, + partition_ids=["default"], + options=None, + created_at=datetime(year=2024, month=11, day=11), + cancelled_at=None, + closed_at=None, + purged_at=None, + deleted_at=None, + duration=timedelta(hours=1), + ), + { + "SessionId": "id", + "Status": 1, + "ClientSubmission": True, + "WorkerSubmission": False, + "PartitionIds": ["default"], + "Options": None, + "CreatedAt": "2024-11-11 00:00:00", + "CancelledAt": None, + "ClosedAt": None, + "PurgedAt": None, + "DeletedAt": None, + "Duration": "1:00:00", + }, + ), + ], +) +def test_serialize(obj, obj_dict): + assert obj_dict == json.loads(json.dumps(obj, cls=CLIJSONEncoder)) diff --git a/tests/outputs/sessions_list.txt b/tests/outputs/sessions_list.txt deleted file mode 100644 index 5f1a274..0000000 --- a/tests/outputs/sessions_list.txt +++ /dev/null @@ -1,18 +0,0 @@ -[ - { - "SessionId": "id", - "Status": "Unspecified", - "ClientSubmission": null, - "WorkerSubmission": null, - "PartitionIds": [], - "Options": null, - "CreatedAt": null, - "CancelledAt": null, - "ClosedAt": null, - "PurgedAt": null, - "DeletedAt": null, - "Duration": null - } -] - -1 sessions found. diff --git a/tests/outputs/sessions_list_empty.txt b/tests/outputs/sessions_list_empty.txt deleted file mode 100644 index 708e170..0000000 --- a/tests/outputs/sessions_list_empty.txt +++ /dev/null @@ -1,2 +0,0 @@ - -0 sessions found. diff --git a/tests/test_cli.py b/tests/test_cli.py index cfd046b..491a546 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,10 +1,10 @@ -from click.testing import CliRunner - -from armonik_cli import cli +from conftest import run_cmd_and_assert_exit_code def test_armonik_version(): - runner = CliRunner() - result = runner.invoke(cli.cli, ["--version"]) - assert result.exit_code == 0 + result = run_cmd_and_assert_exit_code("--version") assert result.output.startswith("armonik, version ") + + +def test_armonik_help(): + run_cmd_and_assert_exit_code("--help") From 943ac778e271a9de95e13874dc1b7445a2b9a911 Mon Sep 17 00:00:00 2001 From: qdelamea Date: Fri, 15 Nov 2024 16:10:36 +0100 Subject: [PATCH 4/4] fix: test coverage report generation --- .coveragerc | 3 +++ .github/workflows/ci.yaml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 .coveragerc diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..6a5e995 --- /dev/null +++ b/.coveragerc @@ -0,0 +1,3 @@ +[run] +omit = + */_version.py diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cd07c1e..046ffa3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -70,7 +70,7 @@ jobs: python -m uv pip install .[tests] - name: Testing - run: pytest tests --cov=armonik_cli --cov-report=term-missing + run: pytest tests --cov=armonik_cli --cov-config=.coveragerc --cov-report=term-missing --cov-append --cov-report xml:coverage.xml --cov-report html:coverage_report - name: Get Report uses: orgoro/coverage@3f13a558c5af7376496aa4848bf0224aead366ac