From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 dev.tarantool.org (Postfix) with ESMTPS id 0813B43D678 for ; Wed, 23 Oct 2019 22:10:48 +0300 (MSK) Date: Wed, 23 Oct 2019 22:10:40 +0300 From: Alexander Turenko Message-ID: <20191023191039.fd4tafb3qdmtc25i@tkn_work_nb> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v3] Add exclude option as suite/test pattern List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Alexander V. Tikhonov" Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org We need to implement it in 'app' and 'unittest' tests too. See AppServer and UnittestServer classes (find_tests() method). See other notes below. WBR, Alexander Turenko. On Fri, Oct 18, 2019 at 10:13:25AM +0300, Alexander V. Tikhonov wrote: > Added exclude option to be able to exclude the tests > by patterns suite/test from testing, it means that > exclude option even excludes the suites given with > option suite. > Also added ability to set the list of exclude patterns > by the environment variable TEST_RUN_EXCLUDE_TESTS. > > Close #54 > --- > > Github: https://github.com/tarantool/test-run/tree/avtikhon/gh-54-exclude-option > Issue: https://github.com/tarantool/test-run/issues/54 > > lib/options.py | 12 ++++++++++++ > lib/tarantool_server.py | 21 +++++++++++++++++++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/lib/options.py b/lib/options.py > index 94dbd17..4dad227 100644 > --- a/lib/options.py > +++ b/lib/options.py > @@ -49,6 +49,18 @@ class Options: > tests starting with "show" in "box" suite. Default: run all > tests in all specified suites.""") > > + parser.add_argument( > + "--exclude", > + metavar = "exclude", Spaces around an equal sign in a call arguments are not recommended by PEP 8, consider `make lint`: | $ make lint | python2 -m flake8 *.py lib/*.py | lib/options.py:54:24: E251 unexpected spaces around keyword / parameter equals | lib/options.py:54:26: E251 unexpected spaces around keyword / parameter equals | lib/options.py:56:24: E251 unexpected spaces around keyword / parameter equals | lib/options.py:56:26: E251 unexpected spaces around keyword / parameter equals | make: *** [Makefile:6: lint] Error 1 > + nargs="*", So it will accepts several arguments? Consider the following command: | ./test-run.py foo --exclude bar baz How do you think, should it include or exclude 'baz' pattern? I tentative myself, to be honest, but I guess I would expect that 'baz' will be included. What do you think? (Yep, I see, the same with --suite, but anyway.) > + default = env_list('TEST_RUN_EXCLUDE_TESTS', ['']), > + help="""Can be empty. List of excluding test names, to look for in > + suites. Each name is used as a substring to look for in the > + path to test file, e.g. "show" will run all tests that have > + "show" in their name in all suites, "box/show" will only enable will run -> will exclude? enable -> disable? > + tests starting with "show" in "box" suite. Default: "" - > + means no tests to exclude""") > + > parser.add_argument( > "--suite", > dest='suites', > diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py > index 0340ada..d857c01 100644 > --- a/lib/tarantool_server.py > +++ b/lib/tarantool_server.py > @@ -1102,13 +1102,30 @@ class TarantoolServer(Server): > else: > tests.append(LuaTest(k, test_suite.args, test_suite.ini)) > > - test_suite.tests = [] > + found_tests = [] > # don't sort, command line arguments must be run in > # the specified order > + # loop the list of test patterns need to run > for name in test_suite.args.tests: > + # loop the list of found tests at the sources > for test in tests: > + # check if the test matches the needed pattern > if test.name.find(name) != -1: > - test_suite.tests.append(test) > + found_tests.append(test) > + > + test_suite.tests = [] > + exclude_tests = test_suite.args.exclude > + # check if any of the exclude patterns set > + if exclude_tests != ['']: I would choose such default value that the check would be `if exclude_tests` (because more simple is more pythonic). An empty list would be good choice I think. Also we can decrease a indentation level using for/else construction. See http://book.pythontips.com/en/latest/for_-_else.html > + # loop the list of test patterns need to exclude > + for name in exclude_tests: > + # loop the list of found tests need to run > + for test in found_tests: > + # check if the test not matches the pattern > + if test.name.find(name) == -1: > + test_suite.tests.append(test) I think we should exclude a test if it matches **any** of exclude patterns. Also I guess the current implementation will add a test several times if several exclude patterns are provided and there are several of them that does not match the test. > + else: > + test_suite.tests = found_tests > > def get_param(self, param=None): > if param is not None: > -- > 2.17.1 >