Skip to content

test(npu): repair stale expectations and stub leaks in the NPU unit tests - #583

Open
Pingzii wants to merge 9 commits into
vllm-project:mainfrom
Pingzii:fix/581-retire-sync-gate-case
Open

test(npu): repair stale expectations and stub leaks in the NPU unit tests#583
Pingzii wants to merge 9 commits into
vllm-project:mainfrom
Pingzii:fix/581-retire-sync-gate-case

Conversation

@Pingzii

@Pingzii Pingzii commented Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Three test repairs around the NPU feature-validation table:

  • [gate-on-attention] (added before feat(npu): transport BF16 Attention-side routing to the FFN role #503): the connector-local compute_gate_on_attention key is a compatibility no-op since feat(npu): transport BF16 Attention-side routing to the FFN role #503 -- the common afd.compute_gate_on_attention field selects routing placement -- so the case can no longer raise. The expectation is deleted; the narrowed validator it once covered is still pinned in test_sync_gate_feature_validation_cpu.py (TP != 1, DBO/ubatching).
  • [async-graph] (stale since [NPU] Enable native target and DSpark draft Attention graphs #545): Async CAM Attention accepts FULL and the native backend resolves it to FULL_DECODE_ONLY, so the config is valid input. The row stays in the table with None as its expectation, documenting the accepted shape.
  • the two connector-contract cases in test_sync_gate_feature_validation_cpu.py ran behind pytest.importorskip("vllm") although nothing at call time needs vLLM; _real_extra_info() now stands in only the modules the import touches, restores sys.modules, and returns the real CAMP2PExtraInfo, so the local run executes the class instead of skipping it (9 passed / 2 skipped -> 11 passed).

Test changes only. Local Windows CPU run: full tests/unit 922 passed / 90 skipped / 0 failed; the vLLM-only file still skips here, so the merge build from #581 remains its verification.

@Pingzii
test(npu): retire the sync-gate case that no longer raises
5a5f580 
`test_npu_feature_validation[gate-on-attention]` asserted that a connector-local
`compute_gate_on_attention` key is rejected. Since vllm-project#503 the key is accepted and
ignored for routing -- the common `afd.compute_gate_on_attention` field selects
the mode -- so the case has been failing on main merge builds since the suite
started collecting again (vllm-project#247 onward, masked before that by the torch_npu
collection error).

The narrowed validator it once covered is still pinned by
`test_sync_gate_feature_validation_cpu.py`, which checks TP != 1 and
DBO/ubatching rejection, so the expectation is deleted rather than replaced.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@github-actions github-actions Bot added the Ascend Ascend NPU platform and related changes label Nov 14, 2026
@Pingzii Pingzii mentioned this pull request Nov 14, 2026
Open
1 of 5 tasks
@Pingzii
test(npu): let the async graph case state the mode vllm-project#545 a…
b89f789 
…llows

vllm-project#545 lets Async CAM Attention run the FULL graph mode and has the native backend
resolve it to FULL_DECODE_ONLY, so the case that asserted a rejection was stale
in the same way as the sync-gate one. The config stays in the table as a valid
input; it is the expectation that changes.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii Pingzii changed the title test(npu): retire the sync-gate case that no longer raises test(npu): retire two feature-validation cases their features outgrew Nov 14, 2026
@Pingzii

Pingzii commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Added the second stale row ([async-graph], from #545) to this PR. Both cases kept their configs; only the expectations changed. On main's side the remaining known mismatch is [quant], which breaks only in #506 because that branch made mode 4 legal and the message became CAMP2P supports quant_mode=[0, 4]; got 1; #506 will update it on rebase.

@Pingzii
test(npu): run the two CAMP2P extra-info cases against the real class
d6f5d89 
The connector-local compatibility flag and the unknown-quant-mode case sat behind
``pytest.importorskip(vllm)`` although they only need the module to import:
``camp2p`` pulls ``vllm.forward_context`` at module scope, nothing at call time
touches vLLM. They were therefore the two cases a bare CPU host never ran, and
the connector-level contract they pin is exactly what vllm-project#581 tripped over.

The helper now stubs the handful of runtime modules the import needs, restores
``sys.modules`` afterwards, and returns the real ``CAMP2PExtraInfo``; parsing and
validation stay real. Local run: 11 passed (previously 9 passed, 2 skipped).

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii Pingzii changed the title test(npu): retire two feature-validation cases their features outgrew test(npu): repair the feature-validation tests their features outgrew Nov 14, 2026
@Pingzii

Pingzii commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

Folded the local-coverage change from #584 in (third commit, d6f5d89): the two extra-info cases now run against the real CAMP2PExtraInfo on a bare CPU host. #584 is closed in favour of this PR; nothing else changed in it.

@Pingzii Pingzii mentioned this pull request Nov 14, 2026
Closed
@Pingzii
test(npu): drop the modules imported under the extra-info stubs
9768caa 
The stubbed import caches ``afd_plugin.connectors.npu.camp2p`` (and friends) in
``sys.modules``, so the placeholder ``get_forward_context`` and the no-op
registration outlive the helper. A later test that imports the package for real
would get the stand-in callables instead -- the leak described in vllm-project#568.

Everything created by that import is now removed from ``sys.modules`` before the
helper returns, next to restoring the stubbed entries.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii
test(npu): drop every afd_plugin module imported under the stubs
b51ecfa 
Removing only the CAMP2P modules left ``afd_plugin.connectors.metadata`` cached
with the stand-in ``DPMetadata``, so a later test in the same process could still
see the stub through the module it reuses.

The cleanup now removes every ``afd_plugin`` module the stubbed import created,
which covers the connector and the shared metadata module alike; a bare-process
check shows none of them, and no stand-in module, survives the call.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii
test(npu): share the stub-bound module dropper with the gate tests
b4428dd 
Dropping the cached child module is not enough: the parent package keeps it as an
attribute, so ``from afd_plugin.connectors import metadata`` still handed back the
module that captured the stand-in ``DPMetadata``.

The ``_drop_stub_bound_modules`` helper already handled that case in the token-id
tests; it moves to ``tests/unit/fakes/stub_modules.py`` and both files use it, so
a module is dropped together with the parent attribute that points at it.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
jiangkuaixue123
jiangkuaixue123 reviewed Nov 15, 2026
View reviewed changes

@jiangkuaixue123 jiangkuaixue123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed b4428dd. One CPU-only test portability issue below. Validation was static source tracing; no project tests or device execution were run.

Comment thread tests/unit/compat/npu/test_sync_gate_feature_validation_cpu.py
before = set(sys.modules)
sys.modules.update(stubs)
try:
from afd_plugin.connectors.npu.camp2p import CAMP2PExtraInfo

@jiangkuaixue123 jiangkuaixue123 Nov 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Keep the torch gate when loading the real connector

On the supported CPU-only development environment without torch, both connector-contract tests now fail instead of skipping: this helper only stubs vLLM, while importing CAMP2PExtraInfo reaches camp2p/connector.py:23 and its unconditional import torch. The previous pytest.importorskip("torch") was removed along with the vLLM gate, and neither conftest supplies a global torch stub. Please retain a local torch gate (or make the extra-info type genuinely torch-independent) so these cases still exercise real production parsing on torch-equipped CPU hosts without breaking the no-torch unit suite required by AGENTS.md and docs/UT_MAINTENANCE.md.

@Pingzii Pingzii Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. b3e367a restores pytest.importorskip("torch") inside _real_extra_info(), before the helper installs the vLLM stubs or imports the CAMP2P connector. On a CPU host with torch and without vLLM, all 11 tests pass, including the two that exercise the real CAMP2PExtraInfo parser. With the torch import blocked, 9 tests pass and those 2 skip with the missing-torch reason.

@Pingzii
test(npu): keep the torch gate on the real extra-info cases
b3e367a 
Importing ``CAMP2PExtraInfo`` reaches ``camp2p/connector.py`` and its
unconditional ``import torch``, so dropping the vLLM gate also dropped the only
guard the no-torch unit suite had: on a CPU host without torch the two cases
failed instead of skipping.

The helper asks for torch again before it stubs vLLM. On a torch-less host they
skip with that reason; on a torch-equipped CPU host they still run real parsing
and validation. Verified both ways: 11 passed with torch, 9 passed / 2 skipped
with torch made unimportable.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii
test(npu): drop the modules that captured the torch_npu stand-in
35adc40 
The proxy test stubs ``torch_npu`` at module scope and removed only the cache
entry afterwards. Every module imported in that window that does ``import
torch_npu`` keeps the stand-in in its globals -- ``deepseek_v4_shared_experts``
does exactly that -- so a later import reuses the cached module and sees the
stand-in instead of a missing extension.

The window is now bracketed and the modules bound to the stand-in are dropped
together with the parent attributes pointing at them, using the helper the other
stub sites already share. Static effect verified on a stand-in package pair;
the file itself needs vLLM-Ascend, so its real-environment behaviour is for the
merge build to confirm.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii Pingzii changed the title test(npu): repair the feature-validation tests their features outgrew test(npu): repair stale expectations and stub leaks in the NPU unit tests Nov 15, 2026
@Pingzii
Pingzii force-pushed the fix/581-retire-sync-gate-case branch from 1058918 to 258691e Compare November 15, 2026 03:59
@Pingzii
test(npu): drop the adapter subtree that keeps the stand-in alive
0202060 
Removing the modules whose globals name the stand-in missed one shape: the NPU
adapter does ``from ... import AFDDeepseekV4SharedExperts`` and holds only the
class, while the class methods keep the defining module's globals -- and the
stand-in in them -- alive. The adapter stayed cached and a later import reused
it.

``drop_new_modules`` forgets a named subtree (parent attributes included, the
package itself kept so the children's attributes are cleared before it goes),
and the proxy test calls it for the adapter tree after the stub-bound scan.
Static check: shared experts, adapter and the stand-in are gone from
``sys.modules`` and the parent holds neither attribute.

Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
@Pingzii
Pingzii force-pushed the fix/581-retire-sync-gate-case branch from 258691e to 0202060 Compare November 15, 2026 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jiangkuaixue123 jiangkuaixue123 jiangkuaixue123 left review comments
@hsliuustc0106 hsliuustc0106 Awaiting requested review from hsliuustc0106 hsliuustc0106 is a code owner

Assignees

No one assigned

Labels

Ascend Ascend NPU platform and related changes

Projects

None yet

Milestone

No milestone

2 participants

@Pingzii @jiangkuaixue123