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

Alexander Turenko alexander.turenko at tarantool.org
Sun Oct 21 23:41:02 MSK 2018

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
- fff
  * ggg

> 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")?

More information about the Tarantool-patches mailing list