Re: [Tarantool-patches] [PATCH luajit 2/3] test: make utils.selfrun usage easier

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Igor,

Thanks for the patch!

LGTM, except a few nits below.

Nit: I suggest to use `` for codenames mentions (if it is a function, then use
brackets, for example `function_name()`), and <> for filenames mentions
in the commit message and comments in the code.
This style has already been used for lua-Harness suite.
Thoughts?

On 05.04.21, Igor Munkin wrote:
> Previous implementation of <utils.selfrun> implicitly obliges the
> developer to pass the arguments to the test script for the second run.
> This has been unnoticed, since the existing tests are invoked for two
> different cases, so the script arguments were kinda obligatory, but they
> are not required in a general case.
> 
> As a result the function signals the testing system that the script is
> being run the second time via TEST_SELFRUN environment variable.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .../gh-4427-ffi-sandwich.test.lua             | 51 +++++++++----------
>  .../lj-flush-on-trace.test.lua                | 51 +++++++++----------
>  test/tarantool-tests/utils.lua                |  7 +++
>  3 files changed, 57 insertions(+), 52 deletions(-)
> 
> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index 56363608..64df5dbd 100644
> --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> @@ -1,31 +1,30 @@

<skipped>

> +----- Test payload. ------------------------------------------------------------

Typo? comment length is too long, isn't it?
But, in other side, we already have the 80-symbols comments inside
<lj_memprof.c>...
But, we have no strict code style for LuaJIT part of the code. So:
Feel free to ignore.

>  
>  local cfg = {
>    hotloop = arg[1] or 1,
> diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> index 46fee533..edc5cf61 100644
> --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
> @@ -1,31 +1,30 @@

<skipped>

> +----- Test payload. ------------------------------------------------------------

Ditto.

>  
>  local cfg = {
>    hotloop = arg[1] or 1,
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index aebbf6ac..d2dd71b0 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua
> @@ -14,6 +14,12 @@ local function luacmd(args)
>  end
>  
>  function M.selfrun(arg, checks)
> +  -- If TEST_SELFRUN is set, just execute the test payload below
> +  -- <selfrun> call, ...
> +  if os.getenv('TEST_SELFRUN') then return end

Typo? should we avoid inline return, shouldn't we?

> +
> +  -- ... otherwise run this chunk via <io.popen>.
> +
>    local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
>  
>    test:plan(#checks)
> @@ -28,6 +34,7 @@ function M.selfrun(arg, checks)
>    local cmd = string.gsub('LUA_PATH="<PATH>/?.lua;$LUA_PATH" ' ..
>                            'LUA_CPATH="<PATH>/?.<SUFFIX>;$LUA_CPATH" ' ..
>                            'LD_LIBRARY_PATH=<PATH>:$LD_LIBRARY_PATH ' ..
> +                          'TEST_SELFRUN=1' ..
>                            '<LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)
>  
>    for _, ch in pairs(checks) do
> -- 
> 2.25.0
> 

-- 
Best regards,
Sergey Kaplun