Re: [Tarantool-patches] [PATCH v6 08/25] Fix luacheck warnings in test/app-tap

[Igor Munkin <imun@tarantool.org>]

Sergey,

Thanks, the patch LGTM, except the couple of comments below.

On 29.05.20, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Part of #4681
> 
> Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> Reviewed-by: Igor Munkin <imun@tarantool.org>
> 
> Co-authored-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> Co-authored-by: Igor Munkin <imun@tarantool.org>
> ---
>  .luacheckrc                                   |   3 +-
>  test/app-tap/cfg.test.lua                     |   2 +-
>  test/app-tap/clock.test.lua                   |   4 +-
>  test/app-tap/console.test.lua                 |  16 +-
>  test/app-tap/console_lua.test.lua             |  22 +-
>  test/app-tap/csv.test.lua                     |  56 +-
>  test/app-tap/debug.test.lua                   |  10 +-
>  test/app-tap/errno.test.lua                   |  24 +-
>  test/app-tap/fail_main.test.lua               |   6 +-
>  .../gh-4761-json-per-call-options.test.lua    |  11 +-
>  test/app-tap/http_client.test.lua             | 297 +++++------
>  test/app-tap/iconv.test.lua                   |   6 +-
>  test/app-tap/init_script.test.lua             |  14 +-
>  test/app-tap/inspector.test.lua               |   5 +-
>  test/app-tap/json.test.lua                    |   1 -
>  test/app-tap/logger.test.lua                  |  21 +-
>  test/app-tap/lua/serializer_test.lua          |  44 +-
>  test/app-tap/minimal.test.lua                 |   4 +-
>  test/app-tap/module_api.test.lua              |  12 +-
>  test/app-tap/msgpackffi.test.lua              |   3 +-
>  test/app-tap/pcall.test.lua                   |   6 +-
>  test/app-tap/popen.test.lua                   |  35 +-
>  test/app-tap/snapshot.test.lua                |  17 +-
>  test/app-tap/string.test.lua                  | 502 +++++++++---------
>  test/app-tap/tap.test.lua                     |  26 +-
>  test/app-tap/tarantoolctl.test.lua            |  82 ++-
>  test/app-tap/trigger.test.lua                 |  48 +-
>  test/app-tap/yaml.test.lua                    |  16 +-
>  28 files changed, 630 insertions(+), 663 deletions(-)
> 
> diff --git a/.luacheckrc b/.luacheckrc
> index 78bb54adb..630834e16 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -7,7 +7,6 @@ exclude_files = {
>      "build/**/*.lua",
>      "src/box/lua/serpent.lua", -- third-party source code
>      "test/app/*.test.lua",
> -    "test/app-tap/lua/serializer_test.lua",

This occurrence still exists on the branch.

>      "test/box/**/*.lua",
>      "test/engine/*.lua",
>      "test/engine_long/*.lua",

<snipped>

> diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua
> index 2a668f898..fdbfb3b24 100644
> --- a/test/app-tap/lua/serializer_test.lua
> +++ b/test/app-tap/lua/serializer_test.lua

<snipped>

> @@ -483,21 +473,21 @@ local function test_decode_buffer(test, s)
>      test:plan(#cases)
>  
>      for _, case in ipairs(cases) do
> -        test:test(case[1], function(test)
> -            test:plan(4)
> +        test:test(case[1], function(testcase)
> +            testcase:plan(4)
>              local args_len = table.maxn(case.args)
>              local res, res_buf = case.func(unpack(case.args, 1, args_len))
> -            test:is_deeply(res, case.exp_res, 'verify result')
> +            testcase:is_deeply(res, case.exp_res, 'verify result')
>              local buf = case.args[1]
>              local rewind = res_buf - buf
> -            test:is(rewind, case.exp_rewind, 'verify resulting buffer')
> +            testcase:is(rewind, case.exp_rewind, 'verify resulting buffer')
>              -- test:iscdata() is not sufficient here, because it

Please, adjust the comment, considering the changes you introduced
around here.

>              -- ignores 'const' qualifier (because of using
>              -- ffi.istype()).
> -            test:is(type(res_buf), 'cdata', 'verify resulting buffer type')
> +            testcase:is(type(res_buf), 'cdata', 'verify resulting buffer type')
>              local buf_ctype = tostring(ffi.typeof(buf))
>              local res_buf_ctype = tostring(ffi.typeof(res_buf))
> -            test:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype')
> +            testcase:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype')
>          end)
>      end
>  end

<snipped>

> -- 
> 2.23.0
> 

-- 
Best regards,
IM