Skip to content

mark vLLM policy integration tests as async #416

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 15, 2025
Merged

mark vLLM policy integration tests as async #416

merged 2 commits into from
Oct 15, 2025

Conversation

ebsmothers
Copy link
Contributor

@ebsmothers ebsmothers commented Oct 14, 2025 •
edited
Loading

Addresses part of #411

Before:

pytest tests/integration_tests/test_vllm_policy_correctness.py
...
FAILED tests/integration_tests/test_vllm_policy_correctness.py::test_same_output - Failed: async def functions are not natively supported.
FAILED tests/integration_tests/test_vllm_policy_correctness.py::test_cache_usage - Failed: async def functions are not natively supported.
========== 2 failed in 4.71s ============

After:

pytest tests/integration_tests/test_vllm_policy_correctness.py
...
========== 2 passed in 84.36s (0:01:24) ================

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 14, 2025
@ebsmothers ebsmothers mentioned this pull request Oct 14, 2025
Open
JenniferWang
JenniferWang reviewed Oct 15, 2025
View reviewed changes
tests/integration_tests/test_vllm_policy_correctness.py Outdated
assert vllm_output != ""
assert policy_output != ""
if vllm_output != policy_output:
print(f"❌ Got different results: {vllm_output} vs. {policy_output}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have a WIP PR but I'll abandon that.
Could you change this to assert so that the test actually fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah duh thanks, will update

JenniferWang
JenniferWang reviewed Oct 15, 2025
View reviewed changes
tests/integration_tests/test_vllm_policy_correctness.py Outdated
assert vllm_output != ""
assert policy_output != ""
if vllm_output != policy_output:
print(f"❌ Got different results: {vllm_output} vs. {policy_output}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@allenwang28
Copy link
Contributor

based on CI it seems this is all just working? does this fully close out #411?

@ebsmothers
Copy link
Contributor Author

based on CI it seems this is all just working? does this fully close out #411?

@allenwang28 nah rn the CI is just running on unit tests. So we need to fix the ones in #411 and then can add the integration tests to CI

@ebsmothers ebsmothers merged commit 17ea795 into meta-pytorch:main Oct 15, 2025
9 checks passed
allenwang28 pushed a commit to allenwang28/forge that referenced this pull request Oct 15, 2025
@ebsmothers @allenwang28
mark vLLM policy integration tests as async (meta-pytorch#416)
a3bf0f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JenniferWang JenniferWang JenniferWang approved these changes

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ebsmothers @allenwang28 @JenniferWang