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

[Sergey Bronnikov <sergeyb@tarantool.org>]

Igor,

thanks for review.

On 14:41 Mon 01 Jun , Igor Munkin wrote:
> 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.

removed in a branch [1]

1. ligurio/gh-4681-fix-luacheck-warnings-test

> >      "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.

adjusted:

             local rewind = res_buf - buf
-            test:is(rewind, case.exp_rewind, 'verify resulting buffer')
-            -- test:iscdata() is not sufficient here, because it
+            testcase:is(rewind, case.exp_rewind, 'verify resulting buffer')
+            -- testcase:iscdata() is not sufficient here, because it
             -- ignores 'const' qualifier (because of using
             -- ffi.istype()).


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