Fix batch submission

src/buildkite_test_collector/collector/api.py

@@ -1,6 +1,6 @@
 """Buildkite Test Analytics API"""
 
-from typing import Optional
+from typing import Any, Generator, Optional
 from os import environ
 import traceback
 from requests import post, Response
@@ -9,18 +9,19 @@
 from ..pytest_plugin.logger import logger
 
 
-def submit(payload: Payload, batch_size=100) -> Optional[Response]:
+def submit(payload: Payload, batch_size=100) -> Generator[Optional[Response], Any, Any]:
     """Submit a payload to the API"""
     token = environ.get("BUILDKITE_ANALYTICS_TOKEN")
     api_url = environ.get("BUILDKITE_ANALYTICS_API_URL", "https://analytics-api.buildkite.com/v1")
     response = None
 
     if not token:
         logger.warning("No `BUILDKITE_ANALYTICS_TOKEN` environment variable present")
+        yield None
 
-    if token:
-        try:
-            for payload_slice in payload.into_batches(batch_size):
+    else:
+        for payload_slice in payload.into_batches(batch_size):
+            try:
                 response = post(api_url + "/uploads",
                                 json=payload_slice.as_json(),
                                 headers={
@@ -29,14 +30,16 @@ def submit(payload: Payload, batch_size=100) -> Optional[Response]:
                                 },
                                 timeout=60)
                 response.raise_for_status()
-                return response
-        except InvalidHeader as error:
-            logger.warning("Invalid `BUILDKITE_ANALYTICS_TOKEN` environment variable")
-            logger.warning(error)
-        except HTTPError as err:
-            logger.warning("Failed to uploads test results to buildkite")
-            logger.warning(err)
-        except Exception:  # pylint: disable=broad-except
-            error_message = traceback.format_exc()
-            logger.warning(error_message)
-    return None
+                yield response
+            except InvalidHeader as error:
+                logger.warning("Invalid `BUILDKITE_ANALYTICS_TOKEN` environment variable")
+                logger.warning(error)
+                yield None
+            except HTTPError as err:
+                logger.warning("Failed to uploads test results to buildkite")
+                logger.warning(err)
+                yield None
+            except Exception:  # pylint: disable=broad-except
+                error_message = traceback.format_exc()
+                logger.warning(error_message)
+                yield None

src/buildkite_test_collector/pytest_plugin/__init__.py

@@ -47,11 +47,11 @@ def pytest_unconfigure(config):
 
         # When xdist is not installed, or when it's installed and not enabled
         if not xdist_enabled:
-            submit(plugin.payload)
+            list(submit(plugin.payload))
 
         # When xdist is activated, we want to submit from worker thread only, because they have access to tag data
         if xdist_enabled and is_xdist_worker:
-            submit(plugin.payload)
+            list(submit(plugin.payload))
 
         # We only want a single thread to write to the json file.
         # When xdist is enabled, that will be the controller thread.

tests/buildkite_test_collector/collector/test_api.py

@@ -16,14 +16,14 @@ def test_submit_with_missing_api_key_environment_variable_returns_none():
     with mock.patch.dict(os.environ, {"CI": "true", "BUILDKITE_ANALYTICS_TOKEN": ""}):
         payload = Payload.init(detect_env())
 
-        assert submit(payload) is None
+        assert next(submit(payload)) is None
 
 
 def test_submit_with_invalid_api_key_environment_variable_returns_none():
     with mock.patch.dict(os.environ, {"CI": "true", "BUILDKITE_ANALYTICS_TOKEN": "\n"}):
         payload = Payload.init(detect_env())
 
-        assert submit(payload) is None
+        assert next(submit(payload)) is None
 
 @responses.activate
 @pytest.mark.skipif(sys.version_info < (3, 9), reason="requires python3.9 or higher")
@@ -39,7 +39,7 @@ def test_submit_with_payload_timeout_captures_ConnectTimeout_error(capfd, succes
 
         payload = payload.push_test_data(successful_test)
 
-        result = submit(payload)
+        result = next(submit(payload))
         captured = capfd.readouterr()
 
         assert captured.err.startswith("buildkite-test-collector - WARNING -")
@@ -59,7 +59,7 @@ def test_submit_with_payload_timeout_captures_ReadTimeout_error(capfd, successfu
 
         payload = payload.push_test_data(successful_test)
 
-        result = submit(payload)
+        result = next(submit(payload))
         captured = capfd.readouterr()
 
         assert captured.err.startswith("buildkite-test-collector - WARNING -")
@@ -84,7 +84,8 @@ def test_submit_with_payload_returns_an_api_response(successful_test):
 
         payload = payload.push_test_data(successful_test)
 
-        result = submit(payload)
+        result = next(submit(payload))
+        assert result
 
         assert result.status_code >= 200
         assert result.status_code < 300
@@ -107,7 +108,7 @@ def test_submit_with_bad_response(successful_test):
 
         payload = payload.push_test_data(successful_test)
 
-        result = submit(payload)
+        result = next(submit(payload))
 
         assert result is None
 
@@ -141,8 +142,52 @@ def test_submit_with_large_payload_batches_requests(successful_test, failed_test
         payload = payload.push_test_data(successful_test)
         payload = payload.push_test_data(failed_test)
 
-        result = submit(payload, batch_size=1)
+        results = [response for response in submit(payload, batch_size=1)]
+        assert len(results) == 2
 
+        for result in results:
+            assert result
+            assert result.status_code >= 200
+            assert result.status_code < 300
+
+            json = result.json()
+            assert len(json["errors"]) == 0
+            assert json['queued'] == 1
+
+@responses.activate
+def test_submit_with_batches_and_errors(capfd, successful_test, failed_test):
+    responses.add(
+        responses.POST,
+        "https://analytics-api.buildkite.com/v1/uploads",
+        body=ConnectTimeout("Error"))
+    responses.add(
+        responses.POST,
+        "https://analytics-api.buildkite.com/v1/uploads",
+        json={'id': str(uuid4()),
+              'run_id': str(uuid4()),
+              'queued': 1,
+              'skipped': 0,
+              'errors': [],
+              'run_url': 'https://buildkite.com/organizations/alembic/analytics/suites/test/runs/52c5d9f6-a4f2-4a2d-a1e6-993335789c92'},
+        status=202)
+
+    with mock.patch.dict(os.environ, {"CI": "true", "BUILDKITE_ANALYTICS_TOKEN": str(uuid4())}):
+        payload = Payload.init(detect_env())
+        payload = Payload.started(payload)
+
+        payload = payload.push_test_data(successful_test)
+        payload = payload.push_test_data(failed_test)
+
+        results = [response for response in submit(payload, batch_size=1)]
+        assert len(results) == 2
+
+        assert not results[0]
+        captured = capfd.readouterr()
+        assert captured.err.startswith("buildkite-test-collector - WARNING -")
+        assert "ConnectTimeout" in captured.err
+
+        result = results[1]
+        assert result
         assert result.status_code >= 200
         assert result.status_code < 300
 
@@ -172,7 +217,8 @@ def test_api_url_override(successful_test):
 
         payload = payload.push_test_data(successful_test)
 
-        result = submit(payload)
+        result = next(submit(payload))
+        assert result
 
         assert result.status_code >= 200
         assert result.status_code < 300