diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 676995431..969f1c6da 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -131,6 +131,7 @@ jobs: - tests/mock_vws/test_model_target_retries.py - tests/mock_vws/vumark_generation_api - tests/mock_vws/test_vumark_generation_failure.py + - tests/mock_vws/test_target_setup_retries.py - tests/mock_vws/test_target_validators.py - tests/mock_vws/requests_mock - tests/mock_vws/respx_mock diff --git a/tests/mock_vws/fixtures/api_contract.py b/tests/mock_vws/fixtures/api_contract.py index 49c2ec99f..62dd56cd4 100644 --- a/tests/mock_vws/fixtures/api_contract.py +++ b/tests/mock_vws/fixtures/api_contract.py @@ -2,6 +2,7 @@ import base64 import binascii +import contextlib import io import uuid from dataclasses import dataclass @@ -12,7 +13,7 @@ from tenacity.retry import retry_if_not_result from tenacity.stop import stop_after_attempt from vws import VWS, CloudRecoService -from vws.reports import TargetStatuses +from vws.reports import TargetStatusAndRecord, TargetStatuses from mock_vws.database import CloudDatabase from tests.mock_vws.utils import Endpoint, ModelTargetEndpoint @@ -106,9 +107,12 @@ def _add_target(*, vws_client: VWS, image: io.BytesIO) -> str: @beartype +@RETRY_ON_TRANSIENT_VWS_FAILURE @RETRY_ON_TARGET_NOT_YET_VISIBLE -def _wait_for_target_processed(*, vws_client: VWS, target_id_: str) -> None: - """Wait for a target to finish processing. +def _get_processed_target( + *, vws_client: VWS, target_id_: str +) -> TargetStatusAndRecord: + """Wait for and read a known target without repeating its creation. Real Vuforia sometimes reports a target which it has just created as unknown, because the target is not yet visible to the read path. We know @@ -116,6 +120,7 @@ def _wait_for_target_processed(*, vws_client: VWS, target_id_: str) -> None: than fail. """ vws_client.wait_for_target_processed(target_id=target_id_) + return vws_client.get_target_record(target_id=target_id_) @dataclass(frozen=True, kw_only=True) @@ -135,7 +140,6 @@ def _target_processed_successfully(result: _TargetProcessingResult) -> bool: @beartype -@RETRY_ON_TRANSIENT_VWS_FAILURE @retry( retry=retry_if_not_result(predicate=_target_processed_successfully), stop=stop_after_attempt(max_attempt_number=_TARGET_SUCCESS_ATTEMPTS), @@ -156,14 +160,21 @@ def _add_target_which_processed_successfully( The ID and processing status of the added target. """ target_id_ = _add_target(vws_client=vws_client, image=image) - _wait_for_target_processed( - vws_client=vws_client, - target_id_=target_id_, - ) - target_details = vws_client.get_target_record(target_id=target_id_) + try: + target_details = _get_processed_target( + vws_client=vws_client, + target_id_=target_id_, + ) + except Exception: + # Setup owns the target until it can return it to the test. Cleanup + # is best effort: a processing target may still reject deletion. + # The real adapter also clears remaining targets before the next test. + with contextlib.suppress(Exception): + vws_client.delete_target(target_id=target_id_) + raise # Failed processing is nondeterministic against the live service. if target_details.status != TargetStatuses.SUCCESS: - vws_client.delete_target(target_id=target_id_) # pragma: no cover + vws_client.delete_target(target_id=target_id_) return _TargetProcessingResult( target_id=target_id_, status=target_details.status, diff --git a/tests/mock_vws/test_target_setup_retries.py b/tests/mock_vws/test_target_setup_retries.py new file mode 100644 index 000000000..2185dc34a --- /dev/null +++ b/tests/mock_vws/test_target_setup_retries.py @@ -0,0 +1,201 @@ +"""Regression tests for retries while preparing a successful target.""" + +import io +import time +from unittest.mock import call, patch + +import pytest +from requests.exceptions import ReadTimeout +from tenacity import RetryError +from vws import VWS +from vws.exceptions.vws_exceptions import ( + TooManyRequestsError, + UnknownTargetError, +) +from vws.reports import TargetRecord, TargetStatusAndRecord, TargetStatuses +from vws.response import Response + +from tests.mock_vws.fixtures.api_contract import ( + _add_target_which_processed_successfully, # pyright: ignore[reportPrivateUsage] +) +from tests.mock_vws.utils.retries import TRANSIENT_VWS_RETRY_ATTEMPTS + + +def _record(*, status: TargetStatuses) -> TargetStatusAndRecord: + """Return a record for a target whose processing has finished.""" + return TargetStatusAndRecord( + status=status, + target_record=TargetRecord( + target_id="target", + active_flag=True, + name="name", + width=1, + tracking_rating=5, + reco_rating="", + ), + ) + + +def _throttle() -> TooManyRequestsError: + """Return the empty 429 response observed during fixture setup.""" + return TooManyRequestsError( + response=Response( + text="", + url="https://example.com/summary/target", + status_code=429, + headers={"x-envoy-ratelimited": "true"}, + request_body=None, + tell_position=0, + content=b"", + ) + ) + + +def _no_sleep(seconds: float) -> None: + """Keep retry tests independent of wall-clock delays.""" + del seconds + + +@pytest.fixture(autouse=True) +def fixture_no_sleep(*, monkeypatch: pytest.MonkeyPatch) -> None: + """Exercise the actual retry decorators without sleeping.""" + monkeypatch.setattr(target=time, name="sleep", value=_no_sleep) + + +@pytest.fixture(name="client") +def fixture_client() -> VWS: + """Use synthetic credentials; all client operations are controlled.""" + return VWS(server_access_key="key", server_secret_key="secret") + + +@pytest.mark.parametrize(argnames="operation", argvalues=["poll", "record"]) +@pytest.mark.parametrize( + argnames="failure", argvalues=["429", "timeout", "unknown"] +) +def test_read_recovery_keeps_target( + *, + client: VWS, + operation: str, + failure: str, +) -> None: + """A transient read or visibility failure never repeats the upload.""" + error = { + "429": _throttle(), + "timeout": ReadTimeout("timed out"), + "unknown": UnknownTargetError(response=_throttle().response), + }[failure] + record = _record(status=TargetStatuses.SUCCESS) + with ( + patch.object( + target=VWS, attribute="add_target", return_value="target" + ) as add, + patch.object( + target=VWS, + attribute="wait_for_target_processed", + side_effect=[error, None] if operation == "poll" else None, + ) as poll, + patch.object( + target=VWS, + attribute="get_target_record", + side_effect=[error, record] if operation == "record" else None, + return_value=record, + ) as read, + patch.object(target=VWS, attribute="delete_target") as delete, + ): + result = _add_target_which_processed_successfully( + vws_client=client, image=io.BytesIO() + ) + assert result.target_id == "target" + add.assert_called_once() + assert poll.call_args_list == [call(target_id="target")] * 2 + assert read.call_args_list == [call(target_id="target")] * ( + 2 if operation == "record" else 1 + ) + delete.assert_not_called() + + +@pytest.mark.parametrize(argnames="operation", argvalues=["poll", "record"]) +@pytest.mark.parametrize(argnames="cleanup_fails", argvalues=[False, True]) +def test_read_exhaustion_preserves_error( + *, + client: VWS, + operation: str, + cleanup_fails: bool, +) -> None: + """Exhaustion creates no replacement and cleanup cannot hide the error.""" + error = _throttle() + with ( + patch.object( + target=VWS, attribute="add_target", return_value="target" + ) as add, + patch.object( + target=VWS, + attribute="wait_for_target_processed", + side_effect=error if operation == "poll" else None, + ) as poll, + patch.object( + target=VWS, attribute="get_target_record", side_effect=error + ) as read, + patch.object( + target=VWS, + attribute="delete_target", + side_effect=RuntimeError("cleanup failed") + if cleanup_fails + else None, + ) as delete, + pytest.raises(expected_exception=TooManyRequestsError) as exc, + ): + _ = _add_target_which_processed_successfully( + vws_client=client, image=io.BytesIO() + ) + assert exc.value is error + add.assert_called_once() + assert poll.call_count == TRANSIENT_VWS_RETRY_ATTEMPTS + assert read.call_count == ( + 0 if operation == "poll" else TRANSIENT_VWS_RETRY_ATTEMPTS + ) + delete.assert_called_once_with(target_id="target") + + +@pytest.mark.parametrize(argnames="recovers", argvalues=[False, True]) +def test_failed_processing_uses_bounded_replacements( + *, client: VWS, recovers: bool +) -> None: + """Only an actual failed processing result triggers a new upload.""" + failed = _record(status=TargetStatuses.FAILED) + success = _record(status=TargetStatuses.SUCCESS) + with ( + patch.object( + target=VWS, + attribute="add_target", + side_effect=["first", "second", "third"], + ) as add, + patch.object(target=VWS, attribute="wait_for_target_processed"), + patch.object( + target=VWS, + attribute="get_target_record", + side_effect=[failed, success] if recovers else None, + return_value=failed, + ), + patch.object(target=VWS, attribute="delete_target") as delete, + ): + if recovers: + result = _add_target_which_processed_successfully( + vws_client=client, image=io.BytesIO() + ) + assert result.target_id == "second" + else: + with pytest.raises(expected_exception=RetryError): + _ = _add_target_which_processed_successfully( + vws_client=client, image=io.BytesIO() + ) + assert add.call_count == (2 if recovers else 3) + assert delete.call_args_list == ( + [call(target_id="first")] + if recovers + else [ + call(target_id="first"), + call(target_id="second"), + call(target_id="third"), + ] + )