ddl, tools, util: migrate pingcap check to testify. by GatorMao · Pull Request #31313 · pingcap/tidb

GatorMao · 2022-01-05T05:28:06Z

What problem does this PR solve?

Issue Number: close #29111 ,close #29108 ,close #29104 ,close #29109 ,close #29114 ,close #28639 ,close #28638 ,close #28631,close #28630

Problem Summary: migrate pingcap check to testify.

What is changed and how it works?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

ti-chi-bot · 2022-01-05T05:28:08Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

tisonkun
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.

Details

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

CLAassistant · 2022-01-05T05:28:11Z

All committers have signed the CLA.

ti-chi-bot · 2022-01-05T05:28:18Z

Welcome @GatorMao!

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. 😃

sre-bot · 2022-01-05T05:28:37Z

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

*: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

GatorMao · 2022-01-05T05:33:15Z

Hi @tisonkun, I draft a PR because I encounter few issues during testing and cannot figure out the reason. One of them is the TestT test function in ddl package ddl_test.go file seems running forever and I don't know the reason for it.
Besides, I create suit.Suite field for each test suite struct and also manually initiate infoSync before each test suite's run method. Not sure if this is the best practice or desired behavior. This is my first contribution to this project and hope could get some help. Thanks !

tisonkun · 2022-01-07T09:12:34Z

@GatorMao thanks for your contribution. The general direction looks good to me. It's a great work!

Please check CI failures and fix it.

sre-bot · 2022-01-08T01:32:27Z

Please follow PR Title Format:

pkg [, pkg2, pkg3]: what is changed

Or if the count of mainly changed packages are more than 3, use

*: what is changed

After you have format title, you can leave a comment /run-check_title to recheck it

GatorMao · 2022-01-08T02:04:32Z

@tisonkun, one of the workflow failed due to following reason. Looks like my change break existing check for test suite in tools/check/check_testSuite.sh file. These test suite are the ones that I modified during this change. Do you know how to fix it ? Possibly need to modify the regular expressions in check_testSuite.sh file ? Thanks.


[2022-01-08T01:59:24.226Z] testStatSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testSerialStatSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testIndexChangeSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testColumnSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testDDLSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testDDLSerialSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] testTableSuite in ./ddl is not enabled

[2022-01-08T01:59:24.226Z] make: *** [testSuite] Error 1

sre-bot · 2022-01-08T04:10:35Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/bc5a39117c40e7f52e332e376407458f0d12bbfc

GatorMao · 2022-01-08T04:32:38Z

@tisonkun, all the unit tests pass. Just need to fix the check_testSuite.sh file before submission for final PR.

tisonkun · 2022-01-08T06:02:40Z

@GatorMao rename the suite name should work. It's a legacy check which you can take a look at the script. Don't take it serious :)

And after we migrate all tests to testify, remove pingcap/check stuff, we can remove this check also.

GatorMao · 2022-01-08T06:43:20Z

@tisonkun , thanks ! Just renamed the test suites and hopefully it works this time.

GatorMao · 2022-01-08T08:02:56Z

@tisonkun, the unit test process seems failed with unrelated reason. My change doesn't touch the tests for br package and unit tests were success last time.

[2022-01-08T07:55:17.641Z] FAIL: schema_test.go:33: testBackupSchemaSuite.SetUpSuite

[2022-01-08T07:55:17.641Z] 

[2022-01-08T07:55:17.641Z] schema_test.go:37:

[2022-01-08T07:55:17.641Z]     c.Assert(s.mock.Start(), IsNil)

[2022-01-08T07:55:17.641Z] ... value *errors.withStack = listen tcp 0.0.0.0:44642: bind: address already in use ("listen tcp 0.0.0.0:44642: bind: address already in use")

[2022-01-08T07:55:17.641Z] 

[2022-01-08T07:55:17.641Z] OOPS: 7 passed, 1 FAILED, 3 MISSED

tisonkun · 2022-01-08T08:15:06Z

@hawkingrei could you give this PR a review to see whether unit_test's failure is related?

zimulala · 2022-01-10T07:10:54Z

Details

@hawkingrei could you give this PR a review to see whether unit_test's failure is related?

This unit_test failure has nothing to do with this PR, but a test of BR failed

[2022-01-09T21:13:21.958Z]     mock_cluster_test.go:21: 
[2022-01-09T21:13:21.958Z]         	Error Trace:	
[2022-01-09T21:13:21.958Z]         	Error:      	Received unexpected error:
[2022-01-09T21:13:21.958Z]         	            	listen tcp 0.0.0.0:44316: bind: address already in use
[2022-01-09T21:13:21.958Z]         	            	github.com/pingcap/errors.AddStack
[2022-01-09T21:13:21.958Z]         	            		/go/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20211224045212-9687c2b0f87c/errors.go:174
[2022-01-09T21:13:21.958Z]         	            	github.com/pingcap/errors.Trace
[2022-01-09T21:13:21.958Z]         	            		/go/pkg/mod/github.com/pingcap/errors@v0.11.5-0.20211224045212-9687c2b0f87c/juju_adaptor.go:15
[2022-01-09T21:13:21.958Z]         	            	github.com/pingcap/tidb/server.NewServer
[2022-01-09T21:13:21.958Z]         	            		/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/server/server.go:245
[2022-01-09T21:13:21.958Z]         	            	github.com/pingcap/tidb/br/pkg/mock.(*Cluster).Start
[2022-01-09T21:13:21.958Z]         	            		/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/br/pkg/mock/mock_cluster.go:106
[2022-01-09T21:13:21.958Z]         	            	github.com/pingcap/tidb/br/pkg/mock_test.TestSmoke
[2022-01-09T21:13:21.958Z]         	            		/home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/br/pkg/mock/mock_cluster_test.go:21
[2022-01-09T21:13:21.958Z]         	            	testing.tRunner
[2022-01-09T21:13:21.958Z]         	            		/usr/local/go/src/testing/testing.go:1193
[2022-01-09T21:13:21.958Z]         	            	runtime.goexit
[2022-01-09T21:13:21.958Z]         	            		/usr/local/go/src/runtime/asm_amd64.s:1371
[2022-01-09T21:13:21.958Z]         	Test:       	TestSmoke

tangenta · 2022-01-12T03:41:02Z

@GatorMao I have filed a PR to stablize it, FYI: #31591.

ddl/fail_test.go

GatorMao · 2022-01-12T03:46:01Z

Much thanks ! @tangenta. Once that PR goes through, I will rebase.

GatorMao · 2022-01-14T05:27:15Z

@tisonkun , the unit test failure does not come from ddl package. Could you check out and approve my change ? Thanks.

GatorMao · 2022-01-18T16:29:55Z

@Mini256 @hawkingrei @tisonkun , I think the pipeline is in good state right now. Please review and approve this change at your convenience. Thanks.

tisonkun · 2022-01-19T01:51:52Z

@GatorMao thanks for your follow-up. I'm reviewing this PR now.

tisonkun

LGTM.

@hawkingrei @xhebox @tangenta PTAL. Since we run all tests in serial, I think using suite is acceptable.

xhebox · 2022-01-20T05:26:41Z

/merge

ti-chi-bot · 2022-01-20T05:26:46Z

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

Details

Commit hash: da4113d

tisonkun · 2022-02-14T08:48:23Z

This closes #29102.

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 5, 2022

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 5, 2022

GatorMao changed the title ~~Ticket 29111~~ Migrate pingcap check to testify. Jan 5, 2022

GatorMao changed the title ~~Migrate pingcap check to testify.~~ ddl: migrate pingcap check to testify. Jan 8, 2022

GatorMao force-pushed the ticket_29111 branch 2 times, most recently from 94bd2e2 to 734f11c Compare January 8, 2022 01:55

GatorMao force-pushed the ticket_29111 branch 5 times, most recently from 3e79674 to cd409e3 Compare January 8, 2022 07:51

GatorMao changed the title ~~ddl: migrate pingcap check to testify.~~ ddl, tools, util: migrate pingcap check to testify. Jan 8, 2022

GatorMao marked this pull request as ready for review January 8, 2022 08:09

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 8, 2022

hawkingrei reviewed Jan 12, 2022

View reviewed changes

ddl/fail_test.go Outdated Show resolved Hide resolved

ddl/fail_test.go Outdated Show resolved Hide resolved

ddl/fail_test.go Outdated Show resolved Hide resolved

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2022

ddl, util, tools: rebase with latest master branch.

20186a7

GatorMao force-pushed the ticket_29111 branch from a7c9bbc to 20186a7 Compare January 14, 2022 04:59

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2022

ddl: fix fail_test.go based on review comments.

ecada57

Merge branch 'master' into ticket_29111

43a849b

tisonkun approved these changes Jan 19, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 19, 2022

GatorMao and others added 2 commits January 19, 2022 02:07

Merge branch 'master' into ticket_29111

658c0c9

Merge branch 'master' into ticket_29111

da4113d

xhebox approved these changes Jan 20, 2022

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 Jan 20, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 20, 2022

Merge branch 'master' into ticket_29111

bc5a391

hawkingrei approved these changes Jan 20, 2022

View reviewed changes

ti-chi-bot merged commit 06a4dfa into pingcap:master Jan 20, 2022

tisonkun mentioned this pull request Feb 14, 2022

migrate test-infra to testify for ddl/fail_test.go #29102

Closed

tisonkun mentioned this pull request Feb 28, 2022

util/testutil: cleanup testutils depend on pingcap/check #32662

Merged

Conversation

GatorMao commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot commented Jan 5, 2022

Uh oh!

sre-bot commented Jan 5, 2022

Uh oh!

GatorMao commented Jan 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tisonkun commented Jan 7, 2022

Uh oh!

sre-bot commented Jan 8, 2022

Uh oh!

GatorMao commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sre-bot commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GatorMao commented Jan 8, 2022

Uh oh!

tisonkun commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GatorMao commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GatorMao commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tisonkun commented Jan 8, 2022

Uh oh!

zimulala commented Jan 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangenta commented Jan 12, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GatorMao commented Jan 12, 2022

Uh oh!

GatorMao commented Jan 14, 2022

Uh oh!

GatorMao commented Jan 18, 2022

Uh oh!

tisonkun commented Jan 19, 2022

Uh oh!

tisonkun left a comment

Choose a reason for hiding this comment

Uh oh!

xhebox commented Jan 20, 2022

Uh oh!

ti-chi-bot commented Jan 20, 2022

Uh oh!

tisonkun commented Feb 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

GatorMao commented Jan 5, 2022 •

edited

Loading

ti-chi-bot commented Jan 5, 2022 •

edited

Loading

CLAassistant commented Jan 5, 2022 •

edited

Loading

GatorMao commented Jan 5, 2022 •

edited

Loading

GatorMao commented Jan 8, 2022 •

edited

Loading

sre-bot commented Jan 8, 2022 •

edited

Loading

tisonkun commented Jan 8, 2022 •

edited

Loading

GatorMao commented Jan 8, 2022 •

edited

Loading

GatorMao commented Jan 8, 2022 •

edited

Loading

zimulala commented Jan 10, 2022 •

edited

Loading