Skip to content

[Tests] Remove deprecated and compatibility-only coverage#39832

Open
yewentao256 wants to merge 3 commits intomainfrom
remove-outdated-unit-tests
Open

[Tests] Remove deprecated and compatibility-only coverage#39832
yewentao256 wants to merge 3 commits intomainfrom
remove-outdated-unit-tests

Conversation

@yewentao256
Copy link
Copy Markdown
Member

@yewentao256 yewentao256 commented Apr 14, 2026

Purpose

Part of #39829

eg. drop pooling tests that only asserted deprecated warning paths

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 Apr 14, 2026
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 removes various deprecated tests and compatibility layers across the codebase, specifically targeting pooling tasks, weight utilities, argparse utilities, and KV connector integrations. However, feedback indicates that several tests—including those for deprecated KV scale mapping, argparse back-compatibility for the --model flag, and LMCache integration contracts—are being removed while the underlying implementation logic remains active in the repository, resulting in a significant loss of test coverage.

I am having trouble creating individual review comments. Click here to see my feedback.

tests/model_executor/test_weight_utils.py (105-110)

high

The test test_deprecated_kv_scale is being removed, but the corresponding compatibility logic in vllm/model_executor/model_loader/weight_utils.py (lines 1521-1538) is still present in the codebase. Removing tests for features that are still active and supported (even if deprecated) increases the risk of regressions. It is recommended to retain these tests until the underlying code is actually removed from the repository.

tests/utils_/test_argparse_utils.py (279-321)

high

This PR removes the test coverage for the --model argument back-compatibility in vllm serve. However, the remapping logic and the associated warning are still present in vllm/utils/argparse_utils.py (lines 185-217). Furthermore, the commented-out test block intended for when compatibility ends is also being deleted. It is safer to keep these tests active as long as the code they verify remains in the repository, or to update the code to the final state (raising an error) and update the tests accordingly to ensure the transition is correctly enforced.

tests/v1/kv_connector/unit/test_lmcache_integration.py (1-229)

high

Removing test_lmcache_integration.py eliminates critical contract tests that ensure vLLM's internal changes do not break the LMCache integration. These tests verify that key attributes and methods on classes like VllmConfig, ModelConfig, and Request remain stable. Unless the LMCache integration is also being removed or these tests have been moved to a different suite, this represents a significant loss in integration testing coverage for a major external component.

yewentao256 and others added 2 commits April 14, 2026 20:47
Signed-off-by: yewentao256 <zhyanwentao@126.com>
@noooop
Copy link
Copy Markdown
Collaborator

noooop commented Apr 15, 2026

For pooling-related tests, please wait for #37861 to land in v0.20.0.

These tests are important before this, please do not remove them.

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

Labels

kv-connector ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants