

<HTML><BODY><div>Hi Igor, thanks for your comments, I’ve used all of them in the code.<br><br> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Пятница, 28 февраля 2020, 21:42 +03:00 от Igor Munkin <imun@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15829153291376191889_BODY">Sasha,<br><br>Thanks for the patch. I left some comments below, please consider them.<br><br>On 27.02.20, Alexander V. Tikhonov wrote:<br>> Cleaned up the tests code:<br>> - added local definitions for the variables<br>> - added os.exit() routine at the tests finish<br>> - changed added information messages to test:ok() calls<br><br>I propose to reword the commit message the following way:<br>| test: cleanup tests code<br>|<br>| Cleaned up the tests code according to the Lua style guide:<br>| - made scoped variables local<br>| - added os.exit call at the end of the test chunk<br>| - adjusted the messages in test:ok calls<br><br>General comment: please adjust the test names regarding the changes I<br>proposed in the second patch.</div></div></div></div></blockquote></div><div>Ok.</div><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>><br>> Part of #4655<br>> ---<br>> test/fix_string_find_recording.test.lua | 6 +++---<br>> test/fold_bug_LuaJIT_505.test.lua | 7 ++++---<br>> test/fold_bug_LuaJIT_524.test.lua | 4 ++--<br>> test/gh.test.lua | 8 ++++----<br>> test/pairsmm_tarantool_4560.test.lua | 8 +++++---<br>> test/table_chain_bug_LuaJIT_494.test.lua | 23 ++++++++++++-----------<br>> test/unsink_64_kptr.test.lua | 7 ++++---<br>> 7 files changed, 34 insertions(+), 29 deletions(-)<br>><br><br><snipped><br><br>> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/fold_bug_LuaJIT_505.test.lua<br>> index 2fee069..3b7d7df 100755<br>> --- a/test/fold_bug_LuaJIT_505.test.lua<br>> +++ b/test/fold_bug_LuaJIT_505.test.lua<br>> @@ -1,8 +1,8 @@<br>> #!/usr/bin/env tarantool<br>><br>> -tap = require('tap')<br>> +local tap = require('tap')<br>><br>> -test = tap.test("505")<br>> +local test = tap.test("lj-505-fold-icorrect-behavior")<br>> test:plan(1)<br>><br>> -- Test file to demonstrate Lua fold machinery icorrect behavior, details:<br>> @@ -16,5 +16,6 @@ for _ = 1, 20 do<br>> local pos_b = string.find(value2, "b", 2, true)<br>> assert(pos_b == 2, "FAIL: position of 'b' is " .. pos_b)<br>> end<br>> +test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")<br><br>Message is absolutely non-informative. I propose to reword it:<br>| "string.find offset aritmetics wasn't broken while recording"</div></div></div></div></blockquote></div><div>Done.</div><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>> -test:ok("PASS")<br>> +os.exit(test:check() and 0 or 1)<br><br><snipped><br><br>> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/table_chain_bug_LuaJIT_494.test.lua<br>> index 06c0f0d..3007360 100755<br>> --- a/test/table_chain_bug_LuaJIT_494.test.lua<br>> +++ b/test/table_chain_bug_LuaJIT_494.test.lua<br>> @@ -1,8 +1,8 @@<br>> #!/usr/bin/env tarantool<br>><br>> -tap = require('tap')<br>> +local tap = require('tap')<br>><br>> -test = tap.test("494")<br>> +local test = tap.test("lj-494-table-chain-infinite-loop")<br>> test:plan(1)<br>><br>> -- Test file to demonstrate Lua table hash chain bugs discussed in<br>> @@ -11,19 +11,19 @@ test:plan(1)<br>> -- <a href="https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500" target="_blank">https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500</a><br>><br>> --- Plumbing<br>> -ffi = require"ffi"<br>> -ffi.cdef"char* strstr(const char*, const char*)"<br>> -strstr = ffi.C.strstr<br>> -cast = ffi.cast<br>> -str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2<br>> +local ffi = require("ffi")<br>> +ffi.cdef("char* strstr(const char*, const char*)")<br>> +local strstr = ffi.C.strstr<br>> +local cast = ffi.cast<br>> +local str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2<br>> function str_hash(s)<br>> return cast("uint32_t*", strstr(s, "")) - str_hash_offset<br>> end<br>> -table_new = require"table.new"<br>> +local table_new = require("table.new")<br>><br>> --- Prepare some objects<br>> -victims = {}<br>> -orig_hash = {}<br>> +local victims = {}<br>> +local orig_hash = {}<br>> for c in ("abcdef"):gmatch"." do<br>> v = c .. "{09add58a-13a4-44e0-a52c-d44d0f9b2b95}"<br>> victims[c] = v<br>> @@ -174,5 +174,6 @@ collectgarbage()<br>> for c, v in pairs(victims) do<br>> str_hash(v)[0] = orig_hash[c]<br>> end<br>> +test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")<br><br>Message is absolutely non-informative. I propose to reword it:<br>| "table keys collisions are resolved properly (no assertions failed)"</div></div></div></div></blockquote></div><div>Done.</div><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>> -test:ok("PASS")<br>> +os.exit(test:check() and 0 or 1)<br>> diff --git a/test/unsink_64_kptr.test.lua b/test/unsink_64_kptr.test.lua<br>> index <span class="js-phone-number">8995763</span>..a618fd2 100755<br>> --- a/test/unsink_64_kptr.test.lua<br>> +++ b/test/unsink_64_kptr.test.lua<br>> @@ -1,8 +1,8 @@<br>> #!/usr/bin/env tarantool<br>><br>> -tap = require('tap')<br>> +local tap = require('tap')<br>><br>> -test = tap.test("232")<br>> +local test = tap.test("or-232-unsink-64-kptr")<br>> test:plan(1)<br>><br>> --- From: Thibault Charbonnier <<a href="/compose?To=thibaultcha@me.com">thibaultcha@me.com</a>><br>> @@ -40,5 +40,6 @@ end<br>> for i = 1, 1000 do<br>> fn(i)<br>> end<br>> +test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")<br><br>Here is a mess with a message, it totally doesn't describe the check<br>being made. I propose the following comment:<br>| "allocation is unsunk at the trace exit (no platform failures)"</div></div></div></div></blockquote></div><div>Done.</div><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>> -test:ok("PASS")<br>> +os.exit(test:check() and 0 or 1)<br>> --<br>> 2.17.1<br>><br><br>--<br>Best regards,<br>IM</div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Alexander Tikhonov</div></div></div><div> </div></div></BODY></HTML>