Tracking issue for replace github.com/pingcap/check with github.com/stretchr/testify

[ben1009]

Background

as described in the thread https://internals.tidb.io/t/topic/141/6

I did ever use testify in pingcap/errors [[1]](https://github.com/pingcap/errors/pull/36) and it works well with richer matchers and default parallelism support.

I think the modification from gocheck to pingcap/check mainly focuses on parallelism. It is possibly we switch to testify to live with a stronger community instead of a lack of maintenance version of a smaller community.

* [testify ](https://github.com/stretchr/testify) has 188 contributors and latest updated at Apr. 27
* [go-check](https://github.com/go-check/check) has 13 contributors and latest updated at last year.

If you think this direction is good to go, maybe we can draft a pre-RFC to collect wider input.

This is somewhat a subtask of pingcap/tidb#26022 that we make a decision that deprecating pingcap/check and adopting testify. Take a look at the tidb issue for more information.

After an offline discussion with @overvenus we think it is fine we make the same change on this repo and here is the tracking issue for the effort.

If you have any further concern, please comment below and reply will be in days.

We first write down a task list and since I have no time to continue the actual work, let's kick off when there is a contributor ask for starting the subtasks.

Join Forces

If you want to participant the movement, you can comment under this issue to acquire pkg or files to migrate. For example,

Hi, I'd like to migrate tests under `errno` pkg. Please create a subtask and assign to me.

React will be in time and do the favor for you.

Also you can directly comment under any open issues below without assignee to require assignment.

You can take a look at pingcap/tidb#26375 as an example to do the migration.

Note

• new unit-test

  • New uts should use testify and goleak directly. Ref to the Pr 2833
  • Pkg github.com/pingcap/tidb/util/testkit should be replaced by github.com/pingcap/tidb/testkit

• parallel

  • Keep test cases as parallel as possible.
  • Don't use t.Parallel() if pkg has failpoint control. Need review Carefully.
  • When some test cases should be run in serial, take use of go testing subtests (t.Run(name string, f func(t *T))) and parallel the parent test only. In this way, test cases in the same subtests set run in serial.
  • Avoid using suite of testify as discussed errno: migrate test-infra to testify #26082 (comment)

• migrate kits

  • Migrate Script or Migrate Tool can do some replace work quickly, try and make pr.

Progress

Considering test-infra migration is a long-term process，we need to divide the whole process into several phases. After that,
each pkgs will be classified into specific phase.

• Phase0: small pkgs, no kits dependent, migrate script/migrate tool can do most work.
• Phase1: small pkgs dependent on kits, need to deal after we migrate kits
• Phase2: medium pkgs, need to modify suite logic manually
• Phase3: large pkgs, need to modify suite logic manually

Subtasks

• Phase0:

• migrate test-infra to testify for pkg/version pkg #2919
• migrate test-infra to testify for pkg/workerpool pkg #2890
• migrate test-infra to testify for pkg/quotes pkg #2928
• migrate test-infra to testify for pkg/regionspan pkg #2927
• migrate test-infra to testify for pkg/retry pkg #2925
• migrate test-infra to testify for pkg/scheduler pkg #2923
• migrate test-infra to testify for pkg/types pkg #2921
• migrate test-infra to testify for pkg/context pkg #2859
• migrate test-infra to testify for pkg/cyclic pkg #2869
• migrate test-infra to testify for pkg/cyclic/mark pkg #2870
• migrate test-infra to testify for pkg/errors pkg #2871
• migrate test-infra to testify for pkg/filter pkg #2936
• migrate test-infra to testify for pkg/flags pkg #2935
• migrate test-infra to testify for pkg/hash pkg #2934
• migrate test-infra to testify for pkg/httputil pkg #2933
• migrate test-infra to testify for pkg/notify pkg #2930
• migrate test-infra to testify for pkg/config pkg #2825
• migrate test-infra to testify for pkg/pipeline pkg #2954

• Kits

• Extract common skip stack function for goleak #2874
• migrate check.Mkdir() to pingcap/ticdc/pkg/util #2879

• Phase1:

• migrate test-infra to testify for pkg/logutil pkg #2932
• migrate test-infra to testify for pkg/etcd pkg #2878
• migrate test-infra to testify for pkg/orchestrator pkg #4200
• migrate test-infra to testify for pkg/orchestrator/util pkg #4878

• Phase2:

• migrate test-infra to testify for pkg/cmd pkg #2912
• migrate test-infra to testify for pkg/util pkg #4879
• migrate test-infra to testify for cdc/entry pkg #2826
• migrate test-infra to testify for cdc/processor pkg #2902
• migrate test-infra to testify for cdc/processor/pipeline pkg #2903

• Phase3:

• migrate test-infra to testify for cdc/kv pkg #2899
• migrate test-infra to testify for cdc/model pkg #2900
• migrate test-infra to testify for cdc/owner pkg #2901
• migrate test-infra to testify for cdc/puller pkg #2904
• migrate test-infra to testify for cdc/puller/fronter pkg #2905
• migrate test-infra to testify for cdc/sorter pkg #2906
• migrate test-infra to testify for cdc/sink pkg #2907
• migrate test-infra to testify for cdc/sink/codec pkg #2908
• migrate test-infra to testify for cdc/sink/mq pkg #2909
• migrate test-infra to testify for cdc/sink/dispatcher pkg #2910
• migrate test-infra to testify for cdc/sink/producer/kafka pkg #2911
• migrate test-infra to testify for pkg/txnutil/gc package #6107

[Rustin170506]

I have a question:

Why we need a test framework? Can we just use the golang's built-in test framework?

[ben1009]

I have a question:

Why we need a test framework? Can we just use the golang's built-in test framework?

more functionality support i guess, ref https://github.com/stretchr/testify, also could discuss the option in https://internals.tidb.io/t/topic/141/6 with community, i think

[Rustin170506]

Off-line discussion:

I support the use of testify, but the migration costs may be higher. Because there are a lot of tests.

From @overvenus , we can use testify, because pingcap/check requires writing redundant templates. But we need to make sure that leak test is supported.

[tisonkun]

But we need to make sure that leak test is supported.

Unnecessary. As done in pingcap/tidb and tikv/client-go, we can migrate testleak to goleak also, which requires less setups and is able to control (ignore) options per test (package). testleak is a toolkit build on pingcap/check and we don't have to maintain it if find a better replacement.

[tisonkun]

Also I don't think this is a feature request.

[tisonkun]

@ben1009 the next time you can comment this effort on the post or the issue of tidb so that we're aware of its happening.

[tisonkun]

I support the use of testify, but the migration costs may be higher. Because there are a lot of tests.

Agree. I think there are several potential contributors to work on this topic. Let's postpone this issue util there is actually someone ask for under this thread. We should avoid throwing ourselves into a situation that maintain two kinds of test cases for a long time.

[sunxiaoguang]

Looks like I can't convert task into issue. I assume contributor can create task and reply to this issue to create link, am I right?

[tisonkun]

Looks like I can't convert task into issue. I assume contributor can create task and reply to this issue to create link, am I right?

@sunxiaoguang yes. See also pingcap/tidb#27618 (comment) .

[maxshuang]

@tisonkun hello, tisonkun, i am new to the ticdc group. We made offline discussion recently , and wanted to make the migration to testify and goleak step by step. What should we pay attention to the migration? Did you met something bad in the progress?

[tisonkun]

@maxshuang I don't remember with an offline discussion though. What I can say it just go ahead to do it. Maybe start from some utility package like pkg/version.

If you'd like to do it, I'll create a subtask and you can assign yourself by /assign.

[tisonkun]

@maxshuang you can take a look at pingcap/tidb#26082 for best practices collected and previous discussions.

[Rustin170506]

Thank you all for your excellent work!