Test/nlp client api path#426
Closed
MaryChen68 wants to merge 2 commits into
Closed
Conversation
using pytest-asyncio for acync function and pytest-mock to isolate from OpenAI, api/test-error seems not exit so I it comment out
- problems I met:
- test seems to be scanning itself, or it's scanning the wrong content. So I'm using the file path directly here to avoid the “import nlp” command looking for the wrong file
- The path checker is too weak.
- using *hasattr(current, attr)* at the first,made the checker even stricter by using *attr in dir(obj)*
- what I want the code do
Scan the OpenAI paths actually called in nlp
↓
Verify these paths using the actual PostHog wrapper client
↓
If the actual client includes the path, pass
↓
Otherwise, fail
- instantiate the real PostHog-wrapped OpenAI client with a fake API key and a disabled PostHog client
- why I am not using mock
- because I think the purpose of this test isn’t checking the response of OpenAI, but does the PostHog-wrapped have the method path used in nlp.py
- if we need to test the response of OpenAI, I can write another test file
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the backend test suite to validate that backend/nlp.py only calls OpenAI client paths that are actually exposed by the PostHog-wrapped OpenAI client, and adds additional backend endpoint/unit tests. It also updates pytest tooling and configuration to support async tests and mocking.
Changes:
- Add a contract-style AST test that extracts
openai_client.*call paths frombackend/nlp.pyand verifies they exist on the real PostHog OpenAI wrapper client. - Add a broader backend test module covering validation, logging, and several API endpoint behaviors.
- Update project dependencies / lockfile for
pytest-asyncio+pytest-mock, and configure pytest asyncio mode.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Adds locked dependencies for pytest-asyncio and pytest-mock. |
pyproject.toml |
Adds pytest plugins and sets asyncio_mode = "auto" for pytest. |
backend/tests/test_smoke.py |
Disables a route assertion for a non-existent endpoint. |
backend/tests/test_nlp_client_api_path.py |
New contract test validating OpenAI client API surface vs nlp.py usage. |
backend/tests/test_backend.py |
New backend tests for validation, logging, endpoints, and PostHog error capture behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,233 @@ | |||
| """ | |||
| Run with: uv run pytest backend/tests/test_api_endpoints.py -v -s | |||
| async def test_middleware_captures_exception_to_posthog(self, mocker): | ||
| """Verifies unhandled backend logic failures run directly through our middleware telemetry into PostHog.""" | ||
| custom_client = TestClient(app, raise_server_exceptions=False) | ||
| mock_posthog = mocker.patch("posthog_client.posthog_client.capture") |
Comment on lines
+47
to
+50
| # Create test client globally | ||
| client = TestClient(app) | ||
|
|
||
|
|
Comment on lines
+7
to
+16
| import sys | ||
| import tempfile | ||
| import shutil | ||
| from pathlib import Path | ||
| from datetime import datetime | ||
| from unittest.mock import AsyncMock | ||
|
|
||
| # Add backend to path | ||
| backend_path = Path(__file__).parent.parent | ||
| sys.path.insert(0, str(backend_path)) |
Comment on lines
+132
to
+141
| try: | ||
| return PostHogAsyncOpenAI( | ||
| api_key="test-openai-key", | ||
| posthog_client=posthog_client, | ||
| ) | ||
| except TypeError: | ||
| # Some SDK versions may not accept posthog_client as a keyword. | ||
| return PostHogAsyncOpenAI( | ||
| api_key="test-openai-key", | ||
| ) |
| assert "/api/chat" in routes | ||
| assert "/api/log" in routes | ||
| assert "/api/test-error" in routes # PostHog error tracking test endpoint | ||
| # assert "/api/test-error" in routes # PostHog error tracking test endpoint |
Comment on lines
13
to
19
| "tenacity>=9.0.0", | ||
| "uvicorn>=0.30.6", | ||
| "pytest>=9.0.3", | ||
| "pytest-asyncio>=0.24.0", | ||
| "pytest-mock>=3.14.0", | ||
| "aiohttp>=3.11.14", | ||
| "posthog>=7.14", |
Contributor
|
We've since worked out higher-value tests and simplified the logic so there's just not as much to test. And I think PostHog/posthog-python#618 will fix this for us. But let's not close this quite yet, at least not until we're able to upgrade to a Posthog release that includes that fix. |
Contributor
|
With the backend now in Hono, I think this is superceded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
problems I met:
what I want the code do
Scan the OpenAI paths actually called in nlp
↓
Verify these paths using the actual PostHog wrapper client
↓
If the actual client includes the path, pass
↓
Otherwise, fail
instantiate the real PostHog-wrapped OpenAI client with a fake API key and a disabled PostHog client
why I am not using mock