diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..9b38853 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,7 @@ +{ + "python.testing.pytestArgs": [ + "tests" + ], + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true +} \ No newline at end of file diff --git a/README.md b/README.md index 1abd514..84d2c94 100644 --- a/README.md +++ b/README.md @@ -244,15 +244,11 @@ docker compose run --rm manage lint --check ## Testing -Currently, testing and coverage for the project are nonexistent. Only the test skeletons, configuration, commands, and a test-friendly architecture are in place. +To make the generation of test fixtures easier, the `RECORD_TRAFFIC` flag has been added, which creates files with all the incoming/outgoing traffic between this service and Cursor/Azure in the directory `recordings/` -To make the generation of test fixtures easier, the `RECORD_TRAFFIC` flag has been added, which creates files with all the incoming/outgoing traffic between this service and Cursor/Azure. +To avoid violating Cursor's intellectual property, a redaction layer removes any sensitive data, such as: system prompts, tool names, tool descriptions, and any context containing scaffolding from Cursor's prompt-building service. -Currently, those fixtures would include sensitive data, such as system prompts, tools, and the entire scaffolding from Cursor's prompt-building service. - -To avoid violating Cursor's intellectual property, a redaction layer will have to be implemented so the recorded traffic can be published and used in tests while remaining MIT-licensed. - -This is a top priority and will be developed next, before any other features, as traffic recording will also be a valuable tool for users of the service to report issues on GitHub and to improve testing for other contributors to confidently contribute to the project. +Therefore, recorded traffic can be published under `tests/recordings/` to be used as test fixtures while remaining MIT-licensed. ## Production diff --git a/app/common/recording.py b/app/common/recording.py index 9ce0e8e..d344512 100644 --- a/app/common/recording.py +++ b/app/common/recording.py @@ -1,12 +1,14 @@ """Lightweight recording helpers for debugging request/response flows. -Artifacts are stored under the project-level ``recordings/`` folder using a -monotonically increasing numeric prefix so related request/response files are -easy to correlate. +Artifacts are stored under the project-level ``recordings/`` folder. Each +request/response lifecycle is grouped in a subdirectory named by an increasing +numeric index, e.g. +``recordings/9/downstream_request.json``. """ import json import os +import re from functools import wraps from typing import Any, Dict @@ -17,16 +19,26 @@ RECORDINGS_DIR = os.path.join(os.path.dirname(__file__), "..", "..", "recordings # Private, module-level counter tracking the latest recording index. __LAST_RECORDING_INDEX = 0 -# Initialize the counter based on existing files in the recordings directory so -# that subsequent runs continue incrementing from the maximum observed index. -files = os.listdir(RECORDINGS_DIR) -for file in files: +# Initialize the counter based on existing subdirectories in the recordings +# directory so that subsequent runs continue incrementing from the maximum +# observed index. +try: + entries = os.listdir(RECORDINGS_DIR) +except FileNotFoundError: + # Create the recordings directory lazily when first used + os.makedirs(RECORDINGS_DIR, exist_ok=True) + entries = [] + +for entry in entries: + entry_path = os.path.join(RECORDINGS_DIR, entry) + if not os.path.isdir(entry_path): + continue try: - recording_index = int(file.split("_")[0]) + recording_index = int(entry) if recording_index > __LAST_RECORDING_INDEX: __LAST_RECORDING_INDEX = recording_index - except (ValueError, IndexError): - # Ignore unrelated files that do not follow the "_.*" pattern + except ValueError: + # Ignore unrelated folders that do not use a numeric name pass @@ -53,21 +65,50 @@ def increment_last_recording() -> None: __LAST_RECORDING_INDEX += 1 +def anonimize(data: str) -> str: + """Removes sensitive data from recordings.""" + closing = r'(.*?[^\\](?:\\\\)?)(")' + patterns = ( + # Content + r'("role": ?"[a-z]+",\s+"content": ?")', # Completions + r'("instructions": ?")', # Responses + r'("text": ?")', # Responses + r'("role": ?"[a-z]+",\s+"delta": ?")', # Both + # User identifiers + r'("user": ?")', # Completions + r'("prompt_cache_key": ?")', # Responses + # Function calls + r'("name": ?")', # Both + r'("description": ?")', # Both + ) + for pattern in patterns: + data = re.sub(pattern + closing, r"\1REDACTED\3", data) + + return data + + +def _recording_file_path(name: str, ext: str) -> str: + dir_path = os.path.join(RECORDINGS_DIR, str(__LAST_RECORDING_INDEX)) + os.makedirs(dir_path, exist_ok=True) + return os.path.join(dir_path, f"{name}.{ext}") + + @config_bypass def record_payload(payload: Dict[str, Any], name: str) -> None: - """Write a JSON payload for the current recording index.""" + """Write a JSON payload under the current recording index subdirectory.""" - file_name = f"{__LAST_RECORDING_INDEX}_{name}.json" - file_path = os.path.join(RECORDINGS_DIR, file_name) + file_path = _recording_file_path(name, "json") with open(file_path, "w") as f: - json.dump(payload, f, indent=2) + data = json.dumps(payload, indent=2) + data = anonimize(data) + f.write(data) @config_bypass def record_sse(sse: bytes, name: str) -> None: - """Write raw SSE bytes for the current recording index.""" + """Write raw SSE bytes under the current recording index subdirectory.""" - file_name = f"{__LAST_RECORDING_INDEX}_{name}.sse" - file_path = os.path.join(RECORDINGS_DIR, file_name) + file_path = _recording_file_path(name, "sse") with open(file_path, "wb") as f: + sse = anonimize(sse.decode("utf-8")).encode("utf-8") f.write(sse) diff --git a/pyproject.toml b/pyproject.toml index 0aa63cd..90a409a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,3 +9,8 @@ omit = ["tests/*", "autoapp.py"] ignore = ["D401", "D202", "E226", "E302", "E41", "W503", "E203"] max-line-length = 120 max-complexity = 30 + +[tool.pytest.ini_options] +filterwarnings = [ + "ignore::DeprecationWarning", +] \ No newline at end of file diff --git a/recordings/.gitkeep b/recordings/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/requirements/dev.txt b/requirements/dev.txt index 1cba50f..569476e 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -6,6 +6,7 @@ factory-boy==3.3.3 pytest==8.4.2 pytest-cov==7.0.0 WebTest==3.0.6 +requests-mock==1.12.1 # Lint and code style black==25.1.0 diff --git a/tests/conftest.py b/tests/conftest.py index 17939af..f4b138f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,13 +3,14 @@ import logging import pytest +from flask import Flask from webtest import TestApp from app import create_app @pytest.fixture -def app(): +def app() -> Flask: """Create application for the tests.""" _app = create_app("tests.settings") _app.logger.setLevel(logging.CRITICAL) @@ -22,6 +23,6 @@ def app(): @pytest.fixture -def testapp(app): +def testapp(app) -> TestApp: """Create Webtest app.""" return TestApp(app) diff --git a/tests/replay_base.py b/tests/replay_base.py new file mode 100644 index 0000000..f2535d1 --- /dev/null +++ b/tests/replay_base.py @@ -0,0 +1,106 @@ +"""Functional tests using WebTest. + +See: http://webtest.readthedocs.org/ +""" + +import json +import os +import re +from typing import Any + +from requests_mock import MockerCore +from webtest import TestApp + + +class ReplyBase: + """Tests the replay of /recordings//. + + This class is extensible via the ``recording`` attribute, which determines + which subdirectory under ``tests/recordings/`` to load fixtures from. + """ + + # The subdirectory under tests/recordings/ to load fixtures from + recording: str + + # Endpoint to mock for the upstream request + UPSTREAM_URL = "https://test-resource.openai.azure.com/openai/responses?api-version=2025-04-01-preview" + + def _get_request_path(self, kind: str) -> str: + """Return path for a recorded request JSON of given kind. + + Example: kind="upstream" -> tests/recordings//upstream_request.json + """ + return os.path.join( + "tests", "recordings", self.recording, f"{kind}_request.json" + ) + + def _get_response_path(self, kind: str) -> str: + """Return path for a recorded response SSE of given kind. + + Example: kind="downstream" -> tests/recordings//downstream_response.sse + """ + return os.path.join( + "tests", "recordings", self.recording, f"{kind}_response.sse" + ) + + def _normalize_response(self, sse_response: bytes) -> str: + """Normalize the response id and created timestamp (use re.sub).""" + text = sse_response.decode("utf-8") + text = re.sub( + r'data: {"id":"chatcmpl-(.*?)"', 'data: {"id":"chatcmpl-ABC123"', text + ) + text = re.sub(r'"created":(\d+)', '"created":1234567890', text) + return text + + def _mock_upstream(self, requests_mock: MockerCore) -> Any: + """Mock upstream request with recorded SSE upstream response. + + Returns the mock object so callers can inspect ``last_request``. + """ + upstream_response_path = self._get_response_path("upstream") + return requests_mock.post( + self.UPSTREAM_URL, + body=open( + upstream_response_path, "rb" + ), # Yes, we need to pass the file object here, not the .read() result + ) + + def _perform_downstream_request(self, testapp: TestApp): + """Perform recorded downstream request and return the response.""" + downstream_request_path = self._get_request_path("downstream") + with open(downstream_request_path, "r") as f: + downstream_request = f.read() + return testapp.post( + "/chat/completions", + status=200, + params=downstream_request, + headers={ + "Content-Type": "application/json", + "Authorization": "Bearer test-service-api-key", + }, + ) + + def _verify_upstream_request(self, mock: Any) -> None: + """Verify upstream request matches the recorded upstream request.""" + upstream_request_path = self._get_request_path("upstream") + with open(upstream_request_path, "r") as f: + upstream_request = json.load(f) + assert mock.last_request.json() == upstream_request + + def _verify_downstream_response(self, response) -> None: + """Verify downstream response matches the recorded downstream response.""" + downstream_response_path = self._get_response_path("downstream") + with open(downstream_response_path, "rb") as f: + recorded_downstream_response = f.read() + response_normalized = self._normalize_response(response.body) + recorded_response_normalized = self._normalize_response( + recorded_downstream_response + ) + assert response_normalized == recorded_response_normalized + + def test(self, testapp: TestApp, requests_mock: MockerCore): + """Run the replay flow using the configured recording fixtures.""" + mock = self._mock_upstream(requests_mock) + response = self._perform_downstream_request(testapp) + self._verify_upstream_request(mock) + self._verify_downstream_response(response) diff --git a/tests/settings.py b/tests/settings.py index 95959ef..06c36b9 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -6,7 +6,7 @@ TESTING = True SERVICE_API_KEY = "test-service-api-key" AZURE_API_VERSION = "2025-04-01-preview" -AZURE_BASE_URL = "test-base-url" +AZURE_BASE_URL = "https://test-resource.openai.azure.com" AZURE_API_KEY = "test-api-key" AZURE_DEPLOYMENT = "gpt-5" AZURE_SUMMARY_LEVEL = "detailed" diff --git a/tests/test_replays.py b/tests/test_replays.py new file mode 100644 index 0000000..9dc8be8 --- /dev/null +++ b/tests/test_replays.py @@ -0,0 +1,42 @@ +"""Functional tests using WebTest. + +See: http://webtest.readthedocs.org/ +""" + +from .replay_base import ReplyBase + + +class TestOnePingPong(ReplyBase): + """Test a single ping-pong interaction, no tool calls.""" + + recording = "one_ping_pong" + + +class TestMultiplePingPongs(ReplyBase): + """Test multiple ping-pong interactions back and forth, no tool calls.""" + + recording = "one_ping_pong" + + +class TestContextWithSingleToolCalls(ReplyBase): + """Multiple single tool calls in the context.""" + + recording = "context_single_tool_calls" + + +class TestReplyWithSingleToolCall(ReplyBase): + """Single tool call in the reply.""" + + recording = "reply_single_tool_call" + + +class TestContextWithParallelToolCall(ReplyBase): + """Parallel tool calls in the context.""" + + recording = "context_parallel_tool_call" + + +class TestReplyWithParallelToolCall(ReplyBase): + """Parallel tool calls in the reply.""" + + recording = "reply_parallel_tool_call"