SpecFlow incorrectly assumes tests are multi-threaded if test runner is re-used under NUnit3

[remcomulder]

Hi, NCrunch developer here.

I've had reported to me a problem in which SpecFlow kicks up an error when NCrunch is used to run specifications under NUnit3 with single-threaded execution:

TechTalk.SpecFlow.SpecFlowException : The ScenarioContext.Current static accessor cannot be used in multi-threaded execution. Try injecting the scenario context to the binding class. See http://go.specflow.org/doc-multithreaded for details.
at TechTalk.SpecFlow.ScenarioContext.get_Current()
at SpecFlow2NUnit3NCrunch.CustomContext.ScenarioSetUp() in C:\Temp\SpecFlow2NCrunchProblem\SpecFlow2NUnit3NCrunch\CustomContext.cs:line 14
at lambda_method(Closure , IContextManager )
at TechTalk.SpecFlow.Bindings.BindingInvoker.InvokeBinding(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, TimeSpan& duration)
at TechTalk.SpecFlow.Infrastructure.TestExecutionEngine.FireEvents(HookType bindingEvent)
at TechTalk.SpecFlow.Infrastructure.TestExecutionEngine.OnScenarioStart(ScenarioInfo scenarioInfo)
at TechTalk.SpecFlow.TestRunner.OnScenarioStart(ScenarioInfo scenarioInfo)
at SpecFlow2NUnit3NCrunch.CalculateFeature.ScenarioSetup(ScenarioInfo scenarioInfo) in C:\Temp\SpecFlow2NCrunchProblem\SpecFlow2NUnit3NCrunch\CalculatorTest.feature.cs:line 0
at SpecFlow2NUnit3NCrunch.CalculateFeature.MultiplyTwoNumbers(String case, String a, String b, String result, String[] exampleTags) in C:\Temp\SpecFlow2NCrunchProblem\SpecFlow2NUnit3NCrunch\CalculatorTest.feature:line 29

The cause of the problem seems to be in the TechTalk.SpecFlow.TestRunnerManager class, where SpecFlow attempts to identify multi-threaded execution by tracking the IDs of threads used to access the ScenarioContext. The code doesn't behave as expected in situations where the test runner process is re-used.

NCrunch can make multiple calls into a test runner for its batching, invoking the test framework multiple times. The first call into the test framework works fine, but if it calls in again, NUnit3 runs the tests on a different thread and the code throws the above exception.

This was never a problem under NUnit2, because NUnit2 would always run tests on the thread used to invoke the framework, and NCrunch always uses the same thread to invoke the framework. However, NUnit3 introduced significant changes to the threading model and internalises the creation of a new thread for each test run.

A possible solution to this problem would be to 'reset' the tracking of threads at the start of every test run.

Alternatively, it is possible to check for the 'NCrunch' environment variable (which equals '1' under an NCrunch test run) and simply suppress the behaviour. NCrunch always disables the parallel execution features of test frameworks because it handles them at a higher level (across multiple processes). I presume this solution will be undesirable as it is product-specific and I'm not sure if other test runners are doing the same thing as NCrunch.

[qtpvarun]

this happened to me also with NUnit GUI runner with NUnit 2.6.4 while executing in windows server through VDI. in my local laptop it works fine though, I had to change ScenarioContext.Current to injected one and then it worked fine for me. this may also be related issue.

[SabotageAndi]

Let's see if I understand it correctly.
NCrunch starts multiple threads and in each they call into NUnit. NUnit it self starts the execution of the tests in another thread.
And I assume all of this happens in the same AppDomain?

Then this exception is the current expected behavior of SpecFlow.
See: http://www.specflow.org/documentation/Parallel-Execution/ - Thread-safe ScenarioContext, FeatureContext and ScenarioStepContext

If you want to use the ScenarioContext in this case, you have to get it via context injection.

@gasparnagy Please correct me if I am wrong.
But I think we should try to bring the ScenarioContext.Current also to work on multithreaded executions.

@remcomulder
Could you give us the code you have to reproduce it?

[remcomulder]

Sorry, perhaps I haven't explained this very well.

This is not a parallel execution scenario. No code is being executed in parallel inside the same AppDomain.

NCrunch holds onto the same thread and always uses the same thread when it calls into the test framework.

Here is basically what happens:

1. NCrunch creates a new process to run several tests
2. The process is initialised, the test framework (in this case, NUnit3) is loaded
3. NCrunch calls into the process via IPC and tells it to run say, 3 tests
4. The process makes a call into NUnit3 via its API, passing instructions to run 3 tests
5. NUnit3 creates a new thread and executes the 3 tests
6. NUnit3's execution of the 3 tests results in the TechTalk.SpecFlow.TestRunnerManager class being invoked, at which point the ID of NUnit3's execution thread is stored here and remembered for future checking.
7. NUnit3's execution run completes. Results are returned to NCrunch over IPC.
8. The test process is not terminated, it is kept alive by NCrunch for future execution
9. NCrunch calls into the process again, via IPC, to run another 2 tests
10. The process makes a fresh call into NUnit3 via its API, passing instructions to run 2 tests
11. NUnit3 creates a NEW thread to execute the 2 tests
12. NUnit3's execution of these 2 tests results in another call to the TechTalk.SpecFlow.TestRunnerManager class. Note that in this case the ID of the current thread is different, because this is a whole new testing session. The above exception is thrown.

Note that the above contains no actual concurrent execution, although it does involve multiple threads calling into and waiting on each other.

I'll need permission from the user that reported this problem to me before I can share their code with you (they provided me with a sample exhibiting the problem). Note that at present I don't know of a way to reproduce the problem outside NCrunch, as I'm not aware of any other test runners that will re-use a test process after the test framework has completed its run.

[ErikStroeken]

Just for the record, I'm the one who reported the issue on Stackoverflow:http://stackoverflow.com/questions/37520896/specflow-and-ncrunch-multi-threaded-error-while-executing-in-single-thread. By cross-linking both bloggs the users don't miss any information.
Thanks for all your help!

[SabotageAndi]

@remcomulder Thanks for the explanation.
Now I see where the problem is. The testRunnerRegistry is only cleaned when the AppDomain is unloaded. As this never happens with NCrunch and the different threads, you get more than one entry in the registry, which then disables ScenarioContext.Current. And you get the exception.

That will be tricky to fix. :-)

[SabotageAndi]

@ErikStroeken Is this the example that @remcomulder is using?

[ErikStroeken]

You can find the example project that propagates the exception here: http://fair-homepage.ch/data/SpecFlow2NCrunchProblem.zip. Direct click doesn't work from here: you need to copy the link into the address bar of your browser. Its the same project @remcomulder used and Stackoverflow refers to...

[samholder]

@sabotageandi would there be an issue with cleaning the registry at the end (or start) of the test run? I'll try and make some time to have a look at this over the weekend.

[SabotageAndi]

@samholder that could be also a solution, but do we have a way to know when a test run is started? It looks for me, that we currently interpret the first instance of a testrunner as TestRun start.

I thought about trying to enable ScenarioContext.Current in multi threaded modes. This should fix this also.

[samholder]

Yeah maybe having a dictionary of ScenarioContext instances keyed by thread
ID and current returns the one for the current thread. That might fix the
issue as well, though I'd need to look into it in a bit more detail
On 3 Jun 2016 5:55 p.m., "Andreas Willich" [email protected] wrote:

@samholder https://github.com/samholder that could be also a solution,
but do we have a way to know when a test run is started? It looks for me,
that we currently interpret the first instance of a testrunner as TestRun
start.

I thought about trying to enable ScenarioContext.Current in multi threaded
modes. This should fix this also.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/techtalk/SpecFlow/issues/638#issuecomment-223633352,
or mute the thread
https://github.com/notifications/unsubscribe/AB-iytTnP5NCKhwawea-Dfc33Ad0dHLHks5qIFyFgaJpZM4IrEtI
.

[SabotageAndi]

@samholder have a look at #641

[gasparnagy]

@SabotageAndi @samholder I don't understand how making ScenarioContext Current work for parallel would help here. If we store things in static registry, they have to be cleaned up otherwise it will produce invalid result... or?

[samholder]

ok, as I said I was unsure if that would help. Is there some point we could do that clean up? or any other suggestions? I might get a couple of hours tonight and will try and do a bit more investigation, but any pointers in the mean time would be appreciated

[gasparnagy]

@samholder the first we would need to decide is what do we want to achieve in terms of the before/after hooks in the scenario described by @remcomulder. do we want to have a before/after test run for each execution batch? or just a beforetestrun and never an aftertestrun (because we can never be sure if there will be a new batch)? also the same questions about the step definition registry: do we want to build it up for each batch?

The problem is that NCrunch runs the specflow tests through the NUnit interface that lacks a lot of things that would be needed to control the execution.
@remcomulder: would it be also an option (for NCrunch) if you would somehow detect that this is not only an NUnit test but a SpecFlow test wrapped in NUnit, so that you could call some special SpecFlow API directly to reset the state?

[SabotageAndi]

@gasparnagy Yeah, you are right. It would work, as long as you do not have threads of different runs with the same ThreadId. Then you would get an old one. :-/

[remcomulder]

@gasparnagy The problem with calling into APIs in NCrunch is usually accessing them. NCrunch needs to work with all major versions of NUnit, Xunit, .NET, VS, etc, often with different versions at the same time (i.e. solution using different versions of a framework). This makes static binding into 3rd party assemblies difficult, especially when these assemblies could be at any version and are being provided by the user.

It's certainly not impossible, but I prefer to avoid it where I can. So far we've managed to get by very well with only indirect integration with SpecFlow. It would be rather frustrating for everyone if the NCrunch/SpecFlow integration suddenly started breaking when new versions of SpecFlow become available, because of the need to link into APIs.

Another thing to consider is that although NCrunch may be the only runner to re-use a test process, I think it's unlikely to be the last. There is considerable movement now into next generation test runners and I expect that others will also do this (if they don't already).

[gasparnagy]

@remcomulder yeah, I absolutely understand. The problem is not (so much) to support re-use from the SpecFlow side. I think it is somehow already supported, we would just need to expose a nicer API.
The problem is more, that how you can access this API through NUnit that does not support such things. So we have to find a smart solution.
Just to be sure I understand it. You are re-using the test process - that's clear, but is it a must to also re-use the same AppDomain across test runs?
How does NCrunch behave if the test assembly changes between multiple runs?

[remcomulder]

Reusing the AppDomain is important because most of the cost of setting up the test process comes from the initialisation of the test framework (i.e. NUnit). Test framework initialisation is very expensive because it usually involves enumerating all types in an assembly and allocating a large number of objects. By comparison, the cost of recycling just the process itself is actually very small (Windows seems to do this very efficiently).

If the user changes their assembly, NCrunch has no choice but to recycle the entire process with its application domain.

What are the options for fixing this from the SpecFlow side? Is there any point in which SpecFlow becomes aware that a test run has started or stopped under NUnit3?

[gasparnagy]

@remcomulder thx. this is clear now. i have to think about (or at least go to sleep now) what would be the best option to fix this on SpecFlow side. I'll keep you informed.

[StephenFriend]

@gasparnagy just wondering if you'd got any further with this? I use NCrunch, NUnit3 and SpecFlow and this issue means I'm having to reconsider my toolset (which I really like, so I'd prefer not to change it).

[gasparnagy]

@StephenFriend Thx for reminding me. I have forgotten about it. I will try to come up with some idea this week.

[gasparnagy]

@remcomulder I was thinking on this and maybe found an easy way. Currently if a unit test runner supports multi-threading, SpecFlow generates a unit test that calls the TestRunnerManager.GetTestRunner method without a second managedThreadId parameter. If this second parameter is not provided, the code takes the thread id of the current thread (see https://github.com/techtalk/SpecFlow/blob/396e0e40ca06e68c01a4ca46fa0dfc9fbbb5a8c3/TechTalk.SpecFlow/TestRunnerManager.cs#L180). What we could maybe do is to check an environment variable (e.g. SPECFLOW_DISABLE_PARALLEL) and if it is set, we would juts use a fix logical thread id (e.g. 0). As a result, all execution batch would run in the same logical thread and there would be no confusion regarding the before/after events, etc.

For this, the only thing what NCrunch would need to do is set a process-level environment variable for the test execution process. Since it is harmless for non-SpecFlow projects, you can set if without the need of detecting whether the current NUnit test uses SpecFlow or not.

I don't know, however, whether it contradicts to any of the features of https://github.com/samholder/SpecFlow.NCrunch (by @samholder).

/cc @StephenFriend @samholder

[samholder]

This should be fine for the specflow extension. This is a generator extension that adds NCrunch attributes to the generated tests only.

[samholder]

I can try and spike the implementation of this solution tonight I think. We could perhaps detect the NCrunch environment variable temporarily until NCrunch implements setting the SPECFLOW_DISABLE_PARALLEL in order to fix in the next Specflow release?

[gasparnagy]

@samholder yeah, that could be an easy way to test the concept...

[SabotageAndi]

@samholder @gasparnagy Could we add this switch also to the config file?

[remcomulder]

@gasparnagy This makes sense. I can easily make a code change to NCrunch to set the new environment variable for SpecFlow when its detected ...

Although, it would be really nice if we can make this also work for retrospective versions of NCrunch, and the build that people are using currently. If I make a code change now, this problem will still exist for everyone until the upgrade to the next release (which is still a little while away). Some people are also using older versions of the software.

Is using the NCrunch environment variable a viable long term option? Perhaps it would be worthwhile to check both values, as other test runners may have a similar problem and it would seem strange for them to pretend to be NCrunch :)

[gasparnagy]

@remcomulder i see. yes, we can add an additional check for the NCrunch variable for a while I think. Anyway, this is exactly what @samholder will try out. Let's see his results.

@SabotageAndi we can put it to the config file as well, but for NCrunch this is not a solution, because the same project is sometimes executed with NCrunch (where the parallel treatment has to be disabled) and sometimes with another test runner (where we want to have parallel).

[samholder]

@ErikStroeken thanks for the sample project. I have used this to test this fix and it seems to allow the tests to pass in your sample project when run with NCrunch. Once the Appveyor build is complete it would be good if you could grab the version of the nuget package from that feed and let me know if this fixes your issue.

[StephenFriend]

@samholder I'd love to try the fix but I don't know how to use appveyor & nuget to update specflow within VS (2015).

[samholder]

If you go to Tools->options->nuget package manager then going the nuget
feed and add the address of the appveyor feed to the list of available
feeds, you can then just choose to update specflow from that source. You'll
have to enable versions that are non release. I can't recall what they call
that option right now. The app veyor feed is
https://CI.appveyor.com/nuget/specflow-ci I think

On 30 Jul 2016 11:49 a.m., "Stephen Friend" [email protected]
wrote:

@samholder https://github.com/samholder I'd love to try the fix but I
don't know how to use appveyor & nuget to update specflow within VS (2015).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/techtalk/SpecFlow/issues/638#issuecomment-236358753,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB-iymRndrqw4FGuP0Nvb46a9y4bHPNQks5qayxSgaJpZM4IrEtI
.

[StephenFriend]

@samholder well, that was obvious. Sorry for the laziness. I'm happy to report that NCrunch is now working happily with my SpecFlow tests. Thanks for the fix.

[samholder]

@StephenFriend Awesome. Thanks for taking the time to test this, it's much appreciated.

[ErikStroeken]

@samholder 'The option to enable versions that are non release' is the check box 'Include prerelease' in the upper left corner next to the search box in the Nuget dialog "Manage Packages for Solution".
Just installing the pre-release didn't work. What exactly must I do to get your fix working? Declare a string type environment variable NCRUNCH or SPECFLOW_DISABLE_PARALLEL and write True in it? Thanks again for your help!

[samholder]

@ErikStroeken Yes its the 'Include prerelease' checkbox. you should not need to do anything else to get the fix working, as when the tests are run by NCrunch it will set the NCRUNCH environment variable to 1 and so the running of tests in parallel by specflow should then be disabled. I'm wondering if the version at the front of the pre-release feed is one from a branch taken from before this changed was merged to master. I can't think of another reason why this wouldn't work.

I'll try and investigate.

[samholder]

@ErikStroeken do you know which version you are getting from the pre-release feed? The change was merged to master in build 407, and I think most builds since then should have it. But you can take that specific build if you want to test just the fix I think

[ErikStroeken]

@samholder I'm resuming the tests with exactly the same solution (with 9'000 tests) as when I discovered the problem in May to be sure to make only one change at a time. I only updated SpecFlow. Currently the solution has the following versions:

• SpecFlow: 2.1.0-ci424
• NUnit: 3.2.1
• NCrunch VS Extension: 2.23.0.2
  I'm performing the tests with the NCrunch Console Tool that has the same version as the NCrunch VS Extension: 2.23.0

When I define the environment variables NCRUNCH and SPECFLOW_DISABLE_PARALLEL manually with version 3.23.0.2 of NCruch there are only 2 tests failing indicating that the patch works.

@remcomulder does the NCrunch version 2.23.0.2 already sets the NCRUNCH environment variable?

[samholder]

@ErikStroeken and if you don't define those environment variables, then the tests don't pass (ie you get the original error)?

Do the tests pass in VS when run with NCrunch?

BTW defining either of those variables should be enough to activate the patch, no need to define them both

[remcomulder]

@ErikStroeken The NCRUNCH environment variable has been set by NCrunch since its first public release :)

[ErikStroeken]

Yes! Everything works now. For some reason 2 tests reported ‚TechTalk.SpecFlow.MissingStepDefinitionException : No matching step definition found for one or more steps‘ but after generating the feature files for the second time those problems disappeared. I removed both environment variables and all tests work, both in NCrunch Visual Studio and Console.

@samholder When is the next official release of SpecFlow planned that includes this bugfix?

[ErikStroeken]

Good afternoon. Since my last chat in august 2016 only a prerelease 'v2.2.0-preview20161020' appeared. Are you planning to have an official release or is the product not being developed any further? Shall we install the preview?

[SabotageAndi]

You can install the preview without problem.
Or if you need newer binaries, you can use the CI- NuGet Feed: https://ci.appveyor.com/nuget/specflow-ci
We are currently working on a release of 2.2 in the next weeks.

[ErikStroeken]

@SabotageAndi
Wanted to update our project and installed the following combination:

• SpecFlow: Latest prerelease 2.2.0-preview20161020
• NUnit: 3.6.1
• NCrunch VS Extension: 3.6.0.2 (VS 2015)

With that combination I get hundreds failing tests with exception:

'System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
  ----> System.InvalidOperationException : The calling thread cannot access this object because a different thread owns it.'

I tried the SpecFlow Version 2.1.0-ci424 with the special fix of august 2016 together with the NUnit and NCrunch versions as mentioned above and it worked: no tests fail.

Next I switched back to Latest prerelease 2.2.0-preview20161020, generated all feature code behind files and the same tests failed again.

Can it be that you forgot to implement the fix of version 2.1.0-ci424? If not, then version 2.2.0 has a new unknown problem that has to be investigated.

[SabotageAndi]

@ErikStroeken It was merged. See the code here: https://github.com/techtalk/SpecFlow/blob/master/TechTalk.SpecFlow/TestRunnerManager.cs#L196
No idea why it is not working anymore, but this exception is new for me.
Do you have a StackTrace for it?

[ErikStroeken]

Before we deep dive into the problem: after the fix of august 2016 only one single thread should execute each BDD test, right? The 'new' exception indicates that more than one thread is working with the same object so it seems that the multithreading problem is back again.

Here is one of a thousend exceptions:

NCrunch: This test was executed on server '(local)'

Given TW4 is running
-> error: Exception has been thrown by the target of an invocation.
Total Test Execution Time: 00:00:45.8578437
System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
  ----> System.InvalidOperationException : The calling thread cannot access this object because a different thread owns it.
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at BoDi.ObjectContainer.CreateObject(Type type, ResolutionList resolutionPath, RegistrationKey keyToResolve)
   at BoDi.ObjectContainer.TypeRegistration.Resolve(ObjectContainer container, RegistrationKey keyToResolve, ResolutionList resolutionPath)

[ErikStroeken]

It seems to be a new problem. After repeatedly rerunning the tests in VS2015 they succeed.
Another new thing I noticed: it appears to me that the total duration of executing all tests increased drammatically.
I'll try the command line version of NCrunch and I'll measure the difference. I'll report the results in 2 days.

[ErikStroeken]

The command line version of NCrunch has the same problems. Its a cross thread exception in the BDD tests which are not designed to be executed in parallel. Version 2.1.0-ci424 doesn't have this problem, its new in version Latest prerelease 2.2.0-preview20161020.
The test results with stack trace can be found here: test results
Don't click directly on the link but copy it in the browser.
Can it be that the latest version still runs some tests in parallel?

[srosam]

I'm getting the same exception (TechTalk.SpecFlow.SpecFlowException : The ScenarioContext.Current static accessor cannot be used in multi-threaded execution) while running SpecFlow tests using the NUnit 3 Gui Runner.

Test runs fine 1 time, on second run it fails with this error.

Any resolution to this would be great, its blocking our upgrade to NUnit3 at the moment.

[DavidKorn]

We're seeing this too in 2.2.0 Things are working in 2.10.0-ci424, but it seems something has changed since. Since we can repro by leaving everything unchanged and just upgrading SpecFlow to 2.2.0, it appears the current issue lies elsewhere in SpecFlow.

@ErikStroeken 's detailed report above from May seems to explain the current case well, but I'm wondering if it was overlooked since this issue was already closed.

Should we open a new issue for this, or re-open this one?

[DavidKorn]

I opened this reemerging issue for 2.2.0 as #917.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.