Continued work on parallelism

[CharliePoole]

Fixes #164

This continues the work on parallel execution - running test cases in parallel. Because this is a pretty important feature, as well as somewhat tricky to implement, I'm doing it "under review" and inviting comments on each commit as I make it.

The first commit makes some changes to the user-visible classes:

• NonParallelizableAttribute is intended as a simpler way to specify non-parallelism, in lieu of using ParallelizableAttribute with ParallelScope.None.
• [NonParallelizable] as well as [Parallelizable(ParallelScope.None)] now cause both the item on which it appears to be non-parallelizable.

The second commit adds tests of ParallelizableAttribute, which were lacking.

[CharliePoole]

The third commit introduces the implementation of TestCase parallelism.

A new workitem type has been created, nested in CompositeWorkItem, for the purpose of running the one time teardown and other cleanup when all child tests are complete. The new workitem is dispatched in lieu of actually performing the teardown, since we may not be on the right kind of thread when the final nested test case completes. For example, if the OneTimeSetUp was run in the STA, then the OneTimeTearDown should also use it.

Unfortunately, I can't figure out a real unit test for this. Any testing requires collaboration of a lot of classes. Currently, our own tests do not use the feature. I think the next step for me is to make as many of our tests run in parallel as possible, at least temporarily, so as to exercise this feature. It would also be useful to get this implementation out to some of the users who have been asking for it so they can test it for us.

[CharliePoole]

I just rebased on top of master.

[CharliePoole]

I had to skip tests on mono that related to passing the test context from one AppDomain to another and which were failing intermittently. I'm not 100% happy with doing this but it's an extreme edge case and at the moment I have no easy way to debug under Mono.

[michelarbide]

I am very interested in this functionality, would you have an approximate date of when it will appear in nunit?

[CharliePoole]

It would be great if you could exercise this. Having it tried out by users in real situations is what we need to be comfortable releasing it!

You can get the latest build of the PR by following the "Show all checks" link. Current build artifacts are here: https://ci.appveyor.com/project/CharliePoole/nunit/build/3.7.0-ci-03734-pr-2066/artifacts

[matthew-horrocks]

I'm happy to run tests with this build line alongside my normal nunit parallelized tests runs.

After having imported 3.7.0-ci-03734-pr-2066 or newer, do we just need to set [assembly: Parallelizable(ParallelScope.All)] or are there some other steps that need to happen?

[CharliePoole]

You can either do that or start smaller by picking certain fixtures to have their cases run in parallel.

In either case the test cases will only be run in parallel when using the special build.

[tuscias]

Hi, i was able to run 42K tests in parallel with 30 workers.
They were slow tests so ran for ~48h.
It did not crash or anything.
3.7.0-ci-03734-pr-2066
HTH

[CharliePoole]

@tuscias That sounds very promising! Did you see an improvement in throughput for your tests as compared to fixture parallelism?

FYI, the performance of Fixture parallelism may also be improved in this build. You may also discover that using either parallel fixtures or parallel test cases gives the best performance.

[CharliePoole]

@nunit/core-team @nunit/framework-team This is ready to review and merge in my view. Because it's critical I'd like to get two reviews besides myself.

One question: should we pick some test cases of our own to run in parallel, just to make sure this is getting tested all the time? My own experiments tend to show that parallel fixtures gives us the best throughput, so that's still the default. We could pick a set of fixtures with lots of cases and mark them as having parallelizable cases if we want.

[rprouse]

@CharliePoole sorry, I just managed to clear out my emails and reviews, but didn't get to this. I've assigned myself as a reviewer and will get back to it soon. I would also like to see another review from another member of the @nunit/core-team or @nunit/framework-team if possible.

To your question on running our own test cases in parallel, I think we should start exercising parallelism at as many different levels as we can manage within our own tests to expose issues as early as possible. Nothing tastes better than our own dogfood 🐶

[rprouse]

Looks good to me. Minor coding style comments, but just being picky.

[rprouse]

This is very hard to read. if/else if/else would be much clearer.

[jnm2]

Personally I've always thought this was more readable than if, but that's just me.

[CharliePoole]

I go back and forth in situations like this. Maybe it would look better with the ternary operator using a different layout, like...

var msg = condition1 ? message1 :
          condition2 ? message2 :
          condition3 ? message3 : null;

[rprouse]

@CharliePoole I don't think this needs to change, but I do think your proposed format is more readable. I find that the conditions get lost in the long messages, so having them up front might help.

[CharliePoole]

I'll start using that layout from now on and change it in the next PR - which is already in progress.

[jnm2]

I often do:

var msg = condition1 ? message1 :
          condition2 ? message2 :
          null;

[CharliePoole]

Great minds and all that! 😄

[rprouse]

@nunit/core-team or @nunit/framework-team is someone available to give this a second review?