[Tarantool-patches] [PATCH 3/4] test: CI-environment
Igor Munkin
imun at tarantool.org
Tue Jul 6 23:29:47 MSK 2021
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 at tarantool.org>
>
> Part of tarantool/tarantool#5970
> ---
> The patch `fperrad/lua-Harness at 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 at 45ed138` should be
> replaced with `fperrad/lua-Harness at 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
More information about the Tarantool-patches
mailing list