Fix #1433: Only use async waiting for async step definitions (#1449)

* move AsyncHelpers to Bindings folder

* Only call bindings with AsyncHelpers when they are actually async (return Task)

* extract synchronous binding invocation to a service, to be able to customize it more easily

TechTalk.SpecFlow/AsyncHelpers.cs → TechTalk.SpecFlow/Bindings/AsyncHelpers.cs

@@ -4,7 +4,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 
-namespace TechTalk.SpecFlow
+namespace TechTalk.SpecFlow.Bindings
 {
     /// <summary>
     /// https://stackoverflow.com/questions/5095183/how-would-i-run-an-async-taskt-method-synchronously#answer-5097066

TechTalk.SpecFlow/Bindings/BindingInvoker.cs

@@ -5,6 +5,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using System.Threading.Tasks;
 using TechTalk.SpecFlow.Bindings.Reflection;
 using TechTalk.SpecFlow.Compatibility;
 using TechTalk.SpecFlow.Configuration;
@@ -14,17 +15,17 @@
 
 namespace TechTalk.SpecFlow.Bindings
 {
-    using System.Threading.Tasks;
-
     public class BindingInvoker : IBindingInvoker
     {
         protected readonly Configuration.SpecFlowConfiguration specFlowConfiguration;
         protected readonly IErrorProvider errorProvider;
+        protected readonly ISynchronousBindingDelegateInvoker synchronousBindingDelegateInvoker;
 
-        public BindingInvoker(Configuration.SpecFlowConfiguration specFlowConfiguration, IErrorProvider errorProvider)
+        public BindingInvoker(Configuration.SpecFlowConfiguration specFlowConfiguration, IErrorProvider errorProvider, ISynchronousBindingDelegateInvoker synchronousBindingDelegateInvoker)
         {
             this.specFlowConfiguration = specFlowConfiguration;
             this.errorProvider = errorProvider;
+            this.synchronousBindingDelegateInvoker = synchronousBindingDelegateInvoker;
         }
 
         public virtual object InvokeBinding(IBinding binding, IContextManager contextManager, object[] arguments, ITestTracer testTracer, out TimeSpan duration)
@@ -45,13 +46,8 @@ public virtual object InvokeBinding(IBinding binding, IContextManager contextMan
                         Array.Copy(arguments, 0, invokeArgs, 1, arguments.Length);
                     invokeArgs[0] = contextManager;
 
-                    result = AsyncHelpers.RunSync(async () =>
-                    {
-                        var r = bindingAction.DynamicInvoke(invokeArgs);
-                        if (r is Task t)
-                            await t;
-                        return r;
-                    });
+                    result = synchronousBindingDelegateInvoker
+                        .InvokeDelegateSynchronously(bindingAction, invokeArgs);
 
                     stopwatch.Stop();
                 }

TechTalk.SpecFlow/Bindings/SynchronousBindingDelegateInvoker.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Threading.Tasks;
+
+namespace TechTalk.SpecFlow.Bindings
+{
+    public interface ISynchronousBindingDelegateInvoker
+    {
+        object InvokeDelegateSynchronously(Delegate bindingDelegate, object[] invokeArgs);
+    }
+
+    public class SynchronousBindingDelegateInvoker : ISynchronousBindingDelegateInvoker
+    {
+        public virtual object InvokeDelegateSynchronously(Delegate bindingDelegate, object[] invokeArgs)
+        {
+            if (typeof(Task).IsAssignableFrom(bindingDelegate.Method.ReturnType))
+                return InvokeBindingDelegateAsync(bindingDelegate, invokeArgs);
+            return InvokeBindingDelegateSync(bindingDelegate, invokeArgs);
+        }
+
+        protected virtual object InvokeBindingDelegateSync(Delegate bindingDelegate, object[] invokeArgs)
+        {
+            return bindingDelegate.DynamicInvoke(invokeArgs);
+        }
+
+        protected virtual object InvokeBindingDelegateAsync(Delegate bindingDelegate, object[] invokeArgs)
+        {
+            return AsyncHelpers.RunSync(async () =>
+            {
+                var r = bindingDelegate.DynamicInvoke(invokeArgs);
+                if (r is Task t)
+                    await t;
+                return r;
+            });
+        }
+
+    }
+}

TechTalk.SpecFlow/Infrastructure/DefaultDependencyProvider.cs

@@ -34,6 +34,7 @@ public virtual void RegisterGlobalContainerDefaults(ObjectContainer container)
             container.RegisterTypeAs<BindingFactory, IBindingFactory>();
             container.RegisterTypeAs<StepDefinitionRegexCalculator, IStepDefinitionRegexCalculator>();
             container.RegisterTypeAs<BindingInvoker, IBindingInvoker>();
+            container.RegisterTypeAs<SynchronousBindingDelegateInvoker, ISynchronousBindingDelegateInvoker>();
             container.RegisterTypeAs<TestObjectResolver, ITestObjectResolver>();
 
             container.RegisterTypeAs<StepDefinitionSkeletonProvider, IStepDefinitionSkeletonProvider>();

Tests/TechTalk.SpecFlow.RuntimeTests/Bindings/AsyncHelperTests.cs

@@ -1,21 +1,27 @@
 using System;
+using System.Threading.Tasks;
 using FluentAssertions;
+using TechTalk.SpecFlow.Bindings;
 using Xunit;
 
 namespace TechTalk.SpecFlow.RuntimeTests.Bindings
 {
     public class AsyncHelperTests
     {
-        public void NormalMethod()
+        private void NormalMethod()
         {
 
         }
 
-        public void ThrowException()
+        private void ThrowException()
         {
             throw new Exception("Hi from async");
         }
 
+        private Task AsyncMethodThrowsException()
+        {
+            throw new Exception();
+        }
 
         [Fact]
         public void RunSync_NormalMethod()
@@ -31,5 +37,18 @@ public void RunSync_ExceptionIsThrown()
 
             action.Should().Throw<Exception>().WithMessage("Hi from async");
         }
+
+        [Fact]
+        public void RunSync_WhenExceptionIsThrown_ShouldRestoreOriginalStackTrace()
+        {
+            try
+            {
+                AsyncHelpers.RunSync(AsyncMethodThrowsException);
+            }
+            catch (Exception ex)
+            {
+                ex.StackTrace.Should().StartWith($"   at {GetType().FullName}.{nameof(AsyncMethodThrowsException)}()");
+            }
+        }
     }
 }

Tests/TechTalk.SpecFlow.RuntimeTests/StepTransformationTests.cs

@@ -99,7 +99,7 @@ public void UserConverterShouldConvertStringToUser()
 
             stepTransformationBinding.Regex.IsMatch("user xyz").Should().BeTrue();
 
-            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object);
+            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object, new SynchronousBindingDelegateInvoker());
             TimeSpan duration;
             var result = invoker.InvokeBinding(stepTransformationBinding, contextManagerStub.Object, new object[] { "xyz" }, new Mock<ITestTracer>().Object, out duration);
             Assert.NotNull(result);
@@ -116,7 +116,7 @@ public void TypeToTypeConverterShouldConvertStringToStringUsingRegex()
 
             Assert.True(stepTransformationBinding.Regex.IsMatch("string xyz"));
 
-            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object);
+            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object, new SynchronousBindingDelegateInvoker());
             TimeSpan duration;
             var result = invoker.InvokeBinding(stepTransformationBinding, contextManagerStub.Object, new object[] { "xyz" }, new Mock<ITestTracer>().Object, out duration);
             Assert.NotNull(result);
@@ -131,7 +131,7 @@ public void TypeToTypeConverterShouldConvertStringToString()
             var transformMethod = stepTransformationInstance.GetType().GetMethod("StringToStringConvert");
             var stepTransformationBinding = CreateStepTransformationBinding(@"", transformMethod);
 
-            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object);
+            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object, new SynchronousBindingDelegateInvoker());
             TimeSpan duration;
             var result = invoker.InvokeBinding(stepTransformationBinding, contextManagerStub.Object, new object[] { "xyz" }, new Mock<ITestTracer>().Object, out duration);
             Assert.NotNull(result);
@@ -146,7 +146,7 @@ public void TypeToTypeConverterShouldConvertTableToTable()
             var transformMethod = stepTransformationInstance.GetType().GetMethod("TableToTableConvert");
             var stepTransformationBinding = CreateStepTransformationBinding(@"", transformMethod);
 
-            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object);
+            var invoker = new BindingInvoker(ConfigurationLoader.GetDefault(), new Mock<IErrorProvider>().Object, new SynchronousBindingDelegateInvoker());
             TimeSpan duration;
             var result = invoker.InvokeBinding(stepTransformationBinding, contextManagerStub.Object, new object[] { new Table("h1") }, new Mock<ITestTracer>().Object, out duration);
             Assert.NotNull(result);