Hi, Sergey! The second testcase has been added and changes force-pushed to the branch. Sergey On 04.03.2025 18:40, Sergey Kaplun wrote: > 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: > > >>>> 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. >>> >>>