[Tarantool-patches] [PATCH 1/1] test: fix flaky replication/gh-* tests

[Tarantool-patches] [PATCH 1/1] test: fix flaky replication/gh-* tests

[Vladislav Shpilevoy]

The tests didn't cleanup _cluster table. Autocleanup is turned on
only in > 1.10 versions. In 1.10 these tests failed, when were
launched in one test-run worker, because they still remembered
previous already deleted instances.

The patch makes cleanup in the end of these tests. Autocleanup
still works on master, but for the sake of similarity the manual
cleanup is done for all branches, including > 1.10.

Closes #4645
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4645-flaky-replication-test
Issue: https://github.com/tarantool/tarantool/issues/4645

 test/replication/gh-4402-info-errno.result       | 4 ++++
 test/replication/gh-4402-info-errno.test.lua     | 2 ++
 test/replication/gh-4605-empty-password.result   | 4 ++++
 test/replication/gh-4605-empty-password.test.lua | 2 ++
 test/replication/gh-4606-admin-creds.result      | 4 ++++
 test/replication/gh-4606-admin-creds.test.lua    | 2 ++
 6 files changed, 18 insertions(+)

diff --git a/test/replication/gh-4402-info-errno.result b/test/replication/gh-4402-info-errno.result
index 661eea38b..25829634e 100644
--- a/test/replication/gh-4402-info-errno.result
+++ b/test/replication/gh-4402-info-errno.result
@@ -61,3 +61,7 @@ d ~= nil and d.system_message ~= nil and d.message ~= nil or i
 box.schema.user.revoke('guest', 'replication')
  | ---
  | ...
+
+test_run:cleanup_cluster()
+ | ---
+ | ...
diff --git a/test/replication/gh-4402-info-errno.test.lua b/test/replication/gh-4402-info-errno.test.lua
index 1b2d9d814..27348d5fd 100644
--- a/test/replication/gh-4402-info-errno.test.lua
+++ b/test/replication/gh-4402-info-errno.test.lua
@@ -23,3 +23,5 @@ d = i.replication[replica_id].downstream
 d ~= nil and d.system_message ~= nil and d.message ~= nil or i
 
 box.schema.user.revoke('guest', 'replication')
+
+test_run:cleanup_cluster()
diff --git a/test/replication/gh-4605-empty-password.result b/test/replication/gh-4605-empty-password.result
index defdfcfcd..bb4000d41 100644
--- a/test/replication/gh-4605-empty-password.result
+++ b/test/replication/gh-4605-empty-password.result
@@ -60,3 +60,7 @@ test_run:cmd("delete server replica_auth")
 box.schema.user.drop('test_user')
  | ---
  | ...
+
+test_run:cleanup_cluster()
+ | ---
+ | ...
diff --git a/test/replication/gh-4605-empty-password.test.lua b/test/replication/gh-4605-empty-password.test.lua
index f42a55f81..c06e052b0 100644
--- a/test/replication/gh-4605-empty-password.test.lua
+++ b/test/replication/gh-4605-empty-password.test.lua
@@ -25,3 +25,5 @@ test_run:cmd("cleanup server replica_auth")
 test_run:cmd("delete server replica_auth")
 
 box.schema.user.drop('test_user')
+
+test_run:cleanup_cluster()
diff --git a/test/replication/gh-4606-admin-creds.result b/test/replication/gh-4606-admin-creds.result
index fc0fbff44..4b46890ac 100644
--- a/test/replication/gh-4606-admin-creds.result
+++ b/test/replication/gh-4606-admin-creds.result
@@ -61,3 +61,7 @@ box.schema.user.passwd('admin', '')
 box.schema.user.revoke('admin', 'replication')
  | ---
  | ...
+
+test_run:cleanup_cluster()
+ | ---
+ | ...
diff --git a/test/replication/gh-4606-admin-creds.test.lua b/test/replication/gh-4606-admin-creds.test.lua
index 217d46ce1..6894429de 100644
--- a/test/replication/gh-4606-admin-creds.test.lua
+++ b/test/replication/gh-4606-admin-creds.test.lua
@@ -24,3 +24,5 @@ test_run:cmd("delete server replica_auth")
 
 box.schema.user.passwd('admin', '')
 box.schema.user.revoke('admin', 'replication')
+
+test_run:cleanup_cluster()
-- 
2.21.0 (Apple Git-122.2)

Re: [Tarantool-patches] [PATCH 1/1] test: fix flaky replication/gh-* tests

[Alexander Turenko]

There are more such tests, let's check it youself:

$ diff -u \
  <(grep -R cleanup_cluster    test/replication/*.test.lua | cut -d: -f1 | sort -u) \
  <(grep -R rpl_master=default test/replication/*.test.lua | cut -d: -f1 | sort -u) \
  | grep ^[+-]test

+test/replication/consistent.test.lua
+test/replication/errinj.test.lua
+test/replication/gh-4402-info-errno.test.lua
+test/replication/gh-4605-empty-password.test.lua
+test/replication/gh-4606-admin-creds.test.lua
-test/replication/prune.test.lua
+test/replication/transaction.test.lua

prune.test.lua has extra cleanup_cluster() call, it is harmless, let's
ignore. Other tests don't clean up _cluster. Maybe fix them too?

I verified that gh-4402-info-errno.test.lua after transaction.test.lua
fails on master when pretest_clean is set to False. If you'll going to
verify it youself, don't forget that transaction.test.lua is in fragile
tests (at least on master, see suite.ini) and will be run after all
other tests by default.

BTW, transaction.test.lua:vinyl fails after transaction.test.lua:memtx,
because the test does not drop its spaces.

Don't even sure now whether it worth to make commits that will fix some
parts of the whole problem: our tests now lean of pretest_clean option.

BTW, 52c67cccae07d130e197cafca2060bf0327bb6fb ('test: enable cleaning of
a test environment') is in 1.10 now. The issue should be fixed.

I don't mind against this particular patch if you need it for some
reason. However I don't see why this may be necessary now. Technically
unnecessary changes should not be pushed.

WBR, Alexander Turenko.

On Mon, Nov 25, 2019 at 11:43:36PM +0100, Vladislav Shpilevoy wrote:
> The tests didn't cleanup _cluster table. Autocleanup is turned on
> only in > 1.10 versions. In 1.10 these tests failed, when were
> launched in one test-run worker, because they still remembered
> previous already deleted instances.
> 
> The patch makes cleanup in the end of these tests. Autocleanup
> still works on master, but for the sake of similarity the manual
> cleanup is done for all branches, including > 1.10.
> 
> Closes #4645
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4645-flaky-replication-test
> Issue: https://github.com/tarantool/tarantool/issues/4645
> 
>  test/replication/gh-4402-info-errno.result       | 4 ++++
>  test/replication/gh-4402-info-errno.test.lua     | 2 ++
>  test/replication/gh-4605-empty-password.result   | 4 ++++
>  test/replication/gh-4605-empty-password.test.lua | 2 ++
>  test/replication/gh-4606-admin-creds.result      | 4 ++++
>  test/replication/gh-4606-admin-creds.test.lua    | 2 ++
>  6 files changed, 18 insertions(+)
> 
> diff --git a/test/replication/gh-4402-info-errno.result b/test/replication/gh-4402-info-errno.result
> index 661eea38b..25829634e 100644
> --- a/test/replication/gh-4402-info-errno.result
> +++ b/test/replication/gh-4402-info-errno.result
> @@ -61,3 +61,7 @@ d ~= nil and d.system_message ~= nil and d.message ~= nil or i
>  box.schema.user.revoke('guest', 'replication')
>   | ---
>   | ...
> +
> +test_run:cleanup_cluster()
> + | ---
> + | ...
> diff --git a/test/replication/gh-4402-info-errno.test.lua b/test/replication/gh-4402-info-errno.test.lua
> index 1b2d9d814..27348d5fd 100644
> --- a/test/replication/gh-4402-info-errno.test.lua
> +++ b/test/replication/gh-4402-info-errno.test.lua
> @@ -23,3 +23,5 @@ d = i.replication[replica_id].downstream
>  d ~= nil and d.system_message ~= nil and d.message ~= nil or i
>  
>  box.schema.user.revoke('guest', 'replication')
> +
> +test_run:cleanup_cluster()
> diff --git a/test/replication/gh-4605-empty-password.result b/test/replication/gh-4605-empty-password.result
> index defdfcfcd..bb4000d41 100644
> --- a/test/replication/gh-4605-empty-password.result
> +++ b/test/replication/gh-4605-empty-password.result
> @@ -60,3 +60,7 @@ test_run:cmd("delete server replica_auth")
>  box.schema.user.drop('test_user')
>   | ---
>   | ...
> +
> +test_run:cleanup_cluster()
> + | ---
> + | ...
> diff --git a/test/replication/gh-4605-empty-password.test.lua b/test/replication/gh-4605-empty-password.test.lua
> index f42a55f81..c06e052b0 100644
> --- a/test/replication/gh-4605-empty-password.test.lua
> +++ b/test/replication/gh-4605-empty-password.test.lua
> @@ -25,3 +25,5 @@ test_run:cmd("cleanup server replica_auth")
>  test_run:cmd("delete server replica_auth")
>  
>  box.schema.user.drop('test_user')
> +
> +test_run:cleanup_cluster()
> diff --git a/test/replication/gh-4606-admin-creds.result b/test/replication/gh-4606-admin-creds.result
> index fc0fbff44..4b46890ac 100644
> --- a/test/replication/gh-4606-admin-creds.result
> +++ b/test/replication/gh-4606-admin-creds.result
> @@ -61,3 +61,7 @@ box.schema.user.passwd('admin', '')
>  box.schema.user.revoke('admin', 'replication')
>   | ---
>   | ...
> +
> +test_run:cleanup_cluster()
> + | ---
> + | ...
> diff --git a/test/replication/gh-4606-admin-creds.test.lua b/test/replication/gh-4606-admin-creds.test.lua
> index 217d46ce1..6894429de 100644
> --- a/test/replication/gh-4606-admin-creds.test.lua
> +++ b/test/replication/gh-4606-admin-creds.test.lua
> @@ -24,3 +24,5 @@ test_run:cmd("delete server replica_auth")
>  
>  box.schema.user.passwd('admin', '')
>  box.schema.user.revoke('admin', 'replication')
> +
> +test_run:cleanup_cluster()
> -- 
> 2.21.0 (Apple Git-122.2)
> 

Re: [Tarantool-patches] [PATCH 1/1] test: fix flaky replication/gh-* tests

[Vladislav Shpilevoy]

Hi!

On 27/11/2019 00:49, Alexander Turenko wrote:
> There are more such tests, let's check it youself:
> 
> $ diff -u \
>   <(grep -R cleanup_cluster    test/replication/*.test.lua | cut -d: -f1 | sort -u) \
>   <(grep -R rpl_master=default test/replication/*.test.lua | cut -d: -f1 | sort -u) \
>   | grep ^[+-]test
> 
> +test/replication/consistent.test.lua

This test is disabled. It makes no sense to change it now, if it
is broken anyway.

> +test/replication/errinj.test.lua
> +test/replication/transaction.test.lua

Yes, these ones lack cluster cleanup.

> +test/replication/gh-4402-info-errno.test.lua
> +test/replication/gh-4605-empty-password.test.lua
> +test/replication/gh-4606-admin-creds.test.lua

These three I fixed in my patch.

But whatever. Seems like you have already committed your
fix, so there is nothing to discuss.