Re: [Tarantool-patches] [PATCH luajit v4 1/4] test: separate memprof Lua API tests into subtests

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

Hi, Mikhail!

Thanks for the patch!

LGTM, except a few nits above.

On 29.09.21, Mikhail Shishatskiy wrote:
> As the number of memprof test cases is expected to grow,
> memprof tests are separated into subtests to encapsulate
> test cases and be able to skip some of the subtests.
> 
> Also, output generation and parsing separated into a

Typo: s/separated/are separated/

> dedicated function `generate_parsed_output`, which allows
> one to run memprof on different payloads, passing payload
> funtion as an argument.

Typo: s/payload funtion/a payload function/

> 
> Part of tarantool/tarantool#5814
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/5814
> Branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
> CI: https://github.com/tarantool/tarantool/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
> 
>  .../misclib-memprof-lapi.test.lua             | 169 ++++++++++--------
>  1 file changed, 94 insertions(+), 75 deletions(-)
> 
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index 06d96b3b..9de4bd98 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -7,7 +7,7 @@ require("utils").skipcond(

<snipped>

> +-- Test profiler API.
> +test:test("smoke", function(subtest)
> +  subtest:plan(6)
>  
> -res, err = misc.memprof.stop()
> -assert(res, err)
> +  -- Not a directory.
> +  local res, err, errno = misc.memprof.start(BAD_PATH)
> +  subtest:ok(res == nil and err:match("No such file or directory"))
> +  subtest:ok(type(errno) == "number")
>  
> --- Profiler is not running.
> -res, err, errno = misc.memprof.stop()
> -test:ok(res == nil and err:match("profiler is not running"))
> -test:ok(type(errno) == "number")
> +  -- Profiler is running.
> +  res, err = misc.memprof.start(TMP_BINFILE)

Nit: This tmp file will be rewritten and removed later by the "output"
test. Please leave the corresponding comment.

> +  assert(res, err)
> +  res, err, errno = misc.memprof.start(TMP_BINFILE)
> +  subtest:ok(res == nil and err:match("profiler is running already"))
> +  subtest:ok(type(errno) == "number")
>  

<snipped>

> -- 
> 2.33.0
> 

-- 
Best regards,
Sergey Kaplun