Skip to content

[CI] Fix AMD: V1 Others (mi325_1) Amd CI bug#35816

Closed
yewentao256 wants to merge 4 commits intomainfrom
wentao-fix-amd-ci-test-others-bug
Closed

[CI] Fix AMD: V1 Others (mi325_1) Amd CI bug#35816
yewentao256 wants to merge 4 commits intomainfrom
wentao-fix-amd-ci-test-others-bug

Conversation

@yewentao256
Copy link
Member

@yewentao256 yewentao256 commented Mar 3, 2026

Purpose

Fixes https://buildkite.com/vllm/ci/builds/54064#019cb0df-37b0-4ad3-a37b-ad9446da1819

<html>
<body>
<!--StartFragment-->
============================= 3 warnings in 2.22s ==============================
--
ERROR: Wrong expression passed to '-m': "not cpu_test": at column 1: expected not OR left parenthesis OR identifier; got string literal

<!--EndFragment-->
</body>
</html>

Test

Covered in CI

Signed-off-by: yewentao256 <zhyanwentao@126.com>
@yewentao256 yewentao256 added the ready ONLY add when PR is ready to merge/full CI is needed label Mar 3, 2026
@mergify mergify bot added ci/build rocm Related to AMD ROCm bug Something isn't working labels Mar 3, 2026
@github-project-automation github-project-automation bot moved this to Todo in AMD Mar 3, 2026
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 CI failure by extending the check for pre-quoted pytest marker expressions to include double-quoted strings. While this addresses the immediate issue, the underlying heuristic for detecting quoted strings is fragile. My review includes a suggestion to make this check more robust by verifying that the string starts and ends with quotes, which will prevent misinterpretation of unquoted strings containing quote characters (e.g., don't_run). This improvement will help prevent similar CI failures in the future.

Signed-off-by: yewentao256 <zhyanwentao@126.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
@llsj14
Copy link
Contributor

llsj14 commented Mar 3, 2026

I'm also facing following ci failed error. I think it is also related to this.

tests/v1/core/test_single_type_kv_cache_manager.py::test_chunked_local_attention_get_num_blocks_to_allocateERROR: Wrong expression passed to '-m': "not cpu_test": at column 1: expected not OR left parenthesis OR identifier; got string literal

https://buildkite.com/vllm/ci/builds/54094/steps/canvas?jid=019cb172-06da-4804-a67d-801632ef5eff

commands:
# Only run tests that need exactly 2 GPUs
- pytest -v -s v1/e2e/test_spec_decode.py -k "tensor_parallelism"
- pytest -v -s v1/e2e/test_spec_decode.py -k 'tensor_parallelism'
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this for real?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is another bug fix, so ' is needed

# ---- Command source selection ----
# Prefer VLLM_TEST_COMMANDS (preserves all inner quoting intact).
# Fall back to $* for backward compatibility, but warn that inner
# double-quotes will have been stripped by the calling shell.
if [[ -n "${VLLM_TEST_COMMANDS:-}" ]]; then
  commands="${VLLM_TEST_COMMANDS}"
  echo "Commands sourced from VLLM_TEST_COMMANDS (quoting preserved)"

@khluu
Copy link
Collaborator

khluu commented Mar 3, 2026

FYTI we just reverted the changes to use VLLM_TEST_COMMANDS on ci-infra side

@yewentao256
Copy link
Member Author

FYTI we just reverted the changes to use VLLM_TEST_COMMANDS on ci-infra side

Ohh, so you mean this PR is not needed any more? Feel free to close it if so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci/build ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants