[core] refactor: consolidate _common test files under _common/tests directory#53543
[core] refactor: consolidate _common test files under _common/tests directory#53543edoakes merged 2 commits intoray-project:masterfrom
_common test files under _common/tests directory#53543Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and consolidates test utilities by moving SignalActor and Semaphore classes under the _common/tests directory while maintaining a backward compatibility shim. Key changes include moving and renaming test utility files, updating import paths in tests, and updating the build file accordingly.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/ray/_common/tests/test_utils.py | Contains the moved SignalActor and Semaphore definitions; missing an asyncio import |
| python/ray/_common/tests/test_signal_semaphore_utils.py | Updated import path for test utilities |
| python/ray/_common/tests/BUILD | Updated test file mapping to reflect the renamed and consolidated files |
| python/ray/_common/test_utils.py | Now acts as a backward compatibility shim importing from the new location |
Comments suppressed due to low confidence (1)
python/ray/_common/tests/test_utils.py:22
- It appears that 'asyncio' is used in the SignalActor class but is not imported. Please add 'import asyncio' at the beginning of the file to ensure proper functionality.
self.ready_event = asyncio.Event()
@codope I think this part might be problematic because the |
|
The rest LGTM and thank you for splitting this change out into its own PR 👍 |
Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>
Got it, I have moved back SignalActor and Semaphore to |
…n accessible in the distributed Ray package Signed-off-by: Sagar Sumit <sagarsumit09@gmail.com>
Why are these changes needed?
In #53457 we moved signal and semaphore to
_common/which is used inservetests. However, after that,_commonhad twotest_utilswhich was confusing. This PR reorganizes the test utilities and their tests for better clarity:Key Changes:
SignalActorandSemaphoretest utility classes in_common/test_utils.pyto ensure they're included in Ray package distribution (external libraries need access to these)_common/tests/test_utils.py(tests forray._common.utilsfunctions)_common/tests/test_signal_semaphore_utils.py(tests forSignalActorandSemaphore)Final Organization:
_common/test_utils.py: Test utility classes (SignalActor,Semaphore) - distributed with Ray package_common/tests/test_utils.py: Tests for Ray utility functions (get_or_create_event_loop,run_background_task)_common/tests/test_signal_semaphore_utils.py: Tests for test utility classesThis resolves the confusion of having multiple
test_utilsfiles while ensuring test utilities remain accessible to external libraries that depend on Ray. The separation maintains clear boundaries between utility classes (for distribution) and their tests (for development).