Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion contributing/samples/gepa/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
from tau_bench.types import EnvRunResult
from tau_bench.types import RunConfig
import tau_bench_agent as tau_bench_agent_lib

import utils


Expand Down
1 change: 0 additions & 1 deletion contributing/samples/gepa/run_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
from absl import flags
import experiment
from google.genai import types

import utils

_OUTPUT_DIR = flags.DEFINE_string(
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ known_third_party = ["google.adk"]

[tool.pytest.ini_options]
testpaths = ["tests"]
pythonpath = "src"
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding pythonpath = "src" to the [tool.pytest.ini_options] section is a good practice. It ensures that pytest can correctly resolve imports from the src directory, which can prevent import errors during test execution, especially in larger projects with a specific directory structure.

asyncio_default_fixture_loop_scope = "function"
asyncio_mode = "auto"

Expand Down
1 change: 1 addition & 0 deletions src/google/adk/auth/auth_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class OAuth2Auth(BaseModelWithConfig):
auth_response_uri: Optional[str] = None
auth_code: Optional[str] = None
access_token: Optional[str] = None
id_token: Optional[str] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding id_token: Optional[str] = None to the OAuth2Auth class is a direct and necessary change to support the id_token in the OIDC flow, as described in the PR. This correctly extends the data model to accommodate the new token type.

refresh_token: Optional[str] = None
expires_at: Optional[int] = None
expires_in: Optional[int] = None
Expand Down
1 change: 1 addition & 0 deletions src/google/adk/auth/oauth2_credential_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ def update_credential_with_tokens(
"""
auth_credential.oauth2.access_token = tokens.get("access_token")
auth_credential.oauth2.refresh_token = tokens.get("refresh_token")
auth_credential.oauth2.id_token = tokens.get("id_token", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Populating the id_token field from the tokens dictionary is crucial for making the id_token available after an OAuth2/OIDC exchange. This change directly implements the solution for issue #3785.

auth_credential.oauth2.expires_at = (
int(tokens.get("expires_at")) if tokens.get("expires_at") else None
)
Expand Down
9 changes: 5 additions & 4 deletions tests/unittests/agents/test_remote_a2a_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import tempfile
from unittest.mock import AsyncMock
from unittest.mock import create_autospec
from unittest.mock import MagicMock
from unittest.mock import Mock
from unittest.mock import patch

Expand Down Expand Up @@ -1002,7 +1003,7 @@ async def test_handle_a2a_response_with_task_submitted_and_no_update(self):
mock_a2a_task,
self.agent.name,
self.mock_context,
self.mock_a2a_part_converter,
self.agent._a2a_part_converter,
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Changing self.mock_a2a_part_converter to self.agent._a2a_part_converter ensures that the test uses the actual converter instance associated with the agent object, rather than a separate mock. This improves the accuracy of the test by reflecting how the agent would behave in a real scenario.

)
# Check the parts are updated as Thought
assert result.content.parts[0].thought is True
Expand Down Expand Up @@ -1770,7 +1771,7 @@ async def test_run_async_impl_successful_request(self):
) # Tuple with parts and context_id

# Mock A2A client
mock_a2a_client = create_autospec(spec=A2AClient, instance=True)
mock_a2a_client = MagicMock(spec=A2AClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Replacing create_autospec(spec=A2AClient, instance=True) with MagicMock(spec=A2AClient) can simplify mocking in some cases. MagicMock is generally more flexible and less strict about argument matching than create_autospec(..., instance=True), which can be beneficial for tests that don't require strict adherence to the spec's signature for all calls. If create_autospec was causing issues due to strictness, MagicMock is a reasonable alternative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failed if I use create_autospec

mock_response = Mock()
mock_send_message = AsyncMock()
mock_send_message.__aiter__.return_value = [mock_response]
Expand Down Expand Up @@ -1909,7 +1910,7 @@ async def test_run_async_impl_with_meta_provider(self):
) # Tuple with parts and context_id

# Mock A2A client
mock_a2a_client = create_autospec(spec=A2AClient, instance=True)
mock_a2a_client = MagicMock(spec=A2AClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change from create_autospec to MagicMock is consistent with the previous change and likely addresses similar issues related to mocking flexibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failed if I use create_autospec

mock_response = Mock()
mock_send_message = AsyncMock()
mock_send_message.__aiter__.return_value = [mock_response]
Expand Down Expand Up @@ -2046,7 +2047,7 @@ async def test_run_async_impl_successful_request(self):
) # Tuple with parts and context_id

# Mock A2A client
mock_a2a_client = create_autospec(spec=A2AClient, instance=True)
mock_a2a_client = MagicMock(spec=A2AClient)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change from create_autospec to MagicMock is consistent with the previous changes and likely addresses similar issues related to mocking flexibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failed if I use create_autospec

mock_response = Mock()
mock_send_message = AsyncMock()
mock_send_message.__aiter__.return_value = [mock_response]
Expand Down
2 changes: 2 additions & 0 deletions tests/unittests/auth/test_oauth2_credential_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def test_update_credential_with_tokens(self):
tokens = OAuth2Token({
"access_token": "new_access_token",
"refresh_token": "new_refresh_token",
"id_token": "some_id_token",
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Adding `

"expires_at": expected_expires_at,
"expires_in": 3600,
})
Expand All @@ -230,5 +231,6 @@ def test_update_credential_with_tokens(self):

assert credential.oauth2.access_token == "new_access_token"
assert credential.oauth2.refresh_token == "new_refresh_token"
assert credential.oauth2.id_token == "some_id_token"
Copy link
Contributor

Choose a reason for hiding this comment

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

high

This assertion verifies that the id_token is correctly set on the OAuth2Auth object after update_credential_with_tokens is called. This is a critical test to ensure the new functionality works as expected.

assert credential.oauth2.expires_at == expected_expires_at
assert credential.oauth2.expires_in == 3600