Refactor: drop dead sys.path hacks in tests/ut/py/#603
Merged
ChaoWao merged 1 commit intohw-native-sys:mainfrom Apr 20, 2026
Merged
Refactor: drop dead sys.path hacks in tests/ut/py/#603ChaoWao merged 1 commit intohw-native-sys:mainfrom
ChaoWao merged 1 commit intohw-native-sys:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes manual sys.path modifications and related imports from several test files, including test_chip_worker.py, test_task_interface.py, and test_platform_comm.py. It also cleans up ruff: noqa: E402 directives that are no longer necessary. I have no feedback to provide.
docs/python-packaging.md rule 2 bans sys.path.insert outside examples/scripts/build_runtimes.py. Three test files under tests/ut/py carried their own sys.path inserts that duplicated what tests/ut/py/conftest.py already does (and, under any install mode, they are covered by site-packages anyway): - tests/ut/py/test_chip_worker.py: inserted python/ to find _task_interface. Already on sys.path via the sibling conftest and installed at the wheel root under pip install. - tests/ut/py/test_task_interface.py: same hack, same reason. - tests/ut/py/test_worker/test_platform_comm.py: inserted project root and python/. Inherited from the stash of PR hw-native-sys#571 which was written for standalone execution; PR hw-native-sys#597 (L1b) always relies on the installed package or conftest-managed sys.path. tests/ut/py/conftest.py is deliberately left alone — upstream PR hw-native-sys#600 just refreshed its sys.path list for the no-install workflow (\"importable without installing the package\"), so the conftest-level hack is a maintained design choice; the per-test duplicates were simply redundant. Ran `pytest tests/ut/py --ignore=tests/ut/py/test_hostsub_fork_shm.py` before and after the removal: 170 passed, 6 skipped in both cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ChaoWao
force-pushed
the
cleanup-test-sys-path-hacks
branch
from
3b51c6e to
50f72c8
Compare
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
1 participant
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.
Summary
Three test files under `tests/ut/py` carried their own `sys.path.insert` blocks that duplicated what the sibling `tests/ut/py/conftest.py` already does — and, under any `pip install` mode, they're redundant with site-packages anyway.
What's dropped
Intentionally NOT touched
Verification
`pytest tests/ut/py --ignore=tests/ut/py/test_hostsub_fork_shm.py` before and after: 170 passed, 6 skipped in both cases.
Net diff
`3 files changed, 1 insertion(+), 28 deletions(-)`
Test plan
🤖 Generated with Claude Code