-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[AI] Unit tests for urlContext APIs #15343
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
[AI] Unit tests for urlContext APIs #15343
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds unit tests for the urlContext APIs, covering both unary and streaming responses for Google AI and Vertex AI backends. The tests are well-structured and cover important scenarios like success cases and mixed validity responses. I've provided a few suggestions to improve code clarity and correctness. Specifically, I've pointed out some unnecessary uses of XCTUnwrap that can be simplified. I also found a test case where the name and assertion seem to contradict each other, and I've suggested a correction to align them. Overall, these are solid additions to the test suite.
| let candidate = try XCTUnwrap(response.candidates.first) | ||
| let urlContextMetadata = try XCTUnwrap(candidate.urlContextMetadata) | ||
| let urlMetadata = try XCTUnwrap(urlContextMetadata.urlMetadata.first) | ||
| XCTAssertEqual(urlMetadata.retrievedURL?.absoluteString, "https://example.com/8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be a contradiction in this test. The test name testGenerateContent_success_urlContext_missingRetrievedURL and the mock resource name unary-success-url-context-missing-retrievedurl suggest that the retrievedURL should be missing or nil. However, the assertion checks if retrievedURL?.absoluteString is equal to a specific string, which implies retrievedURL is not nil.
If the intention is to test for a missing URL, the assertion should probably be XCTAssertNil(urlMetadata.retrievedURL). If the current assertion is correct, consider renaming the test to avoid confusion.
| XCTAssertEqual(urlMetadata.retrievedURL?.absoluteString, "https://example.com/8") | |
| XCTAssertNil(urlMetadata.retrievedURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed function to testGenerateContent_success_urlContext_retrievedURLPresentOnErrorStatus cc: @dlarocque @andrewheard
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini summarize |
|
I can only process one command at a time. Please let me know which command you'd like me to execute first: |
|
/gemini summarize |
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a robust suite of unit tests for the urlContext APIs, significantly improving test coverage. The changes also include enhancements to the MockURLProtocol to better support testing with static JSON data. The new tests are well-written and cover various scenarios. I have a few minor suggestions to improve the clarity and consistency of some tests.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini summary |
|
/gemini summary |
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a comprehensive set of unit tests for the urlContext APIs for both GoogleAI and VertexAI models, covering unary and streaming responses. The tests validate various scenarios like successful retrieval, mixed validity, and empty metadata. The changes are well-structured and significantly improve test coverage for the URL context feature. I've provided a few minor suggestions to enhance test code clarity, consistency, and maintainability by refactoring duplicated code and improving assertions.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
andrewheard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Paul!
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Andrew Heard <andrewheard@google.com>
Summary of Changes
This pull request introduces a robust suite of unit tests for the
urlContextAPIs within both the GoogleAI and VertexAI generative models. The tests cover various response scenarios, ensuring the correct parsing and handling of URL context metadata by the SDK. To support these new tests, the underlyingMockURLProtocolwas enhanced to handle directDataresponses, complemented by a new utility function for generating mock JSON HTTP handlers.Highlights
GenerativeModelGoogleAITests.swiftto validate the handling ofurlContextmetadata. These tests cover successful retrieval, mixed validity scenarios (success and error), and cases where URL metadata is empty. A new streaming test forurlContextwas also introduced.GenerativeModelVertexAITests.swiftforurlContextAPIs. These tests verify successful retrieval, mixed validity, handling of nonexistent retrieved URLs, and empty URL metadata. A streaming test forurlContextwas also included.Candidatedecoding tests forurlMetadata#15348This behaviour is more likely to be observed in practice since [] is typically omitted during JSON serialization in the backend (since it is a default value).
Changelog
testGenerateContent_success_urlContextto verify successful URL context retrieval.testGenerateContent_success_urlContext_mixedValidityto test scenarios with both successful and erroneous URL metadata.testGenerateContent_success_urlContext_emptyURLMetadatato ensure correct handling when URL metadata is empty.testGenerateContentStream_success_urlContextto test streaming responses with URL context.testGenerateContent_success_urlContextto verify successful URL context retrieval.testGenerateContent_success_urlContext_mixedValidityto test scenarios with both successful and erroneous URL metadata.testGenerateContent_success_urlContext_nonexistentRetrievedURLto test cases whereretrievedURLis missing.testGenerateContent_success_urlContext_emptyURLMetadatato ensure correct handling when URL metadata is empty.testGenerateContentStream_success_urlContextto test streaming responses with URL context.dataRequestHandlerstatic variable to allow mocking HTTP responses with rawData.startLoadingto conditionally userequestHandler(for streaming) ordataRequestHandler(for static data).httpRequestHandler(json:statusCode:)utility function to createURLRequesthandlers that return a specified JSON string asData.