Updates to tests to make sure no test pollution is occuring & add som…

…e Payu related changes to them

.github/workflows/ci.yml

@@ -38,9 +38,9 @@ jobs:
         shell: bash -l {0}
         run: conda list
 
-      - name: Run tests
+      - name: Run tests with randomised order
         shell: bash -l {0}
-        run: coverage run -m --source=access_py_telemetry pytest tests
+        run: coverage run -m --source=access_py_telemetry pytest --random-order tests
 
       - name: Generate coverage report
         shell: bash -l {0}

src/access_py_telemetry/api.py

@@ -22,6 +22,7 @@
     config = yaml.safe_load(f)
 
 ENDPOINTS = {registry: content.get("endpoint") for registry, content in config.items()}
+REGISTRIES = {registry for registry in config.keys()}
 SERVER_URL = "https://tracking-services-d6c2fd311c12.herokuapp.com"
 
 
@@ -49,6 +50,7 @@ def __init__(
         self._initialized = True
         self._server_url = SERVER_URL
         self.endpoints = ENDPOINTS
+        self.registries = REGISTRIES
         self._extra_fields: dict[str, dict[str, Any]] = {
             ep_name: {} for ep_name in self.endpoints.keys()
         }
@@ -148,7 +150,7 @@ async def send_telemetry(endpoint: str, data: dict[str, Any]) -> None:
                     print(f"Posting telemetry to {endpoint}")
                     response = await client.post(endpoint, json=data, headers=headers)
                     response.raise_for_status()
-                except httpx.RequestError as e:
+                except (httpx.RequestError, httpx.HTTPStatusError) as e:
                     warnings.warn(
                         f"Request failed: {e}", category=RuntimeWarning, stacklevel=2
                     )

tests/test_api.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+# type: ignore
 
 """Tests for `access_py_telemetry` package."""
 
@@ -15,7 +16,7 @@ def local_host():
 
 @pytest.fixture
 def default_url():
-    return "https://tracking-services-d6c2fd311c12.herokuapp.com"
+    return access_py_telemetry.api.SERVER_URL
 
 
 def test_session_id_properties():
@@ -41,6 +42,8 @@ def test_api_handler_server_url(local_host, default_url):
     """
     Check that the APIHandler class is a singleton.
     """
+    ApiHandler._instance = None
+
     session1 = ApiHandler()
     session2 = ApiHandler()
 
@@ -53,12 +56,16 @@ def test_api_handler_server_url(local_host, default_url):
     session1.server_url = local_host
     assert session2.server_url == local_host
 
+    ApiHandler._instance = None
+
 
 def test_api_handler_extra_fields(local_host):
     """
     Check that adding extra fields to the APIHandler class works as expected.
     """
 
+    ApiHandler._instance = None
+
     session1 = ApiHandler()
     session2 = ApiHandler()
 
@@ -71,7 +78,9 @@ def test_api_handler_extra_fields(local_host):
 
     session1.add_extra_fields("catalog", {"version": "1.0"})
 
-    assert session2.extra_fields == {"catalog": {"version": "1.0"}}
+    blank_registries = {key: {} for key in session1.registries if key != "catalog"}
+
+    assert session2.extra_fields == {"catalog": {"version": "1.0"}, **blank_registries}
 
     with pytest.raises(KeyError) as excinfo:
         session1.add_extra_fields("catalogue", {"version": "2.0"})
@@ -83,8 +92,7 @@ def test_api_handler_extra_fields(local_host):
     assert session1.server_url == local_host
     assert session3.server_url == local_host
 
-    # Reset the server URL to avoid breaking other tests
-    session1.server_url = default_url
+    ApiHandler._instance = None
 
 
 def test_api_handler_extra_fields_validation():
@@ -93,6 +101,7 @@ def test_api_handler_extra_fields_validation():
     to pass the correct types, and only let us update fields through the
     add_extra_field method.
     """
+    ApiHandler._instance = None
     api_handler = ApiHandler()
 
     # Mock a couple of extra services
@@ -123,11 +132,14 @@ def test_api_handler_extra_fields_validation():
         ep_name: {} for ep_name in api_handler.endpoints.keys()
     }
 
+    ApiHandler._instance = None
+
 
 def test_api_handler_remove_fields():
     """
     Check that we can remove fields from the telemetry record.
     """
+    ApiHandler._instance = None
     api_handler = ApiHandler()
 
     # Pretend we only have catalog & payu services and then mock the initialisation
@@ -164,21 +176,17 @@ def test_api_handler_remove_fields():
 
     assert api_handler._pop_fields == {"payu": ["session_id"]}
 
-    # Reset endpoints to avoid breaking other tests - we have to be careful here
-    # because we're using a singleton
-    api_handler.endpoints = access_py_telemetry.api.ENDPOINTS
-    api_handler._extra_fields = {
-        ep_name: {} for ep_name in api_handler.endpoints.keys()
-    }
-    api_handler._pop_fields = {}
+    ApiHandler._instance = None
 
 
-def test_api_handler_send_api_request_no_loop():
+def test_api_handler_send_api_request_no_loop(local_host):
     """
     Create and send an API request with telemetry data.
     """
 
+    ApiHandler._instance = None
     api_handler = ApiHandler()
+    api_handler.server_url = local_host
 
     # Pretend we only have catalog & payu services and then mock the initialisation
     # of the _extra_fields attribute
@@ -217,20 +225,15 @@ def test_api_handler_send_api_request_no_loop():
         "random_number": 2,
     }
 
-    # Reset endpoints to avoid breaking other tests - we have to be careful here
-    # because we're using a singleton
-    api_handler.endpoints = access_py_telemetry.api.ENDPOINTS
-    api_handler._extra_fields = {
-        ep_name: {} for ep_name in api_handler.endpoints.keys()
-    }
-    api_handler._pop_fields = {}
+    ApiHandler._instance = None
 
 
 def test_api_handler_invalid_endpoint():
     """
     Create and send an API request with telemetry data.
     """
 
+    ApiHandler._instance = None
     api_handler = ApiHandler()
 
     # Pretend we only have catalog & payu services and then mock the initialisation
@@ -253,3 +256,6 @@ def test_api_handler_invalid_endpoint():
         )
 
     assert "Endpoint for 'payu' not found " in str(excinfo.value)
+
+    ApiHandler._instance = None
+    api_handler._instance = None

tests/test_cli.py

@@ -1,3 +1,4 @@
+# type: ignore
 import pytest
 from unittest import mock
 

tests/test_decorators.py

@@ -1,3 +1,5 @@
+# type: ignore
+
 from access_py_telemetry.decorators import ipy_register_func, register_func
 from access_py_telemetry.registry import TelemetryRegister
 from access_py_telemetry.api import ApiHandler
@@ -22,9 +24,11 @@ def my_func():
 
     register = TelemetryRegister("catalog")
     api_handler = ApiHandler()
+    blank_registries = {key: {} for key in api_handler.registries if key != "catalog"}
 
     assert api_handler.extra_fields == {
-        "catalog": {"model": "ACCESS-OM2", "random_number": 2}
+        "catalog": {"model": "ACCESS-OM2", "random_number": 2},
+        **blank_registries,
     }
 
     assert api_handler.pop_fields == {"catalog": ["session_id"]}
@@ -35,9 +39,9 @@ def my_func():
 
     register.deregister(my_func.__name__)
 
-    # Reset the api_handler to avoid breaking other tests
-
-    api_handler._extra_fields = {}
+    # Reset the api_handler and register to avoid breaking other tests
+    api_handler._instance = None
+    register._instances = {}
 
 
 @pytest.mark.asyncio
@@ -57,8 +61,11 @@ def my_func():
     register = TelemetryRegister("catalog")
     api_handler = ApiHandler()
 
+    blank_registries = {key: {} for key in api_handler.registries if key != "catalog"}
+
     assert api_handler.extra_fields == {
-        "catalog": {"model": "ACCESS-OM2", "random_number": 2}
+        "catalog": {"model": "ACCESS-OM2", "random_number": 2},
+        **blank_registries,
     }
 
     assert api_handler.pop_fields == {"catalog": ["session_id"]}
@@ -81,6 +88,6 @@ def my_func():
 
     register.deregister(my_func.__name__)
 
-    # Reset the api_handler to avoid breaking other tests
-
-    api_handler._extra_fields = {}
+    # Reset the api_handler and register to avoid breaking other tests
+    api_handler._instance = None
+    register._instances = {}

tests/test_registry.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+# type: ignore
 
 """Tests for `access_py_telemetry` package."""
 
@@ -12,6 +13,7 @@ def test_telemetry_register_unique():
     Check that the TelemetryRegister class is a singleton & that we can register
     and deregister functions as we would expect.
     """
+    TelemetryRegister._instances = {}
     session1 = TelemetryRegister("catalog")
     session2 = TelemetryRegister("catalog")
 
@@ -46,12 +48,11 @@ def test_telemetry_register_unique():
         "DfFileCatalog.search",
     }
 
-    from access_py_telemetry.registry import REGISTRIES
-
-    session1.registry = REGISTRIES["catalog"]
+    TelemetryRegister._instances = {}
 
 
 def test_telemetry_register_validation():
+    TelemetryRegister._instances = {}
     session_register = TelemetryRegister("catalog")
 
     with pytest.raises(ValidationError):
@@ -88,6 +89,4 @@ def test_function():
 
     assert "test_function" not in session_register
 
-    from access_py_telemetry.registry import REGISTRIES
-
-    session_register.registry = REGISTRIES["catalog"]
+    TelemetryRegister._instances = {}