Consolidate intercept (#5033)

* Removed a bunch of obsolete and unused methods

* Consolidate Intercept methods, and add the new AssertThrows

* Fixed a few tests that where dealing with the AggregateExceptions themselves

Author: ismaelhamed

src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs

@@ -423,11 +423,7 @@ public void ProtocolStateActor_must_give_up_outbound_after_connection_timeout()
 
             Watch(stateActor);
 
-            // inner exception will be a TimeoutException
-            Intercept<AggregateException>(() =>
-            {
-                statusPromise.Task.Wait(TimeSpan.FromSeconds(5)).Should().BeTrue();
-            });
+            Intercept<TimeoutException>(() => statusPromise.Task.Wait(TimeSpan.FromSeconds(5)).Should().BeTrue());
             ExpectTerminated(stateActor);
         }
 

src/core/Akka.Streams.Tests/Dsl/FlowAskSpec.cs

@@ -266,13 +266,11 @@ public void Flow_with_ask_signal_failure_when_target_actor_is_terminated() => th
                 .Ask<Reply>(r, _timeout, 4)
                 .RunWith(Sink.Ignore<Reply>(), _materializer);
 
-            Intercept<AggregateException>(() =>
+            Intercept<WatchedActorTerminatedException>(() =>
             {
                 r.Tell(PoisonPill.Instance);
                 done.Wait(RemainingOrDefault);
-            })
-            .Flatten()
-            .InnerException.Should().BeOfType<WatchedActorTerminatedException>();
+            });
 
         }, _materializer);
 

src/core/Akka.Tests.Shared.Internals/AkkaSpec.cs

@@ -113,7 +113,7 @@ private static string GetCallerName()
         }
 
         public static Config AkkaSpecConfig { get { return _akkaSpecConfig; } }
-        
+
         protected T ExpectMsgPf<T>(TimeSpan? timeout, string hint, Func<object, T> function)
         {
             MessageEnvelope envelope;
@@ -136,30 +136,88 @@ protected T ExpectMsgPf<T>(string hint, Func<object, T> pf)
             return pf.Invoke(t);
         }
 
-
+        /// <summary>
+        /// Intercept and return an exception that's expected to be thrown by the passed function value. The thrown
+        /// exception must be an instance of the type specified by the type parameter of this method. This method
+        /// invokes the passed function. If the function throws an exception that's an instance of the specified type,
+        /// this method returns that exception. Else, whether the passed function returns normally or completes abruptly
+        /// with a different exception, this method throws <see cref="ThrowsException"/>.
+        /// <para>
+        /// Also note that the difference between this method and <seealso cref="AssertThrows{T}"/> is that this method
+        /// returns the expected exception, so it lets you perform further assertions on that exception. By contrast,
+        /// the <seealso cref="AssertThrows{T}"/> indicates to the reader of the code that nothing further is expected
+        /// about the thrown exception other than its type. The recommended usage is to use <seealso cref="AssertThrows{T}"/>
+        /// by default, intercept only when you need to inspect the caught exception further.
+        /// </para>
+        /// </summary>
+        /// <param name="actionThatThrows">The action that should throw the expected exception</param>
+        /// <returns>The intercepted exception, if it is of the expected type</returns>
+        /// <exception cref="ThrowsException">If the passed action does not complete abruptly with an exception that's an instance of the specified type.</exception>
         protected T Intercept<T>(Action actionThatThrows) where T : Exception
         {
-            return Assert.Throws<T>(() => actionThatThrows());
-        }
+            try
+            {
+                actionThatThrows();
+            }
+            catch (Exception ex)
+            {
+                var exception = ex is AggregateException aggregateException
+                    ? aggregateException.Flatten().InnerExceptions[0]
+                    : ex;
+
+                var exceptionType = typeof(T);
+                return exceptionType == exception.GetType()
+                    ? (T)exception
+                    : throw new ThrowsException(exceptionType, exception);
+            }
 
-        protected void Intercept(Action actionThatThrows)
+            throw new ThrowsException(typeof(T));
+        }
+
+        /// <summary>
+        /// Ensure that an expected exception is thrown by the passed function value. The thrown exception must be an
+        /// instance of the type specified by the type parameter of this method. This method invokes the passed
+        /// function. If the function throws an exception that's an instance of the specified type, this method returns
+        /// void. Else, whether the passed function returns normally or completes abruptly with a different
+        /// exception, this method throws <see cref="ThrowsException"/>.
+        /// <para>
+        /// Also note that the difference between this method and <seealso cref="Intercept{T}"/> is that this method
+        /// does not return the expected exception, so it does not let you perform further assertions on that exception.
+        /// It also indicates to the reader of the code that nothing further is expected about the thrown exception
+        /// other than its type. The recommended usage is to use <see cref="AssertThrows{T}"/> by default,
+        /// <seealso cref="Intercept{T}"/> only when you need to inspect the caught exception further.
+        /// </para>
+        /// </summary>
+        /// <param name="actionThatThrows">The action that should throw the expected exception</param>
+        /// <exception cref="ThrowsException">If the passed action does not complete abruptly with an exception that's an instance of the specified type.</exception>
+        protected void AssertThrows<T>(Action actionThatThrows) where T : Exception
         {
             try
             {
                 actionThatThrows();
             }
-            catch(Exception)
+            catch (Exception ex)
             {
-                return;
+                var exception = ex is AggregateException aggregateException
+                    ? aggregateException.Flatten().InnerExceptions[0]
+                    : ex;
+
+                var exceptionType = typeof(T);
+                if (exceptionType == exception.GetType())
+                    return;
+
+                throw new ThrowsException(exceptionType, exception);
             }
-            throw new ThrowsException(typeof(Exception));
+
+            throw new ThrowsException(typeof(T));
         }
-        
-        protected async Task InterceptAsync(Func<Task> asyncActionThatThrows)
+
+        [Obsolete("Use AssertThrows instead.")]
+        protected void Intercept(Action actionThatThrows)
         {
             try
             {
-                await asyncActionThatThrows();
+                actionThatThrows();
             }
             catch(Exception)
             {
@@ -168,24 +226,6 @@ protected async Task InterceptAsync(Func<Task> asyncActionThatThrows)
             throw new ThrowsException(typeof(Exception));
         }
 
-        [Obsolete("Use Intercept instead. This member will be removed.")]
-        protected void intercept<T>(Action actionThatThrows) where T : Exception
-        {
-            Assert.Throws<T>(() => actionThatThrows());
-        }
-
-        [Obsolete("Use ExpectMsgPf instead. This member will be removed.")]
-        protected T expectMsgPF<T>(string hint, Func<object, T> pf)
-        {
-            return ExpectMsgPf<T>(hint, pf);
-        }
-
-        [Obsolete("Use ExpectMsgPf instead. This member will be removed.")]
-        protected T expectMsgPF<T>(TimeSpan duration, string hint, Func<object, T> pf)
-        {
-            return ExpectMsgPf<T>(duration, hint, pf);
-        }
-
         protected void MuteDeadLetters(params Type[] messageClasses)
         {
             if (!Sys.Log.IsDebugEnabled)

src/core/Akka.Tests/Actor/CoordinatedShutdownSpec.cs

@@ -337,18 +337,7 @@ public void CoordinatedShutdown_must_abort_if_recover_is_off()
 
             var result = co.Run(CoordinatedShutdown.UnknownReason.Instance);
             ExpectMsg("B");
-            Intercept<AggregateException>(() =>
-            {
-                if (result.Wait(RemainingOrDefault))
-                {
-                    result.Exception?.Flatten().InnerException.Should().BeOfType<TimeoutException>();
-                }
-                else
-                {
-                    throw new Exception("CoordinatedShutdown task did not complete");
-                }
-            });
-
+            Intercept<TimeoutException>(() => result.Wait(RemainingOrDefault));
             ExpectNoMsg(TimeSpan.FromMilliseconds(200)); // C not run
         }