Add retry decorator to tests that are vulnerable to transient service issues (#421)

Some tests in python-sdk are vulnerable to bad responses from the cloud
service. For example, an image query might get a result of
STILL_PROCESSING, which means the cloud didn't have an answer in time.
This is a transient error and will almost always be resolved with a
retry.

This PR adds a retry decorator to protect such tests. Any test that
submits an image query and asserts anything about the result are
protected with this decorator.

I also found a few instances of tests that were not using our standard
`detector_name` function for naming detectors. I fixed those too.

---------

Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai>

Author: timmarkhuff

test/integration/test_groundlight.py

@@ -29,6 +29,8 @@
 from urllib3.exceptions import ConnectTimeoutError, MaxRetryError, ReadTimeoutError
 from urllib3.util.retry import Retry
 
+from test.retry_decorator import retry_on_failure
+
 DEFAULT_CONFIDENCE_THRESHOLD = 0.9
 IQ_IMPROVEMENT_THRESHOLD = 0.75
 
@@ -273,20 +275,23 @@ def test_get_detector_by_name(gl: Groundlight, detector: Detector):
         gl.get_detector_by_name(name="not a real name")
 
 
+@retry_on_failure()
 def test_ask_confident(gl: Groundlight, detector: Detector):
     _image_query = gl.ask_confident(detector=detector.id, image="test/assets/dog.jpeg", wait=10)
     assert str(_image_query)
     assert isinstance(_image_query, ImageQuery)
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_ask_ml(gl: Groundlight, detector: Detector):
     _image_query = gl.ask_ml(detector=detector.id, image="test/assets/dog.jpeg", wait=10)
     assert str(_image_query)
     assert isinstance(_image_query, ImageQuery)
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_submit_image_query(gl: Groundlight, detector: Detector):
     def validate_image_query(_image_query: ImageQuery):
         assert str(_image_query)
@@ -314,6 +319,7 @@ def validate_image_query(_image_query: ImageQuery):
     assert _image_query.result.confidence >= IQ_IMPROVEMENT_THRESHOLD
 
 
+@retry_on_failure()
 def test_submit_image_query_blocking(gl: Groundlight, detector: Detector):
     _image_query = gl.submit_image_query(
         detector=detector.id, image="test/assets/dog.jpeg", wait=10, human_review="NEVER"
@@ -323,13 +329,15 @@ def test_submit_image_query_blocking(gl: Groundlight, detector: Detector):
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_submit_image_query_returns_yes(gl: Groundlight):
     # We use the "never-review" pipeline to guarantee a confident "yes" answer.
     detector = gl.get_or_create_detector(name="Always a dog", query="Is there a dog?", pipeline_config="never-review")
     image_query = gl.submit_image_query(detector=detector, image="test/assets/dog.jpeg", wait=10, human_review="NEVER")
     assert image_query.result.label == Label.YES
 
 
+@retry_on_failure()
 def test_submit_image_query_returns_text(gl: Groundlight):
     # We use the "never-review" pipeline to guarantee a confident "yes" answer.
     detector = gl.get_or_create_detector(
@@ -339,20 +347,23 @@ def test_submit_image_query_returns_text(gl: Groundlight):
     assert isinstance(image_query.text, str)
 
 
+@retry_on_failure()
 def test_submit_image_query_filename(gl: Groundlight, detector: Detector):
     _image_query = gl.submit_image_query(detector=detector.id, image="test/assets/dog.jpeg", human_review="NEVER")
     assert str(_image_query)
     assert isinstance(_image_query, ImageQuery)
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_submit_image_query_png(gl: Groundlight, detector: Detector):
     _image_query = gl.submit_image_query(detector=detector.id, image="test/assets/cat.png", human_review="NEVER")
     assert str(_image_query)
     assert isinstance(_image_query, ImageQuery)
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_submit_image_query_with_confidence_threshold(gl: Groundlight, detector: Detector):
     confidence_threshold = 0.5234  # Arbitrary specific value
     _image_query = gl.submit_image_query(
@@ -366,6 +377,7 @@ def test_submit_image_query_with_confidence_threshold(gl: Groundlight, detector:
 
 
 @pytest.mark.skip_for_edge_endpoint(reason="The edge-endpoint does not support passing an image query ID.")
+@retry_on_failure()
 def test_submit_image_query_with_id(gl: Groundlight, detector: Detector):
     # submit_image_query
     id = f"iq_{KsuidMs()}"
@@ -380,6 +392,7 @@ def test_submit_image_query_with_id(gl: Groundlight, detector: Detector):
     assert _image_query.metadata.get("is_from_edge")
 
 
+@retry_on_failure()
 def test_submit_image_query_with_human_review_param(gl: Groundlight, detector: Detector):
     # For now, this just tests that the image query is submitted successfully.
     # There should probably be a better way to check whether the image query was escalated for human review.
@@ -451,6 +464,7 @@ def test_create_detector_with_invalid_metadata(gl: Groundlight, metadata_list: A
 
 @pytest.mark.skip_for_edge_endpoint(reason="The edge-endpoint does not support passing image query metadata.")
 @pytest.mark.parametrize("metadata", [None, {}, {"a": 1}, '{"a": 1}'])
+@retry_on_failure()
 def test_submit_image_query_with_metadata(
     gl: Groundlight, detector: Detector, image: str, metadata: Union[Dict, str, None]
 ):
@@ -505,6 +519,7 @@ def test_submit_image_query_with_metadata_returns_user_error(gl: Groundlight, de
     assert is_user_error(exc_info.value.status)
 
 
+@retry_on_failure()
 def test_submit_image_query_jpeg_bytes(gl: Groundlight, detector: Detector):
     jpeg = open("test/assets/dog.jpeg", "rb").read()
     _image_query = gl.submit_image_query(detector=detector.id, image=jpeg, human_review="NEVER")
@@ -543,6 +558,7 @@ def test_submit_image_query_bad_jpeg_file(gl: Groundlight, detector: Detector):
 
 
 @pytest.mark.skipif(MISSING_PIL, reason="Needs pillow")  # type: ignore
+@retry_on_failure()
 def test_submit_image_query_pil(gl: Groundlight, detector: Detector):
     # generates a pil image and submits it
     from PIL import Image
@@ -565,6 +581,7 @@ def test_submit_image_query_wait_and_want_async_causes_exception(gl: Groundlight
         )
 
 
+@retry_on_failure()
 def test_submit_image_query_with_want_async_workflow(gl: Groundlight, detector: Detector):
     """
     Tests the workflow for submitting an image query with the want_async parameter set to True.
@@ -589,6 +606,7 @@ def test_submit_image_query_with_want_async_workflow(gl: Groundlight, detector:
     assert _image_query.result.label in VALID_DISPLAY_LABELS
 
 
+@retry_on_failure()
 def test_ask_async_workflow(gl: Groundlight, detector: Detector):
     """
     Tests the workflow for submitting an image query with ask_async.
@@ -638,19 +656,22 @@ def test_list_image_queries_with_filter(gl: Groundlight, detector_name: Callable
         assert image_query.id in iq_ids
 
 
+@retry_on_failure()
 def test_get_image_query(gl: Groundlight, image_query_yes: ImageQuery):
     _image_query = gl.get_image_query(id=image_query_yes.id)
     assert str(_image_query)
     assert isinstance(_image_query, ImageQuery)
     assert is_valid_display_result(_image_query.result)
 
 
+@retry_on_failure()
 def test_get_image_query_label_yes(gl: Groundlight, image_query_yes: ImageQuery):
     gl.add_label(image_query_yes, Label.YES)
     retrieved_iq = gl.get_image_query(id=image_query_yes.id)
     assert retrieved_iq.result.label == Label.YES
 
 
+@retry_on_failure()
 def test_get_image_query_label_no(gl: Groundlight, image_query_no: ImageQuery):
     gl.add_label(image_query_no, Label.NO)
     retrieved_iq = gl.get_image_query(id=image_query_no.id)
@@ -709,6 +730,7 @@ def test_enum_string_equality():
 
 
 @pytest.mark.skipif(MISSING_NUMPY or MISSING_PIL, reason="Needs numpy and pillow")  # type: ignore
+@retry_on_failure()
 def test_submit_numpy_image(gl: Groundlight, detector: Detector):
     np_img = np.random.uniform(0, 255, (600, 800, 3))  # type: ignore
     _image_query = gl.submit_image_query(detector=detector.id, image=np_img, human_review="NEVER")
@@ -771,6 +793,7 @@ def test_update_inspection_metadata_invalid_inspection_id(gl: Groundlight):
 
 
 @pytest.mark.skip_for_edge_endpoint(reason="The edge-endpoint doesn't support inspection_id")
+@retry_on_failure()
 def test_stop_inspection_pass(gl: Groundlight, detector: Detector):
     """Starts an inspection, submits a query with the inspection ID that should pass, stops
     the inspection, checks the result.
@@ -820,6 +843,7 @@ def test_update_detector_confidence_threshold_failure(gl: Groundlight, detector:
 
 
 @pytest.mark.skip_for_edge_endpoint(reason="The edge-endpoint does not support passing detector metadata.")
+@retry_on_failure()
 def test_submit_image_query_with_inspection_id_metadata_and_want_async(gl: Groundlight, detector: Detector, image: str):
     inspection_id = gl.start_inspection()
     metadata = {"key": "value"}
@@ -852,6 +876,7 @@ def test_submit_image_query_with_empty_inspection_id(gl: Groundlight, detector:
     )
 
 
+@retry_on_failure()
 def test_binary_detector(gl: Groundlight, detector_name: Callable):
     """
     verify that we can create and submit to a binary detector
@@ -863,6 +888,7 @@ def test_binary_detector(gl: Groundlight, detector_name: Callable):
     assert binary_iq.result.label is not None
 
 
+@retry_on_failure()
 def test_counting_detector(gl: Groundlight, detector_name: Callable):
     """
     verify that we can create and submit to a counting detector
@@ -874,6 +900,7 @@ def test_counting_detector(gl: Groundlight, detector_name: Callable):
     assert count_iq.result.count is not None
 
 
+@retry_on_failure()
 def test_counting_detector_async(gl: Groundlight, detector_name: Callable):
     """
     verify that we can create and submit to a counting detector
@@ -893,6 +920,7 @@ def test_counting_detector_async(gl: Groundlight, detector_name: Callable):
     assert _image_query.result is not None
 
 
+@retry_on_failure()
 def test_multiclass_detector(gl: Groundlight, detector_name: Callable):
     """
     verify that we can create and submit to a multi-class detector

test/retry_decorator.py

@@ -0,0 +1,33 @@
+"""Test-only helpers for retrying tests affected by transient cloud timing."""
+
+import functools
+import time
+from typing import Any, Callable, Tuple, Type
+
+
+def retry_on_failure(
+    *,
+    max_attempts: int = 2,
+    exception_types: Tuple[Type[BaseException], ...] = (AssertionError,),
+    retry_delay_seconds: float = 5.0,
+) -> Callable[[Callable[..., Any]], Callable[..., None]]:
+    """Run the wrapped test up to `max_attempts` times when it raises a listed exception."""
+
+    if max_attempts < 1:
+        raise ValueError("max_attempts must be at least 1")
+
+    def decorator(fn: Callable[..., Any]) -> Callable[..., None]:
+        @functools.wraps(fn)
+        def wrapper(*args: object, **kwargs: object) -> None:
+            for attempt in range(max_attempts):
+                try:
+                    fn(*args, **kwargs)
+                    return
+                except exception_types:
+                    if attempt == max_attempts - 1:
+                        raise
+                    time.sleep(retry_delay_seconds)
+
+        return wrapper
+
+    return decorator

test/unit/test_customizable_request.py

@@ -1,12 +1,11 @@
-from datetime import datetime
+from typing import Callable
 
 from groundlight import ExperimentalApi
 
 gl = ExperimentalApi()
 
 
-def test_invalid_endpoint_config():
+def test_invalid_endpoint_config(detector_name: Callable):
     print(gl.make_generic_api_request(endpoint="/v1/me", method="GET"))
     print(gl.make_generic_api_request(endpoint="/v1/detectors", method="GET"))
-    name = f"Test {datetime.utcnow()}"
-    print(gl.make_generic_api_request(endpoint="/v1/detector-groups", method="POST", body={"name": name}))
+    print(gl.make_generic_api_request(endpoint="/v1/detector-groups", method="POST", body={"name": detector_name()}))

test/unit/test_experimental.py

@@ -5,6 +5,8 @@
 from groundlight import ExperimentalApi
 from model import Detector, ImageQuery
 
+from test.retry_decorator import retry_on_failure
+
 
 def test_detector_groups(gl_experimental: ExperimentalApi, detector_name: Callable):
     """
@@ -90,6 +92,7 @@ def test_submit_multiple_rois(gl_experimental: ExperimentalApi, image_query_one:
     gl_experimental.add_label(image_query_one, 3, [roi] * 3)
 
 
+@retry_on_failure()
 def test_text_recognition_detector(gl_experimental: ExperimentalApi, detector_name: Callable):
     """
     verify that we can create and submit to a text recognition detector
@@ -103,6 +106,7 @@ def test_text_recognition_detector(gl_experimental: ExperimentalApi, detector_na
     assert mc_iq.result.text is not None
 
 
+@retry_on_failure()
 def test_bounding_box_detector(gl_experimental: ExperimentalApi, detector_name: Callable):
     """
     Verify that we can create and submit to a bounding box detector
@@ -117,6 +121,7 @@ def test_bounding_box_detector(gl_experimental: ExperimentalApi, detector_name:
     assert bbox_iq.rois is not None
 
 
+@retry_on_failure()
 def test_bounding_box_detector_async(gl_experimental: ExperimentalApi, detector_name: Callable):
     """
     Verify that we can create and submit to a bounding box detector with ask_async

test/unit/test_images.py

@@ -1,12 +1,14 @@
-from datetime import datetime
+from typing import Callable
 
 import PIL
 from groundlight import ExperimentalApi
 
+from test.retry_decorator import retry_on_failure
 
-def test_get_image(gl_experimental: ExperimentalApi):
-    name = f"Test {datetime.utcnow()}"
-    det = gl_experimental.get_or_create_detector(name, "test_query")
+
+@retry_on_failure()
+def test_get_image(gl_experimental: ExperimentalApi, detector_name: Callable):
+    det = gl_experimental.get_or_create_detector(detector_name(), "test_query")
     iq = gl_experimental.submit_image_query(det, image="test/assets/dog.jpeg", wait=10)
     gl_experimental.get_image(iq.id)
     assert isinstance(PIL.Image.open(gl_experimental.get_image(iq.id)), PIL.Image.Image)

test/unit/test_internalapi.py

@@ -1,18 +1,20 @@
+from typing import Callable
+
 from groundlight import ExperimentalApi
 from groundlight.internalapi import iq_is_answered, iq_is_confident
 from model import ImageQuery
 
 
-def test_iq_is_confident(gl_experimental: ExperimentalApi, initial_iq: ImageQuery):
-    det = gl_experimental.get_or_create_detector("Test", "test_query")
+def test_iq_is_confident(gl_experimental: ExperimentalApi, initial_iq: ImageQuery, detector_name: Callable):
+    det = gl_experimental.get_or_create_detector(detector_name(), "test_query")
     iq = gl_experimental.ask_async(det, image="test/assets/dog.jpeg")
     assert not iq_is_confident(iq, 0.9)
 
     assert not iq_is_confident(initial_iq, 0.9)
 
 
-def test_iq_is_answered(gl_experimental: ExperimentalApi, initial_iq: ImageQuery):
-    det = gl_experimental.get_or_create_detector("Test", "test_query")
+def test_iq_is_answered(gl_experimental: ExperimentalApi, initial_iq: ImageQuery, detector_name: Callable):
+    det = gl_experimental.get_or_create_detector(detector_name(), "test_query")
     iq = gl_experimental.ask_async(det, image="test/assets/dog.jpeg")
     assert not iq_is_answered(iq)