From c8b69bf84116d377f8229c57596a9bd24e61f00b Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 19 Jan 2024 12:21:45 +0300 Subject: [PATCH] server: restore exec() sparse output support 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. https://github.com/tarantool/luatest/commit/e6a2093 Closes #350 --- CHANGELOG.md | 1 + luatest/server.lua | 19 ++++++-- luatest/utils.lua | 22 +++++++++ test/autorequire_luatest_test.lua | 9 ++++ test/utils_test.lua | 81 +++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 038c52e6..ad887d77 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/luatest/server.lua b/luatest/server.lua index 193b8951..8ea99d0d 100644 --- a/luatest/server.lua +++ b/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) diff --git a/luatest/utils.lua b/luatest/utils.lua index 38375daf..4f7238df 100644 --- a/luatest/utils.lua +++ b/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 diff --git a/test/autorequire_luatest_test.lua b/test/autorequire_luatest_test.lua index a7bf1bc8..b2f80835 100644 --- a/test/autorequire_luatest_test.lua +++ b/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 diff --git a/test/utils_test.lua b/test/utils_test.lua index d9312b2e..94be856d 100644 --- a/test/utils_test.lua +++ b/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