From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id EE25A21C93 for ; Sun, 23 Dec 2018 06:04:14 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id z-vb_woOHNMd for ; Sun, 23 Dec 2018 06:04:14 -0500 (EST) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 5365921C76 for ; Sun, 23 Dec 2018 06:04:14 -0500 (EST) Date: Sun, 23 Dec 2018 14:04:13 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] test: enable parallel for python tests and long Message-ID: <20181223110413.l4cqxggq67jrrryd@tkn_work_nb> References: <20181218084300.48522-1-sergw@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20181218084300.48522-1-sergw@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Sergei Voronezhskii Cc: tarantool-patches@freelists.org, Kirill Yukhin 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 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. > 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, '') > > 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. > 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. 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.