Which tests do I have to run before submitting a pool request for a new feature? #15157
Replies: 3 comments
|
You don't need to run all tests before submitting a PR. The errors you're seeing are mostly environment-specific issues (missing optional dependencies, gated model access, distributed setup) that aren't related to your feature. For your specific feature (hidden states output from 1. Core functionality tests # Test the offline LLM interface where you modified SamplingParams
pytest tests/entrypoints/llm/test_llm_generate.py -v
# Test sampling params validation
pytest tests/test_sampling_params.py -v2. Model runner tests (where hidden states are extracted) # Test the model runner that handles generate()
pytest tests/v1/core/test_scheduler.py -v
pytest tests/models/ -k "generate" -v3. Basic correctness tests # Ensure your changes don't break basic generation
pytest tests/entrypoints/ -k "not distributed" --ignore=tests/entrypoints/openai -vHow to identify relevant tests for your feature:
For your own tests, you typically need:
Example test structure: def test_hidden_states_output():
llm = LLM(model="facebook/opt-125m")
sampling_params = SamplingParams(
max_tokens=10,
output_hidden_states=True # Your new flag
)
outputs = llm.generate(["Hello world"], sampling_params)
# Check that hidden states are present
assert hasattr(outputs[0], 'hidden_states')
assert outputs[0].hidden_states is not None
# Check shape: (num_layers, num_tokens, hidden_size)
assert len(outputs[0].hidden_states.shape) == 3About the errors you're seeing: Most are unrelated to your feature:
Recommended workflow:
The maintainers will provide feedback if additional tests are needed. You don't need to anticipate every possible test case upfront. |
|
I love how the Anilab app is pushing the boundaries of social networking by leveraging AI to enhance user interactions, the way it connects people anilab across communities is what makes it truly special and I'm curious to know, have you noticed any significant improvements in user engagement since implementing these AI-driven features? |
|
amazing info thanks |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Which tests must the new feature pass before the pull request is made?
I locally implemented output of hidden states from LLM's
generate()function based on a new flag attribute that I added toSamplingParamsin a separate sandboxed version of vLLM.Before I even tried to merge/code it into the official dev version, I tried to run tests as described here:
on an editable vLLM version that I installed using these commands, before implementing my changes:
and I got a ton of errors, and that's the official dev repo's code. When I run all my University research scripts, and when I run all the commands I mentioned above **before
pre-commit run --all-files** within the same environment(that is, with dev version of vllm), I do not get any errors, andpre-commit run --all-filesproduces only a few ones related to lint. When I run models as a part of my research in the same environment, I never have any issues. Apparently, some of the errors are due to peculiarities of my environment, or lack of access to certain Huggingface models that I never use. Anyways, those errors pertain to some aspects that I never encounter and the feature I implemented never interacts with those.Therefore, I wonder do I really have to pass all the tests in
pytest/directory, especially given that currently all of the errors are get are related to my conda environment rather than to the code I wrote/ feature I implemented, since I haven't yet written/merged them into the repository in question.In addition, tests are notoriously slow to run, especially with limited compute resources.
If so, I wonder which subset of the tests do I need to pass before I create pull request? How do I determine those tests that are related to my feature (apart from those that I written myself)?
Also, how many tests of my own should I write to extensively test my feature? I know the criteria is that they should cover the usage of the feature, but still, on the average, how many tests usually does it take quantitatively?
Just for reference, the errors and the environment snapshot. I did not put it into "issues" or on Stackoverflow because my primary question is not asking to help with these errors below (although, if I have to pass all the tests, that would be appreciated), but rather to figure out which, if not all, tests does my code have to pass :
Environment snapshot:
All reactions