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.

Sign up for GitHub

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

Jump to bottom

Llm-463 - Fixed unit tests adding mocks to avoid making actual requests ✨ #18

Merged
merged 2 commits into from
Sep 1, 2023
Merged

Llm-463 - Fixed unit tests adding mocks to avoid making actual requests ✨ #18

merged 2 commits into from
Sep 1, 2023

Conversation

javiertoledo
Copy link
Member

@javiertoledo javiertoledo commented Aug 30, 2023 •
edited
Loading

Warning: #20 should be merged first!

  • Mocked the OpenAiService object to test the generateEmbedding method in the OpenAIEmbeddingsModel class.
  • Improved the test to check for the new Embedding structure and expected metadata.
  • Mocked OkHttpClient to implement positive and negative requests to the Pinecone API.
  • Completed tests for store and similaritySearch methods in the class PineconeEmbeddingsStore.

I've also checked that all tests are run with the command ./gradlew test, so we should be able to easily run the test suite in a github action.

@javiertoledo javiertoledo self-assigned this Aug 30, 2023
juanjoman
juanjoman reviewed Aug 31, 2023
View reviewed changes
Copy link
Member

@juanjoman juanjoman left a comment

Choose a reason for hiding this comment

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

Left some comments, mostly about simplifying the mocks. If we're in a rush to finish this I'm ok with it 😄

...ava/com/theagilemonkeys/ellmental/embeddingsgeneration/openai/OpenAIEmbeddingsModelTest.java Outdated Show resolved Hide resolved
...in/java/com/theagilemonkeys/ellmental/embeddingsgeneration/openai/OpenAIEmbeddingsModel.java Outdated Show resolved Hide resolved
...ain/java/com/theagilemonkeys/ellmental/embeddingsstore/pinecone/PineconeEmbeddingsStore.java Outdated Show resolved Hide resolved
...java/com/theagilemonkeys/ellmental/embeddingsstore/pinecone/PineconeEmbeddingsStoreTest.java Outdated Show resolved Hide resolved
@javiertoledo javiertoledo changed the base branch from main to get_delete_embeddings_space August 31, 2023 17:38
juanjoman
juanjoman approved these changes Sep 1, 2023
View reviewed changes
Copy link
Member

@juanjoman juanjoman left a comment

Choose a reason for hiding this comment

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

LGTM!!

Base automatically changed from get_delete_embeddings_space to main September 1, 2023 09:29
@juanjoman juanjoman merged commit fa1a8f7 into main Sep 1, 2023
2 checks passed
@juanjoman juanjoman deleted the llm-463/unit-tests branch September 1, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NickSeagull NickSeagull NickSeagull approved these changes

@juanjoman juanjoman juanjoman approved these changes

@mnlx mnlx Awaiting requested review from mnlx

Assignees

@javiertoledo javiertoledo

Labels
tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@javiertoledo @NickSeagull @juanjoman