<HTML><BODY><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Среда, 23 октября 2019, 22:10 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY">We need to implement it in 'app' and 'unittest' tests too. See AppServer<br>
and UnittestServer classes (find_tests() method).<br><br>
See other notes below.<br><br>
WBR, Alexander Turenko.<br><br>
On Fri, Oct 18, 2019 at 10:13:25AM +0300, Alexander V. Tikhonov wrote:<br>
> Added exclude option to be able to exclude the tests<br>
> by patterns suite/test from testing, it means that<br>
> exclude option even excludes the suites given with<br>
> option suite.<br>
> Also added ability to set the list of exclude patterns<br>
> by the environment variable TEST_RUN_EXCLUDE_TESTS.<br>
> <br>
> Close #54<br>
> ---<br>
> <br>
> Github: <a href="https://github.com/tarantool/test-run/tree/avtikhon/gh-54-exclude-option" target="_blank">https://github.com/tarantool/test-run/tree/avtikhon/gh-54-exclude-option</a><br>
> Issue: <a href="https://github.com/tarantool/test-run/issues/54" target="_blank">https://github.com/tarantool/test-run/issues/54</a><br>
> <br>
> lib/options.py | 12 ++++++++++++<br>
> lib/tarantool_server.py | 21 +++++++++++++++++++--<br>
> 2 files changed, 31 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/lib/options.py b/lib/options.py<br>
> index 94dbd17..4dad227 100644<br>
> --- a/lib/options.py<br>
> +++ b/lib/options.py<br>
> @@ -49,6 +49,18 @@ class Options:<br>
> tests starting with "show" in "box" suite. Default: run all<br>
> tests in all specified suites.""")<br>
> <br>
> + parser.add_argument(<br>
> + "--exclude",<br>
> + metavar = "exclude",<br><br>
Spaces around an equal sign in a call arguments are not recommended by<br>
PEP 8, consider `make lint`:<br></div></div></div></div></blockquote>Corrected.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br>
| $ make lint<br>
| python2 -m flake8 *.py lib/*.py<br>
| lib/options.py:54:24: E251 unexpected spaces around keyword / parameter equals<br>
| lib/options.py:54:26: E251 unexpected spaces around keyword / parameter equals<br>
| lib/options.py:56:24: E251 unexpected spaces around keyword / parameter equals<br>
| lib/options.py:56:26: E251 unexpected spaces around keyword / parameter equals<br>
| make: *** [Makefile:6: lint] Error 1<br><br>
> + nargs="*",<br><br>
So it will accepts several arguments?<br><br>
Consider the following command:<br><br>
| ./test-run.py foo --exclude bar baz<br><br>
How do you think, should it include or exclude 'baz' pattern?<br><br>
I tentative myself, to be honest, but I guess I would expect that 'baz'<br>
will be included. What do you think?<br></div></div></div></div></blockquote>Changed the logic of the 'exclude' option which can have only single argument, but can be used multiply times.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br>
(Yep, I see, the same with --suite, but anyway.)<br><br>
> + default = env_list('TEST_RUN_EXCLUDE_TESTS', ['']),<br>
> + help="""Can be empty. List of excluding test names, to look for in<br>
> + suites. Each name is used as a substring to look for in the<br>
> + path to test file, e.g. "show" will run all tests that have<br>
> + "show" in their name in all suites, "box/show" will only enable<br><br>
will run -> will exclude?<br>
enable -> disable?</div></div></div></div></blockquote>Fixed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br><br>
> + tests starting with "show" in "box" suite. Default: "" -<br>
> + means no tests to exclude""")<br>
> +<br>
> parser.add_argument(<br>
> "--suite",<br>
> dest='suites',<br>
> diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py<br>
> index 0340ada..d857c01 100644<br>
> --- a/lib/tarantool_server.py<br>
> +++ b/lib/tarantool_server.py<br>
> @@ -1102,13 +1102,30 @@ class TarantoolServer(Server):<br>
> else:<br>
> tests.append(LuaTest(k, test_suite.args, test_suite.ini))<br>
> <br>
> - test_suite.tests = []<br>
> + found_tests = []<br>
> # don't sort, command line arguments must be run in<br>
> # the specified order<br>
> + # loop the list of test patterns need to run<br>
> for name in test_suite.args.tests:<br>
> + # loop the list of found tests at the sources<br>
> for test in tests:<br>
> + # check if the test matches the needed pattern<br>
> if test.name.find(name) != -1:<br>
> - test_suite.tests.append(test)<br>
> + found_tests.append(test)<br>
> +<br>
> + test_suite.tests = []<br>
> + exclude_tests = test_suite.args.exclude<br>
> + # check if any of the exclude patterns set<br>
> + if exclude_tests != ['']:<br><br>
I would choose such default value that the check would be `if<br>
exclude_tests` (because more simple is more pythonic). An empty list<br>
would be good choice I think.<br></div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br>
Also we can decrease a indentation level using for/else construction.<br>
See <a href="http://book.pythontips.com/en/latest/for_-_else.html" target="_blank">http://book.pythontips.com/en/latest/for_-_else.html</a><br></div></div></div></div></blockquote>Changed.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br>
> + # loop the list of test patterns need to exclude<br>
> + for name in exclude_tests:<br>
> + # loop the list of found tests need to run<br>
> + for test in found_tests:<br>
> + # check if the test not matches the pattern<br>
> + if test.name.find(name) == -1:<br>
> + test_suite.tests.append(test)<br><br>
I think we should exclude a test if it matches **any** of exclude<br>
patterns. Also I guess the current implementation will add a test<br>
several times if several exclude patterns are provided and there are<br>
several of them that does not match the test.<br></div></div></div></div></blockquote>Corrected.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15718578480212676227_BODY"><br>
> + else:<br>
> + test_suite.tests = found_tests<br>
> <br>
> def get_param(self, param=None):<br>
> if param is not None:<br>
> -- <br>
> 2.17.1<br>
> <br></div></div></div></div></blockquote>
<br>
<br>-- <br>Alexander Tikhonov<br></BODY></HTML>