Skip to content

Conversation

nurali-techie
Copy link
Contributor

@nurali-techie nurali-techie commented Oct 14, 2025

Pull Request Description

To improve coverage for podautoscaler, added unit tests for WorkloadScale.GetCurrentReplicasFromScale().

Related Issues

Resolves: part of [#1649] (GetCurrentReplicasFromScale() and GetPodSelectorFromScale() unit tests)

Important: Before submitting, please complete the description above and review the checklist below.


Contribution Guidelines (Expand for Details)

We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:

Pull Request Title Format

Your PR title should start with one of these prefixes to indicate the nature of the change:

  • [Bug]: Corrections to existing functionality
  • [CI]: Changes to build process or CI pipeline
  • [Docs]: Updates or additions to documentation
  • [API]: Modifications to aibrix's API or interface
  • [CLI]: Changes or additions to the Command Line Interface
  • [Misc]: For changes not covered above (use sparingly)

Note: For changes spanning multiple categories, use multiple prefixes in order of importance.

Submission Checklist

  • PR title includes appropriate prefix(es)
  • Changes are clearly explained in the PR description
  • New and existing tests pass successfully
  • Code adheres to project style and best practices
  • Documentation updated to reflect changes (if applicable)
  • Thorough testing completed, no regressions introduced

By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.

@nurali-techie nurali-techie changed the title test: added test for GetCurrentReplicasFromScale [Misc] Added unit test for podautoscaler workload scale Oct 14, 2025
@nurali-techie
Copy link
Contributor Author

nurali-techie commented Oct 14, 2025

@googs1025 @Jeffwan I have added two unit tests related to workload scale. Can you plz check and confirm that is this the kind of unit tests make sense for workload scale? I am planning to add more such unit tests in this PR. Thank you.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 14, 2025

leave this to @googs1025 for review.

@Jeffwan Jeffwan requested a review from googs1025 October 14, 2025 21:34
@nurali-techie nurali-techie force-pushed the nur/patch-workload-scale-test branch from a3db5d6 to afa81cf Compare October 15, 2025 17:58
@nurali-techie nurali-techie marked this pull request as ready for review October 15, 2025 18:01
@nurali-techie
Copy link
Contributor Author

@googs1025 this PR is ready for review. I have added all postive unit tests for WorkloadScale.GetCurrentReplicasFromScale() using table test. Request you to review and provide your feedback. If looks fine then this PR is ready to merge.

@nurali-techie nurali-techie changed the title [Misc] Added unit test for podautoscaler workload scale [Misc] Added unit test for WorkloadScale.GetCurrentReplicasFromScale() Oct 15, 2025
@nurali-techie nurali-techie changed the title [Misc] Added unit test for WorkloadScale.GetCurrentReplicasFromScale() [Misc] Added unit test for WorkloadScale.GetCurrentReplicasFromScale Oct 16, 2025
@nurali-techie nurali-techie force-pushed the nur/patch-workload-scale-test branch from afa81cf to 7058a35 Compare October 16, 2025 19:51
@nurali-techie nurali-techie changed the title [Misc] Added unit test for WorkloadScale.GetCurrentReplicasFromScale [Misc] Added unit test for WorkloadScale Oct 16, 2025
Copy link
Collaborator

@googs1025 googs1025 left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for the help

@googs1025 googs1025 force-pushed the nur/patch-workload-scale-test branch from 7058a35 to 9c2ac9e Compare October 17, 2025 01:38
@Jeffwan Jeffwan merged commit d28c621 into vllm-project:main Oct 17, 2025
14 checks passed
@nurali-techie nurali-techie deleted the nur/patch-workload-scale-test branch October 17, 2025 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants