-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: Add few error checks in tests, clean error messages and fix one misspell #8101
Conversation
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
@dvrkps Thanks! |
ddl/db_change_test.go
Outdated
@@ -588,6 +588,7 @@ func (s *testStateChangeSuite) TestShowIndex(c *C) { | |||
result, err = s.execQuery(tk, "show index from tr;") | |||
c.Assert(err, IsNil) | |||
err = checkResult(result, testkit.Rows("tr 1 idx1 1 purchased A 0 <nil> <nil> BTREE ", "t 1 c2 1 c2 A 0 <nil> <nil> YES BTREE ")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test was wrong.
err = checkResult(result, testkit.Rows("tr 1 idx1 1 purchased A 0 <nil> <nil> YES BTREE "))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazycs520 Done. Thx.
util/encrypt/crypt_test.go
Outdated
@@ -72,8 +72,8 @@ func (s *testEncryptSuite) TestSQLEncode(c *C) { | |||
|
|||
for _, t := range tests { | |||
crypted, err := SQLDecode(t.str, t.passwd) | |||
c.Assert(err, NotNil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.Assert(err, IsNil)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazycs520 Done. Thx.
expression/integration_test.go
Outdated
@@ -3127,6 +3127,7 @@ func (s *testIntegrationSuite) TestDateBuiltin(c *C) { | |||
|
|||
tk.MustExec("set sql_mode = 'NO_ZERO_DATE'") | |||
rs, err := tk.Exec("select date '0000-00-00';") | |||
c.Assert(err, NotNil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.Assert(err, IsNil)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazycs520 Done. Thx.
expression/integration_test.go
Outdated
@@ -3156,6 +3157,7 @@ func (s *testIntegrationSuite) TestDateBuiltin(c *C) { | |||
c.Assert(rs.Close(), IsNil) | |||
|
|||
rs, err = tk.Exec("select date '0000-00-00';") | |||
c.Assert(err, NotNil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c.Assert(err, IsNil)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazycs520 Done. Thx.
@dvrkps thank you! Lets see if we can get some of these added to the linting/ci now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@crazycs520 Can we turn on any new linters in the Makefile now? |
No description provided.