[tarantool-patches] Re: [PATCH] net.box: fix 'unique' index flag in net.box schema

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 01/04/2019 13:08, Alexander Turenko wrote:
> On Mon, Apr 01, 2019 at 11:42:03AM +0300, Vladislav Shpilevoy wrote:
>>
>>> diff --git a/test/box-tap/gh-4091-net.box-indexes-are-always-non-unique.test.lua b/test/box-tap/gh-4091-net.box-indexes-are-always-non-unique.test.lua
>>> new file mode 100755
>>> index 000000000..04c576915
>>> --- /dev/null
>>> +++ b/test/box-tap/gh-4091-net.box-indexes-are-always-non-unique.test.lua
>>
>> Please, just add a one line to box/net.box.lua. It does not help at all
>> to have a new file per each issue. This way looks bulky and hard to search
>> for something.
> 
> I disagree. Summarized here:
> https://www.freelists.org/post/tarantool-patches/PATCH-sql-make-SQL-BIND-optional-in-an-iproto-request,2

I had read that, and it was discussed so many times that I do not
see a sense of doing that again. Someone should just prohibit one
of the ways, and standardize the other. Until it is done, any reviewer
can stick to his own favorite way, and mine is to do not clog test
directories with hundreds and thousands of files.

> 
> Anyway, if you feel all-in-one tests as the better approach, I don't
> want to spent time to discuss it.
> 
> Changed. The new patch is below.

It is changed, but in fact it is still the same. When you have one file,
you can reuse some of existing places to make new tests shorter and
simpler. Strictly speaking for such a bug I wouldn't add a test at all.
It is rather an old typo than a real bug which could return in future.

My diff is below. The same diff is in a separate commit on top of your
branch.

=======================================================================

diff --git a/test/box/net.box.result b/test/box/net.box.result
index 8fc668606..c23ab7a3a 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -2001,6 +2001,13 @@ c.space.test:format({})
 ---
 - error: net.box does not support setting space format
 ...
+--
+-- gh-4091: index unique flag is always false.
+--
+c.space.test.index.primary.unique
+---
+- true
+...
 c:close()
 ---
 ...
@@ -3526,35 +3533,3 @@ s:drop()
 box.cfg{readahead = readahead}
 ---
 ...
---
--- gh-4091: index unique flag is always false
---
-s = box.schema.space.create("test")
----
-...
-_ = s:create_index("pk")
----
-...
-_ = s:create_index("sk", {unique = false})
----
-...
-box.schema.user.grant("guest", "read", "space", "test")
----
-...
-c = net.connect(box.cfg.listen)
----
-...
-c.space.test.index.pk.unique -- true
----
-- true
-...
-c.space.test.index.sk.unique -- false
----
-- false
-...
-box.schema.user.revoke("guest", "read", "space", "test")
----
-...
-s:drop()
----
-...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index b81bed239..556d48420 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -814,6 +814,11 @@ format[1].type == "unsigned"
 
 c.space.test:format({})
 
+--
+-- gh-4091: index unique flag is always false.
+--
+c.space.test.index.primary.unique
+
 c:close()
 space:drop()
 
@@ -1435,16 +1440,3 @@ test_run:wait_log('default', 'readahead limit is reached', 1024, 0.1)
 
 s:drop()
 box.cfg{readahead = readahead}
-
---
--- gh-4091: index unique flag is always false
---
-s = box.schema.space.create("test")
-_ = s:create_index("pk")
-_ = s:create_index("sk", {unique = false})
-box.schema.user.grant("guest", "read", "space", "test")
-c = net.connect(box.cfg.listen)
-c.space.test.index.pk.unique -- true
-c.space.test.index.sk.unique -- false
-box.schema.user.revoke("guest", "read", "space", "test")
-s:drop()