Re: [Tarantool-patches] [PATCH v7 1/3] luacheck: fix warnings in test/app-tap

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi! Thanks for review!

On 20.12.2020 16:59, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> See 5 comments below.
>
>> diff --git a/.luacheckrc b/.luacheckrc
>> index 29db9eeeb..ae96576ec 100644
>> --- a/.luacheckrc
>> +++ b/.luacheckrc
>> @@ -102,3 +103,9 @@ files["src/box/lua/console.lua"] = {
>>           "212",
>>       }
>>   }
>> +files["test/app-tap/lua/require_mod.lua"] = {
>> +    globals = {"exports"}
> 1. In require_mod.lua the variable 'exports' should be declared 'local'.
> It is never used out of this file.

Fixed in a branch and force-pushed.

--- a/test/app-tap/lua/require_mod.lua
+++ b/test/app-tap/lua/require_mod.lua
@@ -1,4 +1,4 @@
-exports = {}
+local exports = {}

  function exports.test(a, b)
    return a+b

--- a/.luacheckrc
+++ b/.luacheckrc
@@ -1,5 +1,5 @@
  std = "luajit"
-globals = {"box", "_TARANTOOL", "tonumber64"}
+globals = {"box", "_TARANTOOL", "tonumber64", "utf8"}
  ignore = {
      -- Accessing an undefined field of a global variable <debug>.
      "143/debug",
@@ -102,12 +102,6 @@ files["src/box/lua/console.lua"] = {
          "212",
      }
  }
-files["test/app-tap/lua/require_mod.lua"] = {
-    globals = {"exports"}
-}
  files["test/app/lua/fiber.lua"] = {
      globals = {"box_fiber_run_test"}
  }

>
>> +}
>> +files["test/app-tap/string.test.lua"] = {
>> +    globals = {"utf8"}
> 2. utf8 is global everywhere, it is defined by tarantool executable.
> It should be in the root 'globals' list, among 'box', '_TARANTOOL', etc.

I think it was suppressed for single file because it is now used only there.

Fixed in a branch and force-pushed.

--- a/.luacheckrc
+++ b/.luacheckrc
@@ -102,12 +102,6 @@ files["src/box/lua/console.lua"] = {
          "212",
      }
  }
-files["test/app-tap/string.test.lua"] = {
-    globals = {"utf8"}
-}
  files["test/app/lua/fiber.lua"] = {
      globals = {"box_fiber_run_test"}
  }

>
>> diff --git a/test/app-tap/gh-4761-json-per-call-options.test.lua b/test/app-tap/gh-4761-json-per-call-options.test.lua
>> index 1fb24744e..c6b31ba61 100755
>> --- a/test/app-tap/gh-4761-json-per-call-options.test.lua
>> +++ b/test/app-tap/gh-4761-json-per-call-options.test.lua
>> @@ -25,7 +25,7 @@ local res = tap.test('gh-4761-json-per-call-options', function(test)
>>   
>>       -- Same check for json.encode.
>>       local nan = 1/0
>> -    local ok, err = pcall(json.encode, {a = nan},
>> +    local ok = pcall(json.encode, {a = nan},
>>                             {encode_invalid_numbers = false})
> 3. This line is misaligned now. The same above.

Fixed in a branch and force-pushed.

--- a/test/app-tap/gh-4761-json-per-call-options.test.lua
+++ b/test/app-tap/gh-4761-json-per-call-options.test.lua
@@ -14,7 +14,7 @@ local res = tap.test('gh-4761-json-per-call-options', 
function(test)

      -- Preparation code: call :decode() with a custom option.
      local ok = pcall(json.decode, '{"foo": {"bar": 1}}',
-                          {decode_max_depth = 1})
+                     {decode_max_depth = 1})
      assert(not ok, 'expect "too many nested data structures" error')

      -- Verify that the instance option remains unchanged.
@@ -26,7 +26,7 @@ local res = tap.test('gh-4761-json-per-call-options', 
function(test)
      -- Same check for json.encode.
      local nan = 1/0
      local ok = pcall(json.encode, {a = nan},
-                          {encode_invalid_numbers = false})
+                     {encode_invalid_numbers = false})
      assert(not ok, 'expected "number must not be NaN or Inf" error')
      local exp_res = '{"a":inf}'
      local ok, res = pcall(json.encode, {a = nan})

>
>>       assert(not ok, 'expected "number must not be NaN or Inf" error')
>>       local exp_res = '{"a":inf}'
>> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
>> index 9f1464617..44357484b 100755
>> --- a/test/app-tap/tarantoolctl.test.lua
>> +++ b/test/app-tap/tarantoolctl.test.lua
>> @@ -97,23 +96,14 @@ local function tctl_wait_start(dir, name)
>>               fiber.sleep(0.01)
>>           end
>>           ::again::
>> -        while true do
>> -            local stat, nb = pcall(require('net.box').new, path, {
>> -                wait_connected = true, console = true
>> -            })
>> -            if stat == false then
>> -                fiber.sleep(0.01)
>> -                goto again
>> -            else
>> -                break
>> -            end
>> -            local stat, msg = pcall(nb.eval, nb, 'require("fiber").time()')
>> -            if stat == false then
>> -                fiber.sleep(0.01)
>> -            else
>> -                break
>> -            end
>> +        local stat, _ = pcall(require('net.box').new, path, {
>> +            wait_connected = true, console = true
>> +        })
>> +        if stat == false then
>> +            fiber.sleep(0.01)
>> +            goto again
>>           end
>> +        return
> 4. 'return' is not necessary.

Agree, fixed in a branch and force-pushed.

--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -103,7 +103,6 @@ local function tctl_wait_start(dir, name)
              fiber.sleep(0.01)
              goto again
          end
-        return
      end
  end


>>       end
>>   end
>> diff --git a/test/app-tap/trigger.test.lua b/test/app-tap/trigger.test.lua
>> index a31d45e5f..6cf3fbe53 100755
>> --- a/test/app-tap/trigger.test.lua
>> +++ b/test/app-tap/trigger.test.lua
>> @@ -45,7 +45,7 @@ test:test("simple trigger test", function(test)
>>   
>>   
>>       -- Check that we've failed to delete trigger
>> -    local stat, err = pcall(getmetatable(trigger_list).__call, trigger_list,
>> +    local _, err = pcall(getmetatable(trigger_list).__call, trigger_list,
>>                               nil, trigger_cnt)
> 5. This line became misaligned.

Fixed in a branch and force-pushed.

--- a/test/app-tap/trigger.test.lua
+++ b/test/app-tap/trigger.test.lua
@@ -46,7 +46,7 @@ test:test("simple trigger test", function(test)

      -- Check that we've failed to delete trigger
      local _, err = pcall(getmetatable(trigger_list).__call, trigger_list,
-                            nil, trigger_cnt)
+                         nil, trigger_cnt)
      test:ok(string.find(err, "is not found"), "check error")
  end)