[Tarantool-patches] [PATCH v3 0/3] box/console: Fix multireturn and empty output
Alexander Turenko
alexander.turenko at tarantool.org
Tue Jan 21 18:16:42 MSK 2020
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 at tarantool.org>
| Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
| Reviewed-by: Alexander Turenko <alexander.turenko at 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
>
More information about the Tarantool-patches
mailing list