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

[Igor Munkin <imun@tarantool.org>]

Masha,

Thanks for the patch! I left some nits below, please consider them.
Otherwise, LGTM.

On 27.01.20, Maria Khaydich wrote:
> 
> Thank you for the review! All fixes below
>  
> ----------------------------------------------------------------------
> Branch:
> https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type
> Issue:
> https://github.com/tarantool/tarantool/issues/4513
>  
>  src/box/lua/net_box.lua                       |  8 +++-
>  test/app-tap/debug.result                     |  8 ++--
>  test/box-tap/net.box.test.lua                 |  3 +-
>  test/box/engine.cfg                           |  6 +++
>  ...ox-self-and-connect-interchangeable.result | 44 +++++++++++++++++++
>  ...-self-and-connect-interchangeable.test.lua | 20 +++++++++
>  test/box/suite.ini                            |  1 +
>  7 files changed, 83 insertions(+), 7 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

<snipped>

> diff --git a/test/box-tap/net.box.test.lua b/test/box-tap/net.box.test.lua
> index a46f28ad0..577e97d78 100755
> --- a/test/box-tap/net.box.test.lua
> +++ b/test/box-tap/net.box.test.lua
> @@ -30,5 +30,4 @@ test:ok(not pcall(function() conn.space._vspace:insert() end), "error handling")
>  -- cleanup
>  conn:close()
>  inspector:cmd('stop server second with cleanup=1')
> -test:check()
> -os.exit(0)
> +os.exit(test:check() and 0 or 1)

The changes are fine but look like not related to the subj.

> diff --git a/test/box/engine.cfg b/test/box/engine.cfg

<snipped>

> 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..5e8ae7465
> --- /dev/null
> +++ b/test/box/gh-4513-netbox-self-and-connect-interchangeable.test.lua
> @@ -0,0 +1,20 @@
> +netbox = require('net.box')
> +test_run = require('test_run').new()
> +remote = test_run:get_cfg('remote') == 'true'
> +test_run:cmd("setopt delimiter ';'")
> +
> +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
> +--
> +test_run:cmd("setopt delimiter ''");

Minor: The line above relates to the nb initialization, so it's better
been placed before the comment. It's totally minor, feel free to ignore.

> +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

<snipped>

> -- 
> 2.24.0 

<snipped>

-- 
Best regards,
IM