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

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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.

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