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

Consolidate intercept #5033

Merged
merged 3 commits into from
May 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
6 changes: 2 additions & 4 deletions src/core/Akka.Streams.Tests/Dsl/FlowAskSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
98 changes: 69 additions & 29 deletions src/core/Akka.Tests.Shared.Internals/AkkaSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping this one since there are a bunch of tests using it, as a catch-all-exceptions, which is dubious at best.

{
try
{
await asyncActionThatThrows();
actionThatThrows();
}
catch(Exception)
{
Expand All @@ -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)
Expand Down
13 changes: 1 addition & 12 deletions src/core/Akka.Tests/Actor/CoordinatedShutdownSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down