Skip to content

expression: migrate tests to testify#29276

Merged
ti-chi-bot merged 8 commits intopingcap:masterfrom
tisonkun:expression-testify
Nov 2, 2021
Merged

expression: migrate tests to testify#29276
ti-chi-bot merged 8 commits intopingcap:masterfrom
tisonkun:expression-testify

Conversation

@tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Oct 31, 2021 •
edited
Loading

... also use a thread safe rng for stable test

This closes #29132 .
This closes #29134 .
This closes #29142 .

This closes #29283 . Move TestCIWeightString to serial test.

This closes #29164 . Replace rand.Source with lockedSource.

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

Release note

None

@tisonkun
expression: migrate tests to testify
0de22ad 
... also use a thread safe rng for stable test

This closes pingcap#29132 .
This closes pingcap#29134 .
This closes pingcap#29142 .

Signed-off-by: tison <wander4096@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Oct 31, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • bb7133
  • mmyj

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.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 31, 2021
@tisonkun
Copy link
Contributor Author

tisonkun commented Oct 31, 2021

/cc @xhebox @wjhuang2016

@tisonkun
gofmt
a604fc2 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
sync collate.SetNewCollationEnabledForTest
56e024a 
Signed-off-by: tison <wander4096@gmail.com>
feitian124
feitian124 reviewed Oct 31, 2021
View reviewed changes
@feitian124
Copy link
Contributor

feitian124 commented Oct 31, 2021 •
edited
Loading

LGTM.
just wondering, how do you find some tests must run in serial? some of them are not clear at all.

@tisonkun tisonkun mentioned this pull request Nov 1, 2021
Closed
unconsolable
unconsolable reviewed Nov 1, 2021
View reviewed changes
expression/builtin_string_serial_test.go
"github.com/stretchr/testify/require"
)

func TestCIWeightString(t *testing.T) {
Copy link
Contributor

@unconsolable unconsolable Nov 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall the TestWeightString be moved to here too? it seems that #29283 happens on TestWeightString (Also I have encountered few failures on this function)

Copy link
Contributor Author

@tisonkun tisonkun Nov 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. The root cause of TestWeightString is running concurrently with TestCIWeightString. Thus after we move TestCIWeightString out, TestWeightString will be fixed.

@tisonkun tisonkun mentioned this pull request Nov 1, 2021
Closed
@winoros
Copy link
Member

winoros commented Nov 1, 2021

/run-common-test tidb-test=pr/1307

@tisonkun tisonkun mentioned this pull request Nov 1, 2021
Merged
@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 1, 2021

/run-common-test tidb-test=pr/1307

what does it run for?

@winoros
Copy link
Member

winoros commented Nov 2, 2021

/run-common-test tidb-test=pr/1307

what does it run for?

Select a pr to test the changes(

@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 2, 2021

/run-common-test tidb-test=pr/1307

what does it run for?

Select a pr to test the changes(

It may confuse other reviewers that this PR has a bug =。=

@winoros
Copy link
Member

winoros commented Nov 2, 2021

/run-common-test tidb-test=pr/1307

@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 2, 2021

/run-all-tests

@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 2, 2021

/cc @mmyj @bb7133

@ti-chi-bot ti-chi-bot requested review from bb7133 and mmyj November 2, 2021 10:56
@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 2, 2021

/run-common-test

@bb7133
Copy link
Member

bb7133 commented Nov 2, 2021

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 2, 2021
mmyj
mmyj approved these changes Nov 2, 2021
View reviewed changes
Copy link
Member

@mmyj mmyj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ti-chi-bot 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 Nov 2, 2021
@mmyj
Copy link
Member

mmyj commented Nov 2, 2021

/merge

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Nov 2, 2021

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

DetailsCommit hash: 08705cc

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 2, 2021
@ti-chi-bot ti-chi-bot merged commit 318030a into pingcap:master Nov 2, 2021
@tisonkun tisonkun deleted the expression-testify branch November 2, 2021 15:32
@bb7133 bb7133 mentioned this pull request Jun 14, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bb7133 bb7133 bb7133 approved these changes

@mmyj mmyj mmyj approved these changes

+2 more reviewers

@feitian124 feitian124 feitian124 left review comments

@unconsolable unconsolable unconsolable left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

7 participants

@tisonkun @ti-chi-bot @feitian124 @winoros @bb7133 @mmyj @unconsolable