Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua

[Nikita Pettik <korablev@tarantool.org>]

On 17 Apr 09:05, Alexander Tikhonov wrote:
> 
> 
> 
>   
> >Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik <korablev@tarantool.org>:
> > 
> >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
>