testfx icon indicating copy to clipboard operation
testfx copied to clipboard

Opt-in for parallel-execution on a class level, instead of current assembly level

Open Youssef1313 opened this issue 8 months ago • 8 comments

Discussed in https://github.com/microsoft/testfx/discussions/5553

Originally posted by thomasstormark3shape May 2, 2025 Both Xunit and Nunit has the option to opt-in for parallel test execution on a class-level or method level.

With MSTest its only possible to set [assembly: Parallelize(…)], and then opting out for each class or method by [DoNotParallelize].

When you have an old large test project with houndreds of test classes, and you find out that a lot does NOT parallelize - it would be good to gradually opt-in during migration period, and finally go for assembly level default parallelize.

Any thoughts on this feature?

Youssef1313 avatar May 02 '25 11:05 Youssef1313

I am just wondering what the story would be for the final step:

it would be good to gradually opt-in during migration period, and finally go for assembly level default parallelize.

My assumption here is that the person migrating, is trying to make the assembly better by getting it to parallelize, and will go around setting [Parallelize] on test classes.

But how do they invert that when they are ready to make the last step? They would have to go and find all test classes, that don't have [Parallelize] and add [DoNotParallelize] and find all test classes that have [Parallelize] and remove it, because it is now redundant.

An approach where a code fix for would add DoNotParallelize to all classes, and the OP could remove them, would be simpler way to get to the final test project parallel by default.

I can also see value in having the project not parallel by default, and explicitly enabling it on per-class need, but that is not what was asked here, and imho leads us to a worse end result, where tests don't run in parallel by default.

nohwnd avatar May 02 '25 12:05 nohwnd

An approach where a code fix for would add DoNotParallelize to all classes, and the OP could remove them, would be simpler way to get to the final test project parallel by default.

Well that is currently the only way to do it, when you are coming from a large test assembly with many tests NOT being able to parallelize.

Its cumbersome to annotate with DoNotParallelize attribute for all of these in the beginning of your migration period. Also think when you have many test assemblies.

When you are finished migrating, you try run all tests as parallel - and if all green, you slam the [assembly: Parallelize(…)] on the dll and remove all [DoNotParallelize].

Finished.

I can also see value in having the project not parallel by default, and explicitly enabling it on per-class need, but that is not what was asked here.

Well - it is actually what I mean here. It sound a lot easier when starting with a test assembly that is large and has many tests that do not parallelize.

thomasstormark3shape avatar May 02 '25 12:05 thomasstormark3shape

Well - it is actually what I mean here. It sound a lot easier when starting with a test assembly that is large and has many tests that do not parallelize.

I understood that, but what do you do at the tipping point?

Say you have 1000 test classes. You don't know how many of them are parallelizable. You enable parallelization and stuff breaks.

Ideally you would: Take the result of the run, process it, add DoNotParallelize on each class that had failed test. Run again, this time you should pass the tests, with many of them running in parallel.

Go fix the classes that still need DoNotParallelize, to either keep the attribute or remove it because you fixed how your tests / code are written.


With your approach you would start with 1000 classes, add Parallelize (let's say automatically - because they passed) to 800 of them, then you'd start investigating the rest. You would then go class by class keeping assembly:parallelization disabled, and adding either Parallelize, or DoNotParallelize to the classes as you'd be investigating if they are fixed or not.

Then you would end up with assembly fully annotated with Parallelize / DoNotParallelize on classes, you would switch to assembly:Parallelize, and remove Paralllelize from classes because they are now redundant.

This would work, but it seems like more work than the first approach. And

  1. mstest loses a way to quickly switch between parallel and non-parallel run - because then we have an opt-out from non-parallel: the class level parallel attribute. Unless we add another parameter to the DoNotParallelize assembly level attribute.

  2. Having DoNotParallelize on assembly, and then actually having tests run in parallell in the assembly does not sound very intuitive, and does not push the user towards parallelization by default.

nohwnd avatar May 02 '25 12:05 nohwnd

Interesting. I see your point of view. Think that the problem of having a large assembly (1000 test classes), and finding out which does NOT parallelize just needs to be approached by divide-and-conquer with smal sets of test classes (e.g. 50) that are manageable. And then initially slam the DoNotParallelize on all. And gradually remove them, until you find out all reasons why they are not parallelizable.

There are different reasons why each test is not parallelizable. We may have one (or more) test, that does state change for the others making them not parallelizable. But its easier to work initially on a small subset.

thomasstormark3shape avatar May 02 '25 14:05 thomasstormark3shape

I think that's a really fair request and for the concern of the last step - although I think it would be unlikely to really happen - we could more easily automate the last step (if it ever come through) via analyzers (code refactoring to be precise) that would basically set parallelization at asm level and reverse the state of each classes (i.e. if class is marked as [Parallelize] then move it as [DoNotParallelize]).

For the implementation, just allowing the attribute to be specified on classes might still be unoptimal because we foreach classes instead of collecting so we have some more changes to do but that should be relatively "easy".

Evangelink avatar May 07 '25 10:05 Evangelink

Sounds easier to me to go with my first proposal in https://github.com/microsoft/testfx/issues/5555#issuecomment-2847138228 it also requires some automation, ideally connected to list of failing tests and their classes. But does not require new attribute, only some changes to the mstest engine if we don't support DoNotParallelize on classes yet.

And if we had the automation connected to failing tests, we could use it for other interesting transformations probably, rather than a specific one-off. (it also sounds little bit like something that could be a bigger thing, that tries to run your test suite in multiple scenarios to really figure out which tests can run in parallel, or as something that stryker could use for their thing).

nohwnd avatar May 07 '25 16:05 nohwnd

It's semi related to #4116

stan-sz avatar May 09 '25 09:05 stan-sz

Just my $.02 worth... Most discussions are relating to a migration. I came across this looking for the same behavior as part of new tests using parallelization at the method level. But, the tests for several classes are each so trivial that the overhead of parallelization actually adds up to a major performance decrease. If I had the ability to OPT-IN or otherwise adjust the parallelization at the class level then I could at least use it where there is benefit (long running tests) but NOT for short tests (runtime is < a few ms)

smaillet avatar Oct 26 '25 22:10 smaillet