Parallelizable attribute not invalidating invalid parallel scope combinations

[evancam]

I was looking through the ParallelizableAttribute.cs, and the changes made with #2264. I noticed that there is the functionality to invalidate tests with invalid parallel combinations in the ApplyToTest() method, but it does not appear to be actually invalidating them. Is this intentional?

Using 3.7.1,
I tested and was able to reproduce the hanging found in #2261 by placing [NonParallelizable] on the test fixture, and [Parallelizable(ParallelScope.Self)] on the test method.

I placed [Parallelizable(ParallelScope.Fixture)] on a test method with no parallelization on the fixture, and the tests ran and passed normally without any parallelization.

Instead of invalidating the setting, it could potentially be useful. What are your thoughts on allowing ParallelScope.Fixtures for methods? It would be useful in the case where you would only like to run one particular test one at a time at the fixture level, but run the rest of the tests in the fixture at ParallelScope.Self. It allows for the specific test in the fixture to be isolated without affecting other fixtures in the way ParallelScope.None does.

[CharliePoole]

Both the issues you refer to are clearly marked as fixed for the 3.8 release, so you won't see any sign of the changes when running 3.7.1. You can, if you like, download later dev versions of the framework from our myget feed.

ParallelScope.Fixtures indicates that any fixtures below that point in the tree should be run in parallel. It's not valid on methods because methods don't contain fixtures.

If you want some tests in a fixture to be run in parallel (with one another) those fixtures should be marked as Parallelizable (ParallelScope.Self or omitted).

In general, you should avoid those settings marked as non-browsable even though some versions of VS show them in error in the Intellisense. Use NonParallelizable rather than ParallelScope.None.

[evancam]

Sorry, I should have been more specific, I was looking through the issues just for context. This is currently the ApplyToTest method in the nunit source on the dev branch

        public override void ApplyToTest(Test test)
        {
            // Adjust property to include ParallelScope.Self if a fixture has ParallelScope.Fixtures on it
            if (test is TestFixture && _scope.HasFlag(ParallelScope.Fixtures))
                Properties.Set(PropertyNames.ParallelScope, _scope | ParallelScope.Self);

            base.ApplyToTest(test);

            if (test.RunState == RunState.NotRunnable)
                return;

            if (_scope.HasFlag(ParallelScope.Self) && _scope.HasFlag(ParallelScope.None))
                test.MakeInvalid("Test may not be both parallel and non-parallel");

            if (test is TestMethod && _scope.HasFlag(ParallelScope.ContextMask))
                test.MakeInvalid("ParallelScope of a test method may not specify Children or Fixtures");
        }

Everything after the first if statement that changed in 3.8 the method has not been changed since 3.7. I just tested on the 3.8 dev myget package, and the tests still run and are not invalidated. From the bottom 2 if statements, it would imply that the tests should be made invalid and ignored.

Sorry that was my bad for using ParallelScope.None, I realize it is better to use NonParallelizable. I wasn't trying to suggest I was using it, I was trying to refer to its actual functionality, and the fact that using it can dramatically slow down a test run.

I do have the fixtures marked as ParallelScope.Self, the functionality that would be desired is that there is some way for a particular one of those test in that fixture to only be run one at a time.

I realize one way around this would be to put that particular test in a fixture of its own and have ParallelScope.Fixtures above it in the tree so that single test would one one at a time, but it would be nice to have flexibility within a single fixture to avoid splitting a fixture.

[CharliePoole]

Regarding invalidating the test, please post some code that you believe should have been marked invalid but was not. As indicated, if you have a method with both ParallelScope.None and ParallelScope.Self, then it should be marked invalid. Same thing for a test case with either Children or Fixtures, since a test case can't contain any child tests.

Again... assuming you have a fixture containing some tests that should run in parallel and some that should not, you can do one of two things...

[Parallelizable(ParallelScope.Children)] // Or any other scope that excludes children
public class TestFixture
{
    // Mark non-parallelizable methods with [NonParallelizable]
}

[NonParallelizable] // or anyother attribute that doesn't include Children
public class AnotherFixture
{
    // Mark parallelizable tests with [Parallelizable]
}

If I'm misunderstanding you, please post some code!

[evancam]

Here is a sample fixture I used to test this,

    [TestFixture]
    [TestFixtureSource(nameof(Params))]
    [Parallelizable]
    public class UnitTest2
    {
        private static object[] Params = {
            new object[] { "test 1" },
            new object[] { "test 2" }
        };
        public UnitTest2(string test)
        {
        }

        [Test]
        [NonParallelizable]
        [Parallelizable(ParallelScope.Self)]
        public void NUnitTest1()
        {
            Thread.Sleep(5000);
        }

        [Test]
        [Parallelizable(ParallelScope.Fixtures)]
        public void NUnitTest2()
        {
            Thread.Sleep(5000);
        }
    }

Here are the results of the test run of that code:

NUnit Console Runner 3.6.1
Copyright (C) 2017 Charlie Poole

Runtime Environment
   OS Version: Microsoft Windows NT 10.0.15063.0
  CLR Version: 4.0.30319.42000

Test Files
    NUnit.Tests1\bin\Debug\NUnit.Tests1.dll


Run Settings
    DisposeRunners: True
    WorkDirectory: C:\Users\EvanC\Desktop\NUnit.Tests1
    ImageRuntimeVersion: 4.0.30319
    ImageTargetFrameworkName: .NETFramework,Version=v4.5
    ImageRequiresX86: False
    ImageRequiresDefaultAppDomainAssemblyResolver: False
    NumberOfTestWorkers: 8

Test Run Summary
  Overall result: Passed
  Test Count: 4, Passed: 4, Failed: 0, Warnings: 0, Inconclusive: 0, Skipped: 0
  Start time: 2017-08-16 00:53:37Z
    End time: 2017-08-16 00:53:53Z
    Duration: 15.574 seconds

Results (nunit3) saved as TestResult.xml

For the fixture problem,

Let's assume I have 2 fixtures; fixture 'A' and fixture 'B'. They both have ParallelScope.Self, as well as 3 different test fixture data passed in. I want one of the test methods, 'aa', in fixture 'A' to run the individual fixture data one at a time instead of all in parallel. I want the rest of the tests in fixture 'A' to still run as ParallelScope.Self. I would like the tests in fixture 'B' to be unaffected by anything done in fixture 'A'. This makes NonParallelizable a bad option for the test method 'aa', as fixture 'B' will be affected by this.

[CharliePoole]

I'll assume that you have no higher-level ParallelizableAttribute either on the assembly or a SetUpFixture.

The [Parallelizable] on the fixture class will cause it to run in parallel with any other fixtures that have the same attribute. Since it's parameterized, the two instances will run in parallel with one another.

The [NonParallelizable] and [Parallelizable] attributes on test 1 are in conflict with one another. The test should be marked as non-runnable. Since it isn't, that may be a bug - see below. My guess is that one of the two attributes is taking precedence over the other.

Similarly, the [Parallelizable(ParallelScope.Fixtures)] attribute should cause test 2 to be invalid. Again, that may be a bug. In this case, the attribute will have no effect at all, since there are no fixtures "included" in the test method.

I see you are running an older copy of the console runner. Is it possible you are also running an older copy of the framework? The code that checks for invalid combinations is fairly recent.

As for what you want to achieve with multiple fixtures... here's a diagram of what I understand.

[TestFixture("data"), Parallelizable]
public class A
{
    [NonParallelizable]
    public void aa()

   ...
}

[TestFixture("data"), Parallelizable]
public class B
{
   ...
}

If data provides three sets of arguments, we will have six fixture instances running potentially in parallel. I'll assume that all the methods, except for aa have [Parallelizable] (You could alternatively use ParallelScope.Self | ParallelScope.Children on the fixtures, with the same result)

Assuming we are running with at least six cores and we start in parallel mode, all six fixtures would be initialized in parallel (created and one time setup run). In addition, all the methods except for aa would continue to run in parallel. Onetime teardown for the instances of fixture B would also complete.

Eventually, there would be no more tests able to run in parallel. The dispatcher would then switch to its non-parallel mode (we call it a change of shift) and run aa. It would continue to run any other non-parallel tests until there were no more to run. Then it would switch back to parallel mode to run the teardowns for fixture A.

There is no interference among these tests unless you have coded them to somehow interfere with one another. If that's the case, then they can't be run in parallel at all.

I hope this explains things a bit better.

[evancam]

As I have said previously, I am the most recent version of the framework, v3.8.0-dev-04267. I have been testing by both running the tests in Visual Studio, as well as using the 3.6.1 console runner to ensure it is not one or the other causing the problem.

I also just tested a slightly different setup using the 3.8 dev,

    [TestFixture]
    [TestFixtureSource(nameof(Params))]
    [NonParallelizable]
    public class UnitTest2
    {
        private static object[] Params = {
            new object[] { "test 1" },
            new object[] { "test 2" }
        };
        public UnitTest2(string test)
        {
        }

        [Test]
        [Parallelizable(ParallelScope.Self)]
        public void NUnitTest1() {...}

        [Test]
        [Parallelizable(ParallelScope.Fixtures)]
        public void NUnitTest2(){ ... }
    }

this setup seems to be intermittently failing as well. Sometimes it will fully pass, sometimes it will only run 3 out of 4 tests. Sometimes it will hang and throw the following error:

 System.NullReferenceException: Object reference not set to an instance of an object.
   at NUnit.Framework.Internal.Execution.WorkItemQueue.get_IsEmpty()
   at NUnit.Framework.Internal.Execution.WorkShift.get_HasWork()
   at NUnit.Framework.Internal.Execution.ParallelWorkItemDispatcher.StartNextShift()
   at NUnit.Framework.Internal.Execution.ParallelWorkItemDispatcher.OnEndOfShift(Object sender, EventArgs ea)
   at NUnit.Framework.Internal.Execution.WorkShift.EndShift()
   at NUnit.Framework.Internal.Execution.WorkShift.<StartWorkers>b__26_1(TestWorker s, WorkItem ea)
   at NUnit.Framework.Internal.Execution.TestWorker.TestWorkerThreadProc()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()
   at NUnit.Framework.Internal.Execution.WorkItemQueue.get_IsEmpty()
   at NUnit.Framework.Internal.Execution.WorkShift.get_HasWork()
   at NUnit.Framework.Internal.Execution.ParallelWorkItemDispatcher.StartNextShift()
   at NUnit.Framework.Internal.Execution.ParallelWorkItemDispatcher.OnEndOfShift(Object sender, EventArgs ea)
   at NUnit.Framework.Internal.Execution.WorkShift.EndShift()
   at NUnit.Framework.Internal.Execution.WorkShift.<StartWorkers>b__26_1(TestWorker s, WorkItem ea)
   at NUnit.Framework.Internal.Execution.TestWorker.TestWorkerThreadProc()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

As for the multiple fixtures, All of what you said is fine, except it is essential aa is not NonParallelizable. aa is the longest test out of the entire suite of tests, so forcing it to be NonParallelizable would increase the duration of the test run significantly. I don't want to create another separate test fixture, C, for aa as then I am duplicating a significant amount of code from A. I'd rather not create a base fixture that both C and A derive from to solve this problem, as that would make this significantly different from what has already been implemented elsewhere in the current structure.

[CharliePoole]

If the errors only occur when running with attributes that should be forbidden, then I suggest you stop doing that! We will eventually fix the error and the invalid combos will be flagged as non-runnable anyway.

What I haven't yet understood is exactly what you are hoping to have happen as a result of using ParallelScope.Fixtures on a method. If I understood that, I could perhaps give you an alternative.

WRT method aa, is it the case that you want it to run in parallel, but just not in parallel with certain other tests? For example, not with the other tests in the same fixture?

If so, that's the unimplemented feature we describe in #165. Until it's implemented, I'm afraid that parallelism is basically all or nothing for each method.

Based on your sample code, I'm re-opening this so we can confirm it and figure out why the tests are not being invalidated.

[evancam]

Haha, I know they are wrong, I was more so just curious what would happen.

WRT method aa,
I don't want the different fixture instances of the test aa to run in parallel. However, I would like to have the individual instances of the test aa run in parallel with different tests within the fixture, as well as in parallel with other fixtures. I hope that makes sense.