Re: [Tarantool-patches] [PATCH luajit 1/3] test: remove excess dependency for tests target

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

Hi, Igor!

Thanks for the patch!

LGTM, except a few nits below.

On 05.04.21, Igor Munkin wrote:

Nit: I propose the following commit title rewording:
| test: remove excess dependency for tarantool suite
Feel free to ignore.

> TEST_DEPS list was introduced to prevent <tarantool-tests> target from
> being run in case no dependencies are changed. As a result of review it

Typo: s/result of review/result of the review/

> was decided to make all *-tests targets be .PHONY rules, hence TEST_DEPS

Typo? "be" looks excess here. I propose the following rewording
| As a result of the review it was decided to turn all *-tests targets
| into .PHONY rules, hence TEST_DEPS variable is excess and can be
| dropped.

> variable is excess and can be dropped.
> 
> Follows up tarantool/tarantool#4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  test/tarantool-tests/CMakeLists.txt | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 9bb26f32..3e36ff86 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -69,7 +69,6 @@ set(LUA_PATH
>  )
>  set(LUA_TEST_SUFFIX .test.lua)

Nit: looks like `LUA_TEST_SUFFIX` variable is excess now, and we can
move suffix ".test.lua" to `--ext` flag directly.
Feel free to ignore.

>  set(LUA_TEST_FLAGS --failures --shuffle)
> -file(GLOB TEST_DEPS ${CMAKE_CURRENT_SOURCE_DIR}/*${LUA_TEST_SUFFIX})
>  
>  if(CMAKE_VERBOSE_MAKEFILE)
>    list(APPEND LUA_TEST_FLAGS --verbose)
> @@ -78,7 +77,7 @@ endif()
>  # LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
>  # with dependecies are set in scope of BuildTestLib macro.
>  add_custom_target(tarantool-tests
> -  DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS} ${TEST_DEPS}
> +  DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS}
>  )
>  add_custom_command(TARGET tarantool-tests
>    COMMENT "Running Tarantool tests"
> -- 
> 2.25.0
> 

-- 
Best regards,
Sergey Kaplun