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.

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

expression: let testEvaluatorSuite run serially to avoid affect other unit tests since it uses failpoint #11310

Merged
merged 2 commits into from
Jul 18, 2019

Conversation

qw4990
Copy link
Contributor

@qw4990 qw4990 commented Jul 18, 2019

What problem does this PR solve?

Some unit tests of testEvaluatorSuite use failpoint to control whether to push down some expressions.

Therefore, when it runs parallelly with other unit tests, all other tests will also be affected by this failpoint, which is undesired.

What is changed and how it works?

Let it run serially to avoid affect other unit tests.

@codecov
Copy link

codecov bot commented Jul 18, 2019

Codecov Report

Merging #11310 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11310   +/-   ##
===========================================
  Coverage   81.2469%   81.2469%           
===========================================
  Files           423        423           
  Lines         90156      90156           
===========================================
  Hits          73249      73249           
  Misses        11598      11598           
  Partials       5309       5309

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

XuHuaiyu
XuHuaiyu previously approved these changes Jul 18, 2019
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990
Copy link
Contributor Author

qw4990 commented Jul 18, 2019

/run-all-tests

@qw4990
Copy link
Contributor Author

qw4990 commented Jul 18, 2019

@XuHuaiyu I rebased this PR to the latest Master, so please help me to approve it again~

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason changed the title expression, test: let testEvaluatorSuite run serially to avoid affect other unit tests since it uses failpoint expression: let testEvaluatorSuite run serially to avoid affect other unit tests since it uses failpoint Jul 18, 2019
@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 18, 2019
@zz-jason
Copy link
Member

do we need to cherry pick this PR to release 3.0 or 2.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants