Redundant AggregateException removal#1038
Conversation
|
I agree that it is better to I'm a bit suspicious/worried that we need to alter a test, that means the behavior has changed.
|
|
I agree with your concern. In general, I think we should unwrap an |
|
I've added another passing unit test to make sure we can check for an exception type when an AggregateException contains an instance of it: [Fact]
public void When_async_method_throws_aggregate_exception_containing_expected_exception_it_should_succeed()
{
//-----------------------------------------------------------------------------------------------------------
// Arrange
//-----------------------------------------------------------------------------------------------------------
Func<Task> task = async () =>
{
await Task.Delay(100);
throw new AggregateException(new InvalidOperationException());
};
//-----------------------------------------------------------------------------------------------------------
// Act
//-----------------------------------------------------------------------------------------------------------
Action action = () => task
.Should().Throw<InvalidOperationException>();
//-----------------------------------------------------------------------------------------------------------
// Assert
//-----------------------------------------------------------------------------------------------------------
action.Should().NotThrow();
}The latest build fails due to one of those flaky tests again. Aside from adding this test case, are there any other changes I should make at this stage? Thanks very much |
jnyrup
left a comment
There was a problem hiding this comment.
Aha, I wasn't aware of the AggregateException unwrapping feature of FA.
Then it looks good to me 👍
It also inspired me to find some cases, where we wasn't consistent for NotThrow<TException> (PR coming up af this is merged)
Resolves issue #1037