[tarantool-patches] Re: [PATCH v1 1/1] sql: Tests for CREATE TEMP TABLE and CREATE TEMPORARY TABLE

roman.habibov1 at yandex.ru roman.habibov1 at yandex.ru
Wed May 9 20:20:35 MSK 2018


> Please, read guidelines carefully.
>
>> sql: Tests for CREATE TEMP TABLE and CREATE TEMPORARY TABLE
>
> According to our docs:
> — 2. Try to limit the subject line to 50 characters or so.
> — 3. Start the subject line with a capital letter unless it prefixed with a subsystem name and semicolon…
Sorry. I fixed that.

> Also, it is not clear what have you done with these ’tests’: removed/fixed/added etc.
That too.

>> +
>> +-- gh-2166 Tables with TEMP and TEMPORARY were removed before.
>> +
>
> Leading space at the last line. Enable showing whitespaces to avoid such codestyle violations.
>> + })
>> +test:do_catchsql_test(
>> + "temporary",
>
>> + 1, "near \"TEMPORARY\": syntax error"
>> + -- <temporary>
>> + })
>> test:do_execsql2_test(
That too.

> Separate tests with blank line.
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 70b22d2..84eb884 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -623,6 +623,7 @@ test:do_catchsql_test(
 	1, "near \"TEMP\": syntax error"
 	-- <temp>
 	})
+
 test:do_catchsql_test(
 	"temporary",
 	[[
@@ -632,6 +633,7 @@ test:do_catchsql_test(
 	1, "near \"TEMPORARY\": syntax error"
 	-- <temporary>
 	})
+
 test:do_execsql2_test(
     "table-8.6",
     [[

> I would also add tests (or just rewrite yours) which check that complete CREATE TABLE statement doesn’t support TEMP clause:
>
> CREATE TEMP TABLE t1(…);
>
> CREATE TEMPORARY TABLE t1(…);
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index c71e4b9..70b22d2 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -613,11 +613,11 @@ test:do_execsql2_test(
 -- } {cnt 1 max(b+c) 5}
 
 -- gh-2166 Tables with TEMP and TEMPORARY were removed before.
-	
+
 test:do_catchsql_test(
 	"temp",
 	[[
-		CREATE TEMP TABLE
+		CREATE TEMP TABLE t1();
 	]], {
 	-- <temp>
 	1, "near \"TEMP\": syntax error"
@@ -626,7 +626,7 @@ test:do_catchsql_test(
 test:do_catchsql_test(
 	"temporary",
 	[[
-		CREATE TEMPORARY TABLE
+		CREATE TEMPORARY TABLE t1();
 	]], {
 	-- <temporary>
 	1, "near \"TEMPORARY\": syntax error"




More information about the Tarantool-patches mailing list