Skip to content

Conversation

22quinn
Copy link
Collaborator

@22quinn 22quinn commented Sep 8, 2025

Purpose

Mitigate #24402

Test Plan

CI

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

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 disables several flaky tests in test_structured_output to improve CI stability, which is a reasonable temporary measure. The associated tracking issue is correctly referenced. However, I've identified a latent bug in one of the commented-out test configurations that should be fixed to prevent future errors.

#FIXME: This tests are flaky on CI thus disabled. Tracking in Issue #24402
# ("mistralai/Ministral-8B-Instruct-2410", "outlines", "auto", None),
# ("mistralai/Ministral-8B-Instruct-2410", "outlines", "mistral", None),
#("Qwen/Qwen2.5-1.5B-Instruct", "guidance", "auto"),
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 commented-out test case tuple is malformed. It has 3 elements, but the pytest.mark.parametrize for test_structured_output expects 4 elements (model_name, guided_decoding_backend, tokenizer_mode, speculative_config). If this test is uncommented in the future, it will cause a ValueError during test collection. Please add None as the fourth element to represent the speculative_config.

Suggested change
#("Qwen/Qwen2.5-1.5B-Instruct", "guidance", "auto"),
#("Qwen/Qwen2.5-1.5B-Instruct", "guidance", "auto", None),

@22quinn 22quinn added ready ONLY add when PR is ready to merge/full CI is needed ci-failure Issue about an unexpected test failure in CI labels Sep 8, 2025
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) September 8, 2025 02:16
@DarkLight1337 DarkLight1337 merged commit 3a3e91b into vllm-project:main Sep 8, 2025
26 of 29 checks passed
JasonZhu1313 pushed a commit to JasonZhu1313/vllm that referenced this pull request Sep 8, 2025
eicherseiji pushed a commit to eicherseiji/vllm that referenced this pull request Sep 9, 2025
ChrisYangAI pushed a commit to RichardoMrMu/vllm that referenced this pull request Sep 10, 2025
LopezCastroRoberto pushed a commit to LopezCastroRoberto/vllm that referenced this pull request Sep 11, 2025
skyloevil pushed a commit to skyloevil/vllm that referenced this pull request Sep 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-failure Issue about an unexpected test failure in CI ready ONLY add when PR is ready to merge/full CI is needed v1
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants