reportportal / agent-net-specflow

Report Portal agent for SpecFlow
Apache License 2.0
10 stars 5 forks source link

Parallel Execution support #2

Closed abasau closed 6 years ago

abasau commented 8 years ago

SpecFlow throws the following exception when scenarios are run in parallel:

TechTalk.SpecFlow.SpecFlowException : The FeatureContext.Current static accessor cannot be used in multi-threaded execution. Try injecting the feature context to the binding class. See http://go.specflow.org/doc-multithreaded for details.

At the moment there is no way to resolve the issue because of inherited limitation of SpecFlow that will be addressed in https://github.com/techtalk/SpecFlow/issues/552.

DzmitryHumianiuk commented 7 years ago

@aliaksandrbasau @nvborisenko i see that reffred issue closed at techtalk/SpecFlow#552

will it unblock current issue?

abasau commented 7 years ago

@DzmitryHumianiuk, @nvborisenko, I am going to take care of it eventually. The implementation was unblocked by that fix and I even made some progress there but faced with another issue. It looks like SpecFlow doesn't support running NUnit 3 tests from the same fixture in parallel. Parallelization on the fixture level works fine but not on the test level. An deeper investigation still needs to be done.

abasau commented 7 years ago

The issue is currently blocked by https://github.com/techtalk/SpecFlow/issues/894 and https://github.com/techtalk/SpecFlow/issues/886.

nvborisenko commented 7 years ago

Right, specflow cannot generate [Parallelization] attribute into *.cs files. Only 1 possible way is to specify parallelization on assembly level. I propose to support this in our agent due there are no others ways for parallelization with nunit3 provider.

But there are some another runners which support parallelization, eg SpecRun. This guy can execute tests in the same feature in parallel. Let's try to test our agent with this runner and see results.

My idea here is to support official stuff. Parallelization on feature level will be available in the next releases (might be).

abasau commented 7 years ago

We could generate [Parallelization] attributes for tests by implementing a custom generator class based on NUnit3TestGeneratorProvider (https://groups.google.com/forum/#!topic/specflow/8Tp0QyfgpbU). SpecFlow currently allows to generate ParallelizableAttribute for fixtures (according to https://github.com/techtalk/SpecFlow/issues/886).

I tried to run tests with SpecRun. https://github.com/techtalk/SpecFlow/issues/886 currently blocks the further development.

Integration with RP works when NUnit 3 tests are executed in parallel on fixture level. https://github.com/techtalk/SpecFlow/issues/894 blocks running them in parallel on test level.

xunit only supports running tests in parallel on feature level (these 3 runners are mentioned on https://github.com/techtalk/SpecFlow/wiki/Parallel-Execution). It should not be so difficult to support it.

DzmitryHumianiuk commented 7 years ago

@aliaksandrbasau https://github.com/techtalk/SpecFlow/issues/886 closed on Aug 29. 2017

abasau commented 7 years ago

@DzmitryHumianiuk I saw that one. Still waiting for https://github.com/techtalk/SpecFlow/issues/894 to be resolved.

nvborisenko commented 7 years ago

@aliaksandrbasau Alex, I played with ContextInjection and it seems workable. Only 1 issue that I didn't resolve: I cannot get the context in TraceError() method in our ReportPortalAddin class. Specflow raises the exception about non-initialized object container. Like https://github.com/techtalk/SpecFlow/issues/759

As I understand the current progress of this issue you are waiting until #894 will be resolved. But #894 is about nunit and level of parallelism. I think this is related only to specflow and nunit, no additional work is required from our side. Alex, could you please share your current work in separate branch? So, I think we can release a new version of agent, that supports simple model of parallelization (TestFixture level). It would be better than waiting until deeper level of parallezation will be officially supported.

Thanks in advance.

abasau commented 7 years ago

@nvborisenko Sure. Sounds reasonable. Let's release the agent with feature-level parallelization support. I will work on it on weekend and will publish the branch with my results.

abasau commented 7 years ago

@nvborisenko The issue with getting a context in TraceError() is a major one. I think we will have to re-implement reporting step results differently. Instance of ITestTracer belongs to global container and doesn't have access to FeatureContext or ScenarioContext (see https://github.com/techtalk/SpecFlow/wiki/Available-Containers-&-Registrations). I am working on replacing our implementation of ITestTracer with BeforeStep and AfterStep hooks. StepContext should provide enough information about a step while private property ScenarioContext.TestStatus will give us a scenario status. If the solution works, I will submit an improvement to make ScenarioContext.TestStatus public. Most likely the compatibility with the previous version will be broken. I would remove the following static properties in ReportPortalAddin: CurrentFeature, CurrentScenario and CurrentScenarioDescription. I may replace some of them with similar methods. I will need a few more day to re-implement the integration and test it. Hopefully feature- and, even, scenario-level parallelization will work in supported unit frameworks.

nvborisenko commented 7 years ago

Created pull request into specflow repo about internal TestStatus property https://github.com/techtalk/SpecFlow/pull/963 Not sure in which version it is going to be available.

avarabyeu commented 7 years ago

@nvborisenko Kolya, PR has been merged. Do we expected any activities on the current issue?

nvborisenko commented 7 years ago

Waiting new release of specflow, and then we will rewrite agent.

yurii-hunter commented 6 years ago

SpecFlow 2.3 is released

abasau commented 6 years ago

@yurahunter Thanks! I will work on re-testing and merging the changes for Parallel Execution support I implemented.

yurii-hunter commented 6 years ago

@aliaksandrbasau thanks! Looking forward for this feature

abasau commented 6 years ago

@nvborisenko Could you please release a new version of ReportPortal.Shared that will have dependency on ReportPortal.Client (>= 2.0.0)?

nvborisenko commented 6 years ago

@aliaksandrbasau no big changes in client 2.0.0. You can use beta of shared. New release of shared is coming which will shared across app domains. I propose to start development with current beta, and then migrate to released shared library. Thanks for understanding.

abasau commented 6 years ago

@nvborisenko WellKnownIssueType struct is missing in ReportPortal.Client 2.0.0-beta3 that is referenced by the latest ReportPortal.Shared (2.0.0-beta3). I am okay with having a new beta release of ReportPortal.Shared. Or I can always hard-code issue types :) What do you think?

nvborisenko commented 6 years ago

ReportPortal.Shared.2.0.0-beta4 pushed to nuget

abasau commented 6 years ago

@nvborisenko Many thanks!

abasau commented 6 years ago

I have committed the initial version (pull request #21). It was tested with NUnit3, MSTest, XUnit and SpecRun and with Log4Net logger. MbUnit unit framework seems dead so I didn't spend any time testing it. MSTest is the only one without class level parallelization support. Although the feature was recently introduced in beta build (https://blogs.msdn.microsoft.com/devops/2018/01/30/mstest-v2-in-assembly-parallel-test-execution/). It's still quite raw and doesn't seem to work well with SpecFlow. NUnit3 and SpecRun support test level parallelization. But NUnit3 can't run SpecFlow tests this way because of https://github.com/techtalk/SpecFlow/issues/894. At this point let's aim for class level parallelization support (not test level). Test level will also work but may produce duplicate test items for features on RP. It all depends on how test runner executes tests. I am going to work on documentation tomorrow.

yurii-hunter commented 6 years ago

@aliaksandrbasau I've installed ReportPortal.SpecFlowPlugin beta-20 but the issue is still reproducable. xUnit.net + SpecFlow + report portal. Report portal test run is empty

abasau commented 6 years ago

@yurahunter I am going to try to reproduce it myself later. But for now, could you do a preliminary investigation and post your findings (e.g. exception, Fiddler log, etc.)?

yurii-hunter commented 6 years ago

@aliaksandrbasau sorry, test run is empty for beta-7, but for beta 20 I'm getting following exception

System.NullReferenceException
Object reference not set to an instance of an object.
   at ReportPortal.SpecFlowPlugin.FeatureInfoEqualityComparer.GetFeatureInfoHashCode(FeatureInfo obj)
   at System.Collections.Concurrent.ConcurrentDictionary`2.TryGetValue(TKey key, TValue& value)
   at System.Collections.Concurrent.ConcurrentDictionary`2.ContainsKey(TKey key)
   at ReportPortal.SpecFlowPlugin.ReportPortalAddin.GetFeatureTestReporter(FeatureContext context)
   at ReportPortal.SpecFlowPlugin.ReportPortalHooks.BeforeScenario()
   at TechTalk.SpecFlow.Bindings.BindingInvoker.InvokeBinding(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, TimeSpan& duration)
   at ReportPortal.SpecFlowPlugin.SafeBindingInvoker.InvokeBinding(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, TimeSpan& duration)
   at TechTalk.SpecFlow.Infrastructure.TestExecutionEngine.OnAfterLastStep()
   at Automation.Tests.Tests.SmokeTestFeature.ScenarioCleanup()
   at Automation.Tests.Tests.SmokeTestFeature.SimpleSmokeTest()

ReportPortal.Client version="2.0.0" ReportPortal.Shared version="2.0.0-beta4" ReportPortal.SpecFlow version="1.2.2-beta-20" SpecFlow version="2.3.1" SpecFlow.CustomPlugin version="2.3.1" SpecFlow.xUnit version="2.3.1"

abasau commented 6 years ago

@yurahunter Thanks! I will investigate the issue later. I suspect that either FeatureInfo.Description or FeatureInfo.Tags are empty, and the agent doesn't handle it well. As a potential workaround I would add a description or tags to your feature file.

yurii-hunter commented 6 years ago

@aliaksandrbasau Yes! You are right. It requires feature description

abasau commented 6 years ago

@yurahunter I have created a pull request (#23) to fix the issue. Many thanks for reporting it! And sorry about it. @nvborisenko Please review the pull request and release a new beta version.