Среда, 23 октября 2019, 22:10 +03:00 от Alexander Turenko <alexander.turenko@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`:
Corrected.

 | $ 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?
Changed the logic of the 'exclude' option which can have only single argument, but can be used multiply times.

(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?
Fixed.


> + 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.
Changed.

Also we can decrease a indentation level using for/else construction.
See http://book.pythontips.com/en/latest/for_-_else.html
Changed.

> + # 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.
Corrected.

> + else:
> + test_suite.tests = found_tests
>
> def get_param(self, param=None):
> if param is not None:
> --
> 2.17.1
>


--
Alexander Tikhonov