Re: [Tarantool-patches] [PATCH v2 luajit 3/5] test: adjust lua-Harness test suite for Tarantool

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Igor,

Thanks for the review!

On 15.03.21, Igor Munkin wrote:
> Sergey,
> 
> This is ridiculous: you split two similar renames required by the one
> issue into *two* separate commits, but leaving the changes *totally
> unrelated to each other* within a single commit. Please, split this
> patch into two: one for the test directory tweak and another with
> mocking environment variable in CMake.

Sorry, but why then you said nothing about commits separation/joining
for this [1] draft series, as I asked for?

Offline we came to the agreement that we should use 3 commits for each
test suite. The first to onboard it, the second to adjust for LuaJIT,
the third to adjust it for Tarantool.
Later your asked me to send Mergens changes as is.

I specially send draft to discuss the commits content, order and so on.
You said only about separating patchset for different test suites and
merged LuaJIT test suite, so I thought that commit order is OK for you.
I'd merged changes with Tarantool-related one.

If it is not comfortable for you getting WIP series and discussing them
let's decline this practise.

Back to business, I've split the patch into two, considering your
proposal, see them below (their order is the same).

> 
> On 15.03.21, Sergey Kaplun wrote:
> > This patch makes it possible to run lua-Harness test suite using
> > Tarantool.
> > 
> > 203-lexico.t and 301-basic.t is adjusted to valid working with
> > out-of-source build in Tarantool CI.
> 
> This is done not only for Tarantool CI, but also for LuaJIT out of
> source build.
> 
> > 
> > Inside Tarantool's GitHub-CI there is no defined variable LOGNAME nor
> > USERNAME. This leads to test failure inside CI, because a string is
> > expected. So, now USERNAME is set manually via CMake.
> 
> You mix up "the symptom" and "the root cause" here again. Problem is not
> in CI, but in the test. See the comment at the end.
> 
> > 
> > Part of tarantool/tarantool#5844
> > Part of tarantool/tarantool#4473
> > ---
> >  test/lua-Harness-tests/203-lexico.t   | 14 ++++++++++----
> >  test/lua-Harness-tests/301-basic.t    |  6 +++++-
> >  test/lua-Harness-tests/CMakeLists.txt |  4 ++++
> >  3 files changed, 19 insertions(+), 5 deletions(-)
> > 
> 
> <snipped>
> 
> > diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
> > index 9b35e5a..bac279f 100644
> > --- a/test/lua-Harness-tests/CMakeLists.txt
> > +++ b/test/lua-Harness-tests/CMakeLists.txt
> > @@ -40,6 +40,10 @@ add_custom_command(TARGET lua-Harness-tests
> >      # for more info.
> >      # So use less preferable way for tests.
> >      # See the root CMakeLists.txt for more info.
> > +    # XXX: 309-os.t checks os.getenv() function by examine of
> > +    # USERNAME or LOGNAME enviroment variable. It is not present
> > +    # inside CI by itself, so it is set here manually.
> 
> Strictly saying, the issue was found in CI, but it doesn't relate
> directly to CI: this can be done via `unset USERNAME` in you bash. This
> is nothing else, but just a bad test. You can ask Francois to adjust the
> test using a bit more popular variable (such as PWD or HOME). For now I
> propose to change the comment the way below and adjust the commit
> message regarding the changes made.
> 
> s/It is not present inside CI by itself/These might not be set in the environment/.
> 
> > +    USERNAME="fperrad"
> >      ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
> >        --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
> >        ${LUA_TEST_FLAGS}
> > -- 
> > 2.28.0
> > 
> 
> -- 
> Best regards,
> IM

Patch for test directory tweak. Adjusted considering [2].

===================================================================
commit a84980334dce27243a25508e3bb8fd491689e552
Author: Sergey Kaplun <skaplun@tarantool.org>
Date:   Mon Mar 15 16:24:07 2021 +0300

    test: adjust lua-Harness tests that using dofile

    This patch makes out-of-source execution lua-Harness suite tests that
    using `dofile()` correct.

    There are the following files that used `dofile()` function
    on file in test sources directory:
    * 101-boolean.t
    * 102-function.t
    * 103-nil.t
    * 104-number.t
    * 105-string.t
    * 106-table.t
    * 107-thread.t
    * 108-userdata.t
    * 203-lexico.t
    * 231-metatable.t
    * 301-basic.t
    * 305-utf8.t
    * 404-ext.t

    `dofile()` looks for files to execute in the current working directory,
    that might be not the same as the test source directory.

    This patch introduces the new function `dofile_fullpath()` that
    evaluates full path to file considering arg[0] (i.e. test filename)
    value.

    Part of tarantool/tarantool#5844
    Part of tarantool/tarantool#4473

diff --git a/test/lua-Harness-tests/101-boolean.t b/test/lua-Harness-tests/101-boolean.t
index 0033eff..b9a769f 100755
--- a/test/lua-Harness-tests/101-boolean.t
+++ b/test/lua-Harness-tests/101-boolean.t
@@ -114,7 +114,7 @@ error_like(function () local a = true; a[1] = 1; end,
            "index")
 
 if has_op53 then
-    dofile'lexico53/boolean.t'
+    dofile_fullpath'lexico53/boolean.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/102-function.t b/test/lua-Harness-tests/102-function.t
index 48ed814..2858640 100755
--- a/test/lua-Harness-tests/102-function.t
+++ b/test/lua-Harness-tests/102-function.t
@@ -193,7 +193,7 @@ t[print] = true
 ok(t[print])
 
 if has_op53 then
-    dofile'lexico53/function.t'
+    dofile_fullpath'lexico53/function.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/103-nil.t b/test/lua-Harness-tests/103-nil.t
index 561b101..1e7c134 100755
--- a/test/lua-Harness-tests/103-nil.t
+++ b/test/lua-Harness-tests/103-nil.t
@@ -114,7 +114,7 @@ error_like(function () local a = nil; a[1] = 1; end,
            "index")
 
 if has_op53 then
-    dofile'lexico53/nil.t'
+    dofile_fullpath'lexico53/nil.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/104-number.t b/test/lua-Harness-tests/104-number.t
index 0d4d3fd..affd1a4 100755
--- a/test/lua-Harness-tests/104-number.t
+++ b/test/lua-Harness-tests/104-number.t
@@ -233,7 +233,7 @@ error_like(function () local a = 3.14; a[1] = 1; end,
            "index")
 
 if has_op53 then
-    dofile'lexico53/number.t'
+    dofile_fullpath'lexico53/number.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/105-string.t b/test/lua-Harness-tests/105-string.t
index cd8c88b..f571520 100755
--- a/test/lua-Harness-tests/105-string.t
+++ b/test/lua-Harness-tests/105-string.t
@@ -264,7 +264,7 @@ error_like(function () a = 'text'; a[1] = 1; end,
            "index")
 
 if has_op53 then
-    dofile'lexico53/string.t'
+    dofile_fullpath'lexico53/string.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/106-table.t b/test/lua-Harness-tests/106-table.t
index 0c0ba49..b1a1027 100755
--- a/test/lua-Harness-tests/106-table.t
+++ b/test/lua-Harness-tests/106-table.t
@@ -122,7 +122,7 @@ error_like(function () t = {}; t[0/0] = 42 end,
            "table index is NaN")
 
 if has_op53 then
-    dofile'lexico53/table.t'
+    dofile_fullpath'lexico53/table.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/107-thread.t b/test/lua-Harness-tests/107-thread.t
index 3d4af18..2f332d7 100755
--- a/test/lua-Harness-tests/107-thread.t
+++ b/test/lua-Harness-tests/107-thread.t
@@ -122,7 +122,7 @@ t[co] = true
 ok(t[co])
 
 if has_op53 then
-    dofile'lexico53/thread.t'
+    dofile_fullpath'lexico53/thread.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/108-userdata.t b/test/lua-Harness-tests/108-userdata.t
index b1e3641..cc4134b 100755
--- a/test/lua-Harness-tests/108-userdata.t
+++ b/test/lua-Harness-tests/108-userdata.t
@@ -119,7 +119,7 @@ t[u] = true
 ok(t[u])
 
 if has_op53 then
-    dofile'lexico53/userdata.t'
+    dofile_fullpath'lexico53/userdata.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/203-lexico.t b/test/lua-Harness-tests/203-lexico.t
index c1abebf..e5e89ed 100755
--- a/test/lua-Harness-tests/203-lexico.t
+++ b/test/lua-Harness-tests/203-lexico.t
@@ -118,19 +118,19 @@ do
 end
 
 if _VERSION >= 'Lua 5.2' or jit then
-    dofile'lexico52/lexico.t'
+    dofile_fullpath('lexico52/lexico.t')
 end
 
 if _VERSION >= 'Lua 5.3' or luajit21 then
-    dofile'lexico53/lexico.t'
+    dofile_fullpath('lexico53/lexico.t')
 end
 
 if _VERSION >= 'Lua 5.4' then
-    dofile'lexico54/lexico.t'
+    dofile_fullpath('lexico54/lexico.t')
 end
 
 if jit and pcall(require, 'ffi') then
-    dofile'lexicojit/lexico.t'
+    dofile_fullpath('lexicojit/lexico.t')
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/231-metatable.t b/test/lua-Harness-tests/231-metatable.t
index a2c6499..c5684d2 100755
--- a/test/lua-Harness-tests/231-metatable.t
+++ b/test/lua-Harness-tests/231-metatable.t
@@ -589,7 +589,7 @@ do
 end
 
 if has_anno_toclose then
-    dofile'lexico54/metatable.t'
+    dofile_fullpath'lexico54/metatable.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/301-basic.t b/test/lua-Harness-tests/301-basic.t
index f4f9235..460a02f 100755
--- a/test/lua-Harness-tests/301-basic.t
+++ b/test/lua-Harness-tests/301-basic.t
@@ -843,7 +843,7 @@ do -- xpcall
 end
 
 if jit and pcall(require, 'ffi') then
-    dofile'lexicojit/basic.t'
+    dofile_fullpath('lexicojit/basic.t')
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/305-utf8.t b/test/lua-Harness-tests/305-utf8.t
index 4304b6c..18c57d8 100755
--- a/test/lua-Harness-tests/305-utf8.t
+++ b/test/lua-Harness-tests/305-utf8.t
@@ -40,9 +40,9 @@ if not utf8 then
     nok(has_utf8, "no has_utf8")
 else
     plan'no_plan'
-    dofile'lexico53/utf8.t'
+    dofile_fullpath'lexico53/utf8.t'
     if _VERSION >= 'Lua 5.4' then
-        dofile'lexico54/utf8.t'
+        dofile_fullpath'lexico54/utf8.t'
     end
     done_testing()
 end
diff --git a/test/lua-Harness-tests/404-ext.t b/test/lua-Harness-tests/404-ext.t
index 22a52c7..e48d91a 100755
--- a/test/lua-Harness-tests/404-ext.t
+++ b/test/lua-Harness-tests/404-ext.t
@@ -158,7 +158,7 @@ end
 
 -- thread.exdata
 if pcall(require, 'ffi') and (profile.openresty or jit.version:match'moonjit') then
-    dofile'lexicojit/ext.t'
+    dofile_fullpath'lexicojit/ext.t'
 end
 
 done_testing()
diff --git a/test/lua-Harness-tests/tap.lua b/test/lua-Harness-tests/tap.lua
index e527687..37b9df7 100644
--- a/test/lua-Harness-tests/tap.lua
+++ b/test/lua-Harness-tests/tap.lua
@@ -205,6 +205,14 @@ function get_lua_binary_name ()
     return arg[i + 1]
 end
 
+-- XXX: If tests is run out of the tests source tree, the
+-- relative paths below become invalid. Hence, we need to
+-- prepend the directory from the script (i.e. test) name to
+-- the auxiliary files to be loaded and executed.
+function dofile_fullpath (filename)
+    return dofile(arg[0]:gsub('([^/]+)%.t$', '') .. filename)
+end
+
 --
 -- Copyright (c) 2009-2018 Francois Perrad
 --
===================================================================

Patch for mocking environment variable in CMake.

===================================================================
commit 483508b0a7863efabcde6d232ab9af2033e0011f
Author: Sergey Kaplun <skaplun@tarantool.org>
Date:   Mon Mar 15 22:08:07 2021 +0300

    test: forcify set USERNAME env var for lua-Harness

    309-os.t checks `os.getenv()` function by examining of
    USERNAME or LOGNAME environment variable.
    These variables might not be set in the environment, that leads to test
    failure.

    This patchs sets manually USERNAME environment variable for
    lua-Harness-tests target.

    Part of tarantool/tarantool#5844
    Part of tarantool/tarantool#4473

diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
index f8611ce..b844788 100644
--- a/test/lua-Harness-tests/CMakeLists.txt
+++ b/test/lua-Harness-tests/CMakeLists.txt
@@ -34,6 +34,11 @@ add_custom_command(TARGET lua-Harness-tests
   env
     LUA_PATH="${LUA_PATH}\;"
     LUA_CPATH="${LUA_CPATH}\;"
+    # XXX: 309-os.t checks os.getenv() function by examining of
+    # USERNAME or LOGNAME environment variable.
+    # These variables might not be set in the environment, so
+    # set one of them manually.
+    USERNAME="fperrad"
     ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
       --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
       ${LUA_TEST_FLAGS}
===================================================================

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2021-March/022563.html
[2]: https://lists.tarantool.org/pipermail/tarantool-patches/2021-March/022710.html

-- 
Best regards,
Sergey Kaplun