* [Tarantool-patches] [PATCH v3 1/3] box/console: handle multireturn in lua mode
2020-01-20 17:59 [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Cyrill Gorcunov
@ 2020-01-20 17:59 ` Cyrill Gorcunov
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 2/3] box/console: handle empty output format Cyrill Gorcunov
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-01-20 17:59 UTC (permalink / raw)
To: tml
Currently we handle only first member of multireturn statement.
Fix it processing each element separately.
n.b.: While at this file add vim settings.
| tarantool> \set output lua
| true;
| tarantool> 1,2,3,4
| 1, 2, 3, 4;
Fixes #4604
Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/lua/console.lua | 38 +++++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index d4d8ec984..a4a36c625 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -1,4 +1,6 @@
-- console.lua -- internal file
+--
+-- vim: ts=4 sw=4 et
local serpent = require('serpent')
local internal = require('console')
@@ -75,28 +77,38 @@ local serpent_map_symbols = function(tag, head, body, tail, level)
return tag..head..body..tail
end
-output_handlers["lua"] = function(status, opts, ...)
- --
- -- If no data present at least EOS should be put,
- -- otherwise wire readers won't be able to find
- -- where the end of string is.
- if not ... then
- return output_eos["lua"]
- end
+--
+-- Format a Lua value.
+local function format_lua_value(value, opts)
for k,v in pairs(lua_map_direct_symbols) do
- if k == ... then
- return v .. output_eos["lua"]
+ if k == value then
+ return v
end
end
local serpent_opts = {
custom = serpent_map_symbols,
comment = false,
- nocode = true,
+ nocode = true,
}
if opts == "block" then
- return serpent.block(..., serpent_opts) .. output_eos["lua"]
+ return serpent.block(value, serpent_opts)
+ end
+ return serpent.line(value, serpent_opts)
+end
+
+output_handlers["lua"] = function(status, opts, ...)
+ local collect = {}
+ --
+ -- If no data present at least EOS should be put,
+ -- otherwise wire readers won't be able to find
+ -- where the end of string is.
+ if not ... then
+ return output_eos["lua"]
+ end
+ for i = 1, select('#', ...) do
+ collect[i] = format_lua_value(select(i, ...), opts)
end
- return serpent.line(..., serpent_opts) .. output_eos["lua"]
+ return table.concat(collect, ', ') .. output_eos["lua"]
end
local function output_verify_opts(fmt, opts)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [Tarantool-patches] [PATCH v3 2/3] box/console: handle empty output format
2020-01-20 17:59 [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Cyrill Gorcunov
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 1/3] box/console: handle multireturn in lua mode Cyrill Gorcunov
@ 2020-01-20 17:59 ` Cyrill Gorcunov
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 3/3] test/app-tap: add console_lua test Cyrill Gorcunov
2020-01-21 15:16 ` [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Alexander Turenko
3 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-01-20 17:59 UTC (permalink / raw)
To: tml
In case if output format is not specified we should exit
with more readable error message.
Fixes #4638
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
src/box/lua/console.lua | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index a4a36c625..17e2c91b2 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -126,6 +126,9 @@ end
local function parse_output(value)
local fmt, opts
+ if not value then
+ return 'Specify output format: lua or yaml.'
+ end
if value:match("([^,]+),([^,]+)") ~= nil then
fmt, opts = value:match("([^,]+),([^,]+)")
else
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [Tarantool-patches] [PATCH v3 3/3] test/app-tap: add console_lua test
2020-01-20 17:59 [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Cyrill Gorcunov
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 1/3] box/console: handle multireturn in lua mode Cyrill Gorcunov
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 2/3] box/console: handle empty output format Cyrill Gorcunov
@ 2020-01-20 17:59 ` Cyrill Gorcunov
2020-01-21 15:16 ` [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Alexander Turenko
3 siblings, 0 replies; 6+ messages in thread
From: Cyrill Gorcunov @ 2020-01-20 17:59 UTC (permalink / raw)
To: tml
Test multireturn and empty output in lua mode.
Co-developed-by: Alexander Turenko <alexander.turenko@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
test/app-tap/console_lua.test.lua | 141 ++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
create mode 100755 test/app-tap/console_lua.test.lua
diff --git a/test/app-tap/console_lua.test.lua b/test/app-tap/console_lua.test.lua
new file mode 100755
index 000000000..5848bdf69
--- /dev/null
+++ b/test/app-tap/console_lua.test.lua
@@ -0,0 +1,141 @@
+#!/usr/bin/env tarantool
+--
+-- vim: ts=4 sw=4 et
+
+local console = require('console')
+local socket = require('socket')
+local tap = require('tap')
+local fio = require('fio')
+
+local EOL = ';'
+local CONSOLE_SOCKET = 'console-lua.sock'
+
+--
+-- Set Lua output mode.
+local function set_lua_output(client, opts)
+ local opts = opts or {}
+ local mode = opts.block and 'lua,block' or 'lua'
+ client:write(('\\set output %s\n'):format(mode))
+ assert(client:read(EOL), 'true' .. EOL, 'set lua output mode')
+end
+
+--
+-- Start console and setup a client.
+local function start_console()
+ -- Make sure not stale sockets are
+ -- left from previous runs.
+ fio.unlink(CONSOLE_SOCKET)
+
+ local server = console.listen('unix/:./' .. CONSOLE_SOCKET)
+ assert(server ~= nil, 'console.listen started')
+
+ local client = socket.tcp_connect('unix/', CONSOLE_SOCKET)
+ assert(client ~= nil, 'connect to console')
+ local handshake = client:read({chunk = 128})
+ assert(string.match(handshake, '^Tarantool .*console') ~= nil, 'handshake')
+
+ -- Switch to Lua output mode.
+ set_lua_output(client, {block = false})
+ return server, client
+end
+
+--
+-- Disconnect from console and stop it.
+local function stop_console(server, client)
+ client:close()
+ server:close()
+
+ local new_client = socket.tcp_connect('unix/', CONSOLE_SOCKET)
+ assert(new_client == nil, 'console.listen stopped')
+end
+
+--
+-- Give `{x}` for a scalar `x`, just `x` otherwise.
+local function totable(x)
+ return type(x) == 'table' and x or {x}
+end
+
+--
+-- Execute a list of statements, show requests and responses.
+local function execute_statements(test, client, statements, name)
+ test:test(name, function(test)
+ test:plan(2 * #statements)
+
+ for _, stmt in ipairs(statements) do
+ local request = stmt .. '\n'
+ local res = client:write(request)
+ test:ok(res ~= nil, ('-> [[%s]]'):format(request:gsub('\n', '\\n')))
+
+ local response = client:read(EOL)
+ test:ok(response ~= nil and response:endswith(EOL),
+ ('<- [[%s]]'):format(response))
+ end
+ end)
+end
+
+--
+-- Execute a statement and verify its response.
+local function execute_and_verify(test, client, input, exp_output, name)
+ test:test(name, function(test)
+ test:plan(2)
+
+ local res = client:write(input .. '\n')
+ test:ok(res ~= nil, ('-> [[%s]]'):format(input))
+
+ local exp = exp_output .. EOL
+ local res = client:read(EOL)
+ test:is(res, exp, ('<- [[%s]]'):format(exp:gsub('\n', '\\n')))
+ end)
+end
+
+--
+-- Test cases table:
+-- @name: Name of the test, mandatory
+-- @prepare: Preparation script, optional
+-- @opts: Console options, optional
+-- @input: Input statement to execute, mandatory
+-- @expected: Expected results to compare with, mandatory
+-- @cleanup: Cleanup script to run after the test, optional
+local cases = {
+ {
+ name = 'multireturn line mode',
+ prepare = 'a = {1, 2, 3}',
+ opts = {block = false},
+ input = '1, 2, nil, a',
+ expected = '1, 2, box.NULL, {1, 2, 3}',
+ cleanup = 'a = nil',
+ }, {
+ name = 'multireturn block mode',
+ prepare = 'a = {1, 2, 3}',
+ opts = {block = true},
+ input = '1, 2, nil, a',
+ expected = '1, 2, box.NULL, {\n 1,\n 2,\n 3\n}',
+ cleanup = 'a = nil',
+ }, {
+ name = 'empty output',
+ input = '\\set output',
+ expected = '"Specify output format: lua or yaml."',
+ }
+}
+
+local test = tap.test('console-lua')
+test:plan(#cases)
+
+local server, client = start_console()
+
+for _, case in ipairs(cases) do
+ test:test(case.name, function(test)
+ test:plan(3)
+
+ execute_statements(test, client, totable(case.prepare), 'prepare')
+
+ set_lua_output(client, case.opts)
+ execute_and_verify(test, client, case.input, case.expected, 'run')
+
+ execute_statements(test, client, totable(case.cleanup), 'cleanup')
+ end)
+end
+
+stop_console(server, client)
+
+os.exit(test:check() and 0 or 1)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output
2020-01-20 17:59 [Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output Cyrill Gorcunov
` (2 preceding siblings ...)
2020-01-20 17:59 ` [Tarantool-patches] [PATCH v3 3/3] test/app-tap: add console_lua test Cyrill Gorcunov
@ 2020-01-21 15:16 ` Alexander Turenko
2020-01-21 15:27 ` Cyrill Gorcunov
3 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko @ 2020-01-21 15:16 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: tml
Thanks!
There are several missed comments.
Cited from [1]:
> > From previous review (see [1]):
>
> > I propose to add a case with nils at the end as well as simple 1, 2, 3
> > multireturn.
>
> [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2019-December/013326.html
>
> I meant '1, nil, nil, nil\n' or kinda. Your implementation (after the patch)
> works correctly: it does not cut trailing nils. I don't sure whether it is part
> of our contract, to be honest. But while we're tentative it is good to keep
> this property and add the test case.
Added:
diff --git a/test/app-tap/console_lua.test.lua b/test/app-tap/console_lua.test.lua
index 5848bdf69..3ed6aad97 100755
--- a/test/app-tap/console_lua.test.lua
+++ b/test/app-tap/console_lua.test.lua
@@ -111,6 +111,16 @@ local cases = {
input = '1, 2, nil, a',
expected = '1, 2, box.NULL, {\n 1,\n 2,\n 3\n}',
cleanup = 'a = nil',
+ }, {
+ name = 'trailing nils, line mode',
+ opts = {block = false},
+ input = '1, nil, nil, nil',
+ expected = '1, box.NULL, box.NULL, box.NULL',
+ }, {
+ name = 'trailing nils, block mode',
+ opts = {block = true},
+ input = '1, nil, nil, nil',
+ expected = '1, box.NULL, box.NULL, box.NULL',
}, {
name = 'empty output',
input = '\\set output',
Cited from [2]:
> > test/app-tap: add console_lua test
>
> Nit: we often use 'test:' prefix and it is useful for searching /
> excluding of such commits. I would use it here too.
>
> >
> > Test multireturn and empty output in lua mode.
>
> Nit: I would say "lack of a parameter of '\set output' command" or at
> least "empty output format", because 'empty output' is hard to
> understand w/o looking into previous commits.
Changed to:
| test: add app-tap/console_lua test
|
| Test multireturn in lua output mode and lack of a parameter in '\set
| output <...>' command.
|
| Co-developed-by: Alexander Turenko <alexander.turenko@tarantool.org>
| Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| Reviewed-by: Alexander Turenko <alexander.turenko@tarantool.org>
I don't think that a second review is necessary for those patches, they are
simple.
Pushed to master, 2.3, 2.2. CCed Kirill.
[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013595.html
[2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-January/013672.html
WBR, Alexander Turenko.
On Mon, Jan 20, 2020 at 08:59:47PM +0300, Cyrill Gorcunov wrote:
> In v3 nits for tests are fixed.
>
> The issues are
>
> https://github.com/tarantool/tarantool/issues/4604
> https://github.com/tarantool/tarantool/issues/4638
>
> Branch
>
> gorcunov/console-fixes-3
>
> Cyrill Gorcunov (3):
> box/console: handle multireturn in lua mode
> box/console: handle empty output format
> test/app-tap: add console_lua test
>
> src/box/lua/console.lua | 41 ++++++---
> test/app-tap/console_lua.test.lua | 141 ++++++++++++++++++++++++++++++
> 2 files changed, 169 insertions(+), 13 deletions(-)
> create mode 100755 test/app-tap/console_lua.test.lua
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread