Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

migrate test-infra to testify for br/pkg/lightning/restore #28241

Closed
1 task done
Tracked by #28170
tisonkun opened this issue Sep 22, 2021 · 5 comments · Fixed by #32065
Closed
1 task done
Tracked by #28170

migrate test-infra to testify for br/pkg/lightning/restore #28241

tisonkun opened this issue Sep 22, 2021 · 5 comments · Fixed by #32065
Assignees
@disksing
Labels
component/br This issue is related to BR of TiDB. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.

Comments

@tisonkun
Copy link
Contributor

tisonkun commented Sep 22, 2021 •
edited by disksing
Loading

Sub tasks
@tisonkun tisonkun mentioned this issue Sep 22, 2021
Closed
12 tasks
@tisonkun tisonkun added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Sep 22, 2021
@disksing disksing self-assigned this Jan 26, 2022
@disksing disksing mentioned this issue Jan 27, 2022
Merged
4 tasks
disksing added a commit to oh-my-tidb/tidb that referenced this issue Jan 28, 2022
@disksing
br/lightning: migrate tests to using testify
ca079d7 
ref pingcap#28241

Signed-off-by: disksing <i@disksing.com>
@disksing disksing added component/lightning This issue is related to Lightning of TiDB. component/br This issue is related to BR of TiDB. and removed component/lightning This issue is related to Lightning of TiDB. labels Jan 29, 2022
@hawkingrei hawkingrei added the type/enhancement The issue or PR belongs to an enhancement. label Jan 29, 2022
ti-chi-bot pushed a commit that referenced this issue Jan 29, 2022
@disksing
br/lightning: migrate test to testify for restore (#32019)
bc7304c 
ref #28241
disksing added a commit to oh-my-tidb/tidb that referenced this issue Jan 29, 2022
@disksing
br/lightning: migrate test to using testify
fa91b20 
ref pingcap#28241

Signed-off-by: disksing <i@disksing.com>
@disksing disksing mentioned this issue Jan 29, 2022
Merged
4 tasks
@tisonkun
Copy link
Contributor Author

tisonkun commented Feb 7, 2022

@disksing after some investigate I notice that restore_schema_test.go is buggy as failure occurs but not be reported.

You can step into the gomock.Controller.Call and breakpoint at ctrl.T.Fatalf to verify its failure. Due to bugs in pingcap/check the failure reported in child check.C isn't propagated properly in the suite the it reports false PASS.

That is, we're running tests report false positive for a long time :(

@tisonkun
Copy link
Contributor Author

tisonkun commented Feb 7, 2022

@lichunzhu we need either rewrite this test in testify or just remove it as it actually fails. Could you please take a look?

@tisonkun
Copy link
Contributor Author

tisonkun commented Feb 9, 2022

@lichunzhu @disksing if there is no response, I'll regard it as this test is lack of interest and prepare to remove it. It can be brought back anytime from VCS, though.

@disksing
Copy link
Contributor

@lichunzhu @disksing if there is no response, I'll regard it as this test is lack of interest and prepare to remove it. It can be brought back anytime from VCS, though.

good idea

tisonkun added a commit to tisonkun/tidb that referenced this issue Feb 10, 2022
@tisonkun
br/pkg: clean up pingcap/check dep
ccd7f48 
For removing restore_schema_test.go, see also pingcap#28241.

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun mentioned this issue Feb 10, 2022
Closed
@lichunzhu
Copy link
Contributor

@lichunzhu we need either rewrite this test in testify or just remove it as it actually fails. Could you please take a look?

@disksing filed a PR #32065 to rewrite this to testify. We will try to merge this PR.

ti-chi-bot pushed a commit that referenced this issue Feb 11, 2022
@disksing
br/lightning: migrate test to using testify (#32065)
829a101 
close #28241
YuJuncen pushed a commit to pingcap/br that referenced this issue Mar 1, 2022
@disksing
br/lightning: migrate test to testify for restore (#32019)
e5df066 
ref pingcap/tidb#28241
YuJuncen pushed a commit to pingcap/br that referenced this issue Mar 1, 2022
@disksing
br/lightning: migrate test to using testify (#32065)
1014e7f 
close pingcap/tidb#28241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@disksing disksing

Labels
component/br This issue is related to BR of TiDB. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
4 participants
@hawkingrei @disksing @tisonkun @lichunzhu