Skip to content

[Fix] skip xlite e2e test#4786

Merged
wangxiyuan merged 2 commits intovllm-project:mainfrom
lulina:vllm_ascend_xlite
Dec 8, 2025
Merged

[Fix] skip xlite e2e test#4786
wangxiyuan merged 2 commits intovllm-project:mainfrom
lulina:vllm_ascend_xlite

Conversation

@lulina
Copy link
Copy Markdown
Contributor

@lulina lulina commented Dec 8, 2025

What this PR does / why we need it?

Due to the differences in operators used and execution order between xlite and eager modes, there will be slight precision discrepancies. This patch skip the xlite e2e tests.

Does this PR introduce any user-facing change?

No

How was this patch tested?

vLLM version: v0.12.0
vLLM main: vllm-project/vllm@ad32e3e

Signed-off-by: lulina <lina.lulina@huawei.com>
Copy link
Copy Markdown
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 two end-to-end tests for xlite mode by adding @pytest.mark.skip. The reason provided is to handle precision discrepancies between xlite and eager modes. While this addresses CI failures, a better approach is to mark these tests as expected failures. I have recommended replacing @pytest.mark.skip with @pytest.mark.xfail(reason=...). This approach keeps the tests within the test suite, making the known issue visible, and will alert the team if the tests unexpectedly start passing, which could indicate the underlying issue has been resolved.

]


@pytest.mark.skip
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.

high

Instead of unconditionally skipping this test with @pytest.mark.skip, it is better practice to mark it as an expected failure using @pytest.mark.xfail. This will still prevent the test from failing the CI, but it keeps the test active in the test suite. If the underlying precision issue is ever resolved and the test starts passing, xfail will flag it as an unexpected pass, providing visibility that the issue may be fixed.

Suggested change
@pytest.mark.skip
@pytest.mark.xfail(reason="Known precision discrepancies between xlite and eager modes")

)


@pytest.mark.skip
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.

high

Similar to the other test, please use @pytest.mark.xfail instead of @pytest.mark.skip. This clearly documents that there is a known failure and ensures the test is still executed. It improves tracking of the issue and will notify you if the test's behavior changes from failing to passing in the future.

Suggested change
@pytest.mark.skip
@pytest.mark.xfail(reason="Known precision discrepancies between xlite and eager modes")

@lulina lulina changed the title [BugFix] skip xlite e2e test [Fix] skip xlite e2e test Dec 8, 2025
@wangxiyuan wangxiyuan merged commit afe0050 into vllm-project:main Dec 8, 2025
14 of 16 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 8, 2025

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

weijinqian0 pushed a commit to weijinqian0/vllm-ascend that referenced this pull request Dec 9, 2025
### What this PR does / why we need it?
Due to the differences in operators used and execution order between
xlite and eager modes, there will be slight precision discrepancies.
This patch skip the xlite e2e tests.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
vLLM version: v0.12.0
vLLM main:
vllm-project/vllm@ad32e3e

Signed-off-by: lulina <lina.lulina@huawei.com>
Co-authored-by: wangxiyuan <wangxiyuan1007@gmail.com>
Signed-off-by: weijinqian_v1 <weijinqian@huawei.com>
Clorist33 pushed a commit to Clorist33/vllm-ascend that referenced this pull request Dec 10, 2025
### What this PR does / why we need it?
Due to the differences in operators used and execution order between
xlite and eager modes, there will be slight precision discrepancies.
This patch skip the xlite e2e tests.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
vLLM version: v0.12.0
vLLM main:
vllm-project/vllm@ad32e3e

Signed-off-by: lulina <lina.lulina@huawei.com>
Co-authored-by: wangxiyuan <wangxiyuan1007@gmail.com>
Mercykid-bash pushed a commit to Mercykid-bash/vllm-ascend that referenced this pull request Dec 10, 2025
### What this PR does / why we need it?
Due to the differences in operators used and execution order between
xlite and eager modes, there will be slight precision discrepancies.
This patch skip the xlite e2e tests.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
vLLM version: v0.12.0
vLLM main:
vllm-project/vllm@ad32e3e

Signed-off-by: lulina <lina.lulina@huawei.com>
Co-authored-by: wangxiyuan <wangxiyuan1007@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants