[Tarantool-patches] [PATCH luajit v3 2/5] test: separate memprof Lua API tests into subtests

Igor Munkin imun at tarantool.org
Thu Sep 16 18:29:47 MSK 2021


Misha,

Thanks for the patch! LGTM, with few ignorable nits below.

On 20.08.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.
> 
> Part of tarantool/tarantool#5814
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/5814
> Luajit branch: https://github.com/tarantool/luajit/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
> tarantool branch: https://github.com/tarantool/tarantool/tree/shishqa/gh-5814-group-allocations-on-trace-by-trace-number
> 
>  .../misclib-memprof-lapi.test.lua             | 153 ++++++++++--------
>  1 file changed, 83 insertions(+), 70 deletions(-)
> 
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index 06d96b3b..dbf384ed 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua

<snipped>

> +-- Test profiler API.
> +test:test("base", function(subtest)

Minor: The popular name for this is not "base", but "smoke". Basic test
should also validate the output, but the smoke one can just check that
everything looks or seems to be OK. Anyway, feel free to ignore.

> +  subtest:plan(6)
>  

<snipped>

> +  -- Check allocation reports. The second argument is a line number

Typo: Comment exceeds 66 symbols (however, I'm not sure this is still
required in our guidelines; if it's not, then feel free to ignore).

> +  -- of the allocation event itself. The third is a line number of
> +  -- the corresponding function definition. The last one is
> +  -- the number of allocations.

<snipped>

> +  -- We need to cause stack resize for local variables at function
> +  -- call. Let's create a new coroutine (all slots are free).
> +  -- It has 1 slot for dummy frame + 39 free slots + 5 extra slots
> +  -- (so-called red zone) + 2 * LJ_FR2 slots. So 50 local variables

Typo: Comment exceeds 66 symbols (however, I'm not sure this is still
required in our guidelines; if it's not, then feel free to ignore).

> +  -- is enough.

<snipped>

> -- 
> 2.32.0
> 

-- 
Best regards,
IM


More information about the Tarantool-patches mailing list