[Tarantool-patches] [PATCH v1 0/2] Correct luajit tests

[Tarantool-patches] [PATCH v1 0/2] Correct luajit tests

[Alexander V. Tikhonov]

    test: fix style and code
    
    Renamed test files in the following way:
    [gh-[external repo]-<number>-]comment.test.lua
    where "external repo" is github repository:
      lj - luajit/luajit
      lrc - openresty/lua-resty-core
    
    Added local definitions for the variables, added os.exit()
    routine at the tests finish. Changed added information messages
    to test:ok() calls.
    
    Close #4655

Github: https://github.com/tarantool/luajit/tree/avtikhon/gh-4655-cleanup-tests
Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4655-cleanup-tests
Issue: https://github.com/tarantool/tarantool/issues/4655

Alexander V. Tikhonov (2):
  test: cleanup tests code
  test: rename test files

 test/fix_string_find_recording.test.lua       |  6 ++---
 ... gh-3196-incorrect-string-length.test.lua} |  8 +++----
 ...st.lua => gh-4560-pairsmm-is-set.test.lua} |  8 ++++---
 ...t.lua => gh-lj-494-infinite-loop.test.lua} | 23 ++++++++++---------
 ...gh-lj-505-fold-icorrect-behavior.test.lua} |  7 +++---
 ...gh-lj-524-fold-icorrect-behavior.test.lua} |  2 +-
 ....lua => gh-lrc-64-unsink-64-kptr.test.lua} |  7 +++---
 7 files changed, 33 insertions(+), 28 deletions(-)
 rename test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua} (60%)
 rename test/{pairsmm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} (90%)
 rename test/{table_chain_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} (90%)
 rename test/{fold_bug_LuaJIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} (75%)
 rename test/{fold_bug_LuaJIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} (94%)
 rename test/{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} (88%)

-- 
2.17.1

[Tarantool-patches] [PATCH v1 1/2] test: cleanup tests code

[Alexander V. Tikhonov]

Added local definitions for the variables, added os.exit()
routine at the tests finish. Changed added information messages
to test:ok() calls.

Part of #4655
---
 test/fix_string_find_recording.test.lua  |  6 +++---
 test/fold_bug_LuaJIT_505.test.lua        |  7 ++++---
 test/fold_bug_LuaJIT_524.test.lua        |  2 +-
 test/gh.test.lua                         |  8 ++++----
 test/pairsmm_tarantool_4560.test.lua     |  8 +++++---
 test/table_chain_bug_LuaJIT_494.test.lua | 23 ++++++++++++-----------
 test/unsink_64_kptr.test.lua             |  7 ++++---
 7 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/test/fix_string_find_recording.test.lua b/test/fix_string_find_recording.test.lua
index d3fc9e1..911e611 100755
--- a/test/fix_string_find_recording.test.lua
+++ b/test/fix_string_find_recording.test.lua
@@ -1,8 +1,8 @@
 #!/usr/bin/env tarantool
 
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("fix-string-find-recording")
+local test = tap.test("fix-string-find-recording")
 test:plan(1)
 
 local err = [[module 'kit.1.10.3-136' not found:
@@ -76,4 +76,4 @@ until not e
 
 test:is(count_vm, count_jit)
 
-test:check()
+os.exit(test:check() and 0 or 1)
diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/fold_bug_LuaJIT_505.test.lua
index 2fee069..433efb0 100755
--- a/test/fold_bug_LuaJIT_505.test.lua
+++ b/test/fold_bug_LuaJIT_505.test.lua
@@ -1,8 +1,8 @@
 #!/usr/bin/env tarantool
 
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("505")
+local test = tap.test("505")
 test:plan(1)
 
 -- Test file to demonstrate Lua fold machinery icorrect behavior, details:
@@ -16,5 +16,6 @@ for _ = 1, 20 do
     local pos_b = string.find(value2, "b", 2, true)
     assert(pos_b == 2, "FAIL: position of 'b' is " .. pos_b)
 end
+test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")
 
-test:ok("PASS")
+os.exit(test:check() and 0 or 1)
diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/fold_bug_LuaJIT_524.test.lua
index b056684..c8cc7b0 100755
--- a/test/fold_bug_LuaJIT_524.test.lua
+++ b/test/fold_bug_LuaJIT_524.test.lua
@@ -21,4 +21,4 @@ end
 
 test:is(tonumber(sq), math.fmod(math.pow(42, 8), math.pow(2, 32)))
 
-test:check()
+os.exit(test:check() and 0 or 1)
diff --git a/test/gh.test.lua b/test/gh.test.lua
index 00b71a6..2804d35 100755
--- a/test/gh.test.lua
+++ b/test/gh.test.lua
@@ -1,17 +1,17 @@
 #!/usr/bin/env tarantool
 
 -- Miscellaneous test for LuaJIT bugs
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("gh")
+local test = tap.test("gh")
 test:plan(2)
 --
 -- gh-3196: incorrect string length if Lua hash returns 0
 --
-h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
+local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
 test:is(h:len(), 20)
 
 h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
 test:is(h:len(), 20)
 
-test:check()
+os.exit(test:check() and 0 or 1)
diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/pairsmm_tarantool_4560.test.lua
index bb9835d..f8cf0bb 100755
--- a/test/pairsmm_tarantool_4560.test.lua
+++ b/test/pairsmm_tarantool_4560.test.lua
@@ -1,8 +1,8 @@
 #!/usr/bin/env tarantool
 
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("PAIRSMM-is-set")
+local test = tap.test("PAIRSMM-is-set")
 test:plan(2)
 
 -- There is no Lua way to detect whether LJ_PAIRSMM is enabled. However, in
@@ -46,5 +46,7 @@ for k, v in ipairs(t) do
     ipairs_res = v + ipairs_res
 end
 test:is(pairs_res + ipairs_res, 0)
-os_exec_res = os.execute()
+local os_exec_res = os.execute()
 test:is(os_exec_res, 1)
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/table_chain_bug_LuaJIT_494.test.lua
index 06c0f0d..85df8e0 100755
--- a/test/table_chain_bug_LuaJIT_494.test.lua
+++ b/test/table_chain_bug_LuaJIT_494.test.lua
@@ -1,8 +1,8 @@
 #!/usr/bin/env tarantool
 
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("494")
+local test = tap.test("494")
 test:plan(1)
 
 -- Test file to demonstrate Lua table hash chain bugs discussed in
@@ -11,19 +11,19 @@ test:plan(1)
 --     https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500
 
 --- Plumbing
-ffi = require"ffi"
-ffi.cdef"char* strstr(const char*, const char*)"
-strstr = ffi.C.strstr
-cast = ffi.cast
-str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
+local ffi = require("ffi")
+ffi.cdef("char* strstr(const char*, const char*)")
+local strstr = ffi.C.strstr
+local cast = ffi.cast
+local str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
 function str_hash(s)
     return cast("uint32_t*", strstr(s, "")) - str_hash_offset
 end
-table_new = require"table.new"
+local table_new = require("table.new")
 
 --- Prepare some objects
-victims = {}
-orig_hash = {}
+local victims = {}
+local orig_hash = {}
 for c in ("abcdef"):gmatch"." do
     v = c .. "{09add58a-13a4-44e0-a52c-d44d0f9b2b95}"
     victims[c] = v
@@ -174,5 +174,6 @@ collectgarbage()
 for c, v in pairs(victims) do
     str_hash(v)[0] = orig_hash[c]
 end
+test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")
 
-test:ok("PASS")
+os.exit(test:check() and 0 or 1)
diff --git a/test/unsink_64_kptr.test.lua b/test/unsink_64_kptr.test.lua
index 8995763..d01ddc6 100755
--- a/test/unsink_64_kptr.test.lua
+++ b/test/unsink_64_kptr.test.lua
@@ -1,8 +1,8 @@
 #!/usr/bin/env tarantool
 
-tap = require('tap')
+local tap = require('tap')
 
-test = tap.test("232")
+local test = tap.test("232")
 test:plan(1)
 
 --- From: Thibault Charbonnier <thibaultcha@me.com>
@@ -40,5 +40,6 @@ end
 for i = 1, 1000 do
     fn(i)
 end
+test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")
 
-test:ok("PASS")
+os.exit(test:check() and 0 or 1)
-- 
2.17.1

[Tarantool-patches] [PATCH v1 2/2] test: rename test files

[Alexander V. Tikhonov]

Renamed test files in the following way:
[gh-[external repo]-<number>-]comment.test.lua
where "external repo" is github repository:
  lj - luajit/luajit
  lrc - openresty/lua-resty-core

Close #4655
---
 test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua}    | 0
 ...mm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} | 0
 ...n_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} | 0
 ...JIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} | 0
 ...JIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} | 0
 ...{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} | 0
 6 files changed, 0 insertions(+), 0 deletions(-)
 rename test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua} (100%)
 rename test/{pairsmm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} (100%)
 rename test/{table_chain_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} (100%)
 rename test/{fold_bug_LuaJIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} (100%)
 rename test/{fold_bug_LuaJIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} (100%)
 rename test/{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} (100%)

diff --git a/test/gh.test.lua b/test/gh-3196-incorrect-string-length.test.lua
similarity index 100%
rename from test/gh.test.lua
rename to test/gh-3196-incorrect-string-length.test.lua
diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/gh-4560-pairsmm-is-set.test.lua
similarity index 100%
rename from test/pairsmm_tarantool_4560.test.lua
rename to test/gh-4560-pairsmm-is-set.test.lua
diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/gh-lj-494-infinite-loop.test.lua
similarity index 100%
rename from test/table_chain_bug_LuaJIT_494.test.lua
rename to test/gh-lj-494-infinite-loop.test.lua
diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/gh-lj-505-fold-icorrect-behavior.test.lua
similarity index 100%
rename from test/fold_bug_LuaJIT_505.test.lua
rename to test/gh-lj-505-fold-icorrect-behavior.test.lua
diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/gh-lj-524-fold-icorrect-behavior.test.lua
similarity index 100%
rename from test/fold_bug_LuaJIT_524.test.lua
rename to test/gh-lj-524-fold-icorrect-behavior.test.lua
diff --git a/test/unsink_64_kptr.test.lua b/test/gh-lrc-64-unsink-64-kptr.test.lua
similarity index 100%
rename from test/unsink_64_kptr.test.lua
rename to test/gh-lrc-64-unsink-64-kptr.test.lua
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v1 1/2] test: cleanup tests code

[Igor Munkin]

Sasha,

Thanks for the patch! I left several comments below, please consider
them.

On 16.12.19, Alexander V. Tikhonov wrote:
> Added local definitions for the variables, added os.exit()
> routine at the tests finish. Changed added information messages
> to test:ok() calls.

IMHO bullet list is a way more readable for listing the changes.

> 
> Part of #4655
> ---
>  test/fix_string_find_recording.test.lua  |  6 +++---
>  test/fold_bug_LuaJIT_505.test.lua        |  7 ++++---
>  test/fold_bug_LuaJIT_524.test.lua        |  2 +-
>  test/gh.test.lua                         |  8 ++++----
>  test/pairsmm_tarantool_4560.test.lua     |  8 +++++---
>  test/table_chain_bug_LuaJIT_494.test.lua | 23 ++++++++++++-----------
>  test/unsink_64_kptr.test.lua             |  7 ++++---
>  7 files changed, 33 insertions(+), 28 deletions(-)

General comments:
* Feel free to enhance the existing test names passed to tap.test:
| $ grep -rF 'tap.test'
| table_chain_bug_LuaJIT_494.test.lua:local test = tap.test("494")
| fix_string_find_recording.test.lua:local test =
| tap.test("fix-string-find-recording")
| gh.test.lua:local test = tap.test("gh")
| pairsmm_tarantool_4560.test.lua:local test = tap.test("PAIRSMM-is-set")
| fold_bug_LuaJIT_505.test.lua:local test = tap.test("505")
| fold_bug_LuaJIT_524.test.lua:local test = tap.test("LuaJIT 524")
| unsink_64_kptr.test.lua:local test = tap.test("232")
* It's a good idea to add more verbose description, but the new ones
  still don't make sense to me:
| $ grep -rnF 'test:ok'
| table_chain_bug_LuaJIT_494.test.lua:177:test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")
| fold_bug_LuaJIT_505.test.lua:19:test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")
| unsink_64_kptr.test.lua:43:test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")

> 
> diff --git a/test/fix_string_find_recording.test.lua b/test/fix_string_find_recording.test.lua
> index d3fc9e1..911e611 100755
> --- a/test/fix_string_find_recording.test.lua
> +++ b/test/fix_string_find_recording.test.lua
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env tarantool
>  
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("fix-string-find-recording")
> +local test = tap.test("fix-string-find-recording")
>  test:plan(1)
>  
>  local err = [[module 'kit.1.10.3-136' not found:
> @@ -76,4 +76,4 @@ until not e
>  
>  test:is(count_vm, count_jit)
>  
> -test:check()
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/fold_bug_LuaJIT_505.test.lua
> index 2fee069..433efb0 100755
> --- a/test/fold_bug_LuaJIT_505.test.lua
> +++ b/test/fold_bug_LuaJIT_505.test.lua
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env tarantool
>  
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("505")
> +local test = tap.test("505")
>  test:plan(1)
>  
>  -- Test file to demonstrate Lua fold machinery icorrect behavior, details:
> @@ -16,5 +16,6 @@ for _ = 1, 20 do
>      local pos_b = string.find(value2, "b", 2, true)
>      assert(pos_b == 2, "FAIL: position of 'b' is " .. pos_b)
>  end
> +test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")
>  
> -test:ok("PASS")
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/fold_bug_LuaJIT_524.test.lua
> index b056684..c8cc7b0 100755
> --- a/test/fold_bug_LuaJIT_524.test.lua
> +++ b/test/fold_bug_LuaJIT_524.test.lua
> @@ -21,4 +21,4 @@ end
>  
>  test:is(tonumber(sq), math.fmod(math.pow(42, 8), math.pow(2, 32)))
>  
> -test:check()
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/gh.test.lua b/test/gh.test.lua
> index 00b71a6..2804d35 100755
> --- a/test/gh.test.lua
> +++ b/test/gh.test.lua
> @@ -1,17 +1,17 @@
>  #!/usr/bin/env tarantool
>  
>  -- Miscellaneous test for LuaJIT bugs
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("gh")
> +local test = tap.test("gh")
>  test:plan(2)
>  --
>  -- gh-3196: incorrect string length if Lua hash returns 0
>  --
> -h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
> +local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>  test:is(h:len(), 20)
>  
>  h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>  test:is(h:len(), 20)
>  
> -test:check()
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/pairsmm_tarantool_4560.test.lua
> index bb9835d..f8cf0bb 100755
> --- a/test/pairsmm_tarantool_4560.test.lua
> +++ b/test/pairsmm_tarantool_4560.test.lua
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env tarantool
>  
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("PAIRSMM-is-set")
> +local test = tap.test("PAIRSMM-is-set")
>  test:plan(2)
>  
>  -- There is no Lua way to detect whether LJ_PAIRSMM is enabled. However, in
> @@ -46,5 +46,7 @@ for k, v in ipairs(t) do
>      ipairs_res = v + ipairs_res
>  end
>  test:is(pairs_res + ipairs_res, 0)
> -os_exec_res = os.execute()
> +local os_exec_res = os.execute()
>  test:is(os_exec_res, 1)
> +
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/table_chain_bug_LuaJIT_494.test.lua
> index 06c0f0d..85df8e0 100755
> --- a/test/table_chain_bug_LuaJIT_494.test.lua
> +++ b/test/table_chain_bug_LuaJIT_494.test.lua
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env tarantool
>  
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("494")
> +local test = tap.test("494")
>  test:plan(1)
>  
>  -- Test file to demonstrate Lua table hash chain bugs discussed in
> @@ -11,19 +11,19 @@ test:plan(1)
>  --     https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500
>  
>  --- Plumbing
> -ffi = require"ffi"
> -ffi.cdef"char* strstr(const char*, const char*)"
> -strstr = ffi.C.strstr
> -cast = ffi.cast
> -str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
> +local ffi = require("ffi")
> +ffi.cdef("char* strstr(const char*, const char*)")
> +local strstr = ffi.C.strstr
> +local cast = ffi.cast
> +local str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
>  function str_hash(s)
>      return cast("uint32_t*", strstr(s, "")) - str_hash_offset
>  end
> -table_new = require"table.new"
> +local table_new = require("table.new")
>  
>  --- Prepare some objects
> -victims = {}
> -orig_hash = {}
> +local victims = {}
> +local orig_hash = {}
>  for c in ("abcdef"):gmatch"." do
>      v = c .. "{09add58a-13a4-44e0-a52c-d44d0f9b2b95}"
>      victims[c] = v
> @@ -174,5 +174,6 @@ collectgarbage()
>  for c, v in pairs(victims) do
>      str_hash(v)[0] = orig_hash[c]
>  end
> +test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")
>  
> -test:ok("PASS")
> +os.exit(test:check() and 0 or 1)
> diff --git a/test/unsink_64_kptr.test.lua b/test/unsink_64_kptr.test.lua
> index 8995763..d01ddc6 100755
> --- a/test/unsink_64_kptr.test.lua
> +++ b/test/unsink_64_kptr.test.lua
> @@ -1,8 +1,8 @@
>  #!/usr/bin/env tarantool
>  
> -tap = require('tap')
> +local tap = require('tap')
>  
> -test = tap.test("232")
> +local test = tap.test("232")
>  test:plan(1)
>  
>  --- From: Thibault Charbonnier <thibaultcha@me.com>
> @@ -40,5 +40,6 @@ end
>  for i = 1, 1000 do
>      fn(i)
>  end
> +test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")
>  
> -test:ok("PASS")
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.17.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH v1 2/2] test: rename test files

[Igor Munkin]

Sasha,

Thanks for the patch!

IMHO gh prefix can be used for the issues filed in Tarantool queue and
is excess for the tests related to issues from other queues. Otherwise
e.g. tnt prefix can be used for the issues origined to Tarantool and gh
prefix can be dropped at all. The naming you propose within this patch
looks too long for me.

On 16.12.19, Alexander V. Tikhonov wrote:
> Renamed test files in the following way:
> [gh-[external repo]-<number>-]comment.test.lua
> where "external repo" is github repository:
>   lj - luajit/luajit
>   lrc - openresty/lua-resty-core

The openresty queue is located in openresty/luajit2 repo. Furthermore
or (for openresty) feems to be more correct for the corresponding
prefix.

> 
> Close #4655
> ---
>  test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua}    | 0
>  ...mm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} | 0
>  ...n_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} | 0
>  ...JIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} | 0
>  ...JIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} | 0
>  ...{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} | 0
>  6 files changed, 0 insertions(+), 0 deletions(-)
>  rename test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua} (100%)
>  rename test/{pairsmm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} (100%)
>  rename test/{table_chain_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} (100%)

table-chain-infinite-loop looks more correct.

>  rename test/{fold_bug_LuaJIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} (100%)
>  rename test/{fold_bug_LuaJIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} (100%)
>  rename test/{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} (100%)
> 
> diff --git a/test/gh.test.lua b/test/gh-3196-incorrect-string-length.test.lua
> similarity index 100%
> rename from test/gh.test.lua
> rename to test/gh-3196-incorrect-string-length.test.lua
> diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/gh-4560-pairsmm-is-set.test.lua
> similarity index 100%
> rename from test/pairsmm_tarantool_4560.test.lua
> rename to test/gh-4560-pairsmm-is-set.test.lua
> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/gh-lj-494-infinite-loop.test.lua
> similarity index 100%
> rename from test/table_chain_bug_LuaJIT_494.test.lua
> rename to test/gh-lj-494-infinite-loop.test.lua
> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/gh-lj-505-fold-icorrect-behavior.test.lua
> similarity index 100%
> rename from test/fold_bug_LuaJIT_505.test.lua
> rename to test/gh-lj-505-fold-icorrect-behavior.test.lua
> diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/gh-lj-524-fold-icorrect-behavior.test.lua
> similarity index 100%
> rename from test/fold_bug_LuaJIT_524.test.lua
> rename to test/gh-lj-524-fold-icorrect-behavior.test.lua
> diff --git a/test/unsink_64_kptr.test.lua b/test/gh-lrc-64-unsink-64-kptr.test.lua
> similarity index 100%
> rename from test/unsink_64_kptr.test.lua
> rename to test/gh-lrc-64-unsink-64-kptr.test.lua

test/fix_string_find_recodring.test.lua is missing.

> -- 
> 2.17.1
> 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH v1 1/2] test: cleanup tests code

[Alexander Tikhonov]

[-- Attachment #1: Type: text/plain, Size: 7976 bytes --]


Igor,

Thanks for the review, I've made all the changes as you suggested.

>Понедельник, 13 января 2020, 15:06 +03:00 от Igor Munkin < imun@tarantool.org >:
>
>Sasha,
>
>Thanks for the patch! I left several comments below, please consider
>them.
>
>On 16.12.19, Alexander V. Tikhonov wrote:
>> Added local definitions for the variables, added os.exit()
>> routine at the tests finish. Changed added information messages
>> to test:ok() calls.
>
>IMHO bullet list is a way more readable for listing the changes.
>
>> 
>> Part of #4655
>> ---
>>  test/fix_string_find_recording.test.lua  |  6 +++---
>>  test/fold_bug_LuaJIT_505.test.lua        |  7 ++++---
>>  test/fold_bug_LuaJIT_524.test.lua        |  2 +-
>>  test/gh.test.lua                         |  8 ++++----
>>  test/pairsmm_tarantool_4560.test.lua     |  8 +++++---
>>  test/table_chain_bug_LuaJIT_494.test.lua | 23 ++++++++++++-----------
>>  test/unsink_64_kptr.test.lua             |  7 ++++---
>>  7 files changed, 33 insertions(+), 28 deletions(-)
>
>General comments:
>* Feel free to enhance the existing test names passed to tap.test:
>| $ grep -rF 'tap.test'
>| table_chain_bug_LuaJIT_494.test.lua:local test = tap.test("494")
>| fix_string_find_recording.test.lua:local test =
>| tap.test("fix-string-find-recording")
>| gh.test.lua:local test = tap.test("gh")
>| pairsmm_tarantool_4560.test.lua:local test = tap.test("PAIRSMM-is-set")
>| fold_bug_LuaJIT_505.test.lua:local test = tap.test("505")
>| fold_bug_LuaJIT_524.test.lua:local test = tap.test("LuaJIT 524")
>| unsink_64_kptr.test.lua:local test = tap.test("232")
>* It's a good idea to add more verbose description, but the new ones
>  still don't make sense to me:
>| $ grep -rnF 'test:ok'
>| table_chain_bug_LuaJIT_494.test.lua:177:test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")
>| fold_bug_LuaJIT_505.test.lua:19:test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")
>| unsink_64_kptr.test.lua:43:test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")
>
>> 
>> diff --git a/test/fix_string_find_recording.test.lua b/test/fix_string_find_recording.test.lua
>> index d3fc9e1..911e611 100755
>> --- a/test/fix_string_find_recording.test.lua
>> +++ b/test/fix_string_find_recording.test.lua
>> @@ -1,8 +1,8 @@
>>  #!/usr/bin/env tarantool
>> 
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("fix-string-find-recording")
>> +local test = tap.test("fix-string-find-recording")
>>  test:plan(1)
>> 
>>  local err = [[module 'kit.1.10.3-136' not found:
>> @@ -76,4 +76,4 @@ until not e
>> 
>>  test:is(count_vm, count_jit)
>> 
>> -test:check()
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/fold_bug_LuaJIT_505.test.lua
>> index 2fee069..433efb0 100755
>> --- a/test/fold_bug_LuaJIT_505.test.lua
>> +++ b/test/fold_bug_LuaJIT_505.test.lua
>> @@ -1,8 +1,8 @@
>>  #!/usr/bin/env tarantool
>> 
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("505")
>> +local test = tap.test("505")
>>  test:plan(1)
>> 
>>  -- Test file to demonstrate Lua fold machinery icorrect behavior, details:
>> @@ -16,5 +16,6 @@ for _ = 1, 20 do
>>      local pos_b = string.find(value2, "b", 2, true)
>>      assert(pos_b == 2, "FAIL: position of 'b' is " .. pos_b)
>>  end
>> +test:ok(true, "LuaJIT/LuaJIT gh-505 assert not occured")
>> 
>> -test:ok("PASS")
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/fold_bug_LuaJIT_524.test.lua
>> index b056684..c8cc7b0 100755
>> --- a/test/fold_bug_LuaJIT_524.test.lua
>> +++ b/test/fold_bug_LuaJIT_524.test.lua
>> @@ -21,4 +21,4 @@ end
>> 
>>  test:is(tonumber(sq), math.fmod(math.pow(42, 8), math.pow(2, 32)))
>> 
>> -test:check()
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/gh.test.lua b/test/gh.test.lua
>> index 00b71a6..2804d35 100755
>> --- a/test/gh.test.lua
>> +++ b/test/gh.test.lua
>> @@ -1,17 +1,17 @@
>>  #!/usr/bin/env tarantool
>> 
>>  -- Miscellaneous test for LuaJIT bugs
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("gh")
>> +local test = tap.test("gh")
>>  test:plan(2)
>>  --
>>  -- gh-3196: incorrect string length if Lua hash returns 0
>>  --
>> -h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>> +local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>>  test:is(h:len(), 20)
>> 
>>  h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>>  test:is(h:len(), 20)
>> 
>> -test:check()
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/pairsmm_tarantool_4560.test.lua
>> index bb9835d..f8cf0bb 100755
>> --- a/test/pairsmm_tarantool_4560.test.lua
>> +++ b/test/pairsmm_tarantool_4560.test.lua
>> @@ -1,8 +1,8 @@
>>  #!/usr/bin/env tarantool
>> 
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("PAIRSMM-is-set")
>> +local test = tap.test("PAIRSMM-is-set")
>>  test:plan(2)
>> 
>>  -- There is no Lua way to detect whether LJ_PAIRSMM is enabled. However, in
>> @@ -46,5 +46,7 @@ for k, v in ipairs(t) do
>>      ipairs_res = v + ipairs_res
>>  end
>>  test:is(pairs_res + ipairs_res, 0)
>> -os_exec_res = os.execute()
>> +local os_exec_res = os.execute()
>>  test:is(os_exec_res, 1)
>> +
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/table_chain_bug_LuaJIT_494.test.lua
>> index 06c0f0d..85df8e0 100755
>> --- a/test/table_chain_bug_LuaJIT_494.test.lua
>> +++ b/test/table_chain_bug_LuaJIT_494.test.lua
>> @@ -1,8 +1,8 @@
>>  #!/usr/bin/env tarantool
>> 
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("494")
>> +local test = tap.test("494")
>>  test:plan(1)
>> 
>>  -- Test file to demonstrate Lua table hash chain bugs discussed in
>> @@ -11,19 +11,19 @@ test:plan(1)
>>  --  https://gist.github.com/corsix/1fc9b13a2dd5f3659417b62dd54d4500
>> 
>>  --- Plumbing
>> -ffi = require"ffi"
>> -ffi.cdef"char* strstr(const char*, const char*)"
>> -strstr = ffi.C.strstr
>> -cast = ffi.cast
>> -str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
>> +local ffi = require("ffi")
>> +ffi.cdef("char* strstr(const char*, const char*)")
>> +local strstr = ffi.C.strstr
>> +local cast = ffi.cast
>> +local str_hash_offset = cast("uint32_t*", strstr("*", ""))[-2] == 1 and 3 or 2
>>  function str_hash(s)
>>      return cast("uint32_t*", strstr(s, "")) - str_hash_offset
>>  end
>> -table_new = require"table.new"
>> +local table_new = require("table.new")
>> 
>>  --- Prepare some objects
>> -victims = {}
>> -orig_hash = {}
>> +local victims = {}
>> +local orig_hash = {}
>>  for c in ("abcdef"):gmatch"." do
>>      v = c .. "{09add58a-13a4-44e0-a52c-d44d0f9b2b95}"
>>      victims[c] = v
>> @@ -174,5 +174,6 @@ collectgarbage()
>>  for c, v in pairs(victims) do
>>      str_hash(v)[0] = orig_hash[c]
>>  end
>> +test:ok(true, "LuaJIT/LuaJIT gh-494 successfully checked commit a7dc9e8d23315217c9fe0029bc8ae12c03306b33")
>> 
>> -test:ok("PASS")
>> +os.exit(test:check() and 0 or 1)
>> diff --git a/test/unsink_64_kptr.test.lua b/test/unsink_64_kptr.test.lua
>> index  8995763 ..d01ddc6 100755
>> --- a/test/unsink_64_kptr.test.lua
>> +++ b/test/unsink_64_kptr.test.lua
>> @@ -1,8 +1,8 @@
>>  #!/usr/bin/env tarantool
>> 
>> -tap = require('tap')
>> +local tap = require('tap')
>> 
>> -test = tap.test("232")
>> +local test = tap.test("232")
>>  test:plan(1)
>> 
>>  --- From: Thibault Charbonnier < thibaultcha@me.com >
>> @@ -40,5 +40,6 @@ end
>>  for i = 1, 1000 do
>>      fn(i)
>>  end
>> +test:ok(true, "openresty-lua/resty-core gh-232 allowed its address to be inlined")
>> 
>> -test:ok("PASS")
>> +os.exit(test:check() and 0 or 1)
>> -- 
>> 2.17.1
>> 
>
>-- 
>Best regards,
>IM


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 10483 bytes --]

Re: [Tarantool-patches] [PATCH v1 2/2] test: rename test files

[Alexander Tikhonov]

[-- Attachment #1: Type: text/plain, Size: 3507 bytes --]

Igor,

Thanks for the review, I've changed the commit message as you suggested.


>Понедельник, 13 января 2020, 15:07 +03:00 от Igor Munkin <imun@tarantool.org>:
>
>Sasha,
>
>Thanks for the patch!
>
>IMHO gh prefix can be used for the issues filed in Tarantool queue and
>is excess for the tests related to issues from other queues. Otherwise
>e.g. tnt prefix can be used for the issues origined to Tarantool and gh
>prefix can be dropped at all. The naming you propose within this patch
>looks too long for me.
>
>On 16.12.19, Alexander V. Tikhonov wrote:
>> Renamed test files in the following way:
>> [gh-[external repo]-<number>-]comment.test.lua
>> where "external repo" is github repository:
>>   lj - luajit/luajit
>>   lrc - openresty/lua-resty-core
>
>The openresty queue is located in openresty/luajit2 repo. Furthermore
>or (for openresty) feems to be more correct for the corresponding
>prefix.
>
>> 
>> Close #4655
>> ---
>>  test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua}    | 0
>>  ...mm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} | 0
>>  ...n_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} | 0
>>  ...JIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} | 0
>>  ...JIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} | 0
>>  ...{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} | 0
>>  6 files changed, 0 insertions(+), 0 deletions(-)
>>  rename test/{gh.test.lua => gh-3196-incorrect-string-length.test.lua} (100%)
>>  rename test/{pairsmm_tarantool_4560.test.lua => gh-4560-pairsmm-is-set.test.lua} (100%)
>>  rename test/{table_chain_bug_LuaJIT_494.test.lua => gh-lj-494-infinite-loop.test.lua} (100%)
>
>table-chain-infinite-loop looks more correct.
>
>>  rename test/{fold_bug_LuaJIT_505.test.lua => gh-lj-505-fold-icorrect-behavior.test.lua} (100%)
>>  rename test/{fold_bug_LuaJIT_524.test.lua => gh-lj-524-fold-icorrect-behavior.test.lua} (100%)
>>  rename test/{unsink_64_kptr.test.lua => gh-lrc-64-unsink-64-kptr.test.lua} (100%)
>> 
>> diff --git a/test/gh.test.lua b/test/gh-3196-incorrect-string-length.test.lua
>> similarity index 100%
>> rename from test/gh.test.lua
>> rename to test/gh-3196-incorrect-string-length.test.lua
>> diff --git a/test/pairsmm_tarantool_4560.test.lua b/test/gh-4560-pairsmm-is-set.test.lua
>> similarity index 100%
>> rename from test/pairsmm_tarantool_4560.test.lua
>> rename to test/gh-4560-pairsmm-is-set.test.lua
>> diff --git a/test/table_chain_bug_LuaJIT_494.test.lua b/test/gh-lj-494-infinite-loop.test.lua
>> similarity index 100%
>> rename from test/table_chain_bug_LuaJIT_494.test.lua
>> rename to test/gh-lj-494-infinite-loop.test.lua
>> diff --git a/test/fold_bug_LuaJIT_505.test.lua b/test/gh-lj-505-fold-icorrect-behavior.test.lua
>> similarity index 100%
>> rename from test/fold_bug_LuaJIT_505.test.lua
>> rename to test/gh-lj-505-fold-icorrect-behavior.test.lua
>> diff --git a/test/fold_bug_LuaJIT_524.test.lua b/test/gh-lj-524-fold-icorrect-behavior.test.lua
>> similarity index 100%
>> rename from test/fold_bug_LuaJIT_524.test.lua
>> rename to test/gh-lj-524-fold-icorrect-behavior.test.lua
>> diff --git a/test/unsink_64_kptr.test.lua b/test/gh-lrc-64-unsink-64-kptr.test.lua
>> similarity index 100%
>> rename from test/unsink_64_kptr.test.lua
>> rename to test/gh-lrc-64-unsink-64-kptr.test.lua
>
>test/fix_string_find_recodring.test.lua is missing.
>
>> -- 
>> 2.17.1
>> 
>
>-- 
>Best regards,
>IM


-- 
Alexander Tikhonov

[-- Attachment #2: Type: text/html, Size: 4347 bytes --]