Skip to content

Conversation

andrewheard
Copy link
Contributor

  • Reverted the changes to MockURLProtocol since GenerativeModelGoogleAITests and GenerativeModelVertexAITests are specifically meant for mock response testing.
  • Added a similar decoding test in GenerateContentResponseTests to verify the same parsing logic when urlMetadata is [] in the Candidate.
  • Added another decoding test in GenerateContentResponseTests to verify that the same behaviour occurs when urlMetadata is omitted (rather than an empty array).
    • This 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).

#no-changelog

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This is a solid pull request that improves the test suite's structure and maintainability. Moving the urlMetadata decoding tests to GenerateContentResponseTests makes them more focused and correctly tests the Decodable implementation in isolation. The simplification of MockURLProtocol by removing the dataRequestHandler is also a welcome cleanup.

I have one minor suggestion in GenerateContentResponseTests.swift to reduce some code duplication in the newly added tests. Overall, great work!

@andrewheard andrewheard marked this pull request as ready for review September 22, 2025 15:56
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up! LGTM on green

@paulb777 paulb777 merged commit 83ee78a into pb-url-context-unit-tests Sep 22, 2025
44 checks passed
@paulb777 paulb777 deleted the ah/pb-url-context-unit-tests branch September 22, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants