volcano icon indicating copy to clipboard operation
volcano copied to clipboard

Published 20 hours ago • verified publisher icon volcano-sh

[good first issue]enrich the unit-test for some package

Open lowang-bh opened this issue 1 year ago • 18 comments

What would you like to be added:

There are many packages which don't have unit-test, such as:

  • actions:

    • [ ] enqueue
    • [ ] backfill

  • plugins:

    • [ ] gang
    • [ ] extender
    • [ ] overcommit
    • [ ] resourcequota
    • [ ] usage

  • others, include some PRs:

UTs should include positive cases and reverse cases.

Why is this needed:

  1. to avoid future changes impacting those function, and make code more strong

Benefits

  1. you can familiar with the coding logic and get started with volcano

how to assign

assign only one package to yourself in each pr.

lowang-bh avatar Aug 14 '23 05:08 lowang-bh

/good first issue /help wanted

lowang-bh avatar Aug 14 '23 05:08 lowang-bh

/assign

Subhasish-Behera avatar Aug 14 '23 10:08 Subhasish-Behera

@lowang-bh I would like to work on unit-tests for resourcequota

Subhasish-Behera avatar Aug 14 '23 10:08 Subhasish-Behera

/assign

rayoluo avatar Aug 15 '23 02:08 rayoluo

@lowang-bh I would like to work on the rest unit-tests

rayoluo avatar Aug 15 '23 02:08 rayoluo

/assign

hzq5477 avatar Oct 08 '23 16:10 hzq5477

@Subhasish-Behera @rayoluo Any updates on this issue?

lowang-bh avatar Nov 05 '23 13:11 lowang-bh

@lowang-bh Just filed a PR for the resourcequota plugin. any feedback about the PR is welcome.

(cc @william-wang @Monokaix )

Subhasish-Behera avatar Feb 17 '24 21:02 Subhasish-Behera

Now we can use the base UT lib in https://github.com/volcano-sh/volcano/pull/3343 to do the test. Examples shows at PR #3340 #3341 #3342

lowang-bh avatar Mar 09 '24 10:03 lowang-bh

/good-first-issue

lowang-bh avatar Apr 08 '24 13:04 lowang-bh

@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 avatar Apr 08 '24 13:04 volcano-sh-bot

/assign

googs1025 avatar Apr 08 '24 13:04 googs1025

@lowang-bh Hello, may I ask if this will allocate a specific part?

googs1025 avatar Apr 08 '24 14:04 googs1025

@lowang-bh Hello, may I ask if this will allocate a specific part?

Welcome! You can work on those packages which have no UTs covered.

lowang-bh avatar Apr 12 '24 13:04 lowang-bh

@googs1025 Hello, I'm currently looking into Issue #3075 and noticed that there's some overlap with current Issue regarding enqueue and backfill.

I was wondering if you're planning to address these actions in a future PR?

MondayCha avatar Apr 17 '24 11:04 MondayCha

@MondayCha Hi, hello, I am currently working on other packages. You can work on the action package.

googs1025 avatar Apr 17 '24 11:04 googs1025

@MondayCha hi, Are you still working on this?

googs1025 avatar Apr 24 '24 04:04 googs1025

@MondayCha hi, Are you still working on this?

Sorry I haven't started yet. I'm still trying to understand the relevant code and might attempt it next week.

If you need to do enqueue and backfill, I will avoid it.

MondayCha avatar Apr 24 '24 05:04 MondayCha

/close

lowang-bh avatar Jun 04 '24 14:06 lowang-bh