From df4fc0f5161b32e11360dd353b98c020cf7f15ba Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Fri, 3 Oct 2025 09:21:19 +0200 Subject: [PATCH] don't add to group SSHWrapper should just define "ssh" method and that's it. Do not modify parent group. Revert recent introduction of clock_group from TMT driver as well (the only other user), witht that we do not need to support that in the Composite driver. (cherry picked from commit ff05456db86402abb843b5222cf53a22dc876f0a) --- .../jumpstarter_driver_composite/client.py | 8 +----- .../jumpstarter_driver_ssh/client.py | 4 +-- .../jumpstarter_driver_tmt/client.py | 4 +-- .../jumpstarter_driver_tmt/driver_test.py | 27 +++++++++---------- uv.lock | 4 ++- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py b/packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py index 9ebd500a0..468fd9450 100644 --- a/packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py +++ b/packages/jumpstarter-driver-composite/jumpstarter_driver_composite/client.py @@ -1,4 +1,3 @@ -import inspect import logging from dataclasses import dataclass @@ -50,11 +49,6 @@ def base(): for k, v in self.children.items(): if hasattr(v, "cli"): - # Check if the cli method accepts a click_group parameter - sig = inspect.signature(v.cli) - if "click_group" in sig.parameters: - base.add_command(v.cli(click_group=base), k) - else: - base.add_command(v.cli(), k) + base.add_command(v.cli(), k) return base diff --git a/packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py b/packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py index ba7e55e7f..c31e36ac8 100644 --- a/packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py +++ b/packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py @@ -18,8 +18,8 @@ class SSHWrapperClient(CompositeClient): This client provides methods to interact with SSH connections via CLI """ - def cli(self, click_group): - @click_group.command(context_settings={"ignore_unknown_options": True}) + def cli(self): + @click.command(context_settings={"ignore_unknown_options": True}) @click.option("--direct", is_flag=True, help="Use direct TCP address") @click.argument("args", nargs=-1) def ssh(direct, args): diff --git a/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/client.py b/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/client.py index 4e827103e..b71490659 100644 --- a/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/client.py +++ b/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/client.py @@ -31,9 +31,9 @@ class TMTClient(CompositeClient): This client provides methods to interact with LocalTMT devices via SSH """ - def cli(self, click_group): + def cli(self): - @click_group.command(context_settings={"ignore_unknown_options": True}) + @click.command(context_settings={"ignore_unknown_options": True}) @click.option("--forward-ssh", is_flag=True) @click.option("--tmt-username", default=None) @click.option("--tmt-password", default=None) diff --git a/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py b/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py index c6c3405df..6cb0a8278 100644 --- a/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py +++ b/packages/jumpstarter-driver-tmt/jumpstarter_driver_tmt/driver_test.py @@ -1,6 +1,5 @@ from unittest.mock import MagicMock, patch -import click import pytest from click.testing import CliRunner from jumpstarter_driver_network.driver import TcpNetwork @@ -24,18 +23,18 @@ def test_drivers_tmt_cli(): with serve(instance) as client: # Test the CLI tmt command without arguments runner = CliRunner() - cli = client.cli(click.Group()) + cli = client.cli() with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 # Success return code - result = runner.invoke(cli, ["tmt"]) + result = runner.invoke(cli, []) assert result.exit_code == 0 mock_run_tmt.assert_called_once() # Test the CLI tmt command with arguments with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 # Success return code - result = runner.invoke(cli, ["tmt", "test", "arg1", "arg2"]) + result = runner.invoke(cli, ["test", "arg1", "arg2"]) assert result.exit_code == 0 mock_run_tmt.assert_called_once() @@ -46,12 +45,12 @@ def test_drivers_tmt_cli_with_options(): with serve(instance) as client: runner = CliRunner() - cli = client.cli(click.Group()) + cli = client.cli() # Test with --forward-ssh flag with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 # Success return code - result = runner.invoke(cli, ["tmt", "--forward-ssh", "test"]) + result = runner.invoke(cli, ["--forward-ssh", "test"]) assert result.exit_code == 0 mock_run_tmt.assert_called_once() @@ -59,7 +58,7 @@ def test_drivers_tmt_cli_with_options(): with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 # Success return code result = runner.invoke( - cli, ["tmt", "--tmt-username", "custom_user", "--tmt-password", "custom_pass", "test"] + cli, ["--tmt-username", "custom_user", "--tmt-password", "custom_pass", "test"] ) assert result.exit_code == 0 mock_run_tmt.assert_called_once() @@ -67,7 +66,7 @@ def test_drivers_tmt_cli_with_options(): # Test with custom tmt command with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 # Success return code - result = runner.invoke(cli, ["tmt", "--tmt-cmd", "custom-tmt", "test"]) + result = runner.invoke(cli, ["--tmt-cmd", "custom-tmt", "test"]) assert result.exit_code == 0 mock_run_tmt.assert_called_once() @@ -78,12 +77,12 @@ def test_drivers_tmt_cli_error_handling(): with serve(instance) as client: runner = CliRunner() - cli = client.cli(click.Group()) + cli = client.cli() # Test CLI with non-zero return code with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 1 # Error return code - result = runner.invoke(cli, ["tmt", "test"]) + result = runner.invoke(cli, ["test"]) assert result.exit_code == 1 mock_run_tmt.assert_called_once() @@ -94,10 +93,10 @@ def test_drivers_tmt_cli_tmt_on_exporter(): with serve(instance) as client: runner = CliRunner() - cli = client.cli(click.Group()) + cli = client.cli() # Test CLI with --tmt-on-exporter flag (should abort) - result = runner.invoke(cli, ["tmt", "--tmt-on-exporter", "test"]) + result = runner.invoke(cli, ["--tmt-on-exporter", "test"]) assert result.exit_code == 1 # click.Abort() returns exit code 1 assert "TMT will be run on the exporter" in result.output assert "Aborted!" in result.output @@ -428,11 +427,11 @@ def test_drivers_tmt_cli_logging(): with serve(instance) as client: runner = CliRunner() - cli = client.cli(click.Group()) + cli = client.cli() with patch.object(client, '_run_tmt_local') as mock_run_tmt: mock_run_tmt.return_value = 0 with patch.object(client.logger, 'debug') as mock_debug: - result = runner.invoke(cli, ["tmt", "test"]) + result = runner.invoke(cli, ["test"]) assert result.exit_code == 0 mock_debug.assert_called_with("TMT result: 0") diff --git a/uv.lock b/uv.lock index a17a76cef..34e74332e 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 2 requires-python = ">=3.11" [manifest] @@ -1079,6 +1079,7 @@ dependencies = [ { name = "jumpstarter-driver-shell" }, { name = "jumpstarter-driver-snmp" }, { name = "jumpstarter-driver-tftp" }, + { name = "jumpstarter-driver-tmt" }, { name = "jumpstarter-driver-uboot" }, { name = "jumpstarter-driver-ustreamer" }, { name = "jumpstarter-driver-yepkit" }, @@ -1113,6 +1114,7 @@ requires-dist = [ { name = "jumpstarter-driver-shell", editable = "packages/jumpstarter-driver-shell" }, { name = "jumpstarter-driver-snmp", editable = "packages/jumpstarter-driver-snmp" }, { name = "jumpstarter-driver-tftp", editable = "packages/jumpstarter-driver-tftp" }, + { name = "jumpstarter-driver-tmt", editable = "packages/jumpstarter-driver-tmt" }, { name = "jumpstarter-driver-uboot", editable = "packages/jumpstarter-driver-uboot" }, { name = "jumpstarter-driver-ustreamer", editable = "packages/jumpstarter-driver-ustreamer" }, { name = "jumpstarter-driver-yepkit", editable = "packages/jumpstarter-driver-yepkit" },