<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey!</p>
<p>The second testcase has been added and</p>
<p>changes force-pushed to the branch.</p>
<p>Sergey<br>
</p>
<div class="moz-cite-prefix">On 04.03.2025 18:40, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite" cite="mid:Z8cfAQZreaGMs76J@root">
<pre wrap="" class="moz-quote-pre">Hi, Sergey!
Thanks for the fixes, LGTM.
On 27.02.25, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">Hi, Sergey,
thanks for review!
Changes applied and force-pushed.
Sergey
On 26.02.2025 14:58, Sergey Kaplun wrote:
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">What is a rationale for this?
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">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.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
How often users unload profiler's module? For me, this case looks artificial
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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]: <a class="moz-txt-link-freetext" href="https://github.com/moonlibs/package-reload">https://github.com/moonlibs/package-reload</a>
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.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">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
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
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.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
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.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
So, just ignore it then.
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">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.
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre"><snipped>
</pre>
</blockquote>
</blockquote>
<pre wrap="" class="moz-quote-pre">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>