meta/autoid: migrate test-infra to testify #26774
Conversation
Signed-off-by: unconsolable <chenzhipeng2012@gmail.com>
|
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
release-note-none
|
/cc @tisonkun |
|
/run-check_dev_2 |
|
Thanks for your contribution @unconsolable ! I'll review this PR tomorrow. When I first come to autoid, I think it is somehow too large to understand. Is it possible to spilt tests into smaller set (file)? By the way, you may take a look at the implementation of autoid which handle almost all id generation logic. There is a long lasting PR #20708 about this module. I don't ask for your extra working but suggest you can dig into TiDB in this direction. |
I think it may split in three parts: rowid tests, bench tests, sequence id tests, but there are about 600 lines about rowid tests, maybe it is still quite large.
If I try to do that, should I need to refer to |
meta/autoid/memid_test.go
Outdated
| "github.com/pingcap/tidb/meta/autoid" | ||
| "github.com/pingcap/tidb/store/mockstore" | ||
| "github.com/pingcap/tidb/types" |
There was a problem hiding this comment.
This error may be related to vscode package matching, I will fix it.
meta/autoid/autoid_test.go
Outdated
| "github.com/pingcap/errors" | ||
| "github.com/pingcap/failpoint" | ||
| "github.com/pingcap/parser/model" | ||
| "github.com/pingcap/tidb/kv" | ||
| "github.com/pingcap/tidb/meta" | ||
| "github.com/pingcap/tidb/meta/autoid" | ||
| "github.com/pingcap/tidb/store/mockstore" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestT(t *testing.T) { |
There was a problem hiding this comment.
You should find a better name instead of TestT. And may try to separate tests as small as possible, may in different files.
|
@tisonkun: Request changes is only allowed for the reviewers in list. 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. |
You can but it may be too far to the real issue. I suggest you contact with @tangenta in the PR to see what you can supersede. |
|
/run-check_dev_2 |
|
/run-check_dev_2 |
You may first comment the cause of failure in the PR and analyze, rather than just rerun... |
But ci fails at other tests instead of |
|
I'd suggest you test locally first, and try to split tests in a well formatted commit #26771 (comment). In current form of this PR it is still thousands of lines to review. Here is an example how to break diff into meaningful commits that helps on reviewing. https://github.com/apache/flink/pull/9832/commits |
Got it, I will close this PR and switch to this way, thank you. |
Signed-off-by: unconsolable chenzhipeng2012@gmail.com
What problem does this PR solve?
Issue Number: close #26741
Release note