executor_test: clean enviroment for each test. #2025
Conversation
| @@ -132,6 +132,8 @@ func (s *testSuite) TestAdmin(c *C) { | |||
| c.Assert(err, IsNil) | |||
| r, err = tk.Exec("admin check table admin_test") | |||
| c.Assert(err, NotNil) | |||
| tk.MustExec("drop table admin_test") | |||
| tk.MustExec("drop table admin_test1") | |||
There was a problem hiding this comment.
Can be written in one statement.
| @@ -259,6 +259,7 @@ func (s *testSuite) TestAggregation(c *C) { | |||
| tk.MustExec("insert into t2 values(22, 2), (3, 12), (38, 98)") | |||
| result = tk.MustQuery("SELECT COALESCE ( + 1, cor0.col0 ) + - CAST( NULL AS DECIMAL ) FROM t2, t1 AS cor0, t2 AS cor1 GROUP BY cor0.col1") | |||
| result.Check(testkit.Rows("<nil>", "<nil>")) | |||
| tk.MustExec("drop table t, t1, t2") | |||
There was a problem hiding this comment.
any benifit of using defer?
There was a problem hiding this comment.
Clean, and you will never forget to clean up table.
|
@ngaut PTAL |
| tk := testkit.NewTestKit(c, s.store) | ||
| defer func() { | ||
| testleak.AfterTest(c)() | ||
| tk.MustExec("drop table if exists t, t1, t2") |
There was a problem hiding this comment.
This should be executed before testleak.AfterTest(c)()
| tk := testkit.NewTestKit(c, s.store) | ||
| defer func() { | ||
| testleak.AfterTest(c)() | ||
| tk.MustExec("drop table if exists t15") |
There was a problem hiding this comment.
These statements can be synthesized into a statement.
|
Usually we call defer right after resource is created, in this case, the pattern would be. So it's very easy to check that resource has been cleaned up. |
|
LGTM |
|
@hanfei1991 Please add description for the PR. |
| tk := testkit.NewTestKit(c, s.store) | ||
| defer func() { |
There was a problem hiding this comment.
I can't understand this change
| tk := testkit.NewTestKit(c, s.store) | ||
| defer func() { | ||
| defer testleak.AfterTest(c)() |
| tk := testkit.NewTestKit(c, s.store) | ||
| defer func() { | ||
| defer testleak.AfterTest(c)() |
|
LGTM There is one concern that if we want run unit test parallel, |
|
LGTM |
We should clean the evironment for every test, so as to avoid one test can use a table that created by another test.
@shenli @coocood @ngaut @XuHuaiyu @zimulala PTAL