Repository navigation
test: make the three platform-dependent unit tests portable - #574
Merged
jiangkuaixue123 merged 1 commit intoNov 13, 2026
Merged
Conversation
They failed on Windows for reasons unrelated to what they check: - `test_ascend_config_lifecycle.py` parsed a source file through `Path.read_text()` without an encoding, so the GBK locale decoded UTF-8 bytes and raised; both reads now state UTF-8. - the source-distribution manifest test built its file list with native separators while setuptools works on posix manifest paths, so every required entry missed; the list is built with `as_posix()`. - the entry-point test needs installation metadata, which a source tree does not have, so it now skips with that reason instead of asserting on an empty list. It still checks the entry point on an installed distribution. Signed-off-by: 宋元盟 <songyuanmeng2@huawei.com>
Pingzii
requested review from
hsliuustc0106 and
jiangkuaixue123
as code owners
November 12, 2026 17:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three unit tests fail on Windows for reasons unrelated to what they check:
tests/unit/compat/npu/test_ascend_config_lifecycle.pyreads a source file withPath.read_text()and no encoding, so the GBK locale decodes UTF-8 bytes and raisesUnicodeDecodeError; both reads now state UTF-8.tests/unit/package/test_cam_async_build.pybuilds its manifest file list with native separators while setuptools works on posix manifest paths, so every required entry misses; the list now usesas_posix().tests/unit/package/test_package.pyneeds installation metadata to find thevllm.general_pluginsentry point, which a source tree does not have; it now skips with that reason, and still checks the entry point when the distribution is installed.No production code changes. With these, full
tests/uniton a Windows CPU host withtorchand novLLMis 900 passed / 89 skipped / 0 failed (previously 8 failed).Where these come from, for review context:
tests/unit/compat/npu/test_ascend_config_lifecycle.pyarrived in Integrate vLLM 0.30.0 upgrade with current main #425 (b7561d7) withast.parse(path.read_text())and no encoding, so it is latent everywhere except a locale that cannot decode UTF-8;f2c79d8) replaced the posix-normalisedrequiredset withset(files.allfiles), whose entries carry native separators, so the comparison only holds where/is the separator. The Add routed-only asynchronous CAM operators to the AFD NPU build #361 version of the test compared normalised paths and was correct;tests/test_package.pyin Migrate AFD into external vLLM plugin #1 (dffdb14, "Add Phase 1 plugin skeleton") and was later moved totests/unit/package/test_package.pybyd1fd84f; it always needed installation metadata.None of the three involves the transports in #506; the tests are older than that branch.
On verification scope: this PR's checks are label, pre-commit and DCO -- none of them installs the plugin or runs pytest. The numbers above come from a local run on this branch; the entry-point test exercising an installed build is therefore not covered by CI, before or after this change.