[tarantool-patches] Re[3]: [PATCH] test: enable parallel for python tests and long

["Sergei Voronezhskii" <sergw@tarantool.org>]

Hello, I've updated branch, and rebase on 2.1

Removed vclock:
diff --git a/test/replication-py/multi.test.py b/test/replication-py/multi.test.py
index 224332266..15d5a409a 100644
--- a/test/replication-py/multi.test.py
+++ b/test/replication-py/multi.test.py
@@ -55,7 +55,6 @@ for server in cluster:
                 fiber.sleep(0.01)
             end;""", server2.id)
         print 'server', server.id, "connected"
-    server.admin("box.info.vclock")

 print 'done'

diff --git a/test/replication-py/multi.result b/test/replication-py/multi.result
index b4f16b699..f90ab22a0 100644
--- a/test/replication-py/multi.result
+++ b/test/replication-py/multi.result
@@ -17,24 +17,12 @@ Make a full mesh
 server 1 connected
 server 1 connected
 server 1 connected
-box.info.vclock
----
-- {1: 4}
-...
 server 2 connected
 server 2 connected
 server 2 connected
-box.info.vclock
----
-- {1: 4}
-...
 server 3 connected
 server 3 connected
 server 3 connected
-box.info.vclock
----
-- {1: 4}
-...
 done
 ----------------------------------------------------------------------
 Test inserts

Fixed typo:
diff --git a/test/replication-py/multi.test.py b/test/replication-py/multi.test.py
index 224332266..15d5a409a 100644
@@ -84,7 +83,7 @@ print 'Synchronize'
 for server1 in cluster:
     for server2 in cluster:
         server1.wait_lsn(server2.id, server2.get_lsn(server2.id))
-    print 'server', server.id, 'done'
+    print 'server', server1.id, 'done'
 print 'done'
 print

diff --git a/test/replication-py/multi.result b/test/replication-py/multi.result
index b4f16b699..f90ab22a0 100644
--- a/test/replication-py/multi.result
+++ b/test/replication-py/multi.result
@@ -54,8 +42,8 @@ Insert records
 inserted 60 records

 Synchronize
-server 3 done
-server 3 done
+server 1 done
+server 2 done
 server 3 done
 done

Squash wait for whole data:
diff --git a/test/box-py/iproto.test.py b/test/box-py/iproto.test.py
index 81cdddb61..ad788eef4 100644
--- a/test/box-py/iproto.test.py
+++ b/test/box-py/iproto.test.py
@@ -226,6 +226,10 @@ def receive_response():
         resp_len = s.recv(5)
         resp_len = msgpack.loads(resp_len)
         resp_headerbody = s.recv(resp_len)
+        # wait for the whole data
+        while len(resp_headerbody) < resp_len:
+            chunk = s.recv(resp_len - len(resp_headerbody))
+            resp_headerbody = resp_headerbody + chunk
         unpacker = msgpack.Unpacker(use_list = True)
         unpacker.feed(resp_headerbody)
         resp_header = unpacker.unpack()


>Четверг, 28 февраля 2019, 10:53 +03:00 от Sergei Voronezhskii <sergw@tarantool.org>:
>
>
>Updated branch:  https://github.com/tarantool/tarantool/commits/sergw/enable-parallel-test-py-long-fixups
>Test passed:  https://travis-ci.org/tarantool/tarantool/builds/499664259
>
>>Воскресенье, 23 декабря 2018, 14:04 +03:00 от Alexander Turenko <  alexander.turenko@tarantool.org >:
>>
>>Hi!
>>
>>See comments below and fixups on the
>>sergw/enable-parallel-test-py-long-fixups branch.
>>
>>I tested it with the command below and after the fixup it passes.
>>
>>```
>>$ ./test-run.py --long -- $(for i in $(seq 1 10); do echo -n "-py/ "; done)
>>```
>>
>>WBR, Alexander Turenko.
>>
>>On Tue, Dec 18, 2018 at 11:43:00AM +0300, Sergei Voronezhskii wrote:
>>> Fixed cleanup for python tests:
>>> - box-py/iproto.test.py need to cleanup created cluster
>>>   reproduce:
>>>   - [box-py/iproto.test.py, null]
>>>   - [box-py/bootstrap.test.py, null]
>>> - box-py/boostrap.test.py should not restart server because of next
>>>   box-py/call.test.py got error:
>>>    `NetworkError: (104, 'Connection reset by peer')`
>>>    at `iproto.authenticate('test', 'test')`
>>>   reproduce:
>>>   - [box-py/iproto.test.py, null]
>>>   - [box-py/bootstrap.test.py, null]
>>>   - [box-py/call.test.py, null]
>>> - replication-py/multi.test.py should not relay on hardcoded server.id
>>
>>relay -> rely
>fixed
>
>>Why it should not? You cleaned up _cluster in box-py/iproto.test.py.
>>Why don't clean up it in replication-py/cluster.test.py too?
>>
>>>   because previous test can create some and `id` will autoincremented,
>>>   instead this we need to calculate vclock_diff
>
>>You use hardcoded server id 1. 'previous test can create some' -- some
>>what? I suppose you are about using vclock difference instead of
>>absolute value, but the message states something different.
>
>I guess it's enough for checking with hardcoded server id 1.
>If master==replica1 and master==replica2, it useless to checking
>replica1 and replica2.
>
>Previous test autoincrement replica id and also moves vclock.
>
>>
>>>   reproduce:
>>>   - [replication-py/cluster.test.py, null]
>>>   - [replication-py/multi.test.py, null]
>>> 
>>> Part of: #3232
>>> ---
>>> BRANCH:  https://github.com/tarantool/tarantool/tree/sergw/enable-parallel-test-py-long
>>>  test/box-py/bootstrap.test.py     |  4 +---
>>>  test/box-py/iproto.result         |  4 ++++
>>>  test/box-py/iproto.test.py        |  1 +
>>>  test/box-py/suite.ini             |  2 +-
>>>  test/engine_long/suite.ini        |  2 +-
>>>  test/long_run-py/suite.ini        |  2 +-
>>>  test/luajit-tap/suite.ini         |  2 +-
>>>  test/replication-py/multi.result  | 19 +++++--------------
>>>  test/replication-py/multi.test.py | 28 +++++++++++++++++-----------
>>>  test/replication-py/suite.ini     |  2 +-
>>>  test/xlog-py/suite.ini            |  2 +-
>>>  11 files changed, 34 insertions(+), 34 deletions(-)
>>> 
>>> diff --git a/test/box-py/bootstrap.test.py b/test/box-py/bootstrap.test.py
>>> index 9d690b03f..dba6f5ae9 100644
>>> --- a/test/box-py/bootstrap.test.py
>>> +++ b/test/box-py/bootstrap.test.py
>>> @@ -9,8 +9,6 @@ cluster_uuid = yaml.load(server.admin('box.space._schema:get("cluster")',
>>>  sys.stdout.push_filter(cluster_uuid, '<cluster uuid>')
>>> 
>>>  server.admin('box.internal.bootstrap()')
>>> -server.restart()
>>> -
>>
>>I suppose now the test case will not test what it was intended. Please,
>>implement waiting for the server startup.
>>
>>You should have strong opinion why this will not broke the test case
>>before propose such kind of things in a patch. When you have this
>>opinion it is easy to write it in the commit message.
>>
>>I don't sure whether it really brokes the test case, but I don't see any
>>justification in the commit message. So I need to investigate the
>>problem from scratch to prove it is good or bad. And so what gain we get
>>from your work?
>>
>>The code cost is zero w/o investigation. The investigation result was
>>not shared. I don't understand why do you behave in this way. Again and
>>again.
>>
>
>I have checked upgrade.lua for bootstrap method for investigation.
>But I did not found any clue why we should do restart here.
>To fix it without delete `restart` we can do:
>
>iproto.py_con.ping()
>server.restart()
>iproto.reconnect()
>iproto.authenticate('test', 'test')
>
>But I have exception TarantoolStartError on OSX.
>After while you can see @locker already removed it at
>https://github.com/tarantool/tarantool/commit/3fe8c2835406c02acb59b6f86721d63cec69fac7
>
>>>  server.admin('box.space._schema:select{}')
>>>  server.admin('box.space._cluster:select{}')
>>>  server.admin('box.space._space:select{}')
>>> @@ -20,4 +18,4 @@ server.admin('box.space._func:select{}')
>>>  server.admin('box.space._priv:select{}')
>>> 
>>>  # Cleanup
>>> -sys.stdout.pop_filter()
>>> +sys.stdout.clear_all_filters()
>>
>>Two filters was pushed, two should be popped, I think.
>>
>>> diff --git a/test/replication-py/multi.test.py b/test/replication-py/multi.test.py
>>> index 224332266..3394d47be 100644
>>> --- a/test/replication-py/multi.test.py
>>> +++ b/test/replication-py/multi.test.py
>>> @@ -10,10 +10,14 @@ ROW_N = REPLICA_N * 20
>>> 
>>>  # master server
>>>  master = server
>>> +
>>>  master.admin("fiber = require('fiber')")
>>>  master.admin("box.schema.user.grant('guest', 'replication')")
>>>  master.admin("box.schema.user.grant('guest', 'execute', 'universe')")
>>> 
>>> +# Get vclock on master
>>> +vclock_cnt = yaml.load(master.admin("box.info.vclock[1]", silent = True))[0]
>>> +
>>>  print '----------------------------------------------------------------------'
>>>  print 'Bootstrap replicas'
>>>  print '----------------------------------------------------------------------'
>>> @@ -48,14 +52,16 @@ for server in cluster:
>>>      server.iproto.py_con.eval("box.cfg { replication = ... }", [sources])
>>> 
>>>  # Wait connections to establish
>>> -for server in cluster:
>>> +for sid, server in enumerate(cluster, 1):
>>>      for server2 in cluster:
>>>          server.iproto.py_con.eval("""
>>>              while #box.info.vclock[...] ~= nil do
>>>                  fiber.sleep(0.01)
>>>              end;""", server2.id)
>>> -        print 'server', server.id, "connected"
>>> -    server.admin("box.info.vclock")
>>> +        print 'server', sid, "connected"
>>> +    vclock_new = yaml.load(server.admin("box.info.vclock[1]", silent = True))[0]
>>> +    print "vclock_diff: {}".format(vclock_new - vclock_cnt)
>>> +
>>> 
>>
>>Re server.id -> sid: Please, cleanup _cluster in
>>replication-py/cluster.test.py instead. The same for all such changes
>>below. 
>Added
>
>diff --git a/test/replication-py/cluster.result b/test/replication-py/cluster.result
>index 3b5566829..04f06f74e 100644
>--- a/test/replication-py/cluster.result
>+++ b/test/replication-py/cluster.result
>@@ -196,6 +196,10 @@ box.info.vclock[2] == 2
> ---
> - true
> ...
>+box.space._cluster:delete{2} ~= nil
>+---
>+- true
>+...
> -------------------------------------------------------------
> JOIN replica to read-only master
> -------------------------------------------------------------
>@@ -216,3 +220,7 @@ Cleanup
> box.schema.user.revoke('guest', 'replication')
> ---
> ...
>+box.space._cluster:delete{2} ~= nil
>+---
>+- true
>+...
>diff --git a/test/replication-py/cluster.test.py b/test/replication-py/cluster.test.py
>index b5f59f40f..dcb05a517 100644
>--- a/test/replication-py/cluster.test.py
>+++ b/test/replication-py/cluster.test.py
>@@ -239,6 +239,7 @@ replica.admin('box.info.vclock[%d] == 2' % replica_id)
>
> replica.stop()
> replica.cleanup()
>+master.admin('box.space._cluster:delete{%d} ~= nil' % replica_id)
>
> print '-------------------------------------------------------------'
> print 'JOIN replica to read-only master'
>@@ -290,3 +291,5 @@ print '-------------------------------------------------------------'
> sys.stdout.pop_filter()
>
> master.admin("box.schema.user.revoke('guest', 'replication')")
>+
>+master.admin('box.space._cluster:delete{2} ~= nil')
>
>>
>>Re vclocks: Why this vector clock difference is a scalar value? Why it
>>is always for server id 1? Please, describe the problem first. It is
>>that cluster.test.py moves vclock? I think we can just don't print it,
>>because it was informational thing.
>
>Yes it because of cluster.test.py moves vclock. I've updated commit message
>
>
>Fixed cleanup for python tests:
>- replication-py/cluster.test.py
>- box-py/iproto.test.py
>  need to cleanup created cluster
>  reproduce:
>  - [box-py/iproto.test.py, null]
>  - [box-py/bootstrap.test.py, null]
>- replication-py/multi.test.py
>  1) should not rely on hardcoded server.id because previous test can
>     create replica and `id` will autoincremented, instead enumerate
>     through cluster list
>  2) because of cluster.test.py moves vclock need to check for vclock
>     difference instead of printing `box.info.vclock`
>  reproduce:
>  - [replication-py/cluster.test.py, null]
>  - [replication-py/multi.test.py, null]
>
>Part of: #3232
>
>-- 
>Sergei Voronezhskii
>


-- 
Sergei Voronezhskii