-
Notifications
You must be signed in to change notification settings - Fork 2
Add tests that send API requests and assert things about API responses #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the first set of API tests for the BERtron FastAPI application, adding test infrastructure to validate endpoint responses. The changes include setting up a test client, implementing basic endpoint tests, and fixing a type inconsistency in the FastAPI version parameter.
- Adds test infrastructure with FastAPI TestClient and pytest fixtures
- Implements tests for root redirect and version endpoint response validation
- Fixes type consistency by ensuring version parameter is explicitly a string
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/test_server.py | New test file with API endpoint tests using FastAPI TestClient |
| src/server.py | Fixes version parameter type by adding explicit string formatting |
| src/models.py | Adds strict validation configuration to prevent extra fields in responses |
| src/README.md | Updates documentation to include tests directory |
| pyproject.toml | Adds httpx dependency required for FastAPI TestClient |
Comments suppressed due to low confidence (1)
src/tests/test_server.py:18
- [nitpick] The variable name
test_clientinside the fixture function shadows the fixture name itself. Consider renaming the variable toclientorapi_clientfor clarity.
test_client = TestClient(app)
jeff-cohere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks @eecavanna !
|
Thanks, @jeff-cohere! Merging. |
On this branch, I added the first few tests that hit the BERtron API.
I also fixed a type inconsistency, where—as far as our type hints indicate—we could have passed a non-string value to a parameter (i.e. the
versionparameter of theFastAPIfunction) designed to be passed a string.I also introduced
httpxas a development dependency. It is a dependency of FastAPI'sTestClientclass.I did not include any tests that involve the MongoDB server. I think that will happen alongside #53, separately from this.