--regex and --blacklist_file does not work together

[Waldemar Znoinski]

1. when specifying both --regex <regex> and --blacklist_file when calling ostestr the regex applied is not correct

--- shell ---
$ cat tempest_tests_blacklist
tempest.api

$ ostestr --blacklist_file tempest_tests_blacklist --regex '(networks)' --list
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list

--- /shell ---

the exclude_regex variable inside os_testr.py seem to be
^((?!tempest.api).)*$(networks)

which doesn't match any of the existing tests under /opt/stack/tempest due to new line character ($)

2. running the below gives same no results:
testr list_tests '^((?!tempest.api).)*$(networks)'

3. removing new line character (which is added by os_testr.py unconditionally):
testr list_tests '^((?!tempest.api).)*(networks)'
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list
tempest.scenario.test_network_basic_ops.TestNetworkBasicOps.test_connectivity_between_vms_on_different_networks[compute,id-1546850e-fbaa-42f5-8b5f-03d8a6a95f15,network]

[Waldemar Znoinski (wznoinsk)]

4) another example
4a) $ ostestr --regex '(?!.*\[.*\bslow\b.*\])(^tempest\.(api|scenario|thirdparty))' --list
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list
tempest.api.baremetal.admin.test_api_discovery.TestApiDiscovery.test_api_versions[id-a3c27e94-f56c-42c4-8600-d6790650b9c5]
tempest.api.baremetal.admin.test_api_discovery.TestApiDiscovery.test_default_version[id-896283a6-488e-4f31-af78-6614286cbe0d]
tempest.api.baremetal.admin.test_api_discovery.TestApiDiscovery.test_version_1_resources[id-abc0b34d-e684-4546-9728-ab7a9ad9f174]
...

4b) same with blacklist_file
$ cat bl
.*\[.*\bslow\b.*\]

4c) ostestr --blacklist_file bl --regex '(^tempest\.(api|scenario|thirdparty))' --list
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list

4d) regex formed from blacklist_file + --regex looks to be:
^((?!.*\[.*\bslow\b.*\]).)*$(^tempest\.(api|scenario|thirdparty))
(?!.*\[.*\bslow\b.*\])(^tempest\.(api|scenario|thirdparty)) (pasted here from 4a) for comparison)

[melanie witt (melwitt)]

I ran into this same problem and changing in os-testr:

 exclude_regex = "^((?!" + exclude_regex + ").)*$"

to

 exclude_regex = "^((?!" + exclude_regex + ").)*"

made it work for me too

[melanie witt (melwitt)]

Argh, I spoke too soon, removing '$' stops it from excluding tests in the blacklist. I had gotten mixed up when I looked at the output of the ostestr --list with the difference in the exclude_regex.

For example:

testr list-tests '^((?!nova.tests.unit.volume.test_cinder.CinderApiTestCase).)*'

will *include* all of the nova.tests.unit.volume.test_cinder.CinderApiTestCase tests.

Whereas:

testr list-tests '^((?!nova.tests.unit.volume.test_cinder.CinderApiTestCase).)*$'

will exclude the nova.tests.unit.volume.test_cinder.CinderApiTestCase tests.

:(

[Attila Fazekas (afazekas)]

testr just accept one reg-exp argument it is not sufficient for the whole logic! (It should have dedicated accept_re + reject_re)

In case if we have defined black_list + regexp specified, the still simple and not too bad thing what we can do now is to have testr list-tests (with one regexp arg) for getting the test case list, and then filtering that list even more based on the blacklist file content.

We need to convert a list of test cases (exact test names) to a regexp.

So the the `normal flow` will be:
 1. testr list-tests
 2. filter the listed tests based on the extra regexps from the black_list
 3. convert the list to regexp

PS.: I also noticed the current code would call testr too many times in case of the print_skips would be used.

[Attila Fazekas (afazekas)]

The linux argument limit is 1/4th of the stack size for all argument,
but one arg (the final regexp) cannot be more 131072 (including \0) .
The regexp needs to be compacted at the end, or we need to pass the test ids in an another way.

[Attila Fazekas (afazekas)]

https://review.openstack.org/#/c/348878

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/348878
Committed: https://git.openstack.org/cgit/openstack/os-testr/commit/?id=a3b403bd4e0ae54c63c85dec9379fa92471cd73c
Submitter: Jenkins
Branch: master

commit a3b403bd4e0ae54c63c85dec9379fa92471cd73c
Author: Attila Fazekas <email address hidden>
Date: Fri Jul 29 14:56:30 2016 +0200

    Construct a list of test cases instead of passing a regexp

    The way how we handled the regular expressions had a lot of
    limitation.

     - We are not able to pass huge arguments to testr, it makes
       difficult to have long list if accepted and/or rejected test cases,
       but we can pass a path to a file of test cases,
       which can be arbitrary big.
     - How we wanted to handle the backlists before was not worked together
       with the regular selecting regex because it consumed the pattern.
       Now the blacklisting happens in a separated phase after the selecting
       regex search.

    This change just allows the new code path to run when both
     a blacklist_file and a selecting regexp specified.

    The new way depends on the usual test discovery and just
    filters the output of the discovery command,
    this strategy can be the default in the future, now I just
    wanted to preserve the old behavior as much as possible in
    all the other cases.

    Change-Id: Ie8e5928e286d0c9076c4eee23319149c9869a6fa
    Closes-Bug: #1506215

[Szymon Datko (sdatko)]

The OpenStack Tempest project is still impacted by the mentioned issue, as it uses the original regex constructor that were used by os-testr before Attila's patch (change#348878).

I proposed a patchset that fixes the original issue: https://review.openstack.org/#/c/522768/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/522768
Committed: https://git.openstack.org/cgit/openstack/os-testr/commit/?id=96db91056aded65abd8e8bb531ebcaf77b32b12a
Submitter: Zuul
Branch: master

commit 96db91056aded65abd8e8bb531ebcaf77b32b12a
Author: Szymon Datko <email address hidden>
Date: Thu Nov 23 15:54:29 2017 +0100

    Fix regex builder

    Currently when the blacklist_file and regex string is provided,
    the constructed regex looks like '^((?!black1|black2|...).)*$regex'.
    This is incorrect, as it will match nothing - some string
    is expected after end of the line [denoted as $ in the regex].

    The proper construction is like ^(?!black1|black2|...).*(regex).*$
    This solves the issue with Tempest, where using blacklist for smoke
    tests is not working now, as it leads into the issue described above.

    Change-Id: Icdeb3c311f7eb414158aedb4c030494b419211c0
    Closes-Bug: #1506215
    Closes-Bug: #1595119
    Closes-Bug: #1622722
    Closes-Bug: #1669455

[Ghanshyam Mann (ghanshyammann)]

nothing special needed in tempest for this.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/os-testr 1.1.0 release.