Skip to content
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

[good first issue]add relative unit testcase for merged PRs #3075

Open
4 tasks
lowang-bh opened this issue Aug 27, 2023 · 7 comments
Open
4 tasks

[good first issue]add relative unit testcase for merged PRs #3075

lowang-bh opened this issue Aug 27, 2023 · 7 comments
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@lowang-bh
Copy link
Member

lowang-bh commented Aug 27, 2023

What would you like to be added:

Add UT to cover the pod's condition message in allocate/preempt/reclaim/backfill action, according to relative issues and PRs as following.

Why is this needed:

To prevent future changes cause new issue and breaking current fix.
The origin idea is from issue #3049

@lowang-bh lowang-bh added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 27, 2023
@lowang-bh lowang-bh changed the title [good first issue]add testcase for pod's condition when it is unschedulable [good first issue]add relative unit testcase for merged PRs Sep 9, 2023
@lowang-bh
Copy link
Member Author

/good-first-issue

@volcano-sh-bot
Copy link
Contributor

@lowang-bh:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@volcano-sh-bot volcano-sh-bot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Apr 12, 2024
@MondayCha
Copy link

Hello, I am hoping to start with this good first issue on Volcano. Could it be assigned to me?

@lowang-bh
Copy link
Member Author

/assign @MondayCha

@JesseStutler
Copy link
Member

@MondayCha Hi, do you still work on this? Could you assign to me? I'm ready to add some UTs to start at volcano.

@lowang-bh
Copy link
Member Author

/assign @JesseStutler

@JesseStutler
Copy link
Member

I think the current testing framework's scalability is a bit poor...Like I want to add UT in reclaim action, I want to build a pod with preemptionPolicy with pod, I need to add a function based on BuildPod, but now the parameter list of BuildPod is a bit too long:

func BuildPod(namespace, name, nodeName string, p v1.PodPhase, req v1.ResourceList, groupName string, labels map[string]string, selector map[string]string) *v1.Pod {

I don’t know what the parameters in which position are used for. On the contrary, the way of writing in the k8s repository is clearer:
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/job/job_controller_test.go#L7791-L7827

It is better to expand the fields in this way. Now it is difficult for BuildPod to expand the fields, parameter list will be longer and longer...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants