-
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>
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 <[email protected]>
Summary of Changes
This pull request introduces a robust suite of unit tests for the
urlContext
APIs 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 underlyingMockURLProtocol
was enhanced to handle directData
responses, complemented by a new utility function for generating mock JSON HTTP handlers.Highlights
GenerativeModelGoogleAITests.swift
to validate the handling ofurlContext
metadata. These tests cover successful retrieval, mixed validity scenarios (success and error), and cases where URL metadata is empty. A new streaming test forurlContext
was also introduced.GenerativeModelVertexAITests.swift
forurlContext
APIs. These tests verify successful retrieval, mixed validity, handling of nonexistent retrieved URLs, and empty URL metadata. A streaming test forurlContext
was also included.Candidate
decoding 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_urlContext
to verify successful URL context retrieval.testGenerateContent_success_urlContext_mixedValidity
to test scenarios with both successful and erroneous URL metadata.testGenerateContent_success_urlContext_emptyURLMetadata
to ensure correct handling when URL metadata is empty.testGenerateContentStream_success_urlContext
to test streaming responses with URL context.testGenerateContent_success_urlContext
to verify successful URL context retrieval.testGenerateContent_success_urlContext_mixedValidity
to test scenarios with both successful and erroneous URL metadata.testGenerateContent_success_urlContext_nonexistentRetrievedURL
to test cases whereretrievedURL
is missing.testGenerateContent_success_urlContext_emptyURLMetadata
to ensure correct handling when URL metadata is empty.testGenerateContentStream_success_urlContext
to test streaming responses with URL context.dataRequestHandler
static variable to allow mocking HTTP responses with rawData
.startLoading
to conditionally userequestHandler
(for streaming) ordataRequestHandler
(for static data).httpRequestHandler(json:statusCode:)
utility function to createURLRequest
handlers that return a specified JSON string asData
.