Re: [Tarantool-patches] [PATCH 3/4] test: CI-environment

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

Max,

Thanks for the patch! I left several comments regarding the commit
message, please consider them in addition to the main comments in the
cover letter.

On 05.07.21, Maxim Kokryashkin wrote:
> From: Maxim Kokryashkin <m.kokryashkin@tarantool.org>
> 
> Part of tarantool/tarantool#5970
> ---
> The patch `fperrad/lua-Harness@6c2aa87` makes 309-os.t check 
> `os.getenv()` function by examining HOME environment variable 
> instead of USER, so we don't need to set USERNAME explicitly anymore, 
> as it stated in POSIX standard that every user must have HOME 
> varibale set. Therefore, `tarantool/luajit@45ed138` should be 
> replaced with `fperrad/lua-Harness@6c2aa87`.

Considering all comments for the first patch, I propose the following
rewording:

| In scope of the commit 45ed138dcb9228d8ac5b8d82331ddd20a0bfada1
| ('test: set USERNAME env var for lua-Harness suite') the environment
| of the testing process is tweaked via CMake, since both LOGNAME and
| USERNAME environment variables might be missing.
|
| Actually, POSIX standard states that every user must have HOME
| environment variable set. It allows to use this in the corresponding
| assertion instead of LOGNAME and USERNAME without additional
| environment setup.
|
| As a result this patch[1] from lua-Harness suite supersedes the
| mentioned commit, and CMake hack can be dropped.
|
| [1]: https://framagit.org/fperrad/lua-Harness/-/commit/6c2aa87
|
| Part of tarantool/tarantool#5970
| Part of tarantool/tarantool#4473

If you are OK with the part above, feel free to paste it as is into the
commit message for the next version of the patch.

> 
>  test/lua-Harness-tests/309-os.t       | 4 ++--
>  test/lua-Harness-tests/CMakeLists.txt | 5 -----
>  2 files changed, 2 insertions(+), 7 deletions(-)

The fix itself is fine and clear, so no comments regarding it.

> 

<snipped>

> -- 
> 2.31.1
> 

-- 
Best regards,
IM