Skip to content

Commit 21e2848

Browse files
test(flows): consolidate None tests and add falsy-value parametrize cases
Per reviewer feedback: collapse the two near-identical None tests into a single @pytest.mark.parametrize test, and add falsy-but-not-None cases (0, '', {}, False) to prove the sentinel is identity-based and does not mishandle any falsy return value from a FunctionTool.
1 parent 815e4da commit 21e2848

1 file changed

Lines changed: 24 additions & 38 deletions

File tree

tests/unittests/flows/llm_flows/test_functions_thread_pool.py

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -278,52 +278,38 @@ def blocking_sleep() -> dict:
278278
), f'Event loop should have ticked at least 5 times, got {event_loop_ticks}'
279279

280280
@pytest.mark.asyncio
281-
async def test_sync_tool_returning_none_executes_exactly_once(self):
282-
"""Test that a sync FunctionTool returning None is not double-executed.
281+
@pytest.mark.parametrize(
282+
'return_value,use_implicit_return',
283+
[
284+
(None, True), # implicit None (no return statement)
285+
(None, False), # explicit `return None`
286+
(0, False), # falsy int
287+
('', False), # falsy str
288+
({}, False), # falsy dict
289+
(False, False), # falsy bool
290+
],
291+
)
292+
async def test_sync_tool_falsy_return_executes_exactly_once(
293+
self, return_value, use_implicit_return
294+
):
295+
"""FunctionTools returning None or other falsy values must execute exactly once.
283296
284297
Regression test for https://github.com/google/adk-python/issues/5284.
285-
A FunctionTool whose underlying function returns None (e.g. side-effect-
286-
only tools with no explicit return statement) must execute exactly once.
287-
Previously the None return was mistaken for the internal sentinel used to
298+
Previously, a None return was mistaken for the internal sentinel used to
288299
signal 'non-FunctionTool, fall back to run_async', causing a second
289-
invocation via tool.run_async().
300+
invocation. The fix uses an identity-based sentinel so that None and other
301+
falsy values (0, '', {}, False) are treated as valid results.
290302
"""
291303
call_count = 0
292304

293-
def side_effect_only() -> None:
305+
def sync_func():
294306
nonlocal call_count
295307
call_count += 1
296-
# No return statement — implicit None.
308+
if not use_implicit_return:
309+
return return_value
310+
# implicit None — no return statement
297311

298-
tool = FunctionTool(side_effect_only)
299-
model = testing_utils.MockModel.create(responses=[])
300-
agent = Agent(name='test_agent', model=model, tools=[tool])
301-
invocation_context = await testing_utils.create_invocation_context(
302-
agent=agent, user_content=''
303-
)
304-
tool_context = ToolContext(
305-
invocation_context=invocation_context,
306-
function_call_id='test_id',
307-
)
308-
309-
result = await _call_tool_in_thread_pool(tool, {}, tool_context)
310-
311-
assert result is None
312-
assert call_count == 1, (
313-
f'Tool function executed {call_count} time(s); expected exactly 1.'
314-
)
315-
316-
@pytest.mark.asyncio
317-
async def test_sync_tool_returning_explicit_none_executes_exactly_once(self):
318-
"""Test that explicit None return from FunctionTool is not double-executed."""
319-
call_count = 0
320-
321-
def explicit_none_return() -> None:
322-
nonlocal call_count
323-
call_count += 1
324-
return None # Explicit None return.
325-
326-
tool = FunctionTool(explicit_none_return)
312+
tool = FunctionTool(sync_func)
327313
model = testing_utils.MockModel.create(responses=[])
328314
agent = Agent(name='test_agent', model=model, tools=[tool])
329315
invocation_context = await testing_utils.create_invocation_context(
@@ -336,7 +322,7 @@ def explicit_none_return() -> None:
336322

337323
result = await _call_tool_in_thread_pool(tool, {}, tool_context)
338324

339-
assert result is None
325+
assert result == return_value
340326
assert call_count == 1, (
341327
f'Tool function executed {call_count} time(s); expected exactly 1.'
342328
)

0 commit comments

Comments
 (0)