From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 D33474696C3 for ; Fri, 17 Apr 2020 15:45:09 +0300 (MSK) Date: Fri, 17 Apr 2020 12:45:08 +0000 From: Nikita Pettik Message-ID: <20200417124508.GA12992@tarantool.org> References: <1b7ea654465f9a92cb0949ce6286fee63e2be1ca.1587063872.git.imeevma@gmail.com> <20200416211547.GA19535@tarantool.org> <20200416220415.GC8455@tarantool.org> <1587103523.376799823@f501.i.mail.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1587103523.376799823@f501.i.mail.ru> Subject: Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Tikhonov Cc: tarantool-patches@dev.tarantool.org On 17 Apr 09:05, Alexander Tikhonov wrote: > > > >   > >Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik : > >  > >On 17 Apr 00:15, Mergen Imeev wrote: > >> On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > >> > On 16 Apr 23:24, Mergen Imeev wrote: > >> > > Hi! Thank you for review. My answer below. > >> > > > >> > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > >> > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > >> > > > > > >> > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > >> > > > > > >> > > > > >> > > > Did you verify that modified test still reproduces initial problem? > >> > > > I hope so, but ask just in case. > >> > > According to history, the main goal of this part of the > >> > > test is to show that sql_create_function() throws proper > >> > > error in case a new function is created when transaction > >> > > is active. Since the error still here, we may say that > >> > > the test works. > >> > > >> > So the answer to my question is - no, you didn't test it. > >> > Better safe than sorry - I'd better verify that updated > >> > test still covers the initial problem. Thanks. > >> > > >> Doesn't my answer says that we checked it? > > > >No, it doesn't. 'reproduce' means that you are supposed to checkout > >to branch, revert particular fix, run updated test without that fix > >and verify that test fails without fix. Any other actions do not refer > >to my original question. > I still don’t understand this check way. Please clarify: > * checkout the branch with fix based on 2.2 release branch (understood) > * revert particular fix (understood, but it is the same as checkout native 2.2 release branch, anyway ok) False. You have to revert this fix: 14ba68f93 > * «run updated test without that fix» — what does it mean «updated», ok if you mean previous revert to native 2.2 release branch, than we did it manually and in regular testings, check gitlab-ci for results, in both variants test fails > Please point what is the difference between the way I understood and the way you suggest. > > > >>However, I agree that it is never wrong to check it again. > > > >Please, check it for current fix and for any other test > >upgrade in future. > >  >   >   > -- > Alexander Tikhonov >