From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <sergeyb@tarantool.org>
Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 36DE8469710
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 21 May 2020 16:01:11 +0300 (MSK)
Date: Thu, 21 May 2020 16:00:32 +0300
From: Sergey Bronnikov <sergeyb@tarantool.org>
Message-ID: <20200521130032.GA81082@pony.bronevichok.ru>
References: <1589822841.925400170@f462.i.mail.ru>
 <20200520080717.GA1400@pony.bronevichok.ru>
 <1590061225.245862957@f454.i.mail.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1590061225.245862957@f454.i.mail.ru>
Subject: Re: [Tarantool-patches] [PATCH v2] gitlab-ci: integration testing
	vshard
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Oleg Piskunov <o.piskunov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, Alexander Turenko <alexander.turenko@tarantool.org>

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