From 2f29ca51be36652d20f09ef50263629f27276ce7 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 9 Jul 2015 21:22:25 -0400 Subject: [PATCH 01/17] Improved docs for os-testr commands This commit adds real docs for using the 3 currently packaged commands in the os-testr project. As part of this change it also updates the sphinx config to generate proper man pages for all the tooling. At a future stage we need to ensure that these get installed properly. Change-Id: I0055c7961203a094590f6cfeb136a2236a2f65cc --- README.rst | 8 ++ doc/source/conf.py | 15 ++- doc/source/ostestr.rst | 217 +++++++++++++++++++++++++++++++++++ doc/source/subunit2html.rst | 33 ++++++ doc/source/subunit_trace.rst | 110 ++++++++++++++++++ doc/source/usage.rst | 13 ++- 6 files changed, 388 insertions(+), 8 deletions(-) create mode 100644 doc/source/ostestr.rst create mode 100644 doc/source/subunit2html.rst create mode 100644 doc/source/subunit_trace.rst diff --git a/README.rst b/README.rst index ebfd8f7..87dc74c 100644 --- a/README.rst +++ b/README.rst @@ -21,6 +21,14 @@ Features Release Notes ============= +0.2.0 +----- + * Adds support for comments in a blacklist file and printing comments for each + exclude + * Several bugfixes for subunit-trace + * Switched subunit-trace behavior to disable printing percent change + in run time by default and make it optional + 0.1.0 ----- * First release which includes: ostestr, subunit-trace, and subunit2html diff --git a/doc/source/conf.py b/doc/source/conf.py index 251d672..45a8546 100755 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -22,7 +22,7 @@ sys.path.insert(0, os.path.abspath('../..')) # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. extensions = [ 'sphinx.ext.autodoc', - #'sphinx.ext.intersphinx', + # 'sphinx.ext.intersphinx', 'oslosphinx' ] @@ -38,7 +38,7 @@ master_doc = 'index' # General information about the project. project = u'os-testr' -copyright = u'2013, OpenStack Foundation' +copyright = u'2015, Matthew Treinish' # If true, '()' will be appended to :func: etc. cross-reference text. add_function_parentheses = True @@ -68,8 +68,15 @@ latex_documents = [ ('index', '%s.tex' % project, u'%s Documentation' % project, - u'OpenStack Foundation', 'manual'), + u'Matthew Treinish', 'manual'), ] +man_pages = [('ostestr', 'ostestr', 'tooling to run OpenStack tests', + ['Matthew Treinish'], 1), + ('subunit_trace', 'subunit-trace', 'pretty output filter for ' + 'subunit streams', ['Matthew Treinish'], 1), + ('subunit2html', 'subunit2html', 'generate a html results page ' + 'from a subunit stream', ['Matthew Treinish'], 1)] + # Example configuration for intersphinx: refer to the Python standard library. -#intersphinx_mapping = {'http://docs.python.org/': None} +# intersphinx_mapping = {'http://docs.python.org/': None} diff --git a/doc/source/ostestr.rst b/doc/source/ostestr.rst new file mode 100644 index 0000000..6c36bac --- /dev/null +++ b/doc/source/ostestr.rst @@ -0,0 +1,217 @@ +.. _ostestr: + +ostestr +======= + +The ostestr command provides a wrapper around the testr command included in +the testrepository package. It's designed to build on the functionality +included in testr and workaround several UI bugs in the short term. By default +it also has output that is much more useful for OpenStack's test suites which +are lengthy in both runtime and number of tests. Please note that the CLI +semantics are still a work in progress as the project is quite young, so +default behavior might change in future version. + +Summary +------- + ostestr [-b|--blacklist_file ] [-r|--regex REGEX] + [-p|--pretty] [--no-pretty] [-s|--subunit] [-l|--list] + [-n|--no-discover ] [--slowest] [--no-slowest] + [--pdb ] [--parallel] [--serial] + [-c|--concurrency ] [--until-failure] [--print-exclude] + +Options +------- + + --blacklist_file BLACKLIST_FILE, -b BLACKLIST_FILE + Path to a blacklist file, this file contains a + separate regex exclude on each newline + --regex REGEX, -r REGEX + A normal testr selection regex. If a blacklist file is + specified, the regex will be appended to the end of + the generated regex from that file + --pretty, -p + Print pretty output from subunit-trace. This is + mutually exclusive with --subunit + --no-pretty + Disable the pretty output with subunit-trace + --subunit, -s + output the raw subunit v2 from the test run this is + mutuall exclusive with --pretty + --list, -l + List all the tests which will be run. + --no-discover TEST_ID, -n TEST_ID + Takes in a single test to bypasses test discover and + just excute the test specified + --slowest + After the test run print the slowest tests + --no-slowest + After the test run don't print the slowest tests + --pdb TEST_ID + Run a single test that has pdb traces added + --parallel + Run tests in parallel (this is the default) + --serial + Run tests serially + --concurrency WORKERS, -c WORKERS + The number of workers to use when running in parallel. + By default this is the number of cpus + --until-failure + Run the tests in a loop until a failure is + encountered. Running with subunit or prettyoutput + enable will force the loop to run testsserially + --print-exclude + If an exclude file is used this option will prints the + comment from the same line and all skipped tests + before the test run + +Running Tests +------------- + +os-testr is primarily for running tests at it's basic level you just invoke +ostestr to run a test suite for a project. (assuming it's setup to run tests +using testr already) For example:: + + $ ostestr + +This will run tests in parallel (with the number of workers matching the number +of CPUs) and with subunit-trace output. If you need to run tests in serial you +can use the serial option:: + + $ ostestr --serial + +Or if you need to adjust the concurrency but still run in parallel you can use +-c/--concurrency:: + + $ ostestr --concurrency 2 + +If you only want to run an individual test module or more specific (a single +class, or test) and parallel execution doesn't matter, you can use the +-n/--no-discover to skip test discovery and just directly calls subunit.run on +the tests under the covers. Bypassing discovery is desirable when running a +small subset of tests in a larger test suite because the discovery time can +often far exceed the total run time of the tests. + +For example:: + + $ ostestr --no-discover test.test_thing.TestThing.test_thing_method + +Additionally, if you need to run a single test module, class, or single test +with pdb enabled you can use --pdb to directly call testtools.run under the +covers which works with pdb. For example:: + + $ ostestr --pdb tests.test_thing.TestThing.test_thing_method + + +Test Selection +-------------- + +ostestr is designed to build on top of the test selection in testr. testr only +exposed a regex option to select tests. This equivalent is functionality is +exposed via the --regex option. For example:: + + $ ostestr --regex 'magic\.regex' + +This will do a straight passthrough of the provided regex to testr. +Additionally, ostestr allows you to specify a a blacklist file to define a set +of regexes to exclude. You can specify a blacklist file with the +--blacklist-file/-b option, for example:: + + $ ostestr --blacklist_file $path_to_file + +The format for the file is line separated regex, with '#' used to signify the +start of a comment on a line. For example:: + + # Blacklist File + ^regex1 # Excludes these tests + .*regex2 # exclude those tests + +Will generate a regex to pass to testr which will exclude both any tests +matching '^regex1' and '.*regex2'. If a blacklist file is used in conjunction +with the --regex option the regex specified with --regex will be appended to +the generated output from the --blacklist_file. Also it's worth noting that the +regex test selection options can not be used in conjunction with the +--no-discover or --pdb options described in the previous section. This is +because the regex selection requires using testr under the covers to actually +do the filtering, and those 2 options do not use testr. + +It's also worth noting that you can use the test list option to dry run any +selection arguments you are using. You just need to use --list/-l with your +selection options to do this, for example:: + + $ ostestr --regex 'regex3.*' --blacklist_file blacklist.txt --list + +This will list all the tests which will be run by ostestr using that combination +of arguments. + +Please not that all of this selection functionality will be expanded on in the +future and a default grammar for selecting multiple tests will be chosen in a +future release. However as of right now all current arguments (which have +guarantees on always remaining in place) are still required to perform any +selection logic while this functionality is still under development. + + +Output Options +-------------- + +By default ostestr will use subunit-trace as the output filter on the test +run. It will also print the slowest tests from the run after the run is +concluded. You can disable the printing the slowest tests with the --no-slowest +flag, for example:: + + $ ostestr --no-slowest + +If you'd like to disable the subunit-trace output you can do this using +--no-pretty:: + + $ ostestr --no-pretty + +ostestr also provides the option to just output the raw subunit stream on +STDOUT with --subunit/-s. Note if you want to use this you also have to +specify --no-pretty as the subunit-trace output and the raw subunit output +are mutually exclusive. For example, to get raw subunit output the arguments +would be:: + + $ ostestr --no-pretty --subunit + +An additional option on top of the blacklist file is --print-exclude option. +When this option is specified when using a blacklist file before the tests are +run ostestr will print all the tests it will be excluding from the blacklist +file. If a line in the blacklist file has a comment that will be printed before +listing the tests which will be excluded by that line's regex. If no comment is +present on a line the regex from that line will be used instead. For example, +if you were using the example blacklist file from the previous section the +output before the regular test run output would be:: + + $ ostestr -b blacklist-file blacklist.txt --print-exclude + Excludes these tests + regex1_match + regex1_exclude + + exclude those tests + regex2_match + regex2_exclude + + ... + +Notes for running with tox +-------------------------- + +If you use `tox`_ for running your tests and call ostestr as the test command +.. _tox: https://tox.readthedocs.org/en/latest/ +it's recommended that you set a posargs following ostestr on the commands + stanza. For example:: + + [testenv] + commands = ostestr {posargs} + +this will enable end users to pass args to configure the output, use the +selection logic, or any other options directly from the tox cli. This will let +tox take care of the venv management and the environment separation but enable +direct access to all of the ostestr options to easily customize your test run. +For example, assuming the above posargs usage you would be to do:: + + $ tox -epy34 -- --regex ^regex1 + +or to skip discovery:: + + $ tox -epy34 -- -n test.test_thing.TestThing.test_thing_method diff --git a/doc/source/subunit2html.rst b/doc/source/subunit2html.rst new file mode 100644 index 0000000..f0ebbc9 --- /dev/null +++ b/doc/source/subunit2html.rst @@ -0,0 +1,33 @@ +.. _subunit2html: + +subunit2html +============ + +subunit2html is a tool that takes in a subunit stream file and will output an +html page + +Summary +------- + + subunit2html subunit_stream [output] + +Usage +----- + +subunit2html takes in 1 mandatory argument. This is used to specify the location +of the subunit stream file. For example:: + + $ subunit2html subunit_stream + +By default subunit2html will store the generated html results file at +results.html file in the current working directory. + +An optional second argument can be provided to set the output path of the html +results file that is generated. If it is provided this will be the output path +for saving the generated file, otherwise results.html in the current working +directory will be used. For example:: + + $ subunit2html subunit_stream test_results.html + +will write the generated html results file to test_results.html in the current +working directory diff --git a/doc/source/subunit_trace.rst b/doc/source/subunit_trace.rst new file mode 100644 index 0000000..1338fe6 --- /dev/null +++ b/doc/source/subunit_trace.rst @@ -0,0 +1,110 @@ +.. _subunit_trace: + +subunit-trace +============= + +subunit-trace is an output filter for subunit streams. It is often used in +conjunction with test runners that emit subunit to enable a consistent and +useful realtime output from a test run. + +Summary +------- + +subunit-trace [--fails|-f] [--failonly] [--perc-diff|-d] [--no-summary] + [--diff-threshold|-t ] + +Options +------- + + --no-failure-debug, -n + Disable printing failure debug information in realtime + --fails, -f + Print failure debug information after the stream is + proccesed + --failonly + Don't print success items + --perc-diff, -d + Print percent change in run time on each test + --diff-threshold THRESHOLD, -t THRESHOLD + Threshold to use for displaying percent change from the + avg run time. If one is not specified the percent + change will always be displayed. + --no-summary + Don't print the summary of the test run after completes + +Usage +----- +subunit-trace will take a subunit stream in via STDIN. This is the only input +into the tool. It will then print on STDOUT the formatted test result output +for the test run information contained in the stream. + +A subunit v2 stream must be passed into subunit-trace. If only a subunit v1 +stream is available you must use the subunit-1to2 utility to convert it before +passing the stream into subunit-trace. For example this can be done by chaining +pipes:: + + $ cat subunit_v1 | subunit-1to2 | subunit-trace + +Adjusting per test output +^^^^^^^^^^^^^^^^^^^^^^^^^ + +subunit-trace provides several options to customize it's output. This allows +users to customize the output from subunit-trace to suit their needs. The output +from subunit-trace basically comes in 2 parts, the per test output, and the +summary at the end. By default subunit-trace will print failure messages during +the per test output, meaning when a test fails it will also print the message +and any traceback and other attachments at that time. However this can be +disabled by using --no-failure-debug, -n. For example:: + + $ testr run --subunit | subunit-trace --no-failure-debug + +Rhere is also the option to print all failures together at the end of the test +run before the summary view. This is done using the --fails/-f option. For +example:: + + $ testr run --subunit | subunit-trace --fails + +Often the --fails and --no-failure-debug options are used in conjunction to +only print failures at the end of a test run. This is useful for large test +suites where an error message might be lost in the noise. To do this :: + + $ testr run --subunit | subunit-trace --fails --no-failure-debug + +By default subunit-trace will print a line for each test after it completes with +the test status. However, if you only want to see the run time output for +failures and not any other test status you can use the --failonly option. For +example:: + + $ testr run --subunit | subunit-trace --failonly + +The last output option provided by subunit-trace is to diable the summary view +of the test run which is normally displayed at the end of a run. You can do +this using the --no-summary option. For example:: + + $ testr run --subunit | subunit-trace --no-summary + + +Show per test run time percent change +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +subunit-trace provides an option to display the percent change in run time +from the previous run. To do this subunit-trace leverages the testr internals +a bit. It uses the times.dbm database which, the file repository type in +testrepository will create, to get the previous run time for a test. If testr +hasn't ever been used before or for whatever reason subunit-trace is unable to +find the times.dbm file from testr no percentages will be displayed even if it's +enabled. Additionally, if a test is run which does not have an entry in the +times.dbm file will not have a percentage printed for it. + +To enable this feature you use --perc-diff/-d, for example:: + + $ testr run --subunit | subunit-trace --perc-diff + +There is also the option to set a threshold value for this option. If used it +acts as an absolute value and only percentage changes that exceed it will be +printed. Use the --diff-threshold/-t option to set a threshold, for example:: + + $ testr run --subunit | subunit-trace --perc-diff --threshold 45 + +This will only display percent differences when the change in run time is either +>=45% faster or <=45% slower. diff --git a/doc/source/usage.rst b/doc/source/usage.rst index a5b21da..b1ab10a 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -1,7 +1,12 @@ -======== +===== Usage -======== +===== -To use os-testr in a project:: +This section contains the documentation for each of tools packaged in os-testr - import os_testr +.. toctree:: + :maxdepth: 2 + + ostestr + subunit_trace + subunit2html From e43001c9e66a45b56c0d4f66537e068a7e7d60b6 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 14 Jul 2015 11:39:11 -0400 Subject: [PATCH 02/17] Add TODO entry for moving away from subprocess in ostestr This commit adds a new entry to the os-testr TODO file to indicate the long term goal for migrating away from using subprocess in ostestr. Everything ostestr uses is a python project so we shouldn't need to call out for everything, it was just faster to bootstrap the project this way, but we should probably move away from doing this. Change-Id: I3b2e54d72514ad92079a893fd7a835e6d446d4bc --- TODO.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO.rst b/TODO.rst index 135bcd6..ed52e84 100644 --- a/TODO.rst +++ b/TODO.rst @@ -15,3 +15,5 @@ Long Term * Add subunit-trace functional tests ** Sample subunit streams and test output from subunit-trace * Add testing for subunit2html + * Stop using subprocess in ostestr, everything it uses is python so there + isn't a need to shell out for everything. From 970e74bfd8c55f054f0f36f684886724b02bf260 Mon Sep 17 00:00:00 2001 From: Kun Huang Date: Fri, 31 Jul 2015 10:12:21 +0800 Subject: [PATCH 03/17] update requirements Current pbr requirements is blocking tempest develop mode installation. http://paste.openstack.org/show/406537/ Change-Id: I14dfe4c66d22d742706acbd1e5a574587eb40039 --- requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6b439b7..39d130e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,8 +2,8 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -pbr>=0.6,!=0.7,<1.0 +pbr>=1.3,<2.0 Babel>=1.3 testrepository>=0.0.18 python-subunit>=0.0.18 -testtools>=0.9.36,!=1.2.0 +testtools>=1.4.0 From 04a393eae28f78c1057a6e81fdc29429ff01d98a Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 5 Aug 2015 18:30:06 -0400 Subject: [PATCH 04/17] Set min pbr version in setup_requires This commit adds a minimum version on the setuptools setup_requires for pbr. This is required to enable markers and also to ensure a consistent version when installing with other packages. Change-Id: I2168d91896ba82581929a2faeebace7419b4539e --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 7363757..ee81ca9 100755 --- a/setup.py +++ b/setup.py @@ -26,5 +26,5 @@ except ImportError: pass setuptools.setup( - setup_requires=['pbr'], + setup_requires=['pbr>=1.3'], pbr=True) From 2ea7c8837bae482bd4a3cc37025baa7e2f456a54 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 6 Aug 2015 12:03:57 -0400 Subject: [PATCH 05/17] Handle incomplete subunit streams If a subunit stream is aborted in the middle (like in the case of a segfault) this causes subunit-trace to emit a stacktrace. This commit attempts to handle this edge case gracefully. Related-Bug: #1482230 Change-Id: I1a8a0a8e2ab65e637c6a5212e324670b7d95d28d --- os_testr/subunit_trace.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/os_testr/subunit_trace.py b/os_testr/subunit_trace.py index 7c00825..967c53f 100755 --- a/os_testr/subunit_trace.py +++ b/os_testr/subunit_trace.py @@ -239,8 +239,13 @@ def run_time(): def worker_stats(worker): tests = RESULTS[worker] num_tests = len(tests) - delta = tests[-1]['timestamps'][1] - tests[0]['timestamps'][0] - return num_tests, delta + stop_time = tests[-1]['timestamps'][1] + start_time = tests[0]['timestamps'][0] + if not start_time or not stop_time: + delta = 'N/A' + else: + delta = stop_time - start_time + return num_tests, str(delta) def print_summary(stream, elapsed_time): @@ -266,8 +271,11 @@ def print_summary(stream, elapsed_time): "Race in testr accounting.\n" % w) else: num, time = worker_stats(w) - stream.write(" - Worker %s (%s tests) => %ss\n" % - (w, num, time)) + out_str = " - Worker %s (%s tests) => %s" % (w, num, time) + if time.isdigit(): + out_str += 's' + out_str += '\n' + stream.write(out_str) def parse_args(): From aee3402c6815f6620f23125f786cbd972ae16ae4 Mon Sep 17 00:00:00 2001 From: TerryHowe Date: Sun, 19 Jul 2015 08:30:14 -0600 Subject: [PATCH 06/17] Convert file names to regular expressions Add the capability to the --no-discover option to convert file names into regular sexpressions. Also add the --path option that converts a file or directory to a regular expression. Create a mutually exclusive argparse group for --regex, --path, and --no-discover. This change will allow the user to specify a file name instead of a regular expression to match a particular set of tests e.g: tox -e py27 -- --path os_testr/tests/test_os_testr.py Will run os_testr.tests.test_os_testr but you can use tab complete to generate the name. Change-Id: Ibfca2bc023aed44b1b87a0444559ab2a00303a70 --- os_testr/os_testr.py | 35 ++++++++++++++++++++++++--------- os_testr/tests/test_os_testr.py | 10 +++++++--- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 353868c..00d7cd4 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -29,11 +29,19 @@ def parse_args(): parser.add_argument('--blacklist_file', '-b', help='Path to a blacklist file, this file contains a' ' separate regex exclude on each newline') - parser.add_argument('--regex', '-r', - help='A normal testr selection regex. If a blacklist ' - 'file is specified, the regex will be appended ' - 'to the end of the generated regex from that ' - 'file') + group = parser.add_mutually_exclusive_group() + group.add_argument('--regex', '-r', + help='A normal testr selection regex. If a blacklist ' + 'file is specified, the regex will be appended ' + 'to the end of the generated regex from that ' + 'file.') + group.add_argument('--path', metavar='FILE_OR_DIRECTORY', + help='A file name or directory of tests to run.') + group.add_argument('--no-discover', '-n', metavar='TEST_ID', + help="Takes in a single test to bypasses test " + "discover and just excute the test specified. " + "A file name may be used in place of a test " + "name.") parser.add_argument('--pretty', '-p', dest='pretty', action='store_true', help='Print pretty output from subunit-trace. This is ' 'mutually exclusive with --subunit') @@ -44,9 +52,6 @@ def parse_args(): 'this is mutuall exclusive with --pretty') parser.add_argument('--list', '-l', action='store_true', help='List all the tests which will be run.') - parser.add_argument('--no-discover', '-n', metavar='TEST_ID', - help="Takes in a single test to bypasses test " - "discover and just excute the test specified") parser.add_argument('--slowest', dest='slowest', action='store_true', help="after the test run print the slowest tests") parser.add_argument('--no-slowest', dest='slowest', action='store_false', @@ -112,6 +117,11 @@ def print_skips(regex, message): print('\n') +def path_to_regex(path): + root, _ = os.path.splitext(path) + return root.replace('/', '.') + + def construct_regex(blacklist_file, regex, print_exclude): if not blacklist_file: exclude_regex = '' @@ -248,7 +258,11 @@ def main(): msg = "You can not use until_failure mode with pdb or no-discover" print(msg) exit(5) - exclude_regex = construct_regex(opts.blacklist_file, opts.regex, + if opts.path: + regex = path_to_regex(opts.path) + else: + regex = opts.regex + exclude_regex = construct_regex(opts.blacklist_file, regex, opts.print_exclude) if not os.path.isdir('.testrepository'): subprocess.call(['testr', 'init']) @@ -259,6 +273,9 @@ def main(): elif opts.pdb: exit(call_testtools_run(opts.pdb)) else: + test_to_run = opts.no_discover + if test_to_run.find('/') != -1: + test_to_run = path_to_regex(test_to_run) exit(call_subunit_run(opts.no_discover, opts.pretty, opts.subunit)) if __name__ == '__main__': diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 3f64916..7b4ce22 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -19,10 +19,14 @@ test_os_testr Tests for `os_testr` module. """ +from os_testr import os_testr from os_testr.tests import base -class TestOs_testr(base.TestCase): +class Test_Construct_Regex(base.TestCase): - def test_something(self): - pass + def test_file_name(self): + result = os_testr.path_to_regex("tests/network/v2/test_net.py") + self.assertEqual("tests.network.v2.test_net", result) + result = os_testr.path_to_regex("openstack/tests/network/v2") + self.assertEqual("openstack.tests.network.v2", result) From 8eb9453815354ad31b523c334e9fca030722195c Mon Sep 17 00:00:00 2001 From: TerryHowe Date: Thu, 6 Aug 2015 16:48:37 -0600 Subject: [PATCH 07/17] Make use of argparse groups and add some tests This change makes use of argparse groups to generate error messages when the user specifies invalid command line option combinations. For example if the user specified --pretty and --no-pretty before it would not complain, but now it will. Change-Id: I07e1edb5c43ff7b0a81879f388770fb732fed43b --- os_testr/os_testr.py | 34 +++++++++++++++++---------------- os_testr/tests/test_os_testr.py | 28 ++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 00d7cd4..6e9aeb6 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -23,7 +23,7 @@ from subunit import run as subunit_run from testtools import run as testtools_run -def parse_args(): +def get_parser(): parser = argparse.ArgumentParser( description='Tool to run openstack tests') parser.add_argument('--blacklist_file', '-b', @@ -42,27 +42,30 @@ def parse_args(): "discover and just excute the test specified. " "A file name may be used in place of a test " "name.") - parser.add_argument('--pretty', '-p', dest='pretty', action='store_true', + pretty = parser.add_mutually_exclusive_group() + pretty.add_argument('--pretty', '-p', dest='pretty', action='store_true', help='Print pretty output from subunit-trace. This is ' 'mutually exclusive with --subunit') - parser.add_argument('--no-pretty', dest='pretty', action='store_false', + pretty.add_argument('--no-pretty', dest='pretty', action='store_false', help='Disable the pretty output with subunit-trace') parser.add_argument('--subunit', '-s', action='store_true', help='output the raw subunit v2 from the test run ' - 'this is mutuall exclusive with --pretty') + 'this is mutually exclusive with --pretty') parser.add_argument('--list', '-l', action='store_true', help='List all the tests which will be run.') - parser.add_argument('--slowest', dest='slowest', action='store_true', - help="after the test run print the slowest tests") - parser.add_argument('--no-slowest', dest='slowest', action='store_false', - help="after the test run don't print the slowest " - "tests") + slowest = parser.add_mutually_exclusive_group() + slowest.add_argument('--slowest', dest='slowest', action='store_true', + help="after the test run print the slowest tests") + slowest.add_argument('--no-slowest', dest='slowest', action='store_false', + help="after the test run don't print the slowest " + "tests") parser.add_argument('--pdb', metavar='TEST_ID', help='Run a single test that has pdb traces added') - parser.add_argument('--parallel', dest='parallel', action='store_true', - help='Run tests in parallel (this is the default)') - parser.add_argument('--serial', dest='parallel', action='store_false', - help='Run tests serially') + parallel = parser.add_mutually_exclusive_group() + parallel.add_argument('--parallel', dest='parallel', action='store_true', + help='Run tests in parallel (this is the default)') + parallel.add_argument('--serial', dest='parallel', action='store_false', + help='Run tests serially') parser.add_argument('--concurrency', '-c', type=int, metavar='WORKERS', help='The number of workers to use when running in ' 'parallel. By default this is the number of cpus') @@ -76,8 +79,7 @@ def parse_args(): 'prints the comment from the same line and all ' 'skipped tests before the test run') parser.set_defaults(pretty=True, slowest=True, parallel=True) - opts = parser.parse_args() - return opts + return parser def _get_test_list(regex, env=None): @@ -239,7 +241,7 @@ def call_testtools_run(test_id): def main(): - opts = parse_args() + opts = get_parser().parse_args() if opts.pretty and opts.subunit: msg = ('Subunit output and pretty output cannot be specified at the ' 'same time') diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 7b4ce22..8370b4a 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -23,10 +23,36 @@ from os_testr import os_testr from os_testr.tests import base -class Test_Construct_Regex(base.TestCase): +class TestPathToRegex(base.TestCase): def test_file_name(self): result = os_testr.path_to_regex("tests/network/v2/test_net.py") self.assertEqual("tests.network.v2.test_net", result) result = os_testr.path_to_regex("openstack/tests/network/v2") self.assertEqual("openstack.tests.network.v2", result) + + +class TestGetParser(base.TestCase): + def test_pretty(self): + namespace = os_testr.get_parser().parse_args(['--pretty']) + self.assertEqual(True, namespace.pretty) + namespace = os_testr.get_parser().parse_args(['--no-pretty']) + self.assertEqual(False, namespace.pretty) + self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + ['--no-pretty', '--pretty']) + + def test_slowest(self): + namespace = os_testr.get_parser().parse_args(['--slowest']) + self.assertEqual(True, namespace.slowest) + namespace = os_testr.get_parser().parse_args(['--no-slowest']) + self.assertEqual(False, namespace.slowest) + self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + ['--no-slowest', '--slowest']) + + def test_parallel(self): + namespace = os_testr.get_parser().parse_args(['--parallel']) + self.assertEqual(True, namespace.parallel) + namespace = os_testr.get_parser().parse_args(['--serial']) + self.assertEqual(False, namespace.parallel) + self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + ['--parallel', '--serial']) From 01ae2c608b8093ad592a49a050e2822373a3d6fb Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Sun, 2 Aug 2015 17:59:34 -0400 Subject: [PATCH 08/17] Minimize output when --abbreviate is used print '.', 'F', 'S' etc instead of verbose output when --abbreviate is used. Change-Id: Iafd9ca58d846373a9e441b78cabc432475a5367b --- os_testr/subunit_trace.py | 62 ++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/os_testr/subunit_trace.py b/os_testr/subunit_trace.py index 967c53f..2af7376 100755 --- a/os_testr/subunit_trace.py +++ b/os_testr/subunit_trace.py @@ -147,7 +147,7 @@ def find_test_run_time_diff(test_id, run_time): def show_outcome(stream, test, print_failures=False, failonly=False, - enable_diff=False, threshold='0'): + enable_diff=False, threshold='0', abbreviate=False): global RESULTS status = test['status'] # TODO(sdague): ask lifeless why on this? @@ -168,30 +168,42 @@ def show_outcome(stream, test, print_failures=False, failonly=False, if status == 'fail': FAILS.append(test) - stream.write('{%s} %s [%s] ... FAILED\n' % ( - worker, name, duration)) - if not print_failures: - print_attachments(stream, test, all_channels=True) - elif not failonly: - if status == 'success': - out_string = '{%s} %s [%s' % (worker, name, duration) - perc_diff = find_test_run_time_diff(test['id'], duration) - if enable_diff: - if perc_diff and abs(perc_diff) >= abs(float(threshold)): - if perc_diff > 0: - out_string = out_string + ' +%.2f%%' % perc_diff - else: - out_string = out_string + ' %.2f%%' % perc_diff - stream.write(out_string + '] ... ok\n') - print_attachments(stream, test) - elif status == 'skip': - stream.write('{%s} %s ... SKIPPED: %s\n' % ( - worker, name, test['details']['reason'].as_text())) + if abbreviate: + stream.write('F') else: - stream.write('{%s} %s [%s] ... %s\n' % ( - worker, name, duration, test['status'])) + stream.write('{%s} %s [%s] ... FAILED\n' % ( + worker, name, duration)) if not print_failures: print_attachments(stream, test, all_channels=True) + elif not failonly: + if status == 'success': + if abbreviate: + stream.write('.') + else: + out_string = '{%s} %s [%s' % (worker, name, duration) + perc_diff = find_test_run_time_diff(test['id'], duration) + if enable_diff: + if perc_diff and abs(perc_diff) >= abs(float(threshold)): + if perc_diff > 0: + out_string = out_string + ' +%.2f%%' % perc_diff + else: + out_string = out_string + ' %.2f%%' % perc_diff + stream.write(out_string + '] ... ok\n') + print_attachments(stream, test) + elif status == 'skip': + if abbreviate: + stream.write('S') + else: + stream.write('{%s} %s ... SKIPPED: %s\n' % ( + worker, name, test['details']['reason'].as_text())) + else: + if abbreviate: + stream.write('%s' % test['status'][0]) + else: + stream.write('{%s} %s [%s] ... %s\n' % ( + worker, name, duration, test['status'])) + if not print_failures: + print_attachments(stream, test, all_channels=True) stream.flush() @@ -291,6 +303,9 @@ def parse_args(): default=( os.environ.get('TRACE_FAILONLY', False) is not False)) + parser.add_argument('--abbreviate', '-a', action='store_true', + dest='abbreviate', help='Print one character status' + 'for each test') parser.add_argument('--perc-diff', '-d', action='store_true', dest='enable_diff', help="Print percent change in run time on each test ") @@ -312,7 +327,8 @@ def main(): functools.partial(show_outcome, sys.stdout, print_failures=args.print_failures, failonly=args.failonly, - enable_diff=args.enable_diff)) + enable_diff=args.enable_diff, + abbreviate=args.abbreviate)) summary = testtools.StreamSummary() result = testtools.CopyStreamResult([outcomes, summary]) result = testtools.StreamResultRouter(result) From b351bc6c8fa72d2de84ac82aefe3d5a20a53c386 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Sat, 15 Aug 2015 14:41:22 -0400 Subject: [PATCH 09/17] Handle a skipped test without a reason message This commit fixes a bug in subunit-trace where if a test is skipped but does not have a reason associated with the skip it would stack trace. This checks for the existence of a reason before trying to use it. Change-Id: I14dd9fbb40a8c232431e5042aa46f7e521e15311 --- os_testr/subunit_trace.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/os_testr/subunit_trace.py b/os_testr/subunit_trace.py index 2af7376..003299f 100755 --- a/os_testr/subunit_trace.py +++ b/os_testr/subunit_trace.py @@ -194,8 +194,11 @@ def show_outcome(stream, test, print_failures=False, failonly=False, if abbreviate: stream.write('S') else: - stream.write('{%s} %s ... SKIPPED: %s\n' % ( - worker, name, test['details']['reason'].as_text())) + reason = test['details'].get('reason', '') + if reason: + reason = ': ' + reason.as_text() + stream.write('{%s} %s ... SKIPPED%s\n' % ( + worker, name, reason)) else: if abbreviate: stream.write('%s' % test['status'][0]) From c5fd8d37cddc20dc387ec5fb74241610e5d4c294 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 21 Sep 2015 14:41:57 +0000 Subject: [PATCH 10/17] Change ignore-errors to ignore_errors Needed for coverage 4.0 Change-Id: Ib15f741b0bb6d92cea2f4aa0e0ec1d1e6d973842 --- .coveragerc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.coveragerc b/.coveragerc index b97ce41..20677dd 100644 --- a/.coveragerc +++ b/.coveragerc @@ -4,4 +4,4 @@ source = os_testr omit = os_testr/tests/*,os_testr/openstack/* [report] -ignore-errors = True +ignore_errors = True From a8ca37c5234778baa9fef152e496186b9ac1c41c Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 24 Sep 2015 11:51:50 -0400 Subject: [PATCH 11/17] Switch to using autogenerated ChangeLog in docs This commit switches the docs to use the PBR generated ChangeLog instead of a hand curated release notes section. This is a lot less error prone and instantaneous as compared to our current method which often leaves the release notes section stale for months at a time. Change-Id: Iff6586fc0113fd8292967a536d5bd37cb46ec29d --- README.rst | 15 --------------- doc/source/history.rst | 1 + doc/source/index.rst | 1 + 3 files changed, 2 insertions(+), 15 deletions(-) create mode 100644 doc/source/history.rst diff --git a/README.rst b/README.rst index 87dc74c..21f2842 100644 --- a/README.rst +++ b/README.rst @@ -17,18 +17,3 @@ Features * subunit-trace: an output filter for a subunit stream which provides useful information about the run * subunit2html: generates a test results html page from a subunit stream - -Release Notes -============= - -0.2.0 ------ - * Adds support for comments in a blacklist file and printing comments for each - exclude - * Several bugfixes for subunit-trace - * Switched subunit-trace behavior to disable printing percent change - in run time by default and make it optional - -0.1.0 ------ - * First release which includes: ostestr, subunit-trace, and subunit2html diff --git a/doc/source/history.rst b/doc/source/history.rst new file mode 100644 index 0000000..69ed4fe --- /dev/null +++ b/doc/source/history.rst @@ -0,0 +1 @@ +.. include:: ../../ChangeLog diff --git a/doc/source/index.rst b/doc/source/index.rst index 448d655..f1579dc 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -16,6 +16,7 @@ Contents: usage contributing todo + history Indices and tables ================== From f5168086f4885b08721f22229c83c2c776f765dd Mon Sep 17 00:00:00 2001 From: John Griffith Date: Fri, 25 Sep 2015 18:46:08 -0600 Subject: [PATCH 12/17] Minor refactoring to make os_testr more testable While working on some other patches and trying to add some unit tests there were a few things in os_testr that made it difficult to test. Rather than try and be super tricky, just refactor os_testr a bit. * Refactor get_parser to handle parse_args for us We'll need to explicitly pass in argv here * Move the logic of selecting/calling the test runner into its own helper method (_select_call_runner) * Modify existing tests to pass args in to get_parser * Add a new test for runner selection This cleans up the logic a bit and makes testing a good deal easier. Change-Id: I40527601613e6064cf6220f218bef1876ec69cda --- os_testr/os_testr.py | 39 +++++++++++++++++++-------------- os_testr/tests/test_os_testr.py | 37 +++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 6e9aeb6..3c604e4 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -23,7 +23,7 @@ from subunit import run as subunit_run from testtools import run as testtools_run -def get_parser(): +def get_parser(args): parser = argparse.ArgumentParser( description='Tool to run openstack tests') parser.add_argument('--blacklist_file', '-b', @@ -79,7 +79,7 @@ def get_parser(): 'prints the comment from the same line and all ' 'skipped tests before the test run') parser.set_defaults(pretty=True, slowest=True, parallel=True) - return parser + return parser.parse_args(args) def _get_test_list(regex, env=None): @@ -240,8 +240,27 @@ def call_testtools_run(test_id): testtools_run.main([sys.argv[0], test_id], sys.stdout) +def _select_and_call_runner(opts, exclude_regex): + ec = 1 + if not os.path.isdir('.testrepository'): + subprocess.call(['testr', 'init']) + + if not opts.no_discover and not opts.pdb: + ec = call_testr(exclude_regex, opts.subunit, opts.pretty, opts.list, + opts.slowest, opts.parallel, opts.concurrency, + opts.until_failure) + elif opts.pdb: + ec = call_testtools_run(opts.pdb) + else: + test_to_run = opts.no_discover + if test_to_run.find('/') != -1: + test_to_run = path_to_regex(test_to_run) + ec = call_subunit_run(opts.no_discover, opts.pretty, opts.subunit) + return ec + + def main(): - opts = get_parser().parse_args() + opts = get_parser(sys.argv[1:]) if opts.pretty and opts.subunit: msg = ('Subunit output and pretty output cannot be specified at the ' 'same time') @@ -266,19 +285,7 @@ def main(): regex = opts.regex exclude_regex = construct_regex(opts.blacklist_file, regex, opts.print_exclude) - if not os.path.isdir('.testrepository'): - subprocess.call(['testr', 'init']) - if not opts.no_discover and not opts.pdb: - exit(call_testr(exclude_regex, opts.subunit, opts.pretty, opts.list, - opts.slowest, opts.parallel, opts.concurrency, - opts.until_failure)) - elif opts.pdb: - exit(call_testtools_run(opts.pdb)) - else: - test_to_run = opts.no_discover - if test_to_run.find('/') != -1: - test_to_run = path_to_regex(test_to_run) - exit(call_subunit_run(opts.no_discover, opts.pretty, opts.subunit)) + exit(_select_and_call_runner(opts, exclude_regex)) if __name__ == '__main__': main() diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 8370b4a..e9bbb10 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -18,6 +18,7 @@ test_os_testr Tests for `os_testr` module. """ +import mock from os_testr import os_testr from os_testr.tests import base @@ -34,25 +35,43 @@ class TestPathToRegex(base.TestCase): class TestGetParser(base.TestCase): def test_pretty(self): - namespace = os_testr.get_parser().parse_args(['--pretty']) + namespace = os_testr.get_parser(['--pretty']) self.assertEqual(True, namespace.pretty) - namespace = os_testr.get_parser().parse_args(['--no-pretty']) + namespace = os_testr.get_parser(['--no-pretty']) self.assertEqual(False, namespace.pretty) - self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + self.assertRaises(SystemExit, os_testr.get_parser, ['--no-pretty', '--pretty']) def test_slowest(self): - namespace = os_testr.get_parser().parse_args(['--slowest']) + namespace = os_testr.get_parser(['--slowest']) self.assertEqual(True, namespace.slowest) - namespace = os_testr.get_parser().parse_args(['--no-slowest']) + namespace = os_testr.get_parser(['--no-slowest']) self.assertEqual(False, namespace.slowest) - self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + self.assertRaises(SystemExit, os_testr.get_parser, ['--no-slowest', '--slowest']) def test_parallel(self): - namespace = os_testr.get_parser().parse_args(['--parallel']) + namespace = os_testr.get_parser(['--parallel']) self.assertEqual(True, namespace.parallel) - namespace = os_testr.get_parser().parse_args(['--serial']) + namespace = os_testr.get_parser(['--serial']) self.assertEqual(False, namespace.parallel) - self.assertRaises(SystemExit, os_testr.get_parser().parse_args, + self.assertRaises(SystemExit, os_testr.get_parser, ['--parallel', '--serial']) + + +class TestCallers(base.TestCase): + def test_no_discover(self): + namespace = os_testr.get_parser(['-n', 'project.tests.foo']) + + def _fake_exit(arg): + self.assertTrue(arg) + + def _fake_run(*args, **kwargs): + return 'project.tests.foo' in args + + with mock.patch.object(os_testr, 'exit', side_effect=_fake_exit), \ + mock.patch.object(os_testr, 'get_parser', return_value=namespace), \ + mock.patch.object(os_testr, + 'call_subunit_run', + side_effect=_fake_run): + os_testr.main() From b135c5c80e26bb2808b7d98efb62e67c7137ea06 Mon Sep 17 00:00:00 2001 From: John Griffith Date: Mon, 28 Sep 2015 11:08:45 -0600 Subject: [PATCH 13/17] Use test_to_run var in no-discover The use of -n or --pdb was broken if a user specified the test by path. The problem is that even though in the case of -n we parse out path and convert to import notation, we weren't using the parsed our variable in the call_subunit_run call. Also, it turns out that --pdb and -n are really both work just fine calling call_subunit_run. So we can remove the def call_testtools_run and just consolidate pdb and -n into a single statement and runner call. I looked at using a secondary exclusive group between no-discover and pdb, but it it shouldn't matter if a user tries to specify both, the or statement should evaluate correctly and things should just work even if it's pointless to type in `tox -epy27 -- -n --pdb project.tests.test_name`. Change-Id: Icd61d9fb710807a508c95e8080865429a1f34fb4 Closes-Bug: #1499891 --- os_testr/os_testr.py | 10 ++----- os_testr/tests/test_os_testr.py | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 3c604e4..83d4140 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -236,10 +236,6 @@ def call_subunit_run(test_id, pretty, subunit): testtools_run.main([sys.argv[0], test_id], sys.stdout) -def call_testtools_run(test_id): - testtools_run.main([sys.argv[0], test_id], sys.stdout) - - def _select_and_call_runner(opts, exclude_regex): ec = 1 if not os.path.isdir('.testrepository'): @@ -249,13 +245,11 @@ def _select_and_call_runner(opts, exclude_regex): ec = call_testr(exclude_regex, opts.subunit, opts.pretty, opts.list, opts.slowest, opts.parallel, opts.concurrency, opts.until_failure) - elif opts.pdb: - ec = call_testtools_run(opts.pdb) else: - test_to_run = opts.no_discover + test_to_run = opts.no_discover or opts.pdb if test_to_run.find('/') != -1: test_to_run = path_to_regex(test_to_run) - ec = call_subunit_run(opts.no_discover, opts.pretty, opts.subunit) + ec = call_subunit_run(test_to_run, opts.pretty, opts.subunit) return ec diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index e9bbb10..9ae6dc5 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -75,3 +75,51 @@ class TestCallers(base.TestCase): 'call_subunit_run', side_effect=_fake_run): os_testr.main() + + def test_no_discover_path(self): + namespace = os_testr.get_parser(['-n', 'project/tests/foo']) + + def _fake_exit(arg): + self.assertTrue(arg) + + def _fake_run(*args, **kwargs): + return 'project.tests.foo' in args + + with mock.patch.object(os_testr, 'exit', side_effect=_fake_exit), \ + mock.patch.object(os_testr, 'get_parser', return_value=namespace), \ + mock.patch.object(os_testr, + 'call_subunit_run', + side_effect=_fake_run): + os_testr.main() + + def test_pdb(self): + namespace = os_testr.get_parser(['--pdb', 'project.tests.foo']) + + def _fake_exit(arg): + self.assertTrue(arg) + + def _fake_run(*args, **kwargs): + return 'project.tests.foo' in args + + with mock.patch.object(os_testr, 'exit', side_effect=_fake_exit), \ + mock.patch.object(os_testr, 'get_parser', return_value=namespace), \ + mock.patch.object(os_testr, + 'call_subunit_run', + side_effect=_fake_run): + os_testr.main() + + def test_pdb_path(self): + namespace = os_testr.get_parser(['--pdb', 'project/tests/foo']) + + def _fake_exit(arg): + self.assertTrue(arg) + + def _fake_run(*args, **kwargs): + return 'project.tests.foo' in args + + with mock.patch.object(os_testr, 'exit', side_effect=_fake_exit), \ + mock.patch.object(os_testr, 'get_parser', return_value=namespace), \ + mock.patch.object(os_testr, + 'call_subunit_run', + side_effect=_fake_run): + os_testr.main() From d533445b2673d9c98578b63f73e740667721718b Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 28 Aug 2015 20:06:42 -0400 Subject: [PATCH 14/17] Fix issues with the blacklist file regex generation This commit fixes a couple of bugs in the blacklist file regex generator. The first where a comment line was accidently made mandatory. If a comment wasn't specified an IndexError would be raised. The second was related to the variable naming in the function's blacklist file if branch. Variable reuse there was causing the last blacklist regex to take precendence over a regex passed in and would be used instead. Change-Id: Ib80a0c1781db7c8c9e4449b4773258fe3348411a Closes-Bug: #1488700 --- os_testr/os_testr.py | 17 +++--- os_testr/tests/test_os_testr.py | 100 ++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 7 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 83d4140..9751c02 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -134,15 +134,18 @@ def construct_regex(blacklist_file, regex, print_exclude): raw_line = line.strip() split_line = raw_line.split('#') # Before the # is the regex - regex = split_line[0].strip() - # After the # is a comment - comment = split_line[1].strip() - if regex: + line_regex = split_line[0].strip() + if len(split_line) > 1: + # After the # is a comment + comment = split_line[1].strip() + else: + comment = '' + if line_regex: if print_exclude: - print_skips(regex, comment) - exclude_regex = '|'.join([regex, exclude_regex]) + print_skips(line_regex, comment) + exclude_regex = '|'.join([line_regex, exclude_regex]) if exclude_regex: - exclude_regex = "'(?!.*" + exclude_regex + ")" + exclude_regex = "(?!.*" + exclude_regex + ")" if regex: exclude_regex += regex return exclude_regex diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 9ae6dc5..f820ed1 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -18,7 +18,9 @@ test_os_testr Tests for `os_testr` module. """ + import mock +import six from os_testr import os_testr from os_testr.tests import base @@ -123,3 +125,101 @@ class TestCallers(base.TestCase): 'call_subunit_run', side_effect=_fake_run): os_testr.main() + + +class TestConstructRegex(base.TestCase): + def test_regex_passthrough(self): + result = os_testr.construct_regex(None, 'fake_regex', False) + self.assertEqual(result, 'fake_regex') + + def test_blacklist_regex_with_comments(self): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s # A Comment\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', None, False) + self.assertEqual( + result, + "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") + + def test_blacklist_regex_without_comments(self): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', None, False) + self.assertEqual( + result, + "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") + + def test_blacklist_regex_with_comments_and_regex(self): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s # Comments\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', 'fake_regex', False) + + expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0|)fake_regex") + self.assertEqual(result, expected_regex) + + def test_blacklist_regex_without_comments_and_regex(self): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', 'fake_regex', False) + + expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0|)fake_regex") + self.assertEqual(result, expected_regex) + + @mock.patch.object(os_testr, 'print_skips') + def test_blacklist_regex_with_comment_print_skips(self, print_mock): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s # Comment\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', None, True) + + expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0|)") + self.assertEqual(result, expected_regex) + calls = print_mock.mock_calls + self.assertEqual(len(calls), 4) + args = list(map(lambda x: x[1], calls)) + self.assertIn(('fake_regex_0', 'Comment'), args) + self.assertIn(('fake_regex_1', 'Comment'), args) + self.assertIn(('fake_regex_2', 'Comment'), args) + self.assertIn(('fake_regex_3', 'Comment'), args) + + @mock.patch.object(os_testr, 'print_skips') + def test_blacklist_regex_without_comment_print_skips(self, print_mock): + blacklist_file = six.StringIO() + for i in range(4): + blacklist_file.write('fake_regex_%s\n' % i) + blacklist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=blacklist_file): + result = os_testr.construct_regex('fake_path', None, True) + + expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0|)") + self.assertEqual(result, expected_regex) + calls = print_mock.mock_calls + self.assertEqual(len(calls), 4) + args = list(map(lambda x: x[1], calls)) + self.assertIn(('fake_regex_0', ''), args) + self.assertIn(('fake_regex_1', ''), args) + self.assertIn(('fake_regex_2', ''), args) + self.assertIn(('fake_regex_3', ''), args) From 368a2553cd3e309445a58b129a74c8751fd3e5f7 Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Tue, 25 Aug 2015 20:10:57 -0400 Subject: [PATCH 15/17] Add whitelist file support A whitelist files allows the user to specify a list of regexes that ostestr will expand and run. It is mutually exclusive with blacklist, but not mutually exclusive with --regex/path. The value of --regex/path is appended to the whitelist. Co-Authored-By: Davanum Srinivas Change-Id: Ic397212ef3127c176a5870676f67d8a5e0de0441 --- os_testr/os_testr.py | 23 +++++++++++++---- os_testr/tests/test_os_testr.py | 45 ++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 9751c02..0e5176d 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -26,9 +26,14 @@ from testtools import run as testtools_run def get_parser(args): parser = argparse.ArgumentParser( description='Tool to run openstack tests') - parser.add_argument('--blacklist_file', '-b', - help='Path to a blacklist file, this file contains a' - ' separate regex exclude on each newline') + list_files = parser.add_mutually_exclusive_group() + list_files.add_argument('--blacklist_file', '-b', + help='Path to a blacklist file, this file ' + 'contains a separate regex exclude on each ' + 'newline') + list_files.add_argument('--whitelist_file', '-w', + help='Path to a whitelist file, this file ' + 'contains a separate regex on each newline.') group = parser.add_mutually_exclusive_group() group.add_argument('--regex', '-r', help='A normal testr selection regex. If a blacklist ' @@ -124,7 +129,11 @@ def path_to_regex(path): return root.replace('/', '.') -def construct_regex(blacklist_file, regex, print_exclude): +def get_regex_from_whitelist_file(file_path): + return '|'.join(open(file_path).read().splitlines()) + + +def construct_regex(blacklist_file, whitelist_file, regex, print_exclude): if not blacklist_file: exclude_regex = '' else: @@ -148,6 +157,8 @@ def construct_regex(blacklist_file, regex, print_exclude): exclude_regex = "(?!.*" + exclude_regex + ")" if regex: exclude_regex += regex + if whitelist_file: + exclude_regex += '%s' % get_regex_from_whitelist_file(whitelist_file) return exclude_regex @@ -280,7 +291,9 @@ def main(): regex = path_to_regex(opts.path) else: regex = opts.regex - exclude_regex = construct_regex(opts.blacklist_file, regex, + exclude_regex = construct_regex(opts.blacklist_file, + opts.whitelist_file, + regex, opts.print_exclude) exit(_select_and_call_runner(opts, exclude_regex)) diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index f820ed1..85d23ff 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -129,7 +129,7 @@ class TestCallers(base.TestCase): class TestConstructRegex(base.TestCase): def test_regex_passthrough(self): - result = os_testr.construct_regex(None, 'fake_regex', False) + result = os_testr.construct_regex(None, None, 'fake_regex', False) self.assertEqual(result, 'fake_regex') def test_blacklist_regex_with_comments(self): @@ -139,7 +139,7 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', None, False) + result = os_testr.construct_regex('fake_path', None, None, False) self.assertEqual( result, "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") @@ -151,7 +151,7 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', None, False) + result = os_testr.construct_regex('fake_path', None, None, False) self.assertEqual( result, "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") @@ -163,7 +163,8 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', 'fake_regex', False) + result = os_testr.construct_regex('fake_path', None, + 'fake_regex', False) expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" "fake_regex_0|)fake_regex") @@ -176,7 +177,8 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', 'fake_regex', False) + result = os_testr.construct_regex('fake_path', None, + 'fake_regex', False) expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" "fake_regex_0|)fake_regex") @@ -190,7 +192,8 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', None, True) + result = os_testr.construct_regex('fake_path', None, + None, True) expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" "fake_regex_0|)") @@ -211,7 +214,8 @@ class TestConstructRegex(base.TestCase): blacklist_file.seek(0) with mock.patch('six.moves.builtins.open', return_value=blacklist_file): - result = os_testr.construct_regex('fake_path', None, True) + result = os_testr.construct_regex('fake_path', None, + None, True) expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" "fake_regex_0|)") @@ -223,3 +227,30 @@ class TestConstructRegex(base.TestCase): self.assertIn(('fake_regex_1', ''), args) self.assertIn(('fake_regex_2', ''), args) self.assertIn(('fake_regex_3', ''), args) + + +class TestWhitelistFile(base.TestCase): + def test_read_whitelist_file(self): + file_contents = """regex_a +regex_b""" + whitelist_file = six.StringIO() + whitelist_file.write(file_contents) + whitelist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=whitelist_file): + regex = os_testr.get_regex_from_whitelist_file('/path/to/not_used') + self.assertEqual('regex_a|regex_b', regex) + + def test_whitelist_regex_without_comments_and_regex(self): + file_contents = """regex_a +regex_b""" + whitelist_file = six.StringIO() + whitelist_file.write(file_contents) + whitelist_file.seek(0) + with mock.patch('six.moves.builtins.open', + return_value=whitelist_file): + result = os_testr.construct_regex(None, 'fake_path', + None, False) + + expected_regex = 'regex_a|regex_b' + self.assertEqual(result, expected_regex) From d577844b7bc9d7fd25d11affe3e17609a843d9b2 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 29 Sep 2015 21:44:50 -0400 Subject: [PATCH 16/17] Better blacklist - tested with Nova So in Nova, i wanted to add a text file with a list of test ids like so: nova.tests.unit.api.ec2.test_api.ApiEc2TestCase nova.tests.unit.api.ec2.test_apirequest.APIRequestTestCase nova.tests.unit.api.ec2.test_cinder_cloud.CinderCloudTestCase nova.tests.unit.api.ec2.test_cloud.CloudTestCase functional to skip those specific test cases and any functional tests and the current code would not work, so digging through the regexp(s) on stack overflow and found this: http://stackoverflow.com/questions/406230/regular-expression-to-match-line-that-doesnt-contain-a-word Works like a charm! Change-Id: I0e947c599ab19276f35150749d559487d9790028 --- os_testr/os_testr.py | 7 +++++-- os_testr/tests/test_os_testr.py | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 0e5176d..db02446 100755 --- a/os_testr/os_testr.py +++ b/os_testr/os_testr.py @@ -152,9 +152,12 @@ def construct_regex(blacklist_file, whitelist_file, regex, print_exclude): if line_regex: if print_exclude: print_skips(line_regex, comment) - exclude_regex = '|'.join([line_regex, exclude_regex]) + if exclude_regex: + exclude_regex = '|'.join([line_regex, exclude_regex]) + else: + exclude_regex = line_regex if exclude_regex: - exclude_regex = "(?!.*" + exclude_regex + ")" + exclude_regex = "^((?!" + exclude_regex + ").)*$" if regex: exclude_regex += regex if whitelist_file: diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 85d23ff..65fb0a9 100644 --- a/os_testr/tests/test_os_testr.py +++ b/os_testr/tests/test_os_testr.py @@ -142,7 +142,7 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, None, False) self.assertEqual( result, - "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") + "^((?!fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0).)*$") def test_blacklist_regex_without_comments(self): blacklist_file = six.StringIO() @@ -154,7 +154,7 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, None, False) self.assertEqual( result, - "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)") + "^((?!fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0).)*$") def test_blacklist_regex_with_comments_and_regex(self): blacklist_file = six.StringIO() @@ -166,8 +166,8 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, 'fake_regex', False) - expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" - "fake_regex_0|)fake_regex") + expected_regex = ("^((?!fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0).)*$fake_regex") self.assertEqual(result, expected_regex) def test_blacklist_regex_without_comments_and_regex(self): @@ -180,8 +180,8 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, 'fake_regex', False) - expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" - "fake_regex_0|)fake_regex") + expected_regex = ("^((?!fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0).)*$fake_regex") self.assertEqual(result, expected_regex) @mock.patch.object(os_testr, 'print_skips') @@ -195,8 +195,8 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, None, True) - expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" - "fake_regex_0|)") + expected_regex = ("^((?!fake_regex_3|fake_regex_2|fake_regex_1|" + "fake_regex_0).)*$") self.assertEqual(result, expected_regex) calls = print_mock.mock_calls self.assertEqual(len(calls), 4) @@ -217,8 +217,8 @@ class TestConstructRegex(base.TestCase): result = os_testr.construct_regex('fake_path', None, None, True) - expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|" - "fake_regex_0|)") + expected_regex = ("^((?!fake_regex_3|fake_regex_2|" + "fake_regex_1|fake_regex_0).)*$") self.assertEqual(result, expected_regex) calls = print_mock.mock_calls self.assertEqual(len(calls), 4) From 65e1e1867c30fc70e4963df7f5bd9a15acf54a68 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 30 Sep 2015 13:00:23 -0400 Subject: [PATCH 17/17] Force utf8 encoding on subunit attachments output On certain environments if the locale is set to ASCII when printing an attachment things will explode because it's potentially trying to use unicode chars outside of ASCII. This commit forces the attachment output to be unicode encoded to avoid this issue. Change-Id: I79ff7482ec3b1fd3ce83b8acf3137119b3db39a9 Closes-Bug: #1501415 --- os_testr/subunit_trace.py | 1 + 1 file changed, 1 insertion(+) diff --git a/os_testr/subunit_trace.py b/os_testr/subunit_trace.py index 003299f..315d850 100755 --- a/os_testr/subunit_trace.py +++ b/os_testr/subunit_trace.py @@ -120,6 +120,7 @@ def print_attachments(stream, test, all_channels=False): # indent attachment lines 4 spaces to make them visually # offset for line in detail.as_text().split('\n'): + line = line.encode('utf8') stream.write(" %s\n" % line)