From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6CFF4469710 for ; Wed, 20 May 2020 11:03:02 +0300 (MSK) Date: Wed, 20 May 2020 11:03:00 +0300 From: "Alexander V. Tikhonov" Message-ID: <20200520080259.GA15557@hpalx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v2] Add option to update file with reference output List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sergeyb@tarantool.org Cc: tarantool-patches@dev.tarantool.org Hi Sergey, thanks for the patch. In general LGTM, but want to ask if it is needed to mention in help message of the new --update-result option that the new result file will be in the new result style, I mean the following difference, please check: mv replication/misc.result replication/misc.saved ./test-run.py replication/misc.test.lua --update-result diff replication/misc.result replication/misc.saved On Tue, May 19, 2020 at 01:25:46PM +0300, sergeyb@tarantool.org wrote: > From: Sergey Bronnikov > > New option covers two cases: > - in case of test failure test-run.py create a file .reject with > actual test output and one need to move .reject file to .result manually > when test has a valid behaviour. With option --update-result test-run.py > will do it automatically. > - in case of abcense reference result file test-run.py forced to create > such file in a source directory and set test status "new". This patch > changes behaviour. With option --update-result test status is "new" > and result file is created, without option test status is "fail" and > result file is not created. > > Fixes https://github.com/tarantool/tarantool/issues/4654 > Fixes https://github.com/tarantool/tarantool/issues/4258 > Closes #194 > --- > GH branch: https://github.com/tarantool/test-run/tree/ligurio/gh-4654-update-ref-output > > lib/options.py | 8 ++++++++ > lib/test.py | 21 +++++++++++++++++---- > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/lib/options.py b/lib/options.py > index 8bacb4a..47bbc0f 100644 > --- a/lib/options.py > +++ b/lib/options.py > @@ -201,6 +201,14 @@ class Options: > help="""Run the server under 'luacov'. > Default: false.""") > > + parser.add_argument( > + "--update-result", > + dest="update_result", > + action="store_true", > + default=False, > + help="""Update or create file with reference output (.result). > + Default: false.""") > + > # XXX: We can use parser.parse_intermixed_args() on > # Python 3.7 to understand commands like > # ./test-run.py foo --exclude bar baz > diff --git a/lib/test.py b/lib/test.py > index 8bc1feb..733b90c 100644 > --- a/lib/test.py > +++ b/lib/test.py > @@ -15,6 +15,7 @@ except ImportError: > from StringIO import StringIO > > import lib > +from lib.options import Options > from lib.colorer import color_stdout > from lib.utils import non_empty_valgrind_logs > from lib.utils import print_tail_n > @@ -152,7 +153,7 @@ class Test(object): > it to stdout. > > Returns short status of the test as a string: 'skip', 'pass', > - 'new', or 'fail'. There is also one possible value for > + 'new', 'updated' or 'fail'. There is also one possible value for > short_status, 'disabled', but it returned in the caller, > TestSuite.run_test(). > """ > @@ -235,9 +236,20 @@ class Test(object): > elif (self.is_executed_ok and not > self.is_equal_result and not > os.path.isfile(self.result)) and not is_tap: > + if lib.Options().args.update_result: > + shutil.copy(self.tmp_result, self.result) > + short_status = 'new' > + color_stdout("[ new ]\n", schema='test_new') > + else: > + short_status = 'fail' > + color_stdout("[ fail ]\n", schema='test_fail') > + elif (self.is_executed_ok and > + not self.is_equal_result and > + os.path.isfile(self.result) and > + lib.Options().args.update_result): > shutil.copy(self.tmp_result, self.result) > - short_status = 'new' > - color_stdout("[ new ]\n", schema='test_new') > + short_status = 'updated' > + color_stdout("[ updated ]\n", schema='test_new') > else: > has_result = os.path.exists(self.tmp_result) > if has_result: > @@ -256,7 +268,8 @@ class Test(object): > server.print_log(15) > where = ": test execution aborted, reason " \ > "'{0}'".format(diagnostics) > - elif not self.is_crash_reported and not self.is_equal_result: > + elif (not self.is_crash_reported and not self.is_equal_result and > + not lib.Options().args.update_result): > self.print_unidiff() > server.print_log(15) > where = ": wrong test output" > -- > 2.23.0 >