- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 10.9k
[Misc] Replace os environ to monkeypatch in test suite #14516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Misc] Replace os environ to monkeypatch in test suite #14516
Conversation
| 👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run  Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add  🚀 | 
| Please fix the pre-commit errors | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you cant pass monkeypatch to a module level fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
| I would suggest installing the pre-commit which will automatically sign off for you. Run  | 
| 
 okie will test locally first | 
a6ade47    to
    1e64dc0      
    Compare
  
    Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
… variable removal Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Head branch was pushed to by a user without write access
e9c8188    to
    b791b51      
    Compare
  
    …ld push the changes and pass the pre-commit check Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com>
4932bcd    to
    7139925      
    Compare
  
    | 
 Thank you so much I will apply the patch | 
| hmm, can you add me as collaborator to your branch? I can help with expedite this PR and fix CI error :) | 
| 
 Thank you I really appreciate your help. I have sent you an invite. | 
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
| @t-sibiraj can you add the following to the PR description? Thanks | 
Signed-off-by: Aaron Pham <contact@aarnphm.xyz>
| Done, and thanks for fixing this! | 
| The CI failures are unrelated so let's merge this | 
…14516) Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com> Signed-off-by: Aaron Pham <contact@aarnphm.xyz> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Co-authored-by: Aaron Pham <contact@aarnphm.xyz> Signed-off-by: DefTruth <31974251+DefTruth@users.noreply.github.com>
| This PR broke the PyTorch Fullgraph tests: The fixture cannot be used in this simple way to generate an array of parameters | 
…14516) Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com> Signed-off-by: Aaron Pham <contact@aarnphm.xyz> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Co-authored-by: Aaron Pham <contact@aarnphm.xyz> Signed-off-by: Louis Ulmer <ulmerlouis@gmail.com>
Pass pooling_metadata to pooler head in gritlm. This was broken by PR vllm-project#16331 broke gritlm. PR vllm-project#14516 broke gritlm tests due to changing xformers to flash_atnn Signed-off-by: Pooya Davoodi <pooya.davoodi@parasail.io>
…14516) Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com> Signed-off-by: Aaron Pham <contact@aarnphm.xyz> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Co-authored-by: Aaron Pham <contact@aarnphm.xyz>
…14516) Signed-off-by: sibi <85477603+t-sibiraj@users.noreply.github.com> Signed-off-by: Aaron Pham <contact@aarnphm.xyz> Co-authored-by: Cyrus Leung <cyrus.tl.leung@gmail.com> Co-authored-by: Aaron Pham <contact@aarnphm.xyz> Signed-off-by: Mu Huai <tianbowen.tbw@antgroup.com>
Convert all os.environ(xxx) to monkeypatch.setenv in test suite
Submitting pull request again because I forget to sign my original pull request
FIXES #14499
Co-authored-by: Aaron Pham contact@aarnphm.xyz
Signed-off-by: Aaron Pham contact@aarnphm.xyz