Re: [Tarantool-patches] [PATCH v2 luajit 2/5] test: adjust lua-Harness suite for LuaJIT

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

Thanks for the patch!

This is why it's so important to take the suite intact at first: I
didn't even notice these changes in the previous series and now I regret
a bit regarding the solution with LUAJIT_TEST_INIT: for this case it
occurs to be ugly a little. Nevertheless, I believe Francois would be
interested in this use case, so it's worth to file a bug against
lua-Harness, IMHO.

On 15.03.21, Sergey Kaplun wrote:
> LUAJIT_TEST_COMMAND always extends an amount of line argument by two
> causing child test failures.

I don't get the line above. I guess you mind something similar to this:
| LUAJIT_TEST_COMMAND always extends <arg> table containing command line
| arguments by two. It leads to child test failures, since Lua
| interpreter name is misfetched from the table.

> So, a new universal way to detect program name is required.
> 
> This patch introduces the new function `get_lua_binary_name()` inside
> lua-Harness tap.lua module to get binary for test execution correctly.

Side note: I doubt the function location is right, but I guess there is
no other place that is required by all test chunks.

> 
> The following tests are adjusted with the new function:
> * 241-standalone.t
> * 242-luac.t
> * 301-basic.t
> * 308-io.t
> * 309-os.t
> * 320-stdin.t
> * 411-luajit.t
> 
> Part of tarantool/tarantool#5844
> Part of tarantool/tarantool#4473
> ---
>  test/lua-Harness-tests/241-standalone.t | 2 +-
>  test/lua-Harness-tests/242-luac.t       | 2 +-
>  test/lua-Harness-tests/301-basic.t      | 2 +-
>  test/lua-Harness-tests/308-io.t         | 2 +-
>  test/lua-Harness-tests/309-os.t         | 2 +-
>  test/lua-Harness-tests/320-stdin.t      | 2 +-
>  test/lua-Harness-tests/411-luajit.t     | 2 +-
>  test/lua-Harness-tests/tap.lua          | 9 +++++++++
>  8 files changed, 16 insertions(+), 7 deletions(-)
> 

<snipped>

> diff --git a/test/lua-Harness-tests/tap.lua b/test/lua-Harness-tests/tap.lua
> index 08a99b8..5ce95e6 100644
> --- a/test/lua-Harness-tests/tap.lua
> +++ b/test/lua-Harness-tests/tap.lua
> @@ -195,6 +195,15 @@ function todo (reason, count)
>      todo_reason = reason
>  end
>  
> +-- The last arg element is guaranteed name of tested binary.

Typo: s/last/least/.
Typo: s/guaranteed name of tested/guaranteed to be the name of the tested/.

> +function get_lua_binary_name ()
> +    local i = 0
> +    while arg[i] do

You make an excess lookup to a _G for each iteration. It's better to
pass <arg> as an argument to this function. Furthermore, it makes the
signature clearer, so user knows you're using <arg> to detect the
interpreter name.

> +        i = i - 1
> +    end
> +    return arg[i + 1]
> +end
> +
>  --
>  -- Copyright (c) 2009-2018 Francois Perrad
>  --
> -- 
> 2.28.0
> 

-- 
Best regards,
IM