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

test: port lock table ut from mysql #10376

Merged
merged 9 commits into from
Jul 23, 2019

Conversation

marsishandsome
Copy link
Contributor

@marsishandsome marsishandsome commented May 7, 2019

What problem does this PR solve?

port some unit test from mysql for lock table (#10343)

What is changed and how it works?

add some unit test

Tests

  • Unit test

@marsishandsome
Copy link
Contributor Author

@razycs520 PTAL

@marsishandsome
Copy link
Contributor Author

/run-all-tests

@marsishandsome
Copy link
Contributor Author

@winkyao PTAL

@marsishandsome
Copy link
Contributor Author

/run-all-tests

@marsishandsome
Copy link
Contributor Author

@winkyao PTAL

@codecov
Copy link

codecov bot commented Jul 11, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #10376   +/-   ##
===========================================
  Coverage   81.4098%   81.4098%           
===========================================
  Files           423        423           
  Lines         90973      90973           
===========================================
  Hits          74061      74061           
  Misses        11606      11606           
  Partials       5306       5306

ddl/db_test.go Outdated Show resolved Hide resolved
ddl/db_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@marsishandsome
Copy link
Contributor Author

@crazycs520 PTAL again

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

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520
Copy link
Contributor

/run-all-tests

@zz-jason
Copy link
Member

@marsishandsome thanks for your contribution, please follow CONTRIBUTING.md to refine the PR title.

@zz-jason
Copy link
Member

@crazycs520 The two LGTM are all given by you, I think we need another LGTM from another different reviewer.

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 18, 2019
@zz-jason zz-jason dismissed crazycs520’s stale review July 18, 2019 12:59

need one more reviewer

@crazycs520
Copy link
Contributor

@tangenta PTAL

Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

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

Rest LGTM

// Test: allow read
tk.MustExec("lock tables t1 write local")
tk.MustExec("insert into t1 values(NULL)")
tk2.MustQuery("select count(*) from t1")
Copy link
Contributor

@tangenta tangenta Jul 18, 2019

Choose a reason for hiding this comment

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

Do we need to check the result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we do not need to check the result (see mysql lock table test).

@marsishandsome marsishandsome changed the title port lock table ut from mysql test: port lock table ut from mysql Jul 19, 2019
@marsishandsome
Copy link
Contributor Author

@marsishandsome thanks for your contribution, please follow CONTRIBUTING.md to refine the PR title.

done

Copy link
Contributor

@tangenta tangenta 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 merged commit e0e5b50 into pingcap:master Jul 23, 2019
@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 23, 2019
marsishandsome added a commit to marsishandsome/tidb that referenced this pull request Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/test status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants