<HTML><BODY><div>Hi, Igor!</div><div>Thanks for the patch!</div><div>LGTM, except for the nits Sergey has already mentioned</div><div>and the two additional nits below.</div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16699761381559810790_BODY">Sometimes TAP functions become hot spots for JIT compiler and the</div></div></div></div></blockquote><div>Typo: s/for JIT compiler/for the JIT compiler/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>corresponding traces spoils test assertions with their side effects.<br>To avoid such misbehaviour and fix fragile test, <misc.getmetrics> is<br>explicitly called in this particular case. There is no need to fix the<br>corresponding test for Lua C API interface, since there is no TAP<br>helpers used to check whether <jit_snap_restore> is counted right. There<br>is also no need to fix other subtests nearby, since their assertions are<br>no affected by TAP helpers side effects.</div></div></div></div></blockquote><div>Typo: s/no/not/</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>Relates to tarantool/tarantool#7762<br><br>Signed-off-by: Igor Munkin <<a href="/compose?To=imun@tarantool.org">imun@tarantool.org</a>><br>---<br> test/tarantool-tests/misclib-getmetrics-lapi.test.lua | 4 +++-<br> 1 file changed, 3 insertions(+), 1 deletion(-)<br><br>diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>index 19dfd199..0c170d0c 100644<br>--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua<br>@@ -363,7 +363,9 @@ test:test("snap-restores-scalar", function(subtest)<br> -- No exits triggering snap restore so far: snapshot<br> -- restoration was inlined into the machine code.<br> subtest:is(new_metrics.jit_snap_restore - old_metrics.jit_snap_restore, 0)<br>- old_metrics = new_metrics<br>+ -- XXX: obtain actual metrics to avoid side effects that are<br>+ -- caused by Lua code and JIT engine fine tuning above.<br>+ old_metrics = misc.getmetrics()<br> <br> -- Simply 2 side exits from the trace:<br> foo(20)<br>--<br>2.34.0</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></div></blockquote></BODY></HTML>