Skip to content

chore(test): add uts for coscheduling plugin.#2582

Merged
google-oss-prow[bot] merged 3 commits into
kubeflow:masterfrom
IRONICBo:feat/add-coscheduling-uts
Sep 16, 2025
Merged

chore(test): add uts for coscheduling plugin.#2582
google-oss-prow[bot] merged 3 commits into
kubeflow:masterfrom
IRONICBo:feat/add-coscheduling-uts

Conversation

@IRONICBo
Copy link
Copy Markdown
Member

@IRONICBo IRONICBo commented Apr 4, 2025 •
edited by tenzen-y
Loading

What this PR does / why we need it:

Other plugins in pkg/runtime/framework/plugins have corresponding UTs, but the scheduling plugin does not have a UT for itself. I want to add some UTs for it.

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Part-of: #2582

Checklist:

  • Docs included if any changes are user facing

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Apr 4, 2025

/rerun-all

@IRONICBo IRONICBo changed the title feat: add uts for coscheduling plugin. test(coscheduling): add uts for coscheduling plugin. Apr 4, 2025
@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Apr 4, 2025 •
edited
Loading

The PodGroupRuntimeClassHandler and PodGroupLimitRangeHandler UTs are a bit complex. I will update these later. I would be pleased if you had any suggestions.

cc @Electronic-Waste

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 4, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 17738714361

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.0%) to 55.137%
Totals Coverage Status
Change from base Build 17657982734: 3.0%
Covered Lines: 1084
Relevant Lines: 1966
💛 - Coveralls

@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from 9b313ae to a0065e4 Compare April 4, 2025 04:41
@Electronic-Waste
Copy link
Copy Markdown
Member

Electronic-Waste commented Apr 9, 2025

The PodGroupRuntimeClassHandler and PodGroupLimitRangeHandler UTs are a bit complex. I will update these later. I would be pleased if you had any suggestions.

/cc @kubeflow/wg-training-leads @astefanutti

@google-oss-prow google-oss-prow Bot requested review from a team and astefanutti April 9, 2025 08:31
@tenzen-y tenzen-y mentioned this pull request Apr 15, 2025
Closed
6 tasks
tenzen-y
tenzen-y reviewed Apr 15, 2025
View reviewed changes
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
@tenzen-y
Copy link
Copy Markdown
Member

tenzen-y commented Apr 25, 2025

@IRONICBo Do you have any updates?

@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from a0065e4 to ff1239a Compare April 26, 2025 04:55
@google-oss-prow google-oss-prow Bot added size/XL and removed size/L labels Apr 26, 2025
@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Apr 26, 2025

@IRONICBo Do you have any updates?

Sorry I've been preparing for final exams this week and have now updated, please take a look about it. @tenzen-y @Electronic-Waste

@IRONICBo IRONICBo requested a review from tenzen-y April 26, 2025 12:24
@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from ff1239a to 326ff75 Compare April 27, 2025 03:29
@google-oss-prow google-oss-prow Bot added size/L and removed size/XL labels Apr 27, 2025
@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Apr 28, 2025

/rerun-all

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented May 3, 2025

@Electronic-Waste @tenzen-y Hi, do you have time to review my PR?

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented May 3, 2025 •
edited
Loading

Besides, the /rerun-all command doesn't seem to work.

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented May 4, 2025

/retest

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 2, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Aug 2, 2025

/retest

@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from 50838fe to 763f341 Compare August 23, 2025 07:38
@IRONICBo IRONICBo changed the title chore(coscheduling): add uts for coscheduling plugin. chore(test): add uts for coscheduling plugin. Aug 23, 2025
@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from 763f341 to e390c5d Compare August 23, 2025 07:58
andreyvelich
andreyvelich reviewed Aug 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thank you for this great contribution @IRONICBo!
/lgtm
/assign @tenzen-y @astefanutti

@google-oss-prow google-oss-prow Bot added the lgtm label Aug 24, 2025
astefanutti
astefanutti reviewed Aug 25, 2025
View reviewed changes
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
Comment thread pkg/runtime/framework/plugins/coscheduling/coscheduling_test.go Outdated
@Doris-xm Doris-xm mentioned this pull request Sep 15, 2025
Merged
1 task
andreyvelich
andreyvelich reviewed Sep 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

@IRONICBo Did you get a chance to address the comments, so we can move forward with this PR ?

@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Sep 15, 2025

@IRONICBo Did you get a chance to address the comments, so we can move forward with this PR ?

I will update it today, sorry for this late reply. @andreyvelich

@google-oss-prow google-oss-prow Bot removed the lgtm label Sep 15, 2025
@IRONICBo
feat: add uts for coscheduling plugin.
92111cf 
Signed-off-by: Asklv <boironic@gmail.com>
@IRONICBo
fix: add suspend, multiple podset, min resource case for coscheduling…
8382566 
… test.

Signed-off-by: Asklv <boironic@gmail.com>
@IRONICBo
fix: update ut comments for coscheduling.
b9e765d 
Signed-off-by: Asklv <boironic@gmail.com>
@IRONICBo IRONICBo force-pushed the feat/add-coscheduling-uts branch from 994633d to b9e765d Compare September 15, 2025 15:42
@IRONICBo
Copy link
Copy Markdown
Member Author

IRONICBo commented Sep 16, 2025

@andreyvelich This PR is now ready. Please take a look when you have a moment. Thank you!

andreyvelich
andreyvelich reviewed Sep 16, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thanks @IRONICBo!
/lgtm
/assign @astefanutti @tenzen-y

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 16, 2025
@astefanutti
Copy link
Copy Markdown
Contributor

astefanutti commented Sep 16, 2025

Thanks @IRONICBo!

/approve

@google-oss-prow
Copy link
Copy Markdown

google-oss-prow Bot commented Sep 16, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astefanutti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow Bot merged commit aca6366 into kubeflow:master Sep 16, 2025
29 of 30 checks passed
@google-oss-prow google-oss-prow Bot added this to the v2.1 milestone Sep 16, 2025
alexxfan pushed a commit to red-hat-data-services/trainer that referenced this pull request Nov 24, 2025
@IRONICBo @alexxfan
chore(test): add uts for coscheduling plugin. (kubeflow#2582)
f51980c 
* feat: add uts for coscheduling plugin.

Signed-off-by: Asklv <boironic@gmail.com>

* fix: add suspend, multiple podset, min resource case for coscheduling test.

Signed-off-by: Asklv <boironic@gmail.com>

* fix: update ut comments for coscheduling.

Signed-off-by: Asklv <boironic@gmail.com>

---------

Signed-off-by: Asklv <boironic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreyvelich andreyvelich andreyvelich left review comments

@Electronic-Waste Electronic-Waste Awaiting requested review from Electronic-Waste

@jinchihe jinchihe Awaiting requested review from jinchihe

@tenzen-y tenzen-y Awaiting requested review from tenzen-y

+1 more reviewer

@astefanutti astefanutti astefanutti left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@astefanutti astefanutti

@andreyvelich andreyvelich

@tenzen-y tenzen-y

Labels

approved lgtm ok-to-test size/XL

Projects

None yet

Milestone

v2.1

Development

Successfully merging this pull request may close these issues.

6 participants

@IRONICBo @coveralls @Electronic-Waste @tenzen-y @astefanutti @andreyvelich