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

Throw<TException>, NotThrow, NotThrow<TException> and NotThrowAfter are throwing redundant AggregateExceptions #1037

Closed
davidomid opened this issue May 3, 2019 · 1 comment

Comments

@davidomid
Copy link
Contributor

davidomid commented May 3, 2019

Resolved in PR #1038

Description

This is a minor issue but tasks being called synchronously with .Wait() will throw AggregateExceptions. This means that certain FluentAssertions methods are throwing AggregateExceptions, when they should only be throwing the inner exception.

Example

Take the following test case:

E.g.

public void When_async_method_throws_the_expected_inner_exception_it_should_succeed()
        {
            //-----------------------------------------------------------------------------------------------------------
            // Arrange
            //-----------------------------------------------------------------------------------------------------------
            Func<Task> task = async () =>
            {
                await Task.Delay(100);
                throw new InvalidOperationException();
            };

            //-----------------------------------------------------------------------------------------------------------
            // Act
            //-----------------------------------------------------------------------------------------------------------
            Action action = () => task
                .Should().Throw<AggregateException>()
                .WithInnerException<InvalidOperationException>();

            //-----------------------------------------------------------------------------------------------------------
            // Assert
            //-----------------------------------------------------------------------------------------------------------
            action.Should().NotThrow();
        }

Expected behavior:

I would expect this test to fail, given that the task doesn't throw an AggregateException and instead explicitly just throws an InvalidOperationException.

Actual behavior:

The test passes, because currently certain methods are executing the task synchronously using .Wait(), so if it throws an exception it will wrap it in an AggregateException,

The methods which do this are:

  • public ExceptionAssertions<TException> Throw<TException>(string because = "", params object[] becauseArgs) where TException : Exception

  • public void NotThrowAfter(TimeSpan waitTime, TimeSpan pollInterval, string because = "", params object[] becauseArgs)

  • public void NotThrow(string because = "", params object[] becauseArgs)

  • public void NotThrow<TException>(string because = "", params object[] becauseArgs) where TException : Exception

Proposal

What I propose is replacing all calls to .Wait(), with .GetAwaiter().GetResult().
This should make sure only the top exception is thrown. If the given task does in fact throw an AggregateException, it will still work in the same way. The only difference is that Fluent Assertions itself does not throw AggregateExceptions internally, unnecessarily.

This would require changing 2 tests which, like the one shown above, expect an AggregateException to be thrown with an inner exception. The tests would be modified so the given tasks explicitly throws an AggregateException.

@jnyrup
Copy link
Member

jnyrup commented May 4, 2019

Closed in #1038

@jnyrup jnyrup closed this as completed May 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants