Re: [Tarantool-patches] [PATCH v2] gitlab-ci: integration testing vshard

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hi, Oleg

Thanks for the patch! See my four comments inline.

On 20:27 Mon 18 May , Oleg Piskunov wrote:
> 
> - Adding new stage "integr_test" for integration
>   testing connectors and modules with Tarantool.
> - Adding options "-DENABLE_DIST=ON" into
>   "build_debian" target to enable tarantoolctl
>   during T installation.
> - Adding module VSHARD for testing in Tarantool
>   development cycle.
>  
> Closes #4972
> ---
>  
> Github: https://github.com/tarantool/tarantool/tree/opiskunov/gh-4972-integration-test-vshard
> Issue: https://github.com/tarantool/tarantool/issues/4972
> 
>  .gitlab-ci.yml | 11 +++++++++++
>  .travis.mk     |  9 ++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>  
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 256b368..8576f91 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -1,5 +1,6 @@
>  stages:
>    - test
> +  - integr_test

1. We have no string length limitation here, please replace it with
"integration_testing".

<snipped>

>  build_debian:
> -       cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON ${CMAKE_EXTRA_PARAMS}
> +       cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON -DENABLE_DIST=ON ${CMAKE_EXTRA_PARAMS}

2. What is purpose of enabling ENABLE_DIST here?

>         make -j
>  
>  test_debian_no_deps: build_debian
> @@ -87,6 +87,13 @@ test_debian: deps_debian test_debian_no_deps
>  
>  test_debian_clang8: deps_debian deps_buster_clang_8 test_debian_no_deps
>  
> +# Integration testing
> +
> +test_module_vshard: build_debian
> +       make install
> +       git clone --recurse-submodules https://github.com/tarantool/vshard.git vshard

3. We don't need full GIT history here, so I propose to limit it for 1
level with '--depth=1'. It reduces time a bit on clone repository.

4. You are using latest version of vshard from a master branch, so you
are testing latest version of tarantool against latest *non-released*
version of vshard. Why? From my point of view we should test against
released and supported versions of vshard. Latest release was on Feb 24
and we must use it in testing.

> +       cd vshard && cmake . && make test
> +
>  # Debug with coverage
>
>  build_coverage_debian:

Sergey