Hi, Sergey!

The second testcase has been added and

changes force-pushed to the branch.

Sergey

Hi, Sergey!
Thanks for the fixes, LGTM.

On 27.02.25, Sergey Bronnikov wrote:

Hi, Sergey,

thanks for review!

Changes applied and force-pushed.

Sergey

On 26.02.2025 14:58, Sergey Kaplun wrote:

<snipped>

What is a rationale for this?

As I said before, this is an additional possible demonstration of the
bug. I see nothing bad to add it as well, since it simulates a little
bit different workload -- flushing the output before we finish the
process or if we unload the module. I am not insisting on it, though.

How often users unload profiler's module? For me, this case looks artificial

The following module [1] is used to reload all Lua modules (including
`jit.p`). It was used for any Lua package update or code modification.

[1]: https://github.com/moonlibs/package-reload

Also, it is not only about unloading the module but also about finishing
the process too (as you first discovered). This looks like a real use
case. This variant of the test is just simpler than creating a child
process and checking its result files.

and the final goal for us is not a demonstration, but covering a code 
touched by the patch,

it is done by proposed test.

(I'm really confused that on review I learn more and more new 
requirements to the patches

It is not a requirement, just a suggestion. See the last line of my
comment. I'm OK with reproducing the issue only since it is not the
vital part of the code.

like adding all available testcases that cover a patch or that code 
should follow C89 or

that using etc.)

The bug fixed by the patch is quite minor, it will not break production 
and will not kill humans etc.

I believe a single testcase and time that we both spend on doing 
backport, test and two iterations

of review is more than enough for this patch. Moreover, jit.p is not 
used by Tarantool users,

we provide our own profilers to them.

So, just ignore it then.

This is a helper script for LuaJIT profiler and according to our
backporting rules

test must cover a backported patch. This rule is fulfilled by my test.

<snipped>