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

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

[Oleg Piskunov]

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]


- 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
   - perf
   - cleanup
 
@@ -183,6 +184,16 @@ freebsd_12_release:
   script:
     - ${GITLAB_MAKE} vms_test_freebsd
 
+# ###################
+# Integration testing
+# ###################
+
+release_module_vshard_test:
+  <<: *docker_test_definition
+  stage: integr_test
+  script:
+    - ${GITLAB_MAKE} test_module_vshard
+
 # ####
 # Perf
 # ####
diff --git a/.travis.mk b/.travis.mk
index 063537f..1603242 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -77,7 +77,7 @@ deps_buster_clang_8: deps_debian
 # Release
 
 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}
        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
+       cd vshard && cmake . && make test
+
 # Debug with coverage
 
 build_coverage_debian:
--
1.8.3.1
 

[-- Attachment #2: Type: text/html, Size: 2711 bytes --]

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

[Sergey Bronnikov]

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

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

[Alexander V. Tikhonov]

Hi Oleg, thanks for the patch, LGTM.

On Mon, May 18, 2020 at 08:27:21PM +0300, 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
>    - perf
>    - cleanup
>  
> @@ -183,6 +184,16 @@ freebsd_12_release:
>    script:
>      - ${GITLAB_MAKE} vms_test_freebsd
>  
> +# ###################
> +# Integration testing
> +# ###################
> +
> +release_module_vshard_test:
> +  <<: *docker_test_definition
> +  stage: integr_test
> +  script:
> +    - ${GITLAB_MAKE} test_module_vshard
> +
>  # ####
>  # Perf
>  # ####
> diff --git a/.travis.mk b/.travis.mk
> index 063537f..1603242 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -77,7 +77,7 @@ deps_buster_clang_8: deps_debian
>  # Release
>  
>  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}
>         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
> +       cd vshard && cmake . && make test
> +
>  # Debug with coverage
>  
>  build_coverage_debian:
> --
> 1.8.3.1
>  

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

[Oleg Piskunov]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]


Sergey, thanks for review. See my four comments below:
  
>Среда, 20 мая 2020, 11:07 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
>>    - test
>> +  - integr_test
>
>1. We have no string length limitation here, please replace it with
>"integration_testing".
Sure. will do.
>
><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?
The reason is that tarantoolctl needed for testing vshard module.
-DENABLE_DIST=ON option is used to enable tarantoolctl during «make install».
>> +       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.
It doesn’t work, because some tests use git history:
TEST: reload_evolution/storage.test.lua
...
  vshard_copy_path = util.git_checkout('vshard_git_tree_copy',                    \
                                       last_compatible_commit)
>
>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.
As we discussed in chat — latest version of vshard will be used.
 
--
Oleg Piskunov
 

[-- Attachment #2: Type: text/html, Size: 3087 bytes --]

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

[Sergey Bronnikov]

Hi, Oleg

see comments inline

On 14:40 Thu 21 May , Oleg Piskunov wrote:
>
> Sergey, thanks for review. See my four comments below:
>
> >Среда, 20 мая 2020, 11:07 +03:00 от Sergey Bronnikov <sergeyb@tarantool.org>:
> >>    - test
> >> +  - integr_test
> >
> >1. We have no string length limitation here, please replace it with
> >"integration_testing".
> Sure. will do.
> >
> ><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?
> The reason is that tarantoolctl needed for testing vshard module.
> -DENABLE_DIST=ON option is used to enable tarantoolctl during «make install».
> >> +       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.
> It doesn’t work, because some tests use git history:
> TEST: reload_evolution/storage.test.lua
> ...
>   vshard_copy_path = util.git_checkout('vshard_git_tree_copy',                    \
>                                        last_compatible_commit)

Got it, thanks!

> >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.
> As we discussed in chat — latest version of vshard will be used.

Actually it was not so. We decided that you will go to person who knows
more than me about vshard and ask him about preferred versions of vshard
for testing.

Once again: information that new tarantool release is compatible with
vshard from master is useless for tarantool users, because no one take
git version for more or less serious usage. Users relies on released
versions.

> --
> Oleg Piskunov

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

[Sergey Bronnikov]

On 16:00 Thu 21 May , Sergey Bronnikov wrote:

<snipped>

> > >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.
> > As we discussed in chat — latest version of vshard will be used.
> 
> Actually it was not so. We decided that you will go to person who knows
> more than me about vshard and ask him about preferred versions of vshard
> for testing.
> 
> Once again: information that new tarantool release is compatible with
> vshard from master is useless for tarantool users, because no one take
> git version for more or less serious usage. Users relies on released
> versions.

According to answers from MRG users they uses vshard provided by
luarocks. So I propose to switch testing to the latest vshard version in
luarocks.

> > --
> > Oleg Piskunov

-- 
sergeyb@