This repository was archived by the owner on Sep 30, 2024. It is now read-only.
test(cody): Add unit tests for the Completions API#63434
Merged
Conversation
chrsmith
commented
Jun 23, 2024
| case types.HUMAN_MESSAGE_SPEAKER: | ||
| anthropicRole = "user" | ||
| default: | ||
| return nil, errors.Errorf("unexpected role: %s", text) |
Contributor
Author
There was a problem hiding this comment.
These changes are just me fixing something I found while writing the tests. We duplicated incorrect code across all of our API provider implementations.
In all of these the "unexpected role" is referring to the speaker field. (i.e. what the switch statement is checking, and not the text of the message itself.)
emidoots
approved these changes
Jun 23, 2024
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The Server-side Cody Model Selection project requires refactoring a lot of how the Completions API endpoint works for the Sourcegraph backend. It will need to update any place where we interact with the site config as it relates to LLMs, or any time we figure out which models can-or-can-not be used.
In order to safely land those refactoring without breaking existing users, who will be using the "older config format", we need tests. LOTS and LOTS of tests.
This PR adds the necessary infrastructure for writing unit tests against the Completions API, and adds a few basic ones for the Anthropic API provider. I wouldn't say it's as easy as we'd like to write these tests, but at least now it is possible. And we can further streamline things from here.
Overview
The bulk of functionality is in
entconfig_base_test.go. That file contains some basic unit tests for how the enterprise configuration data is loaded, and provides the mocks and test infrastructure.The crux of which is the
apiProviderTestInfrastruct. It will bundle mocks and the HTTP handlers to test, and provides high-level methods for invoking the completion API.What gets trick however, is how we actually hook into the implementation details of the completions API endpoint. You see, the user makes an HTTP request to the Sourcegraph instance. But we then figure out which LLM model to use, and then build an HTTP request to send to the specific LLM API Provider.
So the test infrastructure allows you to mock out that "middle part". The
AssertCodyGatewayReceivesRequestWithResponse/AssertGenericExternalAPIRequestWithResponsefunctions will:Unfortunately it's super gnarly because we aren't really exposing the API data types from the API providers in a useful way. (e.g. perhaps we should just use the standard Anthropic Golang API client library.) So generating the test data relies on a lot of
map[string]anyto make it easier to construct arbitrary data types that can serialize to JSON the way that we need them to.Anyways, with all of this test infrastructure in-place, you can write API provider tests like the following. Here's one such test from
entconfig_anthropic_test.gowhich confirms that various aspects of the site-configuration are honored correctly when configured to use BYOK mode.I've added "ℹ️ comments" to highlight some of the tricker parts.
Next steps
Once this gets checked-in, my plan is to carefully add new unit tests for the existing functionality before DISMANTLING the code to write through an entirely different site configuration object. 🤞 this will allow me to do so in such a way that I can confirm my changes won't alter any existing Sourcegraph instances that are using the older configuration format.
Test plan
Adds more tests.
Changelog
NA. Just trivial changes and adding more tests.