[tarantool-patches] Re[3]: [PATCH] test: enable parallel for python tests and long
Sergei Voronezhskii
sergw at tarantool.org
Wed Mar 6 15:27:18 MSK 2019
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 at 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 at 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
More information about the Tarantool-patches
mailing list