Re: [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably

["Maria Khaydich" <maria.khaydich@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 7578 bytes --]


Thank you for the review! Fixes are done:
----------------------------------------------------------------------
Branch:
https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type  
 
 src/box/lua/net_box.lua                       |  8 +++-
 test/app-tap/debug.result                     |  8 ++--
 test/box/engine.cfg                           |  6 +++
 ...ox-self-and-connect-interchangeable.result | 45 +++++++++++++++++++
 ...-self-and-connect-interchangeable.test.lua | 23 ++++++++++
 test/box/suite.ini                            |  1 +
 6 files changed, 86 insertions(+), 5 deletions(-)
 create mode 100644 test/box/engine.cfg
 create mode 100644 test/box/gh-4513-netbox-self-and-connect-interchangeable.result
 create mode 100644 test/box/gh-4513-netbox-self-and-connect-interchangeable.test.lua
 
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index b4811edfa..3f611c027 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1550,7 +1550,13 @@ local function handle_eval_result(status, ...)
         rollback()
         return box.error(E_PROC_LUA, (...))
     end
-    return ...
+    local results = {...}
+    for i = 1, select('#', ...) do
+        if type(results[i]) == 'cdata' then
+            results[i] = msgpack.decode(msgpack.encode(results[i]))
+        end
+    end
+    return unpack(results)
 end
 
 this_module.self = {
 
diff --git a/test/app-tap/debug.result b/test/app-tap/debug.result
index 72934b076..cdc990307 100644
--- a/test/app-tap/debug.result
+++ b/test/app-tap/debug.result
@@ -15,15 +15,15 @@ nil
 Exec: tarantool -e "print(debug.__dir__); os.exit(0)"
 .
 Exec: tarantool -e "print(require('net.box').self:call('debug.sourcefile')); os.exit(0)"
-nil
+
 Exec: tarantool -e "print(require('net.box').self:call('debug.sourcedir')); os.exit(0)"
 .
 Exec: tarantool -e "fn = function() return debug.__file__ end; print(require('net.box').self:call('fn')); os.exit(0)"
-nil
+
 Exec: tarantool -e "fn = function() return debug.__dir__ end; print(require('net.box').self:call('fn')); os.exit(0)"
 .
 Exec: tarantool -e "fn = function() local res = debug.sourcefile(); return res end; print(require('net.box').self:call('fn')); os.exit(0)"
-nil
+
 Exec: tarantool -e "fn = function() local res = debug.sourcedir(); return res end; print(require('net.box').self:call('fn')); os.exit(0)"
 .
 Exec: tarantool -e "print(loadstring('return debug.sourcefile()')()); os.exit(0)"
@@ -52,7 +52,7 @@ debug/test.lua
 Source: print(debug.__dir__)
 debug
 Source: print(require('net.box').self:call('debug.sourcefile'))
-nil
+
 Source: print(require('net.box').self:call('debug.sourcedir'))
 .
 Source: fn = function() return debug.__file__ end; print(require('net.box').self:call('fn'))
diff --git a/test/box/engine.cfg b/test/box/engine.cfg
new file mode 100644
index 000000000..2d1d450e3
--- /dev/null
+++ b/test/box/engine.cfg
@@ -0,0 +1,6 @@
+{
+    "gh-4513-netbox-self-and-connect-interchangeable.test.lua": {
+        "remote": {"remote": "true"},
+        "local": {"remote": "false"}
+    }
+}
 
diff --git a/test/box/gh-4513-netbox-self-and-connect-interchangeable.result b/test/box/gh-4513-netbox-self-and-connect-interchangeable.result
new file mode 100644
index 000000000..67000f9c8
--- /dev/null
+++ b/test/box/gh-4513-netbox-self-and-connect-interchangeable.result
@@ -0,0 +1,45 @@
+-- test-run result file version 2
+netbox = require('net.box')
+ | ---
+ | ...
+test_run = require('test_run').new()
+ | ---
+ | ...
+remote = test_run:get_cfg('remote') == 'true'
+ | ---
+ | ...
+
+nb = nil
+ | ---
+ | ...
+if remote then \
+    box.schema.user.grant('guest','super') \
+    nb = netbox.connect(box.cfg.listen) \
+else \
+    nb = netbox.self \
+end
+ | ---
+ | ...
+
+--
+-- netbox:self and netbox:connect should work interchangeably
+--
+type(nb:eval('return box.tuple.new{1}')) -- table
+ | ---
+ | - table
+ | ...
+type(nb:eval('return box.error.new(1, "test error")')) -- string
+ | ---
+ | - string
+ | ...
+type(nb:eval('return box.NULL')) -- cdata
+ | ---
+ | - cdata
+ | ...
+
+if remote then \
+    box.schema.user.revoke('guest', 'super') \
+    nb:close() \
+end
+ | ---
+ | ...
 
diff --git a/test/box/gh-4513-netbox-self-and-connect-interchangeable.test.lua b/test/box/gh-4513-netbox-self-and-connect-interchangeable.test.lua
new file mode 100644
index 000000000..010d6cf34
--- /dev/null
+++ b/test/box/gh-4513-netbox-self-and-connect-interchangeable.test.lua
@@ -0,0 +1,23 @@
+netbox = require('net.box')
+test_run = require('test_run').new()
+remote = test_run:get_cfg('remote') == 'true'
+
+nb = nil
+if remote then \
+    box.schema.user.grant('guest','super') \
+    nb = netbox.connect(box.cfg.listen) \
+else \
+    nb = netbox.self \
+end
+
+--
+-- netbox:self and netbox:connect should work interchangeably
+--
+type(nb:eval('return box.tuple.new{1}')) -- table
+type(nb:eval('return box.error.new(1, "test error")')) -- string
+type(nb:eval('return box.NULL')) -- cdata
+
+if remote then \
+    box.schema.user.revoke('guest', 'super') \
+    nb:close() \
+end
 
diff --git a/test/box/suite.ini b/test/box/suite.ini
index 9aa30ede6..01f07236b 100644
--- a/test/box/suite.ini
+++ b/test/box/suite.ini
@@ -3,6 +3,7 @@ core = tarantool
 description = Database tests
 script = box.lua
 disabled = rtree_errinj.test.lua tuple_bench.test.lua
+config = engine.cfg
 release_disabled = errinj.test.lua errinj_index.test.lua rtree_errinj.test.lua upsert_errinj.test.lua iproto_stress.test.lua gh-4648-func-load-unload.test.lua
 lua_libs = lua/fifo.lua lua/utils.lua lua/bitset.lua lua/index_random_test.lua lua/push.lua lua/identifier.lua
 use_unix_sockets = True
-- 
2.24.0 
>Среда, 4 марта 2020, 1:49 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
> 
>Thanks for the patch!
>
>See 2 comments below.
>
>> diff --git a/test/box/gh-4513-netbox-self-and-connect-interchangeable.result b/test/box/gh-4513-netbox-self-and-connect-interchangeable.result
>> new file mode 100644
>> index 000000000..fa41b2442
>> --- /dev/null
>> +++ b/test/box/gh-4513-netbox-self-and-connect-interchangeable.result
>> @@ -0,0 +1,44 @@
>> +-- test-run result file version 2
>> +netbox = require('net.box')
>> + | ---
>> + | ...
>> +test_run = require('test_run').new()
>> + | ---
>> + | ...
>> +remote = test_run:get_cfg('remote') == 'true'
>> + | ---
>> + | ...
>> +test_run:cmd("setopt delimiter ';'")
>> + | ---
>> + | - true
>> + | ...
>> +
>> +nb = nil
>> +if remote then
>> + box.schema.user.grant('guest','super')
>
>1. Grant was given, but was not revoked. This can affect next
>tests running on the same instance.
>
>> + nb = netbox.connect(box.cfg.listen)
>> +else
>> + nb = netbox.self
>> +end;
>> + | ---
>> + | ...
>> +test_run:cmd("setopt delimiter ''");
>
>2. We have a nice syntax of '\' to create multi-line expressions
>like this. For such a small piece of code this would look better
>than 2 'setopt delimiter', IMO. Up to you.
>
>> + | ---
>> + | - true
>> + | ...
>> +
>> +--
>> +-- netbox:self and netbox:connect should work interchangeably
>> +--
>> +type(nb:eval('return box.tuple.new{1}')) -- table
>> + | ---
>> + | - table
>> + | ...
>> +type(nb:eval('return box.error.new(1, "test error")')) -- string
>> + | ---
>> + | - string
>> + | ...
>> +type(nb:eval('return box.NULL')) -- cdata
>> + | ---
>> + | - cdata
>> + | ...
>  
 
 
--
Maria Khaydich
 

[-- Attachment #2: Type: text/html, Size: 9641 bytes --]