

<HTML><BODY><div><br><br> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik <korablev@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15870746562098080639_BODY">On 17 Apr 00:15, Mergen Imeev wrote:<br>> On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote:<br>> > On 16 Apr 23:24, Mergen Imeev wrote:<br>> > > Hi! Thank you for review. My answer below.<br>> > ><br>> > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote:<br>> > > > On 16 Apr 22:09, Alexander Tikhonov wrote:<br>> > > > ><br>> > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM.<br>> > > > ><br>> > > ><br>> > > > Did you verify that modified test still reproduces initial problem?<br>> > > > I hope so, but ask just in case.<br>> > > According to history, the main goal of this part of the<br>> > > test is to show that sql_create_function() throws proper<br>> > > error in case a new function is created when transaction<br>> > > is active. Since the error still here, we may say that<br>> > > the test works.<br>> ><br>> > So the answer to my question is - no, you didn't test it.<br>> > Better safe than sorry - I'd better verify that updated<br>> > test still covers the initial problem. Thanks.<br>> ><br>> Doesn't my answer says that we checked it?<br><br>No, it doesn't. 'reproduce' means that you are supposed to checkout<br>to branch, revert particular fix, run updated test without that fix<br>and verify that test fails without fix. Any other actions do not refer<br>to my original question.</div></div></div></div></blockquote></div><div>I still don’t understand this check way. Please clarify:</div><ol><li>checkout the branch with fix based on 2.2 release branch (understood)</li><li>revert particular fix (understood, but it is the same as checkout native 2.2 release branch, anyway ok)</li><li>«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</li></ol><div>Please point what is the difference between the way I understood and the way you suggest.</div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>>However, I agree that it is never wrong to check it again.<br><br>Please, check it for current fix and for any other test<br>upgrade in future.<br> </div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Alexander Tikhonov</div></div></div><div> </div></div></BODY></HTML>