Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

*: migrate test-infra to testify #344

Merged
merged 14 commits into from
Sep 1, 2021
Merged

*: migrate test-infra to testify #344

merged 14 commits into from
Sep 1, 2021

Conversation

tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Aug 31, 2021 •
edited
Loading

What problem does this PR solve?

pingcap/check is no longer under maintenance, migrate to testify.

This PR can be somewhat regarded as a subtask of pingcap/tidb#26022

This closes #346 .

Release note

  • No release note

@tisonkun
*: migrate test-infra to testify
6bdb5a0 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
fix leaks
6ba3801 
Signed-off-by: tison <wander4096@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 31, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • lichunzhu

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.

@tisonkun
go mod tidy
dadc275 
Signed-off-by: tison <wander4096@gmail.com>
kennytm
kennytm reviewed Aug 31, 2021
View reviewed changes
v4/export/dump_test.go Outdated Show resolved Hide resolved
v4/export/ir_impl_test.go Show resolved Hide resolved
v4/export/consistency_test.go
defer db.Close()
require.NoError(t, err)
defer func() {
_ = db.Close()
Copy link
Collaborator

Choose a reason for hiding this comment

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

why some are converted to require.NoError(t, x.Close()) while these are left as _ = x.Close()

Copy link
Contributor Author

@tisonkun tisonkun Aug 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Because several cases fail with require.NoError(t, x.Close()). The original code doesn't check the err but it is a net win we check it if tests are well written. For those failed, I don't have time to investigate deeply.

v4/export/sql_test.go Outdated Show resolved Hide resolved
v4/export/writer_serial_test.go Show resolved Hide resolved
tisonkun and others added 10 commits August 31, 2021 13:24
@tisonkun @kennytm
Update v4/export/sql_test.go
a3885fd 
Co-authored-by: kennytm <kennytm@gmail.com>
@tisonkun
adopt require.ErrorIs
9108044 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
gofmt
b56f719 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun @kennytm
Update v4/export/ir_impl_test.go
c74c6c5 
Co-authored-by: kennytm <kennytm@gmail.com>
@tisonkun
use goimports
0ca6055 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
fix build
9924127 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
gofumpt false positive
4c8eebd 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
interfacer false positive
8bcc66b 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
fix style warning
fdb6b2e 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
fix data race
29f9b8e 
Signed-off-by: tison <wander4096@gmail.com>
lichunzhu
lichunzhu approved these changes Aug 31, 2021
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 One reviewer approved (LGTM1) label Aug 31, 2021
@tisonkun
Copy link
Contributor Author

/cc @kennytm

would you like to give another look?

@ti-chi-bot ti-chi-bot requested a review from kennytm August 31, 2021 16:07
lichunzhu
lichunzhu reviewed Aug 31, 2021
View reviewed changes
v4/export/sql_test.go Outdated Show resolved Hide resolved
v4/export/sql_test.go Outdated Show resolved Hide resolved
v4/export/sql_test.go Outdated Show resolved Hide resolved
lichunzhu
lichunzhu reviewed Aug 31, 2021
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

Rest LGTM

v4/export/block_allow_list_test.go Outdated Show resolved Hide resolved
@tisonkun
address style comments
8943240 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Contributor Author

@lichunzhu updated. PTAL

@lichunzhu
Copy link
Contributor

lichunzhu commented Aug 31, 2021 •
edited
Loading

The group imports in other files are also not in the same style. But it seems not a big problem? Or do we have lint tools to do this job easily?

@tisonkun
Copy link
Contributor Author

@lichunzhu I think they are in the same style "goimports" while goimports doesn't have a strict rule to group imports as much as possible.

@tisonkun
Copy link
Contributor Author

BTW, pingcap/tidb also use goimports

@ti-chi-bot ti-chi-bot merged commit 85c4dee into pingcap:master Sep 1, 2021
ti-chi-bot pushed a commit to ti-chi-bot/dumpling that referenced this pull request Sep 1, 2021
@tisonkun @ti-chi-bot
This is an automated cherry-pick of pingcap#344
d3fbf0e 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #347.

@ti-chi-bot ti-chi-bot mentioned this pull request Sep 1, 2021
Merged
ti-chi-bot pushed a commit to ti-chi-bot/dumpling that referenced this pull request Sep 1, 2021
@tisonkun @ti-chi-bot
This is an automated cherry-pick of pingcap#344
d9714c3 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Sep 1, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #348.

ti-chi-bot pushed a commit to ti-chi-bot/dumpling that referenced this pull request Sep 1, 2021
@tisonkun @ti-chi-bot
This is an automated cherry-pick of pingcap#344
1ed4664 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Sep 1, 2021
Merged
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #349.

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: cannot checkout 5.2: error checking out 5.2: exit status 1. output: error: pathspec '5.2' did not match any file(s) known to git

ti-chi-bot added a commit that referenced this pull request Sep 7, 2021
@ti-chi-bot
*: migrate test-infra to testify (#344) (#349)
4b40fbc
ti-chi-bot added a commit that referenced this pull request Sep 7, 2021
@ti-chi-bot
*: migrate test-infra to testify (#344) (#347)
8109418
ti-chi-bot added a commit that referenced this pull request Sep 7, 2021
@ti-chi-bot
*: migrate test-infra to testify (#344) (#348)
4b6a20a
@lichunzhu
Copy link
Contributor

/cherry-pick release-5.2

ti-chi-bot pushed a commit to ti-chi-bot/dumpling that referenced this pull request Oct 12, 2021
@tisonkun @ti-chi-bot
This is an automated cherry-pick of pingcap#344
d18e7a1 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Oct 12, 2021
Merged
@ti-chi-bot
Copy link
Member

@lichunzhu: new pull request created: #364.

In response to this:

/cherry-pick release-5.2

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.

ti-chi-bot added a commit that referenced this pull request Oct 13, 2021
@ti-chi-bot
*: migrate test-infra to testify (#344) (#364)
8f6607b
tisonkun added a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@tisonkun
*: migrate test-infra to testify (pingcap#344)
8ba2d56
tisonkun added a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@tisonkun
*: migrate test-infra to testify (pingcap#344)
d260b3a
tisonkun added a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@tisonkun
*: migrate test-infra to testify (pingcap#344)
a15dda8
tisonkun added a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
@tisonkun
*: migrate test-infra to testify (pingcap#344)
649be09
tisonkun added a commit to tisonkun/tidb that referenced this pull request Oct 20, 2021
@tisonkun
*: migrate test-infra to testify (pingcap/dumpling#344)
18185bf
@tisonkun tisonkun mentioned this pull request Mar 25, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lichunzhu lichunzhu lichunzhu approved these changes

@kennytm kennytm kennytm approved these changes

Assignees
No one assigned
Labels
cherry-pick-approved needs-cherry-pick-release-4.0 needs-cherry-pick-release-5.0 needs-cherry-pick-release-5.1 needs-cherry-pick-release-5.2 Type: Need cherry pick to release-5.2 size/XXL status/can-merge status/LGT1 One reviewer approved (LGTM1)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate test-infra to testify
5 participants
@tisonkun @ti-chi-bot @lichunzhu @kennytm @nongfushanquan