From 4d0b04eabb7b8ef5dc54ef7928b642771fdb0e2b Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 21 Sep 2026 09:32:30 -0300 Subject: [PATCH] [Bug] th-cli: don't call GET /api/v1/version on every invocation (#1005) click.version_option's message param is evaluated eagerly at decoration (i.e. import) time, so get_extended_help() -> get_versions() hit the backend on every th-cli invocation (--help, subcommands, etc.), not just --version. When the backend is unreachable, this stalled every command until the HTTP client timed out. Replace it with a hand-rolled eager --version option whose callback only runs get_extended_help() when --version is actually passed. --- tests/test_main.py | 66 ++++++++++++++++++++++++++++++++++++++++++++++ th_cli/main.py | 22 +++++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/test_main.py diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..6bc8c76 --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,66 @@ +# +# Copyright (c) 2025-2026 Project CHIP Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Tests for the th-cli root group and its --version handling. + +Regression tests for #1005: th-cli used to call GET /api/v1/version on every +invocation (not just --version) because click.version_option's `message` was +evaluated eagerly at decoration/import time. +""" + +from unittest.mock import Mock, patch + +import pytest +from click.testing import CliRunner + +from th_cli.main import root + + +@pytest.mark.unit +@pytest.mark.cli +class TestRootVersionOption: + """Test cases for th-cli's --version handling.""" + + def test_help_does_not_query_server_version(self, cli_runner: CliRunner) -> None: + """--help (and, by extension, any other invocation) must not hit the backend.""" + with patch("th_cli.main.get_versions") as mock_get_versions: + result = cli_runner.invoke(root, ["--help"]) + + assert result.exit_code == 0 + mock_get_versions.assert_not_called() + + def test_no_args_does_not_query_server_version(self, cli_runner: CliRunner) -> None: + """Invoking the CLI with no subcommand must not hit the backend either.""" + with patch("th_cli.main.get_versions") as mock_get_versions: + cli_runner.invoke(root, []) + + mock_get_versions.assert_not_called() + + def test_version_queries_server_version(self, cli_runner: CliRunner) -> None: + """--version is the only invocation expected to reach out to the backend.""" + with patch("th_cli.main.get_versions", return_value={"Backend Version": "1.0.0"}) as mock_get_versions: + result = cli_runner.invoke(root, ["--version"]) + + assert result.exit_code == 0 + mock_get_versions.assert_called_once() + assert "Backend Version" in result.output + + def test_version_exits_cleanly_when_server_unreachable(self, cli_runner: CliRunner) -> None: + """--version must still print CLI-only info and exit 0 if the backend call fails.""" + with patch("th_cli.main.get_versions", side_effect=Mock(side_effect=Exception("unreachable"))): + result = cli_runner.invoke(root, ["--version"]) + + assert result.exit_code == 0 + assert "Not able to retrieve versions from server." in result.output diff --git a/th_cli/main.py b/th_cli/main.py index 2e88f9a..9b5e6bc 100644 --- a/th_cli/main.py +++ b/th_cli/main.py @@ -51,8 +51,28 @@ def get_extended_help() -> str: return help_text +def _print_version(ctx: click.Context, param: click.Parameter, value: bool) -> None: + """Eager --version callback. + + Computed lazily (only when --version is actually passed) so that other + invocations (--help, subcommands, etc.) don't pay the cost of a server + round-trip in get_extended_help() -> get_versions(). + """ + if not value or ctx.resilient_parsing: + return + click.echo(get_extended_help()) + ctx.exit() + + @click.group(help=colorize_cmd_help("th-cli", "A CLI tool for Matter Test Harness")) -@click.version_option(message=get_extended_help()) +@click.option( + "--version", + is_flag=True, + expose_value=False, + is_eager=True, + callback=_print_version, + help="Show the version and exit.", +) def root() -> None: pass