Skip to content

test(fault_tolerance): skip flaky sglang migration NATS combo, drop stale graceful_shutdown xfail #8404

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.

Sign up for GitHub

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

Merged
dmitry-tokarev-nv merged 2 commits into from
Apr 21, 2026
Merged

test(fault_tolerance): skip flaky sglang migration NATS combo, drop stale graceful_shutdown xfail #8404

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b869e4
test(fault_tolerance): skip flaky sglang migration NATS combo, drop s…
dmitry-tokarev-nv Apr 20, 2026
9d88a2e
Apply suggestion from @dmitry-tokarev-nv
dmitry-tokarev-nv Apr 21, 2026
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
24 changes: 17 additions & 7 deletions tests/fault_tolerance/migration/test_sglang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,7 @@
"immediate_kill",
[
pytest.param(True, id="worker_failure"),
pytest.param(
False,
id="graceful_shutdown",
marks=pytest.mark.xfail(
strict=False, reason="SGLang graceful shutdown not yet implemented"
),
),
pytest.param(False, id="graceful_shutdown"),
],
),
pytest.mark.parametrize(
Expand Down Expand Up @@ -241,6 +235,22 @@ def test_request_migration_sglang_aggregated(
stream: True for streaming, False for non-streaming
"""

# TODO(<LINEAR-ID>): Flaky on NATS transport — first-token delay routinely
# exceeds the 6s threshold in utils.validate_response. Other parameter
# combinations (including the TCP variant) are stable.
if (
migration_limit == 3
and migration_max_seq_len is None
and immediate_kill is True
and request_api == "chat"
and stream is True
and request.getfixturevalue("request_plane") == "nats"
):
pytest.skip(
"Flaky on NATS transport: first-token delay > 6s threshold. "
"TODO(<LINEAR-ID>)"
Comment thread
dmitry-tokarev-nv marked this conversation as resolved.
Resolved
)
Comment on lines +238 to +252

@coderabbitai coderabbitai Bot Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace the Linear placeholder before merge.

The skip is targeted correctly, but both the comment and skip reason still contain TODO(<LINEAR-ID>), which makes the tracked flaky-test debt unactionable once merged.

If helpful, I can update this once you provide the real Linear ticket ID.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/fault_tolerance/migration/test_sglang.py` around lines 238 - 252, The
TODO marker "TODO(<LINEAR-ID>)" in both the skip message and the preceding
comment should be replaced with the actual Linear ticket ID (or a real issue
identifier) so the flaky-test debt is trackable; update the comment above the
conditional and the pytest.skip(...) reason string that guard the branch
checking migration_limit, migration_max_seq_len, immediate_kill, request_api,
stream, and request.getfixturevalue("request_plane") to include the real ticket
ID (e.g., TODO(LINEAR-1234) or LINEAR-1234) or an actionable issue link and keep
the existing conditional logic unchanged.


# Step 1: Start the frontend
with DynamoFrontendProcess(
request,
Expand Down
Loading