Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrized testcases not running in parallel #2471

Closed
masaeedu opened this issue Oct 4, 2017 · 27 comments

Comments

@masaeedu
Copy link

commented Oct 4, 2017

Issue

I am trying to run several cases of a parametrized test in parallel. Here is the code I'm using:

class Dummy
{
    static TestCaseData Case(int i)
        => new TestCaseData(TimeSpan.FromSeconds(2)).SetName($"Case {i}");

    public static IEnumerable<TestCaseData> Cases()
        => Enumerable.Range(1, 5).Select(Case);

    [TestCaseSource(nameof(Cases)), Parallelizable]
    public void ItShouldSleep(TimeSpan t)
        => Thread.Sleep(t);
}

When I run this using the Visual Studio test runner, I can see the individual cases being executed one at a time, and the overall test run takes ~10 seconds. I would expect all the cases to be executed simultaneously on separate threads, and for the overall run to take ~2 seconds.

Setup

I am using the following .NET Standard based csproj file alongside the code above:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Library</OutputType>
    <TargetFramework>netcoreapp2.0</TargetFramework>
    <ApplicationIcon />
    <StartupObject />
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.3.0" />
    <PackageReference Include="NUnit" Version="3.8.1" />
    <PackageReference Include="NUnit3TestAdapter" Version="3.8.0" />
  </ItemGroup>

</Project>

NuGet packages:

  • Microsoft.NET.Test.Sdk: v15.3.0
  • Microsoft.NETCore.App: v2.0.0
  • NUnit: v3.8.1
  • NUnit3TestAdapter: v3.8.0

Visual Studio:

  • NUnit 3 Test Adapter: 3.8.0.0
  • Visual Studio: 15.3.5
@CharliePoole

This comment has been minimized.

Copy link
Member

commented Oct 4, 2017

See https://github.com/nunit/docs/wiki/Framework-Parallel-Test-Execution

"Parallel execution is supported by the NUnit framework on desktop .NET runtimes. It is not supported in our Portable or .NET Standard builds at this time, although the attributes are recognized without error in order to allow use in projects that build against multiple targets."

IOW, parallel execution is not supported in your .NET Core test assembly.

@nunit/framework-team The above quote is at the very bottom of a usage note that many users will never read, so I think this is a bit of a documentation issue. The warning should be moved up in the usage not and added to the doc page for the attribute itself.

@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 4, 2017

@CharliePoole Gotcha, thanks. Is there an issue tracking .NET Standard support for parallelism? Is the code just ifdefed out?

@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 4, 2017

@CharliePoole Also, changing to <TargetFramework>net461</TargetFramework> doesn't seem to enable parallelism for me.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Oct 4, 2017

@nunit/framework-team I updated the usage note and the descriptions of all three parallelism-related attributes in the docs.

@masaeedu Check to see which build of the framework is referenced in your test. If it's .NET Standard or (for older NUnit) Portable, then you get no parallelism. This is not exactly an issue for us to track. We can't support parallelism in either the .NET standard 1.3 or 1.6 build because they don't provide the support we would need for it. We have an issue for creation of a .NET standard 2.0 build, which would support it.

@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 4, 2017

@CharliePoole I've gone down to <TargetFramework>net45</TargetFramework> and I'm still not getting any parallelism.

Here's the DLL that my NUnit package dependency is resolving to (you can see the metadata indicates it is the full framework build):

image

A full repro project is here: https://github.com/masaeedu/nunit-issue

@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 6, 2017

Issue here was that I wasn't applying the attribute correctly. You need to have [Parallelizable(ParallelScope.Children)] on the containing class.

@masaeedu masaeedu closed this Oct 6, 2017

@ChrisMaddock

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

I would have through Parallelizable(ParallelScope.Children) on the TestCaseSource method would have worked - as the method is technically a 'test-suite'. @CharliePoole - is this a bug, or have a mis-understood the API?

@masaeedu's code is a little different here to what he posted in Gitter - the example I was looking at was this:

    [TestFixture]
    class Deserialization
    {
        public static IEnumerable<TimeSpan> ShouldDeserializeAllCases() 
              => Enumerable.Repeat(0, 5).Select(x => TimeSpan.FromSeconds(2));

        [TestCaseSource("ShouldDeserializeAllCases"), Parallelizable(ParallelScope.Children)]
        public void ShouldDeserializeAll(TimeSpan t)
        {
            Thread.Sleep(t);
            Assert.AreEqual(1, 1);
        }
    }
@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 6, 2017

@ChrisMaddock I've tried both those versions and neither works. The Parallelizable(ParallelScope.Children) is on the test method itself though, and not on the method that is the testcase source. Maybe that's what the issue is?

@ChrisMaddock

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

No - I don't think putting the parallelizable attribute on the testcasesource method itself would do anything at all.

As you pointed out - the docomments do say ParallelScope.Children can't be used on methods - so the behaviour your seeing may be as expected - but it's surprising to me!

@masaeedu

This comment has been minimized.

Copy link
Author

commented Oct 6, 2017

Perhaps there should be some runtime validation of attributes when nUnit reflects over the test assembly, and an error raised if nonsensical usage is detected.

@CharliePoole CharliePoole reopened this Oct 6, 2017

@CharliePoole CharliePoole self-assigned this Oct 6, 2017

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Oct 6, 2017

I reopened this to look at both attribute validation and the handling of parameterized methods.At very least, I'll add it to the set of parallel tests I'm creating.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Oct 29, 2017

I've determined that [Parallelizable(ParallelScope.Children)] works correctly on parameterized methods in the latest build of #2476, so I'm counting this as being fixed when that PR is merged.

We have a general set of problems around attributes that appear on a .test method but which are expected to somehow affect all the test cases for that method.

There are three potential situations

  1. Attributes that specifically apply to the cases
  2. Attributes that do not apply to the cases
  3. Attributes that do not apply to the cases but still impact them in some way (e.g. IgnoreAttribute causes the parameterized test method suite to be ignored, resulting in the cases being ignored as well even though they don't have that setting.)

We may want to address this problem more generally at some point. For this issue, we are only looking at the ParallelizableAttribute and NonParallelizable attribute.

Starting with [NonParallelizable]... this one is unambiguous. It sets the method to be non-parallelizable as well as the context in which the cases are run. Nothing is parallelized once this attribute is used.

[Parallelizable] used with a scope is also pretty clear.

  • If the scope is Self, then the method may run in parallel with other methods of the same class. The individual cases under the method are run sequentially.
  • If the scope is Children then the method may not run in parallel with other methods but all it's cases may run in parallel with one another.
  • If the scope is All then the cases may run in parallel with one another as well as with other parallel methods.

If one remembers that [Parallelizable] is equivalent to [Parallelizable(ParallelScope.Self)], then this case is also clear. However, it seems to be very common for users to mistake the meaning in this case. Our choices are:

  1. Make the bare attribute mean something else when it's applied to a parameterized method. This seems like a potential source of even greater confusion to me.

  2. Make using [Parallelizable] on a parameterized method an error.

  3. Give a warning message of some kind when [Parallelizable] is used on a parameterized method, in order to encourage the user to provide a ParallelScope argument.

  4. Just improve the documentation.

I prefer approach number 4. What do others think?

rprouse added a commit to rprouse/NUnit.Parallelism.Integration.Tests that referenced this issue Oct 31, 2017

@ChrisMaddock

This comment has been minimized.

Copy link
Member

commented Oct 31, 2017

I agree, 4 sounds good. The current situation is logical with a deeper understanding of NUnit, to me.

This stuff could really use examples of these cases - I'm not sure I would have been able to guess that behaviour based on spec alone.

@PYasonau

This comment has been minimized.

Copy link

commented Nov 2, 2017

@CharliePoole I am sorry that i am here in closed issue, but it is still not clear for me. I need work against .net core 2.0 and now i don't have parallelism. But in future, will i got it? Any time predictions? Issues to track?

@rprouse rprouse added the closed:done label Nov 3, 2017

@rprouse rprouse added this to the 3.9 milestone Nov 3, 2017

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Nov 28, 2017

@PYasonau Just noticed you were never answered. @rprouse Is it possible to give him some idea of when this might come?

@mikkelbu

This comment has been minimized.

Copy link
Member

commented Nov 28, 2017

Not I can answer on @rprouse behalf, but I think that work on a .NET Standard 2.0 version will begin shortly (after the smaller tasks - due to the major cleanup in #2380 - has been done). Please track this issue #2542.

@gamerka

This comment has been minimized.

Copy link

commented Apr 12, 2018

@CharliePoole hi, I'm here concerning:

If the scope is Children then the method may not run in parallel with other methods but all it's cases may run in parallel with one another.

It seems, it's not working for me.
I have a [NonParallelizable] Fixture containing 2 [Parallelizable(ParallelScope.Children)] Tests, each has 3 TestCases.
With default LevelOfParallelism = 4 I have all 3 Cases of Test1 and one Case from Test2 starting simultaniously, and I can't understand why.
Tried both VS test adapter and nunit-console, latest versions.
My test code:

    [TestFixture]
    [NonParallelizable]
    public class Tests
    {
        [Parallelizable(ParallelScope.Children)]
        [TestCase(1)]
        [TestCase(2)]
        [TestCase(3)]
        public async Task Test1(int i)
        {
            Console.WriteLine($"{DateTime.Now} Start1 {i}");
            await Task.Delay(TimeSpan.FromSeconds(5));
            Console.WriteLine($"{DateTime.Now} Stop1 {i}");
        }
        [Parallelizable(ParallelScope.Children)]
        [TestCase(1)]
        [TestCase(2)]
        [TestCase(3)]
        public async Task Test2(int i)
        {
            Console.WriteLine($"{DateTime.Now} Start2 {i}");
            await Task.Delay(TimeSpan.FromSeconds(5));
            Console.WriteLine($"{DateTime.Now} Stop2 {i}");
        }
    }

I'm returning to this issue third time for the last few months, previously I thought it's just not possible, as docs say that [Parallelizable(ParallelScope.Children)] is only valid for assemblies and classes. But after seeng your post I tried one more time, on latest version, with same result.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

@gamerka
Remove the attributes from your tests and put [Parallelizable(ParallelScope.Children)] on the class.

Explanation...

ParallelScope.Children on the class means (1) the class is non-parallel (2) its children (i.e. the methods) are all parallel with respect to one another.

ParallelScope.Children on the method would indicate that any children of the method run in parallel. But methods do not have any children!

Hope that clarifies for you.

@nunit/framework-team
I've seen this confusion come up a few times. Shouldn't we give an error message or perhaps a warning when invalid scopes are used? I thought I had written code to do that, but maybe it was never submitted as a PR.

@jnm2

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2018

@CharliePoole Yes, diagnostics would be helpful. I'm confused right now because I thought methods did have children in the case of parameterization as in gamerka's example.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

@jnm2 I'm not in the code right now, but IIRC the attribute originally applied to the "hidden" parameterized suite but that confused users so we changed it. I could be remembering wrongly though.

@jnm2

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2018

In ReSharper's runner, which is the only test UI I've put much attention into, the parameterized method is shown at the same level as non-parameterized methods and the test cases show up as child nodes of the parameterized method's node. I thought they were just blindly following the test hierarchy we output, so I thought that's what we do. I suppose I'd have to look at the test XML to verify that.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

There is a suite and the cases are children. I'm saying we handle different attributes differently and I believe Parallelizable is different.

@gamerka

This comment has been minimized.

Copy link

commented Apr 12, 2018

@CharliePoole

Remove the attributes from your tests and put [Parallelizable(ParallelScope.Children)] on the class.

Then I get exactly the same result - all testcases are running in parallel.
Maybe I need to clarify my expectations:
I need to make all Tests run sequentially, but TestCases inside this methods to run in parallel.
I thought that by this quote you meant exactly that:

If the scope is Children then the method may not run in parallel with other methods but all it's cases may run in parallel with one another.

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

I went back to find that quote and I'm pretty sure it was correct at teatime and in that context. AFAICS it applied to the then current master, not to a particular release. I'll take a closer look at the current code and docs. What framework version are you using?

@gamerka

This comment has been minimized.

Copy link

commented Apr 12, 2018

I'm using .Net framework 4.6

@CharliePoole

This comment has been minimized.

Copy link
Member

commented Apr 12, 2018

Which nunit framework version?

@gamerka

This comment has been minimized.

Copy link

commented Apr 12, 2018

nunit 3.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.