Skip to content

[train][tests] Fix hanging test test_abort_generation_vllm_engine #1202

Merged
CharlieFRuan merged 3 commits intomainfrom
remove-abort-test
Feb 24, 2026
Merged

[train][tests] Fix hanging test test_abort_generation_vllm_engine #1202
CharlieFRuan merged 3 commits intomainfrom
remove-abort-test

Conversation

@SumanthRH
Copy link
Member

@SumanthRH SumanthRH commented Feb 24, 2026

What does this PR do?

Fixes hanging test test_abort_generation_vllm_engine. Currently this test sends 4 requests to InferenceEngineInterface.chat_completion, calls pause_generation and waits for the requests to finish. However, chat_completion method has an infinite retry loop to wait until the engine is back online - causing a deadlock.

We need to use client.engines[0].chat_completion here .


Open with Devin

Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
@SumanthRH SumanthRH changed the title [train][tests] Remove hanging test test_abort_generation_vllm_engine [train][tests] Fix hanging test test_abort_generation_vllm_engine Feb 24, 2026
@SumanthRH SumanthRH marked this pull request as ready for review February 24, 2026 22:56
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

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 pull request fixes a hanging test, test_abort_generation_vllm_engine, by correctly modifying the calls to chat_completion and completion. The original implementation called these methods on the client object, which contains retry logic that led to a deadlock when generation was paused. The fix is to call the methods directly on the underlying engine (client.engines[0]), bypassing the problematic retry logic. This change is correct and effectively resolves the issue in the test. The changes look good.

x
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Copy link
Member

@CharlieFRuan CharlieFRuan left a comment

Choose a reason for hiding this comment

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

thanks!

@CharlieFRuan CharlieFRuan merged commit 5b27358 into main Feb 24, 2026
4 checks passed
@CharlieFRuan CharlieFRuan deleted the remove-abort-test branch February 24, 2026 22:59
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.

2 participants