table/tables: migrate test-infra to testify for `index_test.go` #27557

feitian124 · 2021-08-24T15:41:39Z

What problem does this PR solve?

Issue Number: closes #27588

resolve above issue in 3 parts, this is part 0:

migrate index_test.go
migrate partition_test.go
migrate tables_test.go

Release note

None

This reverts commit aa63353.

ti-chi-bot · 2021-08-24T15:41:40Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

tiancaiamao
xhebox

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.

feitian124 · 2021-08-24T15:59:57Z

The following test cases take too long to finish:
[2021-08-24T15:47:43.269Z] PASS: tidb_test.go:878: tidbTestSerialSuite.TestTLSAuto 7.539s

/run-check_dev_2

feitian124 · 2021-08-24T16:14:32Z

not sure what is the best practise to setup and teardown the shared variables.
i try to use setup teardown style in commit aa63353 but unit test fails so i reverted it.

/cc @tisonkun

feitian124 · 2021-08-25T02:00:35Z

/cc @tisonkun

tisonkun

You can comment in #26866 and ask me for creating subtasks. I'd prefer avoid using part 0,1,2 where number is hard to understand.

Sorry for my previous bad example.

See #26863 as an example.

ti-chi-bot · 2021-08-25T10:48:42Z

@tisonkun: Request changes is only allowed for the reviewers in list.

In response to this:

You can comment in #26866 and ask me for creating subtasks. I'd prefer avoid using part 0,1,2 where number is hard to understand.

Sorry for my previous bad example.

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

Comments inline. Even if you migrate tests by script, please do a human check and avoid push the reviewer to review a mechanical patch with corner case uncovered.

table/tables/index_test.go

ti-chi-bot · 2021-08-25T11:08:10Z

@tisonkun: Request changes is only allowed for the reviewers in list.

In response to this:

Comments inline. Even if you migrate tests by script, please do a human check and avoid push the reviewer to review a mechanical patch with corner case uncovered.

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.

Signed-off-by: tison <wander4096@gmail.com>

tisonkun

Generally looks good, a question on parallelism.

table/tables/index_test.go

feitian124 · 2021-09-05T05:15:38Z

/run-check_dev_2

ti-chi-bot · 2021-09-06T02:01:19Z

@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:

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 · 2021-09-06T02:02:32Z

/cc @xhebox @tiancaiamao

xhebox · 2021-09-06T13:46:51Z

/merge

ti-chi-bot · 2021-09-06T13:46:53Z

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

Commit hash: c79f51a

ti-chi-bot · 2021-09-06T14:23:33Z

@feitian124: 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.

xiongjiwei · 2021-11-01T09:05:16Z

table/tables/index_test.go

 	}
 }

-func (s *testIndexSuite) TestMultiColumnCommonHandle(c *C) {
+func TestMultiColumnCommonHandle(t *testing.T) {
+	t.Parallel()


please see #29125 (comment)

feitian124 added 4 commits August 24, 2021 21:32

table/tables: migrate test-infra to testify

2d4e920

make test pass

23fb97f

use setup teardown

aa63353

Revert "use setup teardown"

8a07dab

This reverts commit aa63353.

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 24, 2021

ti-chi-bot requested a review from tisonkun August 25, 2021 02:00

tisonkun requested changes Aug 25, 2021

View reviewed changes

feitian124 mentioned this pull request Aug 25, 2021

migrate test-infra to testify for table/tables #26866

Closed

3 tasks

tisonkun changed the title ~~table/tables: migrate test-infra to testify (part 0)~~ table/tables: migrate test-infra to testify for index_test.go Aug 25, 2021

tisonkun requested changes Aug 25, 2021

View reviewed changes

table/tables/index_test.go Outdated Show resolved Hide resolved

table/tables/index_test.go Outdated Show resolved Hide resolved

table/tables/index_test.go Outdated Show resolved Hide resolved

table/tables/index_test.go Outdated Show resolved Hide resolved

feitian124 added 2 commits August 25, 2021 23:47

use require.NoError

5c837e2

use buildMock to get mock store and close

e1babfa

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 26, 2021

feitian124 and others added 2 commits August 26, 2021 23:05

fix failed test

f487249

reuse existing testkits

36b727a

Signed-off-by: tison <wander4096@gmail.com>

tisonkun reviewed Aug 26, 2021

View reviewed changes

table/tables/index_test.go Outdated Show resolved Hide resolved

table/tables/index_test.go Outdated Show resolved Hide resolved

feitian124 mentioned this pull request Sep 2, 2021

maybe common mistakes with table driven tests and t.Parallel() #27779

Closed

mv 2 test cases which can not parallel to separate file

c79f51a

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 5, 2021

feitian124 mentioned this pull request Sep 5, 2021

table/tables: migrate test-infra to testify for partition_test.go #27812

Merged

tisonkun approved these changes Sep 6, 2021

View reviewed changes

ti-chi-bot requested review from tiancaiamao and xhebox September 6, 2021 02:02

tiancaiamao approved these changes Sep 6, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 6, 2021

xhebox approved these changes Sep 6, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 6, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 6, 2021

ti-chi-bot added 2 commits September 6, 2021 21:46

Merge branch 'master' into issue-26866-part1

bb3a639

Merge branch 'master' into issue-26866-part1

7af04d3

ti-chi-bot merged commit 28b6f52 into pingcap:master Sep 6, 2021

xiongjiwei reviewed Nov 1, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

table/tables: migrate test-infra to testify for `index_test.go` #27557

table/tables: migrate test-infra to testify for `index_test.go` #27557

feitian124 commented Aug 24, 2021 •

edited by tisonkun

Loading

ti-chi-bot commented Aug 24, 2021 •

edited

Loading

feitian124 commented Aug 24, 2021

feitian124 commented Aug 24, 2021 •

edited

Loading

feitian124 commented Aug 25, 2021

tisonkun left a comment •

edited

Loading

ti-chi-bot commented Aug 25, 2021

tisonkun left a comment

ti-chi-bot commented Aug 25, 2021

tisonkun left a comment

feitian124 commented Sep 5, 2021

ti-chi-bot commented Sep 6, 2021

tisonkun commented Sep 6, 2021

xhebox commented Sep 6, 2021

ti-chi-bot commented Sep 6, 2021

ti-chi-bot commented Sep 6, 2021

xiongjiwei Nov 1, 2021

table/tables: migrate test-infra to testify for index_test.go #27557

table/tables: migrate test-infra to testify for index_test.go #27557

Conversation

feitian124 commented Aug 24, 2021 • edited by tisonkun Loading

What problem does this PR solve?

Release note

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

feitian124 commented Aug 24, 2021

feitian124 commented Aug 24, 2021 • edited Loading

feitian124 commented Aug 25, 2021

tisonkun left a comment • edited Loading

Choose a reason for hiding this comment

ti-chi-bot commented Aug 25, 2021

tisonkun left a comment

Choose a reason for hiding this comment

ti-chi-bot commented Aug 25, 2021

tisonkun left a comment

Choose a reason for hiding this comment

feitian124 commented Sep 5, 2021

ti-chi-bot commented Sep 6, 2021

tisonkun commented Sep 6, 2021

xhebox commented Sep 6, 2021

ti-chi-bot commented Sep 6, 2021

ti-chi-bot commented Sep 6, 2021

xiongjiwei Nov 1, 2021

Choose a reason for hiding this comment

table/tables: migrate test-infra to testify for `index_test.go` #27557

table/tables: migrate test-infra to testify for `index_test.go` #27557

feitian124 commented Aug 24, 2021 •

edited by tisonkun

Loading

ti-chi-bot commented Aug 24, 2021 •

edited

Loading

feitian124 commented Aug 24, 2021 •

edited

Loading

tisonkun left a comment •

edited

Loading