Functional tests for /azure_byod (#589)

* Functional tests for /azure_byod

* Split tests into two

* Refactor + README

code/tests/functional/backend_api/app_config.py

@@ -1,33 +1,40 @@
 import logging
 import os
-from typing import Any, Dict
 
 
 class AppConfig:
-    config: Dict[str, Any] = {
+    config: dict[str, str | None] = {
         "AZURE_SPEECH_SERVICE_KEY": "some-azure-speech-service-key",
         "AZURE_SPEECH_SERVICE_REGION": "some-azure-speech-service-region",
         "APPINSIGHTS_ENABLED": "False",
         "AZURE_OPENAI_API_KEY": "some-azure-openai-api-key",
+        "AZURE_OPENAI_API_VERSION": "2024-02-01",
+        "AZURE_SEARCH_INDEX": "some-azure-search-index",
         "AZURE_SEARCH_KEY": "some-azure-search-key",
         "AZURE_CONTENT_SAFETY_KEY": "some-content_safety-key",
         "AZURE_OPENAI_EMBEDDING_MODEL": "some-embedding-model",
         "AZURE_OPENAI_MODEL": "some-openai-model",
         "AZURE_SEARCH_CONVERSATIONS_LOG_INDEX": "some-log-index",
+        "AZURE_OPENAI_STREAM": "True",
         "LOAD_CONFIG_FROM_BLOB_STORAGE": "False",
         "TIKTOKEN_CACHE_DIR": f"{os.path.dirname(os.path.realpath(__file__))}/resources",
+        # These values are set directly within EnvHelper, adding them here ensures
+        # that they are removed from the environment when remove_from_environment() runs
+        "OPENAI_API_TYPE": None,
+        "OPENAI_API_KEY": None,
+        "OPENAI_API_VERSION": None,
     }
 
-    def __init__(self, config_overrides: Dict[str, Any] = {}) -> None:
+    def __init__(self, config_overrides: dict[str, str | None] = {}) -> None:
         self.config = self.config | config_overrides
 
-    def set(self, key: str, value: Any) -> None:
+    def set(self, key: str, value: str | None) -> None:
         self.config[key] = value
 
-    def get(self, key: str) -> Any:
+    def get(self, key: str) -> str | None:
         return self.config[key]
 
-    def get_all(self) -> Dict[str, Any]:
+    def get_all(self) -> dict[str, str | None]:
         return self.config
 
     def apply_to_environment(self) -> None:

code/tests/functional/backend_api/common.py

@@ -0,0 +1,42 @@
+import logging
+import socket
+import threading
+import time
+import requests
+from threading import Thread
+from create_app import create_app
+
+
+def start_app(app_port: int) -> Thread:
+    logging.info(f"Starting application on port {app_port}")
+    app = create_app()
+    app_process = threading.Thread(target=lambda: app.run(port=app_port), daemon=True)
+    app_process.start()
+    wait_for_app(app_port)
+    logging.info("Application started")
+    return app_process
+
+
+def wait_for_app(port: int, initial_check_delay: int = 2):
+    attempts = 0
+    time.sleep(initial_check_delay)
+    while attempts < 10:
+        try:
+            response = requests.get(f"http://localhost:{port}/api/config")
+            if response.status_code == 200:
+                return
+        except Exception:
+            pass
+
+        attempts += 1
+        time.sleep(1)
+
+    raise Exception("App failed to start")
+
+
+def get_free_port() -> int:
+    s = socket.socket(socket.AF_INET, type=socket.SOCK_STREAM)
+    s.bind(("localhost", 0))
+    _, port = s.getsockname()
+    s.close()
+    return port

code/tests/functional/backend_api/conftest.py

@@ -1,15 +1,8 @@
-import logging
-import socket
 import ssl
-import threading
-import time
 import pytest
 from pytest_httpserver import HTTPServer
-import requests
 from tests.functional.backend_api.app_config import AppConfig
-from threading import Thread
 import trustme
-from create_app import create_app
 
 
 @pytest.fixture(scope="session")
@@ -43,42 +36,6 @@ def httpclient_ssl_context(ca):
         return ssl.create_default_context(cafile=ca_temp_path)
 
 
-@pytest.fixture(scope="session")
-def app_port() -> int:
-    logging.info("Getting free port")
-    return get_free_port()
-
-
-@pytest.fixture(scope="session")
-def app_url(app_port: int) -> str:
-    return f"http://localhost:{app_port}"
-
-
-@pytest.fixture(scope="session")
-def app_config(make_httpserver, ca):
-    logging.info("Creating APP CONFIG")
-    with ca.cert_pem.tempfile() as ca_temp_path:
-        app_config = AppConfig(
-            {
-                "AZURE_OPENAI_ENDPOINT": f"https://localhost:{make_httpserver.port}",
-                "AZURE_SEARCH_SERVICE": f"https://localhost:{make_httpserver.port}",
-                "AZURE_CONTENT_SAFETY_ENDPOINT": f"https://localhost:{make_httpserver.port}",
-                "SSL_CERT_FILE": ca_temp_path,
-                "CURL_CA_BUNDLE": ca_temp_path,
-            }
-        )
-        logging.info(f"Created app config: {app_config.get_all()}")
-        yield app_config
-
-
-@pytest.fixture(scope="session", autouse=True)
-def manage_app(app_port: int, app_config: AppConfig):
-    app_config.apply_to_environment()
-    start_app(app_port)
-    yield
-    app_config.remove_from_environment()
-
-
 @pytest.fixture(scope="function", autouse=True)
 def setup_default_mocking(httpserver: HTTPServer, app_config: AppConfig):
     httpserver.expect_request(
@@ -154,39 +111,3 @@ def setup_default_mocking(httpserver: HTTPServer, app_config: AppConfig):
     yield
 
     httpserver.check()
-
-
-def start_app(app_port: int) -> Thread:
-    logging.info(f"Starting application on port {app_port}")
-    app = create_app()
-    app_process = threading.Thread(target=lambda: app.run(port=app_port))
-    app_process.daemon = True
-    app_process.start()
-    wait_for_app(app_port)
-    logging.info("Application started")
-    return app_process
-
-
-def wait_for_app(port: int, initial_check_delay: int = 10):
-    attempts = 0
-    time.sleep(initial_check_delay)
-    while attempts < 10:
-        try:
-            response = requests.get(f"http://localhost:{port}/api/config")
-            if response.status_code == 200:
-                return
-        except Exception:
-            pass
-
-        attempts += 1
-        time.sleep(1)
-
-    raise Exception("App failed to start")
-
-
-def get_free_port() -> int:
-    s = socket.socket(socket.AF_INET, type=socket.SOCK_STREAM)
-    s.bind(("localhost", 0))
-    _, port = s.getsockname()
-    s.close()
-    return port

code/tests/functional/backend_api/tests/README.md

@@ -0,0 +1,56 @@
+# Backend API Tests
+
+At present, there are two sets of tests: `with_data` and `without_data`.
+Each set of tests starts its own instance of the backend API on a different port.
+The difference between the two is the environment variables, namely the lack of the
+`AZURE_SEARCH_SERVICE` variable for the `without_data` tests.
+
+When adding new tests, first check to see if it is possible to add the tests to an
+existing set of tests, rather than creating a new set, as this removes the need for
+starting up a new instance of the application on another port.
+
+New environment variables common to all tests can be directly added to the `config`
+dict in [app_config.py](../app_config.py), while variables only needed for one set
+of tests can be added to the `app_config` fixture in the respective `conftest.py`
+file, e.g. [./with_data/conftest.py](./with_data/conftest.py).
+
+```py
+@pytest.fixture(scope="package")
+def app_config(make_httpserver, ca):
+    logging.info("Creating APP CONFIG")
+    with ca.cert_pem.tempfile() as ca_temp_path:
+        app_config = AppConfig(
+            {
+                "AZURE_OPENAI_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_SEARCH_SERVICE": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_CONTENT_SAFETY_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "SSL_CERT_FILE": ca_temp_path,
+                "CURL_CA_BUNDLE": ca_temp_path,
+                "NEW_ENV_VAR": "VALUE",
+            }
+        )
+        logging.info(f"Created app config: {app_config.get_all()}")
+        yield app_config
+```
+
+To remove an environment variable from the default defined in the `AppConfig` class,
+set its value to `None`.
+
+```py
+@pytest.fixture(scope="package")
+def app_config(make_httpserver, ca):
+    logging.info("Creating APP CONFIG")
+    with ca.cert_pem.tempfile() as ca_temp_path:
+        app_config = AppConfig(
+            {
+                "AZURE_OPENAI_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_SEARCH_SERVICE": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_CONTENT_SAFETY_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "SSL_CERT_FILE": ca_temp_path,
+                "CURL_CA_BUNDLE": ca_temp_path,
+                "ENV_VAR_TO_REMOVE": None,
+            }
+        )
+        logging.info(f"Created app config: {app_config.get_all()}")
+        yield app_config
+```

code/tests/functional/backend_api/tests/with_data/conftest.py

@@ -0,0 +1,40 @@
+import logging
+import pytest
+from tests.functional.backend_api.app_config import AppConfig
+from tests.functional.backend_api.common import get_free_port, start_app
+
+
+@pytest.fixture(scope="package")
+def app_port() -> int:
+    logging.info("Getting free port")
+    return get_free_port()
+
+
+@pytest.fixture(scope="package")
+def app_url(app_port: int) -> str:
+    return f"http://localhost:{app_port}"
+
+
+@pytest.fixture(scope="package")
+def app_config(make_httpserver, ca):
+    logging.info("Creating APP CONFIG")
+    with ca.cert_pem.tempfile() as ca_temp_path:
+        app_config = AppConfig(
+            {
+                "AZURE_OPENAI_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_SEARCH_SERVICE": f"https://localhost:{make_httpserver.port}/",
+                "AZURE_CONTENT_SAFETY_ENDPOINT": f"https://localhost:{make_httpserver.port}/",
+                "SSL_CERT_FILE": ca_temp_path,
+                "CURL_CA_BUNDLE": ca_temp_path,
+            }
+        )
+        logging.info(f"Created app config: {app_config.get_all()}")
+        yield app_config
+
+
+@pytest.fixture(scope="package", autouse=True)
+def manage_app(app_port: int, app_config: AppConfig):
+    app_config.apply_to_environment()
+    start_app(app_port)
+    yield
+    app_config.remove_from_environment()