Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov <sergeyb@tarantool.org>
To: Oleg Piskunov <o.piskunov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org,
	Alexander Turenko <alexander.turenko@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v2] gitlab-ci: integration testing vshard
Date: Thu, 21 May 2020 16:00:32 +0300	[thread overview]
Message-ID: <20200521130032.GA81082@pony.bronevichok.ru> (raw)
In-Reply-To: <1590061225.245862957@f454.i.mail.ru>

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

  reply	other threads:[~2020-05-21 13:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 17:27 Oleg Piskunov
2020-05-20  8:07 ` Sergey Bronnikov
2020-05-21 11:40   ` Oleg Piskunov
2020-05-21 13:00     ` Sergey Bronnikov [this message]
2020-05-21 15:53       ` Sergey Bronnikov
2020-05-20 16:51 ` Alexander V. Tikhonov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200521130032.GA81082@pony.bronevichok.ru \
    --to=sergeyb@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=o.piskunov@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2] gitlab-ci: integration testing vshard' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox