Skip to content

Add live tests#4

Merged
johanste merged 6 commits into
clientfrom
live-tests
Jun 22, 2023
Merged

Add live tests#4
johanste merged 6 commits into
clientfrom
live-tests

Conversation

@kristapratico
Copy link
Copy Markdown
Collaborator

@kristapratico kristapratico commented Jun 14, 2023

Test real calls to the APIs through the client using azure and openai endpoints

@kristapratico kristapratico marked this pull request as ready for review June 19, 2023 22:13
@kristapratico kristapratico requested a review from johanste June 19, 2023 22:13
Comment thread openai/tests/test_client.py Outdated
auth=OPENAI_API_KEY,
backend="openai"
)
kwargs = {"client": client}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We are silently dropping any kwargs here. Is that intentional? If so, I'd explicitly pass in client to the wrapped f on the next line and not do a assign-new-value-to-kwargs-and-spread-that-in.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Simplified it. Didn't realize I could pass the values of parametrize to a fixture, but pytest lets you do it cleanly.

Comment thread openai/tests/test_client.py Outdated
Comment thread openai/tests/test_client.py Outdated
@pytest.mark.parametrize("api_type", API_TYPE)
@configure_client
def test_client_completion(api_type, **kwargs):
client = kwargs.pop("client")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why not make client a keyword-only argument to the test

Copy link
Copy Markdown
Collaborator Author

@kristapratico kristapratico Jun 20, 2023

Choose a reason for hiding this comment

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

Was a workaround, but instead I just made client a fixture now 😆

@johanste johanste merged commit 24dad1d into client Jun 22, 2023
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