[Test] Add qwen3-omni tests for audio_in_video and one word prompt#2097
Conversation
Signed-off-by: yenuo26 <410167048@qq.com>
|
@Shirley125 @amy-why-3459 PTAL |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42387a32de
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d increasing max tokens in CI configuration Signed-off-by: yenuo26 <410167048@qq.com>
|
Please add a multi-concurrency precision test case for audio_in_video. |
Signed-off-by: yenuo26 <410167048@qq.com>
done |
|
@Gaohan123 @tzhouam please help to add a ready label |
hsliuustc0106
left a comment
There was a problem hiding this comment.
BLOCKER scan:
- Correctness: ISSUES:
test_audio_in_video_001/002still do not assert on audio-derived content, so the newuse_audio_in_videopath can break while the tests continue to pass by describing only the video. - Reliability/Safety: ISSUES:
.buildkite/test-ready.ymlnow hard-switches the H100 job to an advanced expansion test with an in-file note saying it is for debug and should be removed before merge. That changes the repo's normal CI signal rather than adding stable coverage. - Breaking Changes: PASS
- Test Coverage: needs tests/evidence that specifically fail when embedded audio is ignored.
- Documentation: PASS (test-only PR)
- Security: PASS
OVERALL: 2 BLOCKERS FOUND
VERDICT: REQUEST_CHANGES
I validated the new use_audio_in_video wiring in send_omni_request() and the new expansion tests. The remaining blockers are that the assertions still don't depend on extracted audio content, and the Buildkite change is explicitly checked in as a temporary debug path instead of stable CI coverage.
| timeout 20m bash -c ' | ||
| export VLLM_WORKER_MULTIPROC_METHOD=spawn | ||
| export VLLM_TEST_CLEAN_GPU_MEMORY="1" | ||
| #pytest -s -v tests/e2e/online_serving/test_qwen3_omni.py -m "core_model" --run-level "core_model" |
There was a problem hiding this comment.
This swaps the normal H100 omni test job over to test_qwen3_omni_expansion.py and even leaves a for debug, will be removed before merging note in the committed pipeline. That means the PR is changing repository CI coverage in a temporary/debug-only way instead of adding a stable test signal. Could you revert the debug pipeline override and keep the new coverage in a dedicated test/job if it needs CI coverage?
Review SummaryVerdict: ✅ LGTM (No Blockers) Test-only PR for Qwen3-omni edge cases. Good coverage expansion! Test Quality Check✅ Test correctness: Tests use standard
✅ Test evidence: PR shows 1 passed in 132.98s Non-blocking Observationsconftest.py: Large refactoring (121 additions, 32 deletions) - test infrastructure improvements. Not reviewing in detail since it's test infrastructure. test-ready.yml: Modified CI config (39 additions, 38 deletions) - appears to be test scheduling changes. Ready to merge. |
…ken configuration Signed-off-by: yenuo26 <410167048@qq.com>
…to include audio in the output MP4. Update tests to utilize the new feature and adjust CI configuration to reduce `max_tokens` for improved performance. Signed-off-by: yenuo26 <410167048@qq.com>
Signed-off-by: wangyu <410167048@qq.com>
|
Please fix docs |
Actually, I didn't modify any document. I will retry it. |
|
@yenuo26 Please resolve conflicts |
|
| openai_client.send_omni_request(request_config, request_num=get_max_batch_size()) | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="There is a known issue: https://github.com/vllm-project/vllm-omni/pull/2019") |
There was a problem hiding this comment.
PR #2019 has been merged. Please remove the skip option and test whether the test cases pass.
There was a problem hiding this comment.
During local testing, I found that when called concurrently, this error is not resolved.
There was a problem hiding this comment.
OK, I will continue to follow up on this issue and will remove the skip flag once it is resolved.
|
@Gaohan123 @david6666666 please help to add nightly-test |
|
Can this be merged? I've already run L2+L4. The vllm-omni CI failed because the Diffusion L4 performance test didn't pass. @Gaohan123 |
…ecision for unique timestamps, preventing file overwrites during concurrent calls. Signed-off-by: wangyu <410167048@qq.com>
…d checks for similarity and normalized matching against audio_transcript_key_words. Updated test cases to reflect new requirements. Signed-off-by: wangyu <410167048@qq.com>
…ness. Signed-off-by: wangyu <410167048@qq.com>
I just modify test case in "Omni Model Test with H100", and it is success in nightly test. |
Signed-off-by: wangyu <53896905+yenuo26@users.noreply.github.com>
…llm-project#2097) Signed-off-by: yenuo26 <410167048@qq.com> Signed-off-by: wangyu <410167048@qq.com> Signed-off-by: wangyu <53896905+yenuo26@users.noreply.github.com> Co-authored-by: Alicia <115451386+congw729@users.noreply.github.com>
…llm-project#2097) Signed-off-by: yenuo26 <410167048@qq.com> Signed-off-by: wangyu <410167048@qq.com> Signed-off-by: wangyu <53896905+yenuo26@users.noreply.github.com> Co-authored-by: Alicia <115451386+congw729@users.noreply.github.com>
…llm-project#2097) Signed-off-by: yenuo26 <410167048@qq.com> Signed-off-by: wangyu <410167048@qq.com> Signed-off-by: wangyu <53896905+yenuo26@users.noreply.github.com> Co-authored-by: Alicia <115451386+congw729@users.noreply.github.com>

PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.
Purpose
Add qwen3-omni tests for audio_in_video and one word prompt
Test Plan
1.run in local env
2.run in ci
Test Result
1.local
2.ci

Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model. Please runmkdocs serveto sync the documentation editions to./docs.BEFORE SUBMITTING, PLEASE READ https://github.com/vllm-project/vllm-omni/blob/main/CONTRIBUTING.md (anything written below this line will be removed by GitHub Actions)