Correct documentation on ParallelScope

[mikkelbu]

Now specifies that ParallelScope.All also can be used on assembly level.

[CharliePoole]

But All makes no sense on the assembly, since it incorporates Self.

[mikkelbu]

But isn't too strict to say that it is not valid on assemblies?

It is correct that Self part will not have any effect, but the Children part will work as intended. Perhaps we should distinguish in the comments - and the documentation - between whether using a particular ParallelScope value will break something - like using ParallelScope.Children on a non-parameterized test method or using ParallelScope.Fixtures on a test method (both will make the test method invalid) - or whether it just does not make much sense.

Also it seems like there are some people using ParallelScope.All on assembly level and not "just" ParallelScope.Children (probably because thay want "all" tests to be parallel).
nunit/nunit3-vs-adapter#443
https://github.com/techtalk/SpecFlow/issues/1052
nunit/docs#272
...

[CharliePoole]

I think the docs should use consistent language. In my view, not valid would mean that it gives an error. I haven't actually compared the docs to the code to see if that is true. If a setting is merely ignored, perhaps that's what we should say instead of not valid.

[CharliePoole]

I think your example of some people misinterpreting All suggests that it was a bad choice for a value.

[jnm2]

I agree that it's confusing to say All is actually invalid for assemblies unless we follow that up by marking the assembly invalid.
This would be a good thing to do if we want the freedom to change what it means for assemblies in the future. E.g. maybe it could be a signal to a future runner to control the assembly-level parallelization?

[CharliePoole]

Of course, that goes for Self as well.

[jnm2]

@mikkelbu Do you mind leaving out assemblies until we figure out the answer?

[jnm2]

We could say it's not meaningful instead of saying it's not valid.

[jnm2]

@nunit/framework-team I like these clarifications. 3.12 is imminent and the PR is still waiting. Functionally speaking, Self on an assembly is meaningless, so you could say that All for an assembly isn't meaningfully different than Self.

If we say All is valid on an assembly, I think that amounts to recommending or confirming it. Should we do that, or should the XML point people to Children for assemblies instead?

[mikkelbu]

@nunit/framework-team Sorry for having left this hanging for so long. What about we rephrased the summaries above ParallelScope.Self and ParallelScope.All in the following manner?

ParallelScope.Self

From

        /// The test may be run in parallel with others at the same level.
        /// Valid on classes and methods but not assemblies.

To

        /// The test may be run in parallel with others at the same level.
        /// Valid on classes and methods but has no effect on assemblies.

ParallelScope.All

From

        /// The test and its descendants may be run in parallel with others at	
        /// the same level. Valid on classes, parameterized methods, and assemblies.

To

        /// The test and its descendants may be run in parallel with others at	
        /// the same level. Valid on classes and parameterized methods.
        /// For assemblies it is recommended to use <see cref="ParallelScope.Children"/>
        /// instead, as <see cref="ParallelScope.Self"/> has no effect on assemblies.

[CharliePoole]

Looks good to me

[jnm2]

@mikkelbu I like that.

[mikkelbu]

@jnm2 @CharliePoole Great. I've pushed a commit with the proposed changes.

[jnm2]

@mikkelbu Thank you!