[CI] use pytest.ini to manage vllm native tests #5
Conversation
| jobs: | ||
| test: | ||
| name: vLLM Ascend test (self-host) | ||
| if: ${{ github.event_name == 'push' || github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && contains(github.event.comment.body, 'recheck_pr_test')) }} |
There was a problem hiding this comment.
comment should be added to on section as well to enable triger
There was a problem hiding this comment.
maybe some meaningful word like /recheck e2e is better.
There was a problem hiding this comment.
qq: the issue comments triggered test will replace pr test results or not (in the end of this PR CI results)? It seems recheck results only show on action page, is it expected?
There was a problem hiding this comment.
@MengqingCao https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#issue_comment-on-issues-only-or-pull-requests-only
I've tried this and same as @Yikun said, the workflow will be triggered with the latest code of branch main at action, not related to pr.
I think we should solve this in next pr so that we do not block CI development.
| jobs: | ||
| test: | ||
| name: vLLM Ascend test (self-host) | ||
| if: ${{ github.event_name == 'push' || github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && contains(github.event.comment.body, 'recheck_pr_test')) }} |
There was a problem hiding this comment.
maybe some meaningful word like /recheck e2e is better.
| jobs: | ||
| test: | ||
| name: vLLM Ascend test (self-host) | ||
| if: ${{ github.event_name == 'push' || github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && contains(github.event.comment.body, 'recheck_pr_test')) }} |
There was a problem hiding this comment.
qq: the issue comments triggered test will replace pr test results or not (in the end of this PR CI results)? It seems recheck results only show on action page, is it expected?
|
BTW, better to use the https://github.com/vllm-project/vllm-ascend/blob/main/.github/PULL_REQUEST_TEMPLATE.md refresh commits msg. |
Signed-off-by: MengqingCao <[email protected]>
There was a problem hiding this comment.
LGTM, compare to https://github.com/vllm-project/vllm-ascend/actions/runs/13178356884/job/36782919013?pr=15 , It seems current all 13 tests are running together rather than 7 tests + 5 tests + 1 tests. and the time reduce from 55s to 18s.
### What this PR does / why we need it? Use `pytest.ini` to manage vllm native tests. This will convert the original test script whitelist to a blacklist to prevent missing the newly added test scripts of the upstream vLLM. **note**: _we do **not** manage the test scripts of vLLM-Ascend in `pytest.ini`, because if we do so, there will be conflicts between vLLM and vLLM-Ascend's `conftest.py`._ ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? CI passed with new existing test. Signed-off-by: MengqingCao <[email protected]>
3 participants
What this PR does / why we need it?
Use
pytest.inito manage vllm native tests.This will convert the original test script whitelist to a blacklist to prevent missing the newly added test scripts of the upstream vLLM.
note: we do not manage the test scripts of vLLM-Ascend in
pytest.ini, because if we do so, there will be conflicts between vLLM and vLLM-Ascend'sconftest.py.Does this PR introduce any user-facing change?
N/A
How was this patch tested?
CI passed with new existing test.