Skip to content

fix: resolve flaky test_nested_subagents_create_recursive_toolparts - #117

Merged
Million-mo merged 3 commits into
wolf1069b:feat/openai-compatible-native-tool-returnfrom
Million-mo:feat/openai-compatible-native-tool-return
Jul 6, 2026
Merged

fix: resolve flaky test_nested_subagents_create_recursive_toolparts#117
Million-mo merged 3 commits into
wolf1069b:feat/openai-compatible-native-tool-returnfrom
Million-mo:feat/openai-compatible-native-tool-return

Conversation

@Million-mo

Copy link
Copy Markdown
Collaborator

Problem

PR #115 的 Unit tests CI 挂了——test_nested_subagents_create_recursive_toolparts 在 CI 慢速环境下死锁,触发 --timeout=60 后 rerun 也卡住,最终 10 分钟 job 超时被 cancel。

Root Cause

Depth=2 spawn event 在 parent consumer 还没完全启动时就发布了,事件丢失导致 assertion 永远等不到,测试挂起。

Fix

在发布 depth=2 spawn event 之前加 await asyncio.sleep(0.2),确保 parent consumer 完全启动。

本地验证 3 次全部 < 1s 通过,无 rerun。

Note: 这是已有的 flaky test,与 OpenAICompatibleModel 无关。fix 在 #116 squash merge 时丢失了,这里单独补上。

Add 200ms sleep before publishing depth=2 spawn event to ensure the
parent consumer is fully started. Without this, the event can be missed
on slow CI runners, causing the test to hang until timeout.

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

Copy link
Copy Markdown

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 hardcoded sleep to prevent a race condition in a subagent events test. The reviewer correctly points out that using a hardcoded sleep is an anti-pattern and suggests replacing it with a polling loop to wait for the parent consumer to initialize, ensuring both speed and robustness.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +567 to +569
# Extra sleep to ensure parent consumer is fully started before publishing
# depth=2 event — without this, the event can be missed on slow CI runners.
await asyncio.sleep(0.2)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using a hardcoded asyncio.sleep(0.2) to resolve race conditions in tests is an anti-pattern. It can still be flaky on extremely slow CI runners, while unnecessarily slowing down the test suite on fast local machines.

Instead, we can use a polling loop with a timeout to wait for the parent consumer to be registered in integration._contexts, followed by a very brief sleep to allow the subscription to complete. This is both faster and more robust.

Suggested change
# Extra sleep to ensure parent consumer is fully started before publishing
# depth=2 event — without this, the event can be missed on slow CI runners.
await asyncio.sleep(0.2)
# Wait for parent consumer to initialize
for _ in range(50):
if parent_id in integration._contexts:
break
await asyncio.sleep(0.01)
await asyncio.sleep(0.05)

Increase wait from 150ms to 300ms to avoid timing flakiness on slow
CI runners where 50ms timeout + two turn executions exceed 150ms.
Replace fixed 200ms sleep with a polling loop that waits for the parent
consumer to register in integration._contexts before publishing the
depth=2 spawn event. Faster on local machines, more robust on slow CI.

Accepts Gemini Code Assist review suggestion.
@Million-mo
Million-mo merged commit 2cb1e58 into wolf1069b:feat/openai-compatible-native-tool-return Jul 6, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant