Skip to content

feat: convert openai related tests to VCR#50

Merged
michaelneale merged 2 commits intosquare:mainfrom
codefromthecrypt:openai-vcr
Sep 23, 2024
Merged

feat: convert openai related tests to VCR#50
michaelneale merged 2 commits intosquare:mainfrom
codefromthecrypt:openai-vcr

Conversation

@codefromthecrypt
Copy link
Contributor

This converts OpenAI tests (including Ollama) to VCR instead of manual patching. What this does is allow us to capture real responses and then be able to know the difference in format between the openai platform and others like Azure OpenAI or clones like ollama or localai.

The first time a VCR test is run, it makes a call to the real service, and then afterwards it re-uses the data. Since the data remains, this also helps us safely refactor as otherwise only those with accounts will know if changes break.

See https://pytest-vcr.readthedocs.io/en/latest/

This converts OpenAI tests (including Ollama) to VCR instead
of manual patching. What this does is allow us to capture real
responses and then be able to know the difference in format
between the openai platform and others like Azure OpenAI or
clones like ollama or localai.

The first time a VCR test is run, it makes a call to the real
service, and then afterwards it re-uses the data. Since the
data remains, this also helps us safely refactor as otherwise
only those with accounts will know if changes break.

See https://pytest-vcr.readthedocs.io/en/latest/

Signed-off-by: Adrian Cole <adrian.cole@elastic.co>
@codefromthecrypt
Copy link
Contributor Author

cc @anuraaga if you want to drive by again ;)



@pytest.mark.vcr()
def test_openai_completion(monkeypatch):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

so you can imagine we can add another test here for reasoning model, and then we can see the output, which would have reasoning tokens in it. like test_openai_completion_reasoning

Signed-off-by: Adrian Cole <adrian.cole@elastic.co>
@anuraaga
Copy link
Contributor

LGTM

def test_openai_completion_integration():
reply = openai_complete()

assert reply[0].content is not None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

main thing here is that in the integration tests, we can't assert on exact values, where we can (and should) on the unit/vcr tests

Copy link
Collaborator

@michaelneale michaelneale left a comment

Choose a reason for hiding this comment

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

I think this looks good

@michaelneale michaelneale merged commit c0114fb into square:main Sep 23, 2024
codefromthecrypt pushed a commit to codefromthecrypt/exchange that referenced this pull request Oct 13, 2024
Co-authored-by: lily-de <119957291+lily-de@users.noreply.github.com>
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.

3 participants