Re[2]: [PATCH v2 1/5] test: cleanup replication tests

Sergei Voronezhskii sergw at tarantool.org
Mon Oct 22 11:07:39 MSK 2018




>Воскресенье, 21 октября 2018, 23:41 +03:00 от Alexander Turenko <alexander.turenko at tarantool.org>:
>
>Ok except minor comments below.
>
>It would be glab if you'll answer to review messages, cite reviewer
>comments and answer what was changed or why it didn't changed. It is
>hard to manually track all questions were raised, esp. when the patch is
>relatively big.
>
>For small changes (eps. for a relatively big patch) you can just answer
>for comments. Ideally also attach diff from the previous version at end
>of the email (just call `git diff` before commit your changes and
>paste). I can see the whole diff on the branch and paste related hunks
>to comment it. Other reviewers can ask you to paste the whole new diff
>explicitly, I personally prefer to run git diff and paste things I want
>to comment.
>
>WBR, Alexander Turenko.
>
>On Fri, Oct 19, 2018 at 07:17:17PM +0300, Sergei Voronezhskii wrote:
>> - at the end of tests which create any replication config need to call:
>>   * `test_run:cmd('delete server ...')` removes server object
>> from `TestState.servers` list, this behaviour was taken
>> from `test_run:drop_cluster()` function
>
>Please, indent it like so (for ease reading):
>
>- xxx
>  * yyy
>    zzz
>    aaa
>- fff
>  eee
>  * ggg
>    hhh 
fixed
>
>> diff --git a/test/replication/local_spaces.test.lua b/test/replication/local_spaces.test.lua
>> index 06e2b0bd2..633cc9f1a 100644
>> --- a/test/replication/local_spaces.test.lua
>> +++ b/test/replication/local_spaces.test.lua
>> @@ -76,6 +76,7 @@ box.space.test3:select()
>>  test_run:cmd("switch default")
>>  test_run:cmd("stop server replica")
>>  test_run:cmd("cleanup server replica")
>> +test_run:cleanup_cluster()
>>  box.schema.user.revoke('guest', 'replication')
>> 
>>  s1:select()
>
>Missed test_run:cmd("delete server replica")?
fixed

-- 
Sergei Voronezhskii
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181022/96702743/attachment.html>


More information about the Tarantool-patches mailing list