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.

Sign up for GitHub

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

Jump to bottom

Non-generic overload for WithInnerExceptionExactly #1769

Merged
merged 9 commits into from Jan 13, 2022
Merged

Non-generic overload for WithInnerExceptionExactly #1769

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1edbd1a
Non-generic overload for WithInnerExceptionExactly
Jan 7, 2022
906fbe1
Adjusted method name and call structure
Jan 10, 2022
622f666
Added failing test for WithInnerException.
Jan 10, 2022
4555dde
Update Release Notes
Jan 10, 2022
79abee5
Remove virtual and make it work for derived exceptions
Jan 10, 2022
b00c4c2
Use extension method
Jan 11, 2022
bf8e7d1
Merge branch 'master' into issue1632
Jan 11, 2022
30ec768
Remove virtual and run AcceptApiChanges.ps1
Jan 11, 2022
0ebb0d5
Readd *verified.txt
Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions Src/FluentAssertions/ExceptionAssertionsExtensions.cs
View file
Open in desktop
Expand Up @@ -80,6 +80,28 @@ public static class ExceptionAssertionsExtensions
return (await task).WithInnerException<TInnerException>(because, becauseArgs);
}

/// <summary>
/// Asserts that the thrown exception contains an inner exception of type <param name="innerException" />.
/// </summary>
/// <typeparam name="TException">The expected type of the exception.</typeparam>
/// <param name="task">The <see cref="ExceptionAssertions{TException}"/> containing the thrown exception.</param>
/// <param name="because">
/// A formatted phrase as is supported by <see cref="string.Format(string,object[])" /> explaining why the assertion
/// is needed. If the phrase does not start with the word <i>because</i>, it is prepended automatically.
/// </param>
/// <param name="becauseArgs">
/// Zero or more objects to format using the placeholders in <paramref name="because" />.
/// </param>
public static async Task<ExceptionAssertions<Exception>> WithInnerException<TException>(
this Task<ExceptionAssertions<TException>> task,
Type innerException,
string because = "",
params object[] becauseArgs)
where TException : Exception
{
return (await task).WithInnerException(innerException, because, becauseArgs);
}

/// <summary>
/// Asserts that the thrown exception contains an inner exception of the exact type <typeparamref name="TInnerException" /> (and not a derived exception type).
/// </summary>
Expand All @@ -103,6 +125,28 @@ public static class ExceptionAssertionsExtensions
return (await task).WithInnerExceptionExactly<TInnerException>(because, becauseArgs);
}

/// <summary>
/// Asserts that the thrown exception contains an inner exception of the exact type <param name="innerException" /> (and not a derived exception type).
/// </summary>
/// <typeparam name="TException">The expected type of the exception.</typeparam>
/// <param name="task">The <see cref="ExceptionAssertions{TException}"/> containing the thrown exception.</param>
/// <param name="because">
/// A formatted phrase as is supported by <see cref="string.Format(string,object[])" /> explaining why the assertion
/// is needed. If the phrase does not start with the word <i>because</i>, it is prepended automatically.
/// </param>
/// <param name="becauseArgs">
/// Zero or more objects to format using the placeholders in <paramref name="because" />.
/// </param>
public static async Task<ExceptionAssertions<Exception>> WithInnerExceptionExactly<TException>(
this Task<ExceptionAssertions<TException>> task,
Type innerException,
string because = "",
params object[] becauseArgs)
where TException : Exception
{
return (await task).WithInnerExceptionExactly(innerException, because, becauseArgs);
}

/// <summary>
/// Asserts that the thrown exception has a parameter which name matches <paramref name="paramName" />.
/// </summary>
Expand Down
115 changes: 82 additions & 33 deletions Src/FluentAssertions/Specialized/ExceptionAssertions.cs
View file
Open in desktop
Expand Up @@ -109,28 +109,24 @@ public ExceptionAssertions(IEnumerable<TException> exceptions)
params object[] becauseArgs)
where TInnerException : Exception
{
Execute.Assertion
.BecauseOf(because, becauseArgs)
.WithExpectation("Expected inner {0}{reason}, but ", typeof(TInnerException))
.ForCondition(Subject is not null)
.FailWith("no exception was thrown.")
.Then
.ForCondition(Subject.Any(e => e.InnerException is not null))
.FailWith("the thrown exception has no inner exception.")
.Then
.ClearExpectation();

TInnerException[] expectedInnerExceptions = Subject
.Select(e => e.InnerException)
.OfType<TInnerException>()
.ToArray();

Execute.Assertion
.ForCondition(expectedInnerExceptions.Any())
.BecauseOf(because, becauseArgs)
.FailWith("Expected inner {0}{reason}, but found {1}.", typeof(TInnerException), SingleSubject.InnerException);
var expectedInnerExceptions = GetExpectedInnerExceptions(typeof(TInnerException), because, becauseArgs);
return new ExceptionAssertions<TInnerException>(expectedInnerExceptions.Cast<TInnerException>());
}

return new ExceptionAssertions<TInnerException>(expectedInnerExceptions);
/// <summary>
/// Asserts that the thrown exception contains an inner exception of type <param name="innerException" />.
/// </summary>
/// <param name="because">
/// A formatted phrase as is supported by <see cref="string.Format(string,object[])" /> explaining why the assertion
/// is needed. If the phrase does not start with the word <i>because</i>, it is prepended automatically.
/// </param>
/// <param name="becauseArgs">
/// Zero or more objects to format using the placeholders in <paramref name="because" />.
/// </param>
public virtual ExceptionAssertions<Exception> WithInnerException(Type innerException, string because = null,
jnyrup marked this conversation as resolved.
Show resolved Hide resolved
params object[] becauseArgs)
{
return new ExceptionAssertions<Exception>(GetExpectedInnerExceptions(innerException, because, becauseArgs));
}

/// <summary>
Expand All @@ -148,19 +144,24 @@ public ExceptionAssertions(IEnumerable<TException> exceptions)
params object[] becauseArgs)
where TInnerException : Exception
{
WithInnerException<TInnerException>(because, becauseArgs);

TInnerException[] expectedExceptions = Subject
.Select(e => e.InnerException)
.OfType<TInnerException>()
.Where(e => e?.GetType() == typeof(TInnerException)).ToArray();

Execute.Assertion
.ForCondition(expectedExceptions.Any())
.BecauseOf(because, becauseArgs)
.FailWith("Expected inner {0}{reason}, but found {1}.", typeof(TInnerException), SingleSubject.InnerException);
var exceptionExpression = GetInnerExceptionExactly(typeof(TInnerException), because, becauseArgs);
return new ExceptionAssertions<TInnerException>(exceptionExpression.Cast<TInnerException>());
}

return new ExceptionAssertions<TInnerException>(expectedExceptions);
/// <summary>
/// Asserts that the thrown exception contains an inner exception of the exact type <param name="innerException" /> (and not a derived exception type).
/// </summary>
/// <param name="because">
/// A formatted phrase as is supported by <see cref="string.Format(string,object[])" /> explaining why the assertion
/// is needed. If the phrase does not start with the word <i>because</i>, it is prepended automatically.
/// </param>
/// <param name="becauseArgs">
/// Zero or more objects to format using the placeholders in <paramref name="because" />.
/// </param>
public virtual ExceptionAssertions<Exception> WithInnerExceptionExactly(Type innerException, string because = null,
Copy link
Member

Choose a reason for hiding this comment

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

Remove virtual here as well.
As a rule-of-thumb, when we review PRs we only point out a single occurrence of something that should be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you for the Patience. It is my first time contributing to anything.

params object[] becauseArgs)
{
return new ExceptionAssertions<Exception>(GetInnerExceptionExactly(innerException, because, becauseArgs));
}

/// <summary>
Expand Down Expand Up @@ -191,6 +192,54 @@ public ExceptionAssertions(IEnumerable<TException> exceptions)
return this;
}

private IEnumerable<Exception> GetExpectedInnerExceptions(Type innerException, string because = null,
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I think we should rename this method to emphasize that it's primary function is to assert something. Same for the other Get method.

params object[] becauseArgs)
{
Guard.ThrowIfArgumentIsNull(innerException, nameof(innerException));

Execute.Assertion
.BecauseOf(because, becauseArgs)
.WithExpectation("Expected inner {0}{reason}, but ", innerException)
.ForCondition(Subject is not null)
.FailWith("no exception was thrown.")
.Then
.ForCondition(Subject.Any(e => e.InnerException is not null))
.FailWith("the thrown exception has no inner exception.")
.Then
.ClearExpectation();

Exception[] expectedInnerExceptions = Subject
.Select(e => e.InnerException)
.Where(e => e?.GetType() == innerException)
Copy link
Member

Choose a reason for hiding this comment

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

❌ This checks that the runtime type of e is exactly innerException.
This breaks checking derived types.

We need a test for this scenario.
You could copy When_subject_throws_an_exception_with_the_expected_inner_exception_it_should_not_do_anything and change the inner exception to a derived one, e.g. ArgumentNullException.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I missed that and thought this code merely moved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is one of the few changes, so that it would work for both call types. Unfortunately I did not fully grasp the full extent of the OfType<> method. Will fix it!

.ToArray();

Execute.Assertion
.ForCondition(expectedInnerExceptions.Any())
.BecauseOf(because, becauseArgs)
.FailWith("Expected inner {0}{reason}, but found {1}.", innerException, SingleSubject.InnerException);

return expectedInnerExceptions;
}

private IEnumerable<Exception> GetInnerExceptionExactly(Type innerException, string because = null,
params object[] becauseArgs)
{
Guard.ThrowIfArgumentIsNull(innerException, nameof(innerException));

WithInnerException(innerException, because, becauseArgs);
Copy link
Member

Choose a reason for hiding this comment

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

🔧 Instead of creating a seemingly recursive call (which it didn't after digging through the code), I would just call the GetExpectedInnerExceptions method (and reverse their order in the file for readability).


Exception[] expectedExceptions = Subject
.Select(e => e.InnerException)
.Where(e => e?.GetType() == innerException).ToArray();

Execute.Assertion
.ForCondition(expectedExceptions.Any())
.BecauseOf(because, becauseArgs)
.FailWith("Expected inner {0}{reason}, but found {1}.", innerException, SingleSubject.InnerException);

return expectedExceptions;
}

private TException SingleSubject
{
get
Expand Down
46 changes: 46 additions & 0 deletions Tests/FluentAssertions.Specs/Exceptions/AsyncFunctionExceptionAssertionSpecs.cs
View file
Open in desktop
Expand Up @@ -832,6 +832,21 @@ public async Task When_async_method_throws_the_expected_inner_exception_it_shoul
await action.Should().NotThrowAsync();
}

[Fact]
public async Task When_async_method_throws_the_expected_inner_exception_from_argument_it_should_succeed()
{
// Arrange
Func<Task> task = () => Throw.Async(new AggregateException(new InvalidOperationException()));

// Act
Func<Task> action = () => task
.Should().ThrowAsync<AggregateException>()
.WithInnerException(typeof(InvalidOperationException));

// Assert
await action.Should().NotThrowAsync();
jnyrup marked this conversation as resolved.
Show resolved Hide resolved
}

[Fact]
public async Task When_async_method_throws_the_expected_inner_exception_exactly_it_should_succeed()
{
Expand All @@ -847,6 +862,21 @@ public async Task When_async_method_throws_the_expected_inner_exception_exactly_
await action.Should().NotThrowAsync();
}

[Fact]
public async Task When_async_method_throws_the_expected_inner_exception_exactly_defined_in_arguments_it_should_succeed()
{
// Arrange
Func<Task> task = () => Throw.Async(new AggregateException(new ArgumentException()));

// Act
Func<Task> action = () => task
.Should().ThrowAsync<AggregateException>()
.WithInnerExceptionExactly(typeof(ArgumentException));

// Assert
await action.Should().NotThrowAsync();
}

[Fact]
public async Task When_async_method_throws_aggregate_exception_containing_expected_exception_it_should_succeed()
{
Expand Down Expand Up @@ -905,6 +935,22 @@ public async Task When_async_method_does_not_throw_the_expected_inner_exception_
await action.Should().ThrowAsync<XunitException>().WithMessage("*ArgumentException*ArgumentNullException*");
}

[Fact]
public async Task
When_async_method_does_not_throw_the_expected_inner_exception_exactly_defined_in_arguments_it_should_fail()
{
// Arrange
Func<Task> task = () => Throw.Async(new AggregateException(new ArgumentNullException()));

// Act
Func<Task> action = () => task
.Should().ThrowAsync<AggregateException>()
.WithInnerExceptionExactly(typeof(ArgumentException));

// Assert
await action.Should().ThrowAsync<XunitException>().WithMessage("*ArgumentException*ArgumentNullException*");
}

[Fact]
public async Task When_async_method_does_not_throw_the_expected_exception_it_should_fail()
{
Expand Down
90 changes: 90 additions & 0 deletions Tests/FluentAssertions.Specs/Exceptions/ExceptionAssertionSpecs.cs
View file
Open in desktop
Expand Up @@ -409,6 +409,22 @@ public void When_asserting_with_an_aggregate_exception_type_the_asserts_should_o
.WithMessage("Inner Message");
}

[Fact]
public void When_asserting_with_an_aggregate_exception_and_inner_exception_type_from_argument_the_asserts_should_occur_against_the_aggregate_exception()
{
// Arrange
Does testSubject = Does.Throw(new AggregateException("Outer Message", new Exception("Inner Message")));

// Act
Action act = testSubject.Do;

// Assert
act.Should().Throw<AggregateException>()
.WithMessage("Outer Message*")
.WithInnerException(typeof(Exception))
.WithMessage("Inner Message");
}

#endregion

#region Inner Exceptions
Expand All @@ -426,6 +442,19 @@ public void When_subject_throws_an_exception_with_the_expected_inner_exception_i
.WithInnerException<ArgumentException>();
}

[Fact]
public void When_subject_throws_an_exception_with_the_expected_inner_exception_from_argument_it_should_not_do_anything()
{
// Arrange
Does testSubject = Does.Throw(new Exception("", new ArgumentException()));

// Act / Assert
testSubject
.Invoking(x => x.Do())
.Should().Throw<Exception>()
.WithInnerException(typeof(ArgumentException));
}

[Fact]
public void WithInnerExceptionExactly_no_parameters_when_subject_throws_subclass_of_expected_inner_exception_it_should_throw_with_clear_description()
{
Expand Down Expand Up @@ -487,6 +516,53 @@ public void WithInnerExceptionExactly_when_subject_throws_subclass_of_expected_i
}
}

[Fact]
public void WithInnerExceptionExactly_with_type_exception_when_subject_throws_expected_inner_exception_it_should_not_do_anything()
{
// Arrange
Action act = () => throw new BadImageFormatException("", new ArgumentNullException());

// Act / Assert
act.Should().Throw<BadImageFormatException>()
.WithInnerExceptionExactly(typeof(ArgumentNullException), "because {0} should do just that", "the action");
}

[Fact]
public void WithInnerExceptionExactly_with_type_exception_no_parameters_when_subject_throws_expected_inner_exception_it_should_not_do_anything()
{
// Arrange
Action act = () => throw new BadImageFormatException("", new ArgumentNullException());

// Act / Assert
act.Should().Throw<BadImageFormatException>()
.WithInnerExceptionExactly(typeof(ArgumentNullException));
}

[Fact]
public void WithInnerExceptionExactly_with_type_exception_when_subject_throws_subclass_of_expected_inner_exception_it_should_throw_with_clear_description()
{
// Arrange
var innerException = new ArgumentNullException();

Action act = () => throw new BadImageFormatException("", innerException);

try
{
// Act
act.Should().Throw<BadImageFormatException>()
.WithInnerExceptionExactly(typeof(ArgumentException), "because {0} should do just that", "the action");

throw new XunitException("This point should not be reached.");
}
catch (XunitException ex)
{
// Assert
var expectedMessage = BuildExpectedMessageForWithInnerExceptionExactly("Expected inner System.ArgumentException because the action should do just that, but found System.ArgumentNullException with message", innerException.Message);

ex.Message.Should().Be(expectedMessage);
}
}

[Fact]
public void WithInnerExceptionExactly_when_subject_throws_expected_inner_exception_it_should_not_do_anything()
{
Expand Down Expand Up @@ -585,6 +661,20 @@ public void When_an_inner_exception_matches_exactly_it_should_allow_chaining_mor
.Where(i => i.Message == "InnerMessage");
}

[Fact]
public void When_an_inner_exception_matches_exactly_it_should_allow_chaining_more_asserts_on_that_exception_type_from_argument()
{
// Act
Action act = () =>
throw new ArgumentException("OuterMessage", new InvalidOperationException("InnerMessage"));

// Assert
act
.Should().ThrowExactly<ArgumentException>()
.WithInnerExceptionExactly(typeof(InvalidOperationException))
.Where(i => i.Message == "InnerMessage");
}

[Fact]
public void When_injecting_a_null_predicate_it_should_throw()
{
Expand Down