planner: migreate expression_test to testify#28893
planner: migreate expression_test to testify#28893ti-chi-bot merged 4 commits intopingcap:masterfrom cr0rc:migrate-expression-test-to-testtify
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
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. DetailsReviewer can indicate their review by submitting an approval review. |
|
/cc @tisonkun |
planner/core/expression_test.go
Outdated
There was a problem hiding this comment.
You should migrate the test until this dependency can be safely removed.
planner/core/expression_test.go
Outdated
There was a problem hiding this comment.
No need for this line, we already verify goroutine leaks by goleak.
|
@tisonkun this should be good to review again. Thanks! |
planner/core/expression_test.go
Outdated
| Tp: f, | ||
| } | ||
|
|
||
| stmtCtx := new(stmtctx.StatementContext) |
There was a problem hiding this comment.
Why do you make these changes about stmtctx? It seems you add new tests.
There was a problem hiding this comment.
OK I get it that you expand DatumEquals. Let me take a closer look.
There was a problem hiding this comment.
yes, I checked other tests and looks like this is the way to migrate DatumEquals. Let me know if there is a better way. Thanks!
There was a problem hiding this comment.
As it is a common checker used in expression, I tend to write a util for it. I'll push a follow up later.
Signed-off-by: tison <wander4096@gmail.com>
|
/cc @mmyj |
|
/merge |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: 883f32a |
What problem does this PR solve?
Issue Number: close #28405
Release note