server: restore exec() sparse output support (#351)

After [1], returning values after nil (for example, classic
`return nil, err` scenario) is broken. This patch restores the behavior.

Since server doesn't always has access to `require('luatest.utils')`,
utility is provided through its dump.

Since `net.box` already handles `nil` args input with `box.NULL`,
everything is fine for exec arguments.

1. e6a2093

Closes #350

CHANGELOG.md

@@ -4,6 +4,7 @@
 
   * Fixed incorrent unix socket path length check (gh-341).
   * Now net_box_uri can be accepted as table (gh-342).
+  * Fixed returning values from `Server:exec()` if some of them are nil (gh-350).
 
 ## 1.0.0
 

luatest/server.lua

@@ -686,6 +686,10 @@ function Server:exec(fn, args, options)
             :format(utils.get_fn_location(fn)))
     end
 
+    local utils_dumps = {
+        unpack_sparse_array = string.dump(utils.unpack_sparse_array),
+    }
+
     -- The function `fn` can return multiple values and we cannot use the
     -- classical approach to work with the `pcall`:
     --
@@ -694,14 +698,21 @@ function Server:exec(fn, args, options)
     -- `result` variable will contain only `1` value, not `1, 2, 3`.
     -- To solve this, we put everything from `pcall` in a table.
     return exec_tail(pcall(self.net_box.eval, self.net_box, [[
-        local dump, args, passthrough_ups = ...
-        local fn = loadstring(dump)
+        local fn_dump, args, passthrough_ups, utils_dumps = ...
+
+        local fn = loadstring(fn_dump)
         for i = 1, debug.getinfo(fn, 'u').nups do
             local name, _ = debug.getupvalue(fn, i)
             if passthrough_ups[name] then
                 debug.setupvalue(fn, i, require(passthrough_ups[name]))
             end
         end
+
+        local utils = {}
+        for k, v in pairs(utils_dumps) do
+            utils[k] = loadstring(v)
+        end
+
         local result
         if args == nil then
             result = {pcall(fn)}
@@ -714,8 +725,8 @@ function Server:exec(fn, args, options)
             end
             error(result[2], 0)
         end
-        return unpack(result, 2)
-    ]], {string.dump(fn), args, passthrough_ups}, options))
+        return utils.unpack_sparse_array(result, 2)
+    ]], {string.dump(fn), args, passthrough_ups, utils_dumps}, options))
 end
 
 function Server:coverage(action)

luatest/utils.lua

@@ -191,4 +191,26 @@ function utils.is_tarantool_binary(path)
     return path:find('^.*/tarantool[^/]*$') ~= nil
 end
 
+function utils.unpack_sparse_array(array, index)
+    -- Embed everything so no upvalues required on server.
+    index = index or 1
+
+    local len = 0
+    for k, _ in pairs(array) do
+        if type(k) == 'number' then
+            len = math.max(len, k)
+        end
+    end
+
+    local function unpack_sparse_array_tail(array, index, len) -- luacheck: ignore
+        if index > len then
+            return
+        end
+
+        return array[index], unpack_sparse_array_tail(array, index + 1, len)
+    end
+
+    return unpack_sparse_array_tail(array, index, len)
+end
+
 return utils

test/autorequire_luatest_test.lua

@@ -104,3 +104,12 @@ end
 g.after_test('test_exec_when_luatest_not_found', function()
     g.bad_env_server:drop()
 end)
+
+g.test_exec_with_sparse_output = function()
+    local res1, res2 = g.server:exec(function()
+        return nil, 'some error'
+    end)
+
+    t.assert_equals(res1, nil)
+    t.assert_equals(res2, 'some error')
+end

test/utils_test.lua

@@ -1,3 +1,5 @@
+local json = require('json')
+
 local t = require('luatest')
 local g = t.group()
 
@@ -22,3 +24,82 @@ g.test_is_tarantool_binary = function()
                         ("Unexpected result for %q"):format(path))
     end
 end
+
+g.test_unpack_sparse_array_with_values = function()
+    local non_sparse_input = {
+        {{1, 2, 3, 4}, nil},
+        {{1, 2, 3, 4}, 3},
+    }
+
+    -- Test unpack_sparse_array() is unpack() if non-sparse input.
+    local non_sparse_cases = {}
+    for _, v in ipairs(non_sparse_input) do
+        table.insert(non_sparse_cases, {v[1], v[2], {unpack(v[1], v[2])}})
+    end
+
+    local sparse_cases = {
+        {{1, nil, 3}, nil, {1, nil, 3}},
+        {{1, nil, 3}, 2, {nil, 3}},
+        {{nil, 2, nil}, nil, {nil, 2}},
+        {{nil, 2, nil}, 2, {2}},
+        {{nil, 2, box.NULL}, nil, {nil, 2, box.NULL}},
+        {{nil, 2, box.NULL}, 3 ,{box.NULL}},
+        {{nil, nil, nil, nil, 5}, 4, {nil, 5}},
+        {{nil, nil, nil, nil, 5}, 5, {5}},
+    }
+
+    local cases = {unpack(non_sparse_cases), unpack(sparse_cases)}
+
+    for _, case in ipairs(cases) do
+        local array, index, result_packed = unpack(case)
+
+        local assert_msg
+        if index ~= nil then
+            assert_msg = ("Unexpected result for unpack_sparse_array(%q, %d)"):format(
+                json.encode(array), index)
+        else
+            assert_msg = ("Unexpected result for unpack_sparse_array(%q)"):format(
+                json.encode(array))
+        end
+
+        t.assert_equals(
+            {utils.unpack_sparse_array(array, index)},
+            result_packed,
+            assert_msg
+        )
+    end
+end
+
+local function assert_return_no_values(func, ...)
+    -- http://lua-users.org/lists/lua-l/2011-09/msg00312.html
+    t.assert_error_msg_contains(
+        "bad argument #1 to 'assert' (value expected)",
+        function(...)
+            assert(func(...))
+        end,
+        ...
+    )
+end
+
+g.test_unpack_sparse_array_no_values = function()
+    local non_sparse_cases = {
+        {{1, 2, 3, 4}, 5},
+        {{}, 1},
+    }
+
+    local sparse_cases = {
+        {{1, nil, 3}, 6},
+    }
+
+    -- Assert built-in unpack() symmetric behavior.
+    for _, case in ipairs(sparse_cases) do
+        local array, index = unpack(case)
+        assert_return_no_values(unpack, array, index)
+    end
+
+    local cases = {unpack(non_sparse_cases), unpack(sparse_cases)}
+    for _, case in ipairs(cases) do
+        local array, index = unpack(case)
+        assert_return_no_values(utils.unpack_sparse_array, array, index)
+    end
+end