Qiskit-terra: Switch to parallel test runner for python unit tests

[mtreinish]

What is the expected enhancement?

We currently are using the default stdlib test runner for running our python unit tests. While this works and will execute the tests correctly it's quite limited in what it can do. The biggest limitations are that it can only serial execute tests and that it doesn't enable any results tracking.

I propose that we switch the test runner to stestr: https://github.com/mtreinish/stestr which addresses these limitations and also provides users with an increased level of flexibility and configurability in how tests are run (admittedly I am a bit biased, since I do maintain the project).The list of features over the stdlib runner is quite long, but the most immediate advantage is that by switching we'll gain a significant speed up in test execution time by default.

[diego-plan9]

Hi @mtreinish ,

and thanks for the suggestion - the topic of replacing the standard unit test runner with something a bit more powerful comes up every now and them (mostly in informal conversations). There are a couple of forces that explain why we still use the standard lib one by default: one of them is mostly for compatibility and allowing contributors to choose the test runner of their choice (ie. try to avoid having constructs in the tests that are specific to a test runner), and the other one is mostly ... coming up with an agreement on what should be the replacement. We might benefit indeed of soft-recommending one or making use of it in travis, etc.

Can you make a compelling argument for using stestr as opposed to the alternatives that are usually floating around and seem to have a higher "market share", such as nose2 or py.test, as you seem to be indeed the best person to ask? :) I'm also a bit biased as I tend to use py.test myself, but would be looking forward to hear your opinion and get to know it a bit better. In particular, is the parallelization of subTest done transparently? How is the support for parametrized tests?

[mtreinish]

The reasons you outlined for sticking with the stdlib runner are actually a big part of the reason I suggested using stestr. It builds off the upstream stdlib unittest constructs (so it uses the same discovery, etc.) so if it runs with stestr it will work with any runner. So if we did switch to stestr it would be basically the soft recommendation approach you mentioned, and just switching what we put in the makefile and what we use for travis. It wouldn't force anyone to use it all the time, and because it just uses stdlib unittest compatible mechanisms people can use whatever runner they like locally.

As for comparing to the more popular ones, I actually wrote a small blog post about it a couple months ago(along with explaining the stack and history behind the project): https://blog.kortar.org/?p=370 . But, to go into a bit more detail, we should avoid nose2 because it's not really maintained anymore (and they actually recommend using pytest). So when comparing to pytest the reasons I always suggest stestr over pytest are for 3 main reasons: it's strict unittest compliance (as talked about above), baked in parallelization, and the built in results repository.

For the parallelization, yes it splits up the tests automatically for execution. By default (because it is configurable, the docs) it does discovery to get a full list of all the tests, and then splits the individual test methods up to run on multiple runners (which by default is the number of cpus) and then multiplexes the results in real time. For parameterized tests do you mean something like a ddt or testscenarios? If so then that works just fine because those all typically work at either import or discovery time using either a generator decorator to split out test methods on load, or use a load_tests hook to generate the tests). So when stestr goes to schedule and execute tests it works just fine.

The other feature stestr provides that is missing from the popular runners, is the results repository. By default stestr stores the results from all the previous runs. It can use that data for scheduling (to optimize how the tests are run in parallel), but also to aid in debugging. For example, a typical workflow is to make a change to the project, run the full test suite and then encounter failures. You fix the issues and go to rerun the tests. But, because stestr keeps track of all the history when you fix the failures you can just do something like "stestr run --failing" which will only run the tests which failed previously. Then you just rinse and repeat until you get to zero failures. The full history also enables so more advanced analysis and other advantages, but that's a longer term thing. (and most people don't touch it)

The place where I see that pytest excel are on the UX, especially for failure debugging. But, by switching to it by default there is the risk of building a test suite that's not cross compatible with another runner of the users choice. Especially because pytest couples a testing library with the runner which isn't always compatible with stdlib, and the pytest runner doesn't use the stdlib discovery mechanisms. It also remains significantly slower because it's just running things serially.

I pushed up a work in progress patch here: https://github.com/mtreinish/qiskit-core/commit/2521a1925f51d2bd218bcb84478882c24217bcc7 that switches make test to use stestr. I suggest giving it a try to get a feel for it. The most noticeable thing for me running locally on my laptop was I got a ~2x speedup running things.

[ajavadia]

I have no experience with any other test runner but parallel execution of tests sounds very nice.

[delapuente]

I sincerely think this is a great addition. Indeed we are reaching points where test execution is becoming a bottleneck and the lack of isolation is a source of tricky bugs. I'm doing some optimization efforts here in #927 and, in the light of this issue and the recorded testing times, I'm adding this to it.

[1ucian0]

Might this affect VCR?

[mtreinish]

@1ucian0 there shouldn't be any changes with VCR. The new test runner doesn't actually change how the tests are executed. It just splits the execution up between multiple workers. We'll still be using python stdlib unittest to run things (well actually a layer on top, but it inherits from unittest and respects the interface). So if it works in unittest there shouldn't be any issue.

The only place I think that we'll have any potential issues is any potential issues between tests when they manipulate a global or external state. The only place I think this happens in our code base is the online tests where we might end up hitting the api in parallel a bit too hard and have resource limitations cause test failures. I've configured the initial patch to only parallelize at the test class level (so things inside a test class will run serially on a single worker) but we might have to add locking between online tests to limit running them in parallel. Or alternatively adjust the test grouping hint (which is just a regex that groups on the results to re.match() with the test_id) to serialize all the online tests.

[diego-plan9]

Let's try to get a consensus before continuing work, in the hopes of getting the rest of the travis and QA related issues in track! I think there is a clear consensus about introducing _parallel running of tests_, which is the core of the issue, but not so clear about how to achieve it (ie. which test runner to pick).

Re-reading your detailed comment (thanks for the depth in it!), I think your arguments for stestr are concentrated in this sentence:

So when comparing to pytest the reasons I always suggest stestr over pytest are for 3 main reasons: it's strict unittest compliance (as talked about above), baked in parallelization, and the built in results repository.

1. _strict unittest compliance_: that argument is also valid for pytest - it can run any suite built for unittest. The fact that it provides helper and constructs on top of it does not force us to use them :) (and as mentioned before, I do explicitly not want to use them, to keep the suite agnostic).
2. _baked in parallelization_ - it's also baked in in pytest ... if you use a widely used plugin maintained by the pytest team, which I think is a reasonable assumption (more on plugins later).
3. _built in results repository_ - I can't comment too much into as you are much more familiar with the features! Future needs aside, for the specific case of re-run failing tests there is also an enabled by default plugin in pytest that would do the trick.

But more importantly, I think the most weight for the decision should be placed on the benefits of using try and tested tools, and if possible not steer away from the beaten path unless we have a strong and well justified incentive for doing so. This has been a constant desire in all aspects of the project, and I hope you agree on the benefits of keeping that point of view, as I think it brings in a ton of benefits implicitly (support, familiarity, flexibility, integration, adoption, etc) that are very valuable.

In the particular case of pytest, it is arguably the "de facto" standard for a "test runner other than python -m unittest" - and probably for good reason! We can evaluate it more in depth if needed or search for numbers, but as a quick example, among the projects that were used as arguments in the tox issue, 3/4 (leaving out the projects for the runners themselves, seemed only fair) use pylint:

• https://github.com/home-assistant/home-assistant/blob/dev/tox.ini#L15
• https://github.com/PyCQA/pylint/blob/master/tox.ini#L33
• https://github.com/scipy/scipy/blob/master/tox.ini#L34

On a more "specific needs" level, pytest also has a rather large (~500) set of plugins that we might benefit from in the future: among them, some integrations (coverage, jupyter, github, lint, etc), modifications to the actual runner functionality (remote testing, distributing, ordering, etc) and a lot of unexpected uses and features of different sorts. Again, rather than the specific plugins, I think this is a tangible example of "things we get 'for free' by taking advantage of a de-facto tool", being able to tap into the solutions that lots of developers faced before us.

TL;DR: I recommend basing the paralellization of terra tests in py.test, as I think the potential advantages outlined for stestr are a bit diluted and not strong enough to justify steering away from a more "industry-standard" test runner.

[mtreinish]

Let's try to get a consensus before continuing work, in the hopes of getting the rest of the travis and QA related issues in track! I think there is a clear consensus about introducing _parallel running of tests_, which is the core of the issue, but not so clear about how to achieve it (ie. which test runner to pick).

Re-reading your detailed comment (thanks for the depth in it!), I think your arguments for stestr are concentrated in this sentence:

So when comparing to pytest the reasons I always suggest stestr over pytest are for 3 main reasons: it's strict unittest compliance (as talked about above), baked in parallelization, and the built in results repository.

1. _strict unittest compliance_: that argument is also valid for `pytest` - it can run any suite built for unittest. The fact that it provides helper and constructs on top of it does not force us to use them :) (and as mentioned before, I do explicitly not want to use them, to keep the suite agnostic).

It's actually not. While, pytest supports running unittest suites, but there is a reason I said strict compliance in my earlier comment. This extends beyond just the testing library packaged in pytest, but also includes things like how pytest does test discovery. It doesn't conform to the standard unittest discovery which can lead you to execute things it thinks are "tests" that are not actually part of the suite. While I agree we can avoid using the testing library features which would couple us to using pytest only by code review the other non-compliance with the spec can't be as easily caught. I know this from first hand experience where working with a pytest CI'd suite stopped working under unittest because of discovery differences. Although that was a pretty weird and specific case, but to be fair the reverse is more common where pytest discovery executes a file not setup in unittest discovery and fails (pytest does a recursive file scan and doesn't actually use the discovery mechanism to find test modules so it often runs things not intended to be included in test discovery).

2. _baked in parallelization_ - it's also baked in in `pytest` ... if you use a widely used [plugin maintained by the pytest team](https://github.com/pytest-dev/pytest-xdist), which I think is a reasonable assumption (more on plugins later).

While xdist has come a long way in the past several years (it used to be completely unusable 3-4 yrs ago) it's still not quite the same thing. It's missing a lot of key features mainly around scheduling flexibility, results introspection, and debugging which are really needed if you start running things in parallel. When running tests in parallel you need to be able to unwind cross interactions between tests that ran at the same time and reproduce those conditions. The specific split of tests between workers changes between runs too so you need to be able to map back that when debugging failures from parallel execution. xdist doesn't do any of this, which is what I meant by baked in parallelization, because stestr was designed with running in parallel by default it is designed to handle these concerns. Adding parallelization to a test runs adds a ton of complexity to debugging and quite frankly this is where xdist doesn't provide any real assistance.

3. _built in results repository_ - I can't comment too much into as you are much more familiar with the features! Future needs aside, for the specific case of re-run failing tests there is also an [enabled by default plugin](https://docs.pytest.org/en/latest/cache.html) in `pytest` that would do the trick.

Again the cache feature of pytest is similar but not actually the same. The pytest cache by default (even with xdist) only contains a list of tests and whether they failed or not. This is not actually enough especially for debugging a ci system running in parallel. What I meant by the results repository is that the actual results stream from the test (using the subunit protocol https://en.wikipedia.org/wiki/Subunit_(format) ) are stored from each run in stestr's cache (called the respository in stestr). This includes all information generated by the test execution, including the test name/id, the worker it ran on, start and stop times of each test, any attachments for a test (normally stdout, stderr, and logging but you have to enable capturing and attaching that to the stream). When debugging a parallel test run that's not being run locally this information is critical to being able to figure out what went wrong.

Also because we have a machine readable stream of the test run stored on disk there are lots of tools available out there to help with this debugging. One of the ones I've always found particularly cool is stackviz to visualize the test run as a timeline. For example:

http://logs.openstack.org/00/605600/4/check/tempest-full-py3/af7bf67/controller/logs/stackviz/#/stdin/timeline

But these are things I don't expect us to need anytime soon (if ever).

But more importantly, I think the most weight for the decision should be placed on the benefits of using try and tested tools, and if possible not steer away from the beaten path unless we have a strong and well justified incentive for doing so. This has been a constant desire in all aspects of the project, and I hope you agree on the benefits of keeping that point of view, as I think it brings in a ton of benefits implicitly (support, familiarity, flexibility, integration, adoption, etc) that are very valuable.

In the particular case of pytest, it is arguably the "de facto" standard for a "test runner other than python -m unittest" - and probably for good reason! We can evaluate it more in depth if needed or search for numbers, but as a quick example, among the projects that were used as arguments in the tox issue, 3/4 (leaving out the projects for the runners themselves, seemed only fair) use pylint:

* https://github.com/home-assistant/home-assistant/blob/dev/tox.ini#L15

* https://github.com/PyCQA/pylint/blob/master/tox.ini#L33

* https://github.com/scipy/scipy/blob/master/tox.ini#L34

On a more "specific needs" level, pytest also has a rather large (~500) set of plugins that we might benefit from in the future: among them, some integrations (coverage, jupyter, github, lint, etc), modifications to the actual runner functionality (remote testing, distributing, ordering, etc) and a lot of unexpected uses and features of different sorts. Again, rather than the specific plugins, I think this is a tangible example of "things we get 'for free' by taking advantage of a de-facto tool", being able to tap into the solutions that lots of developers faced before us.

Yes there is a large plugin ecosystem around pytest, but how much of that will we ever actually use? Also, if we agree that we need parallel testing how many of those plugins are actually compatible with xdist? (which can be a real problem) I also personally tend to follow the unix philosophy and have one tool do one thing and do that well. So stestr doesn't have a plugin interface like pytest to expand it's role. Instead it gets expanded by using standard protocols (like subunit) and/or building tools around it. stestr isn't just used by me, and I can cite a bunch of very useful examples of these if you'd like. (I have already referenced a few in here)

TL;DR: I recommend basing the paralellization of terra tests in py.test, as I think the potential advantages outlined for stestr are a bit diluted and not strong enough to justify steering away from a more "industry-standard" test runner.

I don't contend that it's a "industry-standard" for what we're trying to do here. I will admit it is widely deployed, and probably the most popular python test runner out there. But, it's not used everywhere, and in fact I'd argue that for parallel testing it's far from the de-facto standard. Most projects that use pytest in CI do not run it with xdist. Heck even xdist itself doesn't run its tests in parallel for CI: https://travis-ci.org/pytest-dev/pytest-xdist/jobs/437376435

While stestr on the other hand has been used to run testing at a very large scale [0] in parallel in CI and was built with those concerns in mind. You can look at almost everything that runs python tests in openstack's ci [1] or most of the projects on http://git.openstack.org/cgit [2] and see that they currently use stestr as the test runner there too.

[0] look at the nova repo from the tox issue for example, that project alone has 17k tests in its unittests and ignores that's ignoring functional or integration tests
[1] Which is a far larger scale than any other individual project run in travis, see: http://status.openstack.org/openstack-health/#/ (which is all built using subunit from the stestr results repository) or http://zuul.openstack.org/builds for examples of that scale
[2] Which are not all openstack projects fwiw. It's open to any open source project that wants to use the same infrastructure as openstack. It's actually in the middle of being rebranded to 'opendev' to better reflect this and to better position it as a completely open source alternative to the closed source github and partially closed source gitlab.

Also, the first leg of my argument is important because with the strict unittest compliance this means if you want to use pytest locally (and use all the advantages you outlined here) the switch to stestr in ci and the tox config does not stop you or anyone else from running it locally. It's just that we are using stestr to enable parallel test execution in CI.

[diego-plan9]

Thanks for the detailed answer (really - I'm a kind of "waller of text" too), @mtreinish !

Unfortunately, if I was not able to get an agreement on both py.test and stestr being in practice technically apt solutions for solving the issue at hand, I don't really think pursuing a conversation over smaller details or cherry-picking examples and metrics will accomplish progress - so I'm basically opting-out of the conversation, hoping other members of team jump into it (as again, we do need to solve the CI times!).

I'd also strongly encourage that the decision is made solely by _other_ members of the team, in order to avoid bias and keep the best interests of the project at heart - it's (naturally) hard to keep the arguments neutral when the topic revolves around a tool maintained by a participant in the discussion.

[delapuente]

Although I'm a complete ignorant of testing frameworks in Python I won't say I'm totally unbiased!

Anyway, I would argue that support and adoption are among the things we should consider most. They usually indicate maturity, stability and a live community of contributors for helping you when problems come up. For these reason, I'm inclined to use py.test.

Althouhg running 17k is remarkable, fortunately, we are far from that point and I don't think the performance improvements under that work load worth dismissing the benefits of py.test.

Regarding your main point about being "strictly compliant", it is hard to me to come up with a situation in which we can execute things other than tests when they are kept inside the test folder. I don't think our naming schema would be that different and, given that py.test will become a dev dependency and the execution will be behind tox, I don't see the point of not using py.test locally either.

[mtreinish]

It's not that I think py.test is a bad test runner. It's very good, better than stestr in many ways, there is a reason it's popular. I would be totally fine switching to that if it's what we decided to do. But, if our goal is to switch to parallel execution of our tests for CI, which there seems to be general agreement on (although I'd make the argument that individual test suite run time isn't our real issue, but instead the way our jobs get queued is) then I feel pretty strongly that xdist is not the solution for that (which was actually my main point above, it might have gotten lost in my wall of text :) ). Speaking from my experience having worked on maintaining different test suites (many much larger than qiskit's) and CI systems that all ran the tests in parallel I feel pretty confident saying that xdist and py.test do not provide all the tools necessary to debug failures when executing tests in parallel, especially in a remote environment that is not directly accessible (ie a CI system). There's a reason you very rarely see xdist being used to parallelize test execution in CI (although I expect a large part of that is most people don't think the speed boost is worth the extra complexity of parallel execution, especially if the test suite is small).

I fully admit I'm biased here since I am the primary maintainer of stestr. But I did build it (well forked and built really) to be able to run any python unittest test suite (both large and small) in parallel, and provide the necessary tools to debug failures when running in parallel.

[delapuente]

I feel pretty confident saying that xdist and py.test do not provide all the tools necessary to debug failures when executing tests in parallel, especially in a remote environment that is not directly accessible (ie a CI system)

What debugging tools does xdist lack of, compared to py.test?

[mtreinish]

It's about the complexity from parallel execution. There are a few common non-deterministic failure modes everyone running tests in parallel encounters eventually. For example, test isolation failures that effect other tests/workers when running at the same time. Another common issue that introduces non-determinism is just a straight up race condition when tests are talking to a globally scoped or external resource (like a file or api) and the tests interfere with each other when running in parallel. In both of these cases you need to be able to figure out which tests were running at the same time and try to figure out which tests are interacting or interfering with each other. xdist does not provide this level of information, the closest it gets is when you turn on the verbose output it prints the worker id with the test id after a test finishes. But, that doesn't actually provide sufficient information to discover which tests were running at the same time (especially for n>2 workers or if it's a very subtle timing issue).

[delapuente]

Having thought thoroughly about this, I've changed my mind so I support moving forward with stestr. The main motivations for this changing are as follow:

• OpenStack is a very active project and it relies on stestr for runnin 17k parallel tests which is, indeed, impressive. OpenStack has been using stestr during almost one year with not many issues since there.
• We have an expert on the matter willing to do it. @mtreinish has provided a solution to a long-time desired feature for our codebase. True, he's the maintainer of stestr but that does not negate the fact that the software is performing really well and also implies he's completely aware of the framework internals. Furthermore, he already provided a solution in #737.
• We wanted parallelization. It is possible we want more things in the future but, right now, we want parallelization and stestr provide superior capabilities on this specific subject.