Skip to content
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

Implement FastAPI Unit Tests #34

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

Conversation

NeonDaniel
Copy link
Member

Description

Adds test coverage for all exposed endpoints with mocked backend methods
Addresses some edge-case bugs in request.client object handling
Fixes/completes model type annotations

Issues

Other Notes

Add default value handling to `request.client` params to address test failures
Add httpx test dependency for FastAPI tests
Fix errors in LLMResponse example
Make IP address lookups in util module safe
client_manager.validate_auth("", request.client.host)
return request.client.host
ip_addr = request.client.host if request.client else "127.0.0.1"
if len(ip_addr.split('.')) != 4:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit(non-blocking): There is an outside chance a hostname could have multiple subdomains and end up triggering this conditional. Might be worth using regex, but this won't make or break the PR IMHO

@@ -1,4 +1,5 @@
pytest
mock
httpx

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see this used - was it an initial implementation but removed later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants