diff --git a/monitoring/monitorlib/clients/versioning/client_interuss.py b/monitoring/monitorlib/clients/versioning/client_interuss.py index f1a267c67e..a98233261a 100755 --- a/monitoring/monitorlib/clients/versioning/client_interuss.py +++ b/monitoring/monitorlib/clients/versioning/client_interuss.py @@ -49,9 +49,15 @@ def get_version(self, version_type: str | None) -> GetVersionResponse: "Response to get version didn't return a system identity" ) - if not resp.has_field_with_value("system_version"): + system_version = (query.response.json or {}).get("system_version") + if not isinstance(system_version, str): raise VersionQueryError( - "Response to get version didn't return a system version" + f"Response to get version expected system version to be a string, but instead found a {type(system_version).__name__}" + ) + + if not system_version.strip(): + raise VersionQueryError( + f"Response to get version expected system version to have a value, but instead got {system_version!r}" ) if resp.system_identity != version_type: diff --git a/monitoring/monitorlib/clients/versioning/client_interuss_test.py b/monitoring/monitorlib/clients/versioning/client_interuss_test.py index 751e22240f..2589c2dfab 100644 --- a/monitoring/monitorlib/clients/versioning/client_interuss_test.py +++ b/monitoring/monitorlib/clients/versioning/client_interuss_test.py @@ -1,3 +1,4 @@ +import re from datetime import datetime import pytest @@ -90,7 +91,39 @@ def test_get_version_no_system_version(mocker, client): with pytest.raises( VersionQueryError, - match="Response to get version didn't return a system version", + match="Response to get version expected system version to be a string, but instead found a NoneType", + ): + client.get_version("test") + + +def test_get_version_non_string_system_version(mocker, client): + mocker.patch( + "monitoring.monitorlib.clients.versioning.client_interuss.query_and_describe", + return_value=build_query_response( + 200, {"system_identity": "test", "system_version": {"version": "test"}} + ), + ) + + with pytest.raises( + VersionQueryError, + match="Response to get version expected system version to be a string, but instead found a dict", + ): + client.get_version("test") + + +def test_get_version_blank_system_version(mocker, client): + mocker.patch( + "monitoring.monitorlib.clients.versioning.client_interuss.query_and_describe", + return_value=build_query_response( + 200, {"system_identity": "test", "system_version": "\r\n\t "} + ), + ) + + with pytest.raises( + VersionQueryError, + match=re.escape( + "Response to get version expected system version to have a value, but instead got '\\r\\n\\t '" + ), ): client.get_version("test")