From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 17D296C1AE; Sat, 22 May 2021 00:54:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 17D296C1AE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1621634071; bh=/j4V8zj3nYWIB/4hPjG5GPdfEyH2oDGaEQkpUY8jhrk=; h=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=XjbSTYVn9nseZqGofsJhpByHU5/15L08CEgFRih/YW+XPRL8XPeqDttJl/OARDCaW zHuIlGCJ2MP21Grc8wM+PSP4lHEX3GhHJrnTsk/hG95oKwYqrRGUTm88h6YDDOoSuZ syrKUscj0LMftfds61pEBxCT8iZSDnnZgkw9fIXU= Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154]) (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 4545A6C1AE for ; Sat, 22 May 2021 00:54:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4545A6C1AE Received: by smtp17.mail.ru with esmtpa (envelope-from ) id 1lkD6C-0005pI-Nn for tarantool-patches@dev.tarantool.org; Sat, 22 May 2021 00:54:29 +0300 Received: by mail-lf1-f49.google.com with SMTP id q7so30408928lfr.6 for ; Fri, 21 May 2021 14:54:28 -0700 (PDT) X-Gm-Message-State: AOAM533J+6/4mXtbkEc+wWPaGRSKzjDMsE6HU1az4UKA4AC3woQzv7WB WkUoIEa/MMQk0/ZfYJvMffwZhyGZML+colvkqQ== X-Google-Smtp-Source: ABdhPJwc4yHRXz5tzGowYT+y97cX3ohUF5dmvXQKfNKNe4DW0OSicnfwj0kZ1VHwVIbuA2kaqI0CGte8sWrwKPDjZwI= X-Received: by 2002:a05:6512:5c6:: with SMTP id o6mr3453508lfo.455.1621634068256; Fri, 21 May 2021 14:54:28 -0700 (PDT) MIME-Version: 1.0 References: <2ec9e3f1-6b24-6593-174f-0ea72b56cd6b@tarantool.org> In-Reply-To: <2ec9e3f1-6b24-6593-174f-0ea72b56cd6b@tarantool.org> Date: Sat, 22 May 2021 00:54:17 +0300 X-Gmail-Original-Message-ID: Message-ID: To: Vladislav Shpilevoy Content-Type: multipart/alternative; boundary="000000000000981e1405c2de1aeb" X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD91B019B01C53E51AF113D6D562FBA72891FA115C4A817A10000894C459B0CD1B99B8066F97E6812CE1E87518FD99A3D11EA52DEF3EA6389A9EE80B9AFA0738B6B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70043D879A87EF1BCEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B3D52627AD81B52CEA1F7E6F0F101C67CDEEF6D7F21E0D1D9295C2E9FA3191EE1B59CA4C82EFA658C17B0A4A0B151A8B9DA342C6A6F83B31F6B57BC7E64490618DEB871D839B73339E8FC8737B5C224952D31B9D28593E51CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C0DEC8C2C8BCD2534D8941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD2691661749BA6B97735D94E105876FE7799D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE367F1C1C3ABB44F3A302FCEF25BFAB345C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637F765F39FA4E70FFE43847C11F186F3C59DAA53EE0834AAEE X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824A2A04A2ABAA09D25379311020FFC8D4AD29A9CE9800E4A7B4DC28E0C37135E455 X-C1DE0DAB: 0D63561A33F958A5F4D8D0DBCA04082A666912CE3B847F399620D89DF9620F4FD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752546FE575EB473F1410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D95739AEDB3821B40EEC877A6512580AB2F557A04AC795BBFF7F564C2421B9A6E62065F053BD320F1D7E09C32AA3244CDE6982D042A4D011DD368CE8E8FC0E7F4DBEAD0ED6C55A803EB3F6AD6EA9203E X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj3CIvDNz8QqDUaRemTGvxeg== X-Mailru-Sender: 4C235FE2E5D2D890BBD2AEE38E3B8B37C21FB0E06876A98A6CEE56E5C2BE1D46D72A5496D47674B153E6F1E4007818E061AB7FC983AAE23E992E2169F9161B8DDAB93E5CC8760AFE9437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 1/2] test: fix test output on latest Tarantool X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Yaroslav Dynnikov via Tarantool-patches Reply-To: Yaroslav Dynnikov Cc: Yaroslav Dynnikov , tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" --000000000000981e1405c2de1aeb Content-Type: text/plain; charset="UTF-8" OK, no objections then. On Fri, 21 May 2021, 22:26 Vladislav Shpilevoy, wrote: > Hi! Thanks for the review! > > On 19.05.2021 23:50, Yaroslav Dynnikov wrote: > > HI, Vlad > > > > Thanks for the patch. > > Find one comment below. > > > > On Thu, 13 May 2021 at 14:07, Vladislav Shpilevoy < > v.shpilevoy@tarantool.org > wrote: > > > > Duplicate key error at insertion into a space on the latest > > Tarantool changed its message and it broke of the tests. The patch > > updates the test so it checks only the needed part of the message > > and does not depend on Tarantool version anymore. > > --- > > test/storage/storage.result | 8 +++++--- > > test/storage/storage.test.lua | 3 ++- > > 2 files changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/test/storage/storage.result > b/test/storage/storage.result > > index 2c9784a..d18b7f8 100644 > > --- a/test/storage/storage.result > > +++ b/test/storage/storage.result > > @@ -179,10 +179,12 @@ vshard.storage.buckets_info() > > status: active > > id: 1 > > ... > > -vshard.storage.bucket_force_create(1) -- error > > +ok, err = vshard.storage.bucket_force_create(1) > > --- > > -- null > > -- Duplicate key exists in unique index 'pk' in space '_bucket' > > +... > > +assert(not ok and err.message:match("Duplicate key exists")) > > +--- > > +- Duplicate key exists > > ... > > vshard.storage.bucket_force_drop(1) > > --- > > diff --git a/test/storage/storage.test.lua > b/test/storage/storage.test.lua > > index 33f0498..97558f6 100644 > > --- a/test/storage/storage.test.lua > > +++ b/test/storage/storage.test.lua > > @@ -56,7 +56,8 @@ vshard.storage.sync(100500) > > vshard.storage.buckets_info() > > vshard.storage.bucket_force_create(1) > > vshard.storage.buckets_info() > > -vshard.storage.bucket_force_create(1) -- error > > +ok, err = vshard.storage.bucket_force_create(1) > > +assert(not ok and "err.message:match("Duplicate key exists)) > > > > > > I'd suggest splitting the check in two: > > > > 1. ok -- should be false > > 2. Then check the message matches. > > > > Assertions usually don't provide useful errors. > > In the core we try to use assertions more, because this makes the > tests easier to read. You can see right away what are the target > points of the test. This is kind of a simulation of tap tests. > > I consider vshard be "half-core", so I decided to use assertions > here as well. > --000000000000981e1405c2de1aeb Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
OK, no objections then.

On Fri, 21 May 2021, 22:26 Vlad= islav Shpilevoy, <v.shpilev= oy@tarantool.org> wrote:
Hi!= Thanks for the review!

On 19.05.2021 23:50, Yaroslav Dynnikov wrote:
> HI, Vlad
>
> Thanks for the patch.
> Find one comment below.
>
> On Thu, 13 May 2021 at 14:07, Vladislav Shpilevoy <v.shpilev= oy@tarantool.org <mailto:v.shpilevoy@tarantool.org>>= ; wrote:
>
>=C2=A0 =C2=A0 =C2=A0Duplicate key error at insertion into a space on th= e latest
>=C2=A0 =C2=A0 =C2=A0Tarantool changed its message and it broke of the t= ests. The patch
>=C2=A0 =C2=A0 =C2=A0updates the test so it checks only the needed part = of the message
>=C2=A0 =C2=A0 =C2=A0and does not depend on Tarantool version anymore. >=C2=A0 =C2=A0 =C2=A0---
>=C2=A0 =C2=A0 =C2=A0=C2=A0test/storage/storage.result=C2=A0 =C2=A0| 8 += ++++---
>=C2=A0 =C2=A0 =C2=A0=C2=A0test/storage/storage.test.lua | 3 ++-
>=C2=A0 =C2=A0 =C2=A0=C2=A02 files changed, 7 insertions(+), 4 deletions= (-)
>
>=C2=A0 =C2=A0 =C2=A0diff --git a/test/storage/storage.result b/test/sto= rage/storage.result
>=C2=A0 =C2=A0 =C2=A0index 2c9784a..d18b7f8 100644
>=C2=A0 =C2=A0 =C2=A0--- a/test/storage/storage.result
>=C2=A0 =C2=A0 =C2=A0+++ b/test/storage/storage.result
>=C2=A0 =C2=A0 =C2=A0@@ -179,10 +179,12 @@ vshard.storage.buckets_info()=
>=C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0status: active
>=C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0 =C2=A0id: 1
>=C2=A0 =C2=A0 =C2=A0=C2=A0...
>=C2=A0 =C2=A0 =C2=A0-vshard.storage.bucket_force_create(1) -- error
>=C2=A0 =C2=A0 =C2=A0+ok, err =3D vshard.storage.bucket_force_create(1)<= br> >=C2=A0 =C2=A0 =C2=A0=C2=A0---
>=C2=A0 =C2=A0 =C2=A0-- null
>=C2=A0 =C2=A0 =C2=A0-- Duplicate key exists in unique index 'pk'= ; in space '_bucket'
>=C2=A0 =C2=A0 =C2=A0+...
>=C2=A0 =C2=A0 =C2=A0+assert(not ok and err.message:match("Duplicat= e key exists"))
>=C2=A0 =C2=A0 =C2=A0+---
>=C2=A0 =C2=A0 =C2=A0+- Duplicate key exists
>=C2=A0 =C2=A0 =C2=A0=C2=A0...
>=C2=A0 =C2=A0 =C2=A0=C2=A0vshard.storage.bucket_force_drop(1)
>=C2=A0 =C2=A0 =C2=A0=C2=A0---
>=C2=A0 =C2=A0 =C2=A0diff --git a/test/storage/storage.test.lua b/test/s= torage/storage.test.lua
>=C2=A0 =C2=A0 =C2=A0index 33f0498..97558f6 100644
>=C2=A0 =C2=A0 =C2=A0--- a/test/storage/storage.test.lua
>=C2=A0 =C2=A0 =C2=A0+++ b/test/storage/storage.test.lua
>=C2=A0 =C2=A0 =C2=A0@@ -56,7 +56,8 @@ vshard.storage.sync(100500)
>=C2=A0 =C2=A0 =C2=A0=C2=A0vshard.storage.buckets_info()
>=C2=A0 =C2=A0 =C2=A0=C2=A0vshard.storage.bucket_force_create(1)
>=C2=A0 =C2=A0 =C2=A0=C2=A0vshard.storage.buckets_info()
>=C2=A0 =C2=A0 =C2=A0-vshard.storage.bucket_force_create(1) -- error
>=C2=A0 =C2=A0 =C2=A0+ok, err =3D vshard.storage.bucket_force_create(1)<= br> >=C2=A0 =C2=A0 =C2=A0+assert(not ok and "err.message:match("Du= plicate key exists))
>
>
> I'd suggest splitting the check in two:
>
> 1. ok -- should be false
> 2. Then check the message matches.
>
> Assertions usually don't provide useful errors.

In the core we try to use assertions more, because this makes the
tests easier to read. You can see right away what are the target
points of the test. This is kind of a simulation of tap tests.

I consider vshard be "half-core", so I decided to use assertions<= br> here as well.
--000000000000981e1405c2de1aeb--