Skip to content

Test cgf dir#24614

Closed
zhewenl wants to merge 10 commits intovllm-project:mainfrom
zhewenl:test-cgf-dir
Closed

Test cgf dir#24614
zhewenl wants to merge 10 commits intovllm-project:mainfrom
zhewenl:test-cgf-dir

Conversation

@zhewenl
Copy link
Copy Markdown
Collaborator

@zhewenl zhewenl commented Sep 10, 2025

Purpose

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: zhewenli <zhewenli@meta.com>
Signed-off-by: zhewenli <zhewenli@meta.com>
Signed-off-by: zhewenli <zhewenli@meta.com>
Signed-off-by: zhewenli <zhewenli@meta.com>
Signed-off-by: zhewenli <zhewenli@meta.com>
izaitsevfb pushed a commit to pytorch/test-infra that referenced this pull request Sep 11, 2025
### Summary
This PR adds an option to specify BC linter config path, which will be
used in vllm-project/vllm#21234

### Test plan
Locally test against PR: vllm-project/vllm#24614

Before:
```
../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4

::group::fetch github.event.pull_request.base.sha
From ssh://github.com/zhewenl/vllm
 * branch                b5e383cd8b62975dec605bed05e22d273c296c7a -> FETCH_HEAD
::endgroup::
fatal: path 'vllm/_bc_linter.py' exists on disk, but not in 'b5e383cd8b62975dec605bed05e22d273c296c7a'
::warning file=vllm/v1/core/sched/output.py,line=99::Function CachedRequestData: num_computed_tokens changed from list[int] to list[float]
::warning file=vllm/v1/core/sched/scheduler.py,line=1::Function Scheduler.get_grammar_bitmask: function deleted
::warning file=vllm/v1/core/sched/scheduler.py,line=1::Function Scheduler.update_draft_token_ids: function deleted
```

After:
```
../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4 --config-dir=.github

::group::fetch github.event.pull_request.base.sha
From ssh://github.com/zhewenl/vllm
 * branch                b5e383cd8b62975dec605bed05e22d273c296c7a -> FETCH_HEAD
::endgroup::
BC-linter: Using .bc-linter.yml (parsed successfully)
fatal: path 'vllm/_bc_linter.py' exists on disk, but not in 'b5e383cd8b62975dec605bed05e22d273c296c7a'
::warning file=vllm/v1/core/sched/output.py,line=99::Function CachedRequestData: num_computed_tokens changed from list[int] to list[float]
> ../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4
```

Note the difference in `scheduler.py`, before the we specifying the
local config the bc linter yml was not respected - so the default rules
are applied and we check ALL python files; after specifying the yml the
warnings are restricted to ONLY the functions with annotations
@mergify
Copy link
Copy Markdown

mergify bot commented Sep 12, 2025

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @zhewenl.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant