store/gcworker: migrate test-infra to testify for gc_worker_test.go #27657

[penthaapatel]

What problem does this PR solve?

Issue Number: closes #27657

What is changed and how it works?

What's Changed:

• Migrate from pingcap/check to testify
• Eliminate goroutine leak

Release note

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• disksing
• tiancaiamao

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[ti-chi-bot]

Welcome @penthaapatel!

It looks like this is your first PR to pingcap/tidb 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/tidb. 😃

[penthaapatel]

/cc @tisonkun

[tisonkun]

@penthaapatel please fix CI fails like go fmt.

[penthaapatel]

@tisonkun Just fixed using go fmt.
check_dev_2 CI still seems to be failing. Need some more guidance to proceed as I am a first-time contributor.

[penthaapatel]

/cc @tisonkun
On running the package tests for store/gcworker with the original unchanged gc_worker_test.go file, the following tests for gcworker package were already failing on my system:

FAIL: gc_worker_test.go:1594: testGCWorkerSuite.TestGCLabelRules
FAIL: gc_worker_test.go:1582: testGCWorkerSuite.TestGCPlacementRules
FAIL: gc_worker_test.go:1538: testGCWorkerSuite.TestPhyscailScanLockDeadlock
FAIL: gc_worker_test.go:873: testGCWorkerSuite.TestResolveLockRangeInfine
FAIL: gc_worker_test.go:1350: testGCWorkerSuite.TestResolveLocksPhysical
OOPS: 21 passed, 5 FAILED
--- FAIL: TestT (28.04s)

Is this the reason why check_dev_2 CI is unable to pass?

The only changes I have made with this PR are:

• improving the func SetUpTest() by using s *testGCWorkerSuite and clean() as the return values
• incorporating func TearDownTest() within func SetUpTest()
• as a result of the above change, I used func SetUpTest() at the beginning of each test, to create a new testGCWorkerSuite struct, and used defer clean() to tear down the test
• replacing c.AssertXXX with testify/require package
• adding t.Parallel() for imporved concurrency
• checking for goroutine leaks in store/gcworker/main_test.go

I can assure that no changes have been made to the logic of the original gc_worker_test.go code.

How should I proceed further? First time contributor here.

[tisonkun]

@penthaapatel thanks for your quick update. I'm looking into this.

[tisonkun]

We're trying to avoid suite as mentioned #26082 (comment) .

When you really need to batch some setup/teardown logics and state, you can take a look at #27572 .

[tisonkun]

Let me try to push a follow up base on your change so that you can get the idea. Later you can reuse the experience on other tasks if you like.

[tisonkun]

@penthaapatel I push a follow up which you can take a look to get ideas on idiom. Sorry I don't have conversation for it since code expresses better than words in this case. You can argue or revert the changes if you have other better ideas.

[ti-chi-bot]

@tisonkun: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

@penthaapatel I push a follow up which you can take a look to get ideas on idiom. Sorry I don't have conversation for it since code expresses better than words in this case. You can argue or revert the changes if you have other better ideas.

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 ti-community-infra/tichi repository.

[tisonkun]

/cc @disksing

[penthaapatel]

@tisonkun
I am going through the latest changes you pushed to the code. Working with such a huge codebase for the very first time is a learning experience for me. I will try to understand the code and will try to make further contributions here.
Thank you for patiently guiding me. I will reach out to you if I have any further queries/ideas.

[tisonkun]

/run-check_dev_2

[tiancaiamao]

/merge

[ti-chi-bot]

This pull request has been accepted and is ready to merge.

Commit hash: d7cc89f

[ti-chi-bot]

@penthaapatel: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.

[zhouqiang-cl]

/merge