From ab5e9340f3b2db73ba9a9498d124a75577b67bf1 Mon Sep 17 00:00:00 2001
From: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com>
Date: Tue, 28 Jan 2025 11:07:10 -0600
Subject: [PATCH] fix unit tests that use app url

---
 .secrets.baseline         |  4 ++--
 tests/conftest.py         | 36 ++++++++++++++++++++++++++----------
 tests/test_s3_endpoint.py |  7 -------
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/.secrets.baseline b/.secrets.baseline
index 62574c6..79e5540 100644
--- a/.secrets.baseline
+++ b/.secrets.baseline
@@ -187,7 +187,7 @@
         "filename": "tests/conftest.py",
         "hashed_secret": "0dd78d9147bb410f0cb0199c5037da36594f77d8",
         "is_verified": false,
-        "line_number": 226
+        "line_number": 228
       }
     ],
     "tests/migrations/test_migration_e1886270d9d2.py": [
@@ -209,5 +209,5 @@
       }
     ]
   },
-  "generated_at": "2025-01-27T18:55:41Z"
+  "generated_at": "2025-01-28T17:07:00Z"
 }
diff --git a/tests/conftest.py b/tests/conftest.py
index 188a6e9..d9fb101 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2,10 +2,12 @@
 See https://github.com/uc-cdis/gen3-user-data-library/blob/main/tests/conftest.py#L1
 """
 
+import contextlib
 from datetime import datetime
 from dateutil.tz import tzutc
 import json
 import os
+import time
 from unittest.mock import MagicMock, patch
 from urllib.parse import parse_qsl, urlparse
 
@@ -274,6 +276,25 @@ async def reset_requests_mocks():
     mock_arborist_request.reset_mock()
 
 
+class UvicornServer(uvicorn.Server):
+    """
+    Server that can be stopped when the unit test completes. Used for tests that need to hit
+    the app URL directly.
+    """
+
+    @contextlib.contextmanager
+    def run_in_thread(self):
+        thread = Thread(target=self.run)
+        thread.start()
+        try:
+            while not self.started:
+                time.sleep(1e-3)
+            yield
+        finally:
+            self.should_exit = True
+            thread.join()
+
+
 @pytest_asyncio.fixture(scope="session")
 async def client(request):
     """
@@ -343,16 +364,11 @@ async def handle_request(request: Request):
     if get_url:  # for tests that need to hit the app URL directly
         host = "0.0.0.0"
         port = 8080
-
-        def run_uvicorn():
-            uvicorn.run(app, host=host, port=port)
-
-        # start the app in a separate thread
-        thread = Thread(target=run_uvicorn)
-        thread.daemon = True  # ensures the thread ends when the test ends
-        thread.start()
-
-        yield f"http://{host}:{port}"  # URL to use in the tests
+        server = UvicornServer(
+            config=uvicorn.Config(app, host=host, port=port, log_level="debug")
+        )
+        with server.run_in_thread():
+            yield f"http://{host}:{port}"
     else:
         # the tests use a real httpx client that forwards requests to the app
         async with httpx.AsyncClient(
diff --git a/tests/test_s3_endpoint.py b/tests/test_s3_endpoint.py
index 4d5ced2..88500d5 100644
--- a/tests/test_s3_endpoint.py
+++ b/tests/test_s3_endpoint.py
@@ -43,13 +43,6 @@ def test_s3_endpoint_no_token(s3_client):
         s3_client.list_objects(Bucket=f"gen3wf-{config['HOSTNAME']}-{TEST_USER_ID}")
 
 
-# This test currently doesn't work because the client generated when `get_url` is True is not
-# stopped properly, so generating a different client (with `authorized=False` param) triggers an
-# error:
-# > OSError: [Errno 48] error while attempting to bind on address ('0.0.0.0', 8080): address already
-#   in use
-# TODO fix that
-@pytest.mark.skip(reason="doesn't work")
 @pytest.mark.parametrize(
     "client", [{"get_url": True, "authorized": False}], indirect=True
 )