vllm
vllm copied to clipboard
vllm-project
[CI/Build] Move `test_utils.py` to `tests/utils.py`
Since #4335 was merged, I've noticed that the definition of ServerRunner in the tests is the same as in the test for OpenAI API. I have moved the class to the test utilities to avoid code duplication. (Although it only has been repeated twice so far, I will add another similar test suite in #4200 which would duplicate the code a third time)
Also, I have moved the test utilities file (test_utils.py) to under the test directory (tests/utils.py), since none of its code is actually used in the main package. Note that I have added __init__.py to each test subpackage and updated the ray.init() call in the test utilities file in order to relative import tests/utils.py.
I'm not that familiar with how the nccl library is handled in vLLM. How would using relative imports cause the library to not be loaded?
Apart from the AMD tests failing due to out of storage space and kernels-test-3 being canceled, the tests have passed. I think we can merge this now.
It's a little bit late but I find pytorch also has some testing code in the main library https://github.com/pytorch/pytorch/tree/main/torch/testing . Maybe we need to discuss it. Relative import used in this PR is quite fragile .
Hmm is there a way to just exclude it in actual wheel?
Not sure. But why do we care about this anyway? It's just a single python file with dozens of lines.
Relative import used in this PR is quite fragile.
By this, do you mean that it breaks calling pytest from inner directories? I think this is not necessarily tied to the location of utils. The main reason why I moved it to tests directory is to avoid accidentally making test-only dependencies required for the user, rather than reducing wheel size.
To reinstate this functionality while keeping utilities in the tests directory, we can import the utilities by import tests.utils rather than the original import vllm.testing_utils. Prior to this PR, some test code already referred to the root conftest in a similar manner (import tests.conftest), so it should work.