Re: [Tarantool-patches] [PATCH luajit v2 3/5] test: introduce test:done TAP helper

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

Hi, Igor!
Thanks for the patch!
LGTM, except a minor nits regarding the commit message.

On 21.07.23, Igor Munkin wrote:
> Before the patch all tests (except <lj-603-err-snap-restore.test.lua>)

Typo: s/Before the patch/Before the patch,/

> in tarantool-tests suite terminate their execution via <os.exit> with
> the status depending on the test results. However, the second argument
> of <os.exit> was omitted and Lua universe was not properly finalized as

Typo: s/omitted and/omitted, and/

> a result. This behaviour becomes a problem, when LuaJIT is build with

Typo: s/build/built/

> LUAJIT_USE_SYSMALLOC option and AddressSanitizer support enabled, since
> the sanitizer starts reporting false positive errors about the memory
> allocations without the corresponding memory releases. To resolve these
> errors, the second parameter to each <os.exit> call terminating the test
> has to be added.
> 
> To avoid loss of the aforementioned parameter in future, <test:done>

Typo: s/in future/in the future/

> helper has been added to the TAP module. Depending on the single
> parameter, the new helper either properly finalize the test being run,
> or simply checks all the test assertions and raises an error if any of
> them fail. The latter case is added especially to handle the specifics
> of <lj-603-err-snap-restore.test.lua> and still check that everything
> works fine.
> 
> Needed for tarantool/tarantool#5878
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

> +local function done(test, exit)
> +  if exit then
> +    -- XXX: The second argument is required to properly close Lua
> +    -- universe (i.e. invoke <lua_close> before exiting).
> +    -- XXX: return is added explicitly to force CALLT emitting.

Side note: Is there a place, where it is important?

> +    return os.exit(test:check() and 0 or 1, true)
> +  else
> +    assert(test:check(), 'Test check failed')
> +  end
> +end

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun