Skip to content

Should().Throw and Should().NotThrow throw for Func<T>#951

Merged
dennisdoomen merged 14 commits intofluentassertions:masterfrom
krajek:should_throw_for_func_t
Nov 8, 2018
Merged

Should().Throw and Should().NotThrow throw for Func<T>#951
dennisdoomen merged 14 commits intofluentassertions:masterfrom
krajek:should_throw_for_func_t

Conversation

@krajek
Copy link
Copy Markdown
Contributor

@krajek krajek commented Oct 19, 2018

Closes #422.

This is still work in progress but if you would like to do an early review I would appreciate.
Implemented according to @jnyrup idea of returning AndWhichConstraint from NotThrow which allows continued assertions like this:

//-----------------------------------------------------------------------------------------------------------
// Arrange
//-----------------------------------------------------------------------------------------------------------
Func<int> f = () => 12;
 //-----------------------------------------------------------------------------------------------------------
// Act / Assert
//-----------------------------------------------------------------------------------------------------------
f.Should().NotThrow().Which.Should().Be(12);

All the commented code and tests are part of the WIP.

jnyrup
jnyrup reviewed Oct 21, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

To align with Action assertions we should add
public AndWhichConstraint<FunctionAssertions<T>, T> NotThrow<TException>
public ExceptionAssertions<TException> ThrowExactly<TException>

@krajek krajek changed the title [WIP] Should().Throw and Should_().NotThrow throw for Func<T> [WIP] Should().Throw and Should().NotThrow throw for Func<T> Oct 28, 2018
@krajek krajek changed the title [WIP] Should().Throw and Should().NotThrow throw for Func<T> Should().Throw and Should().NotThrow throw for Func<T> Oct 29, 2018
@krajek
Copy link
Copy Markdown
Contributor Author

krajek commented Oct 29, 2018

Ready for the merge with squash.

To align with Action assertions we should add
public AndWhichConstraint<FunctionAssertions, T> NotThrow

NotThrow<TException> does return void, because it does not guarantee that no exception was thrown and that assertion can be continued.

jnyrup
jnyrup approved these changes Oct 29, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

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

Looks good 👍

I didn't thought about the difficulty of the return type for NotThrow<TException>().
For now I agree with returning void.

At some later point I would like if we could come up with a nice way to represent the three possible outcomes:

  • didn't throw any exception => success and pass on result for chaining further assertions.
  • threw an exception but it was not the unexpected => success but no resulting value.
  • threw an exception and it was the unexpected one => fail.

@krajek krajek changed the title Should().Throw and Should().NotThrow throw for Func<T> [WIP] Should().Throw and Should().NotThrow throw for Func<T> Oct 29, 2018
@krajek
Copy link
Copy Markdown
Contributor Author

krajek commented Nov 2, 2018

@jnyrup tests you requested are added, it was a good idea.

Additionally, FunctionAssertions now inherits from ReferenceTypeAssertions. It was not extremely important to do it, but at least it is consistent with ActionAssertions among others.

Ready for a merge with squash.

@krajek krajek changed the title [WIP] Should().Throw and Should().NotThrow throw for Func<T> Should().Throw and Should().NotThrow throw for Func<T> Nov 2, 2018
@jnyrup jnyrup requested a review from dennisdoomen November 5, 2018 22:27
@dennisdoomen dennisdoomen self-assigned this Nov 7, 2018
@dennisdoomen dennisdoomen merged commit a1673b0 into fluentassertions:master Nov 8, 2018
@krajek krajek deleted the should_throw_for_func_t branch November 12, 2018 06:40
@dennisdoomen dennisdoomen mentioned this pull request Nov 15, 2018
Closed
@jnyrup jnyrup mentioned this pull request Jan 8, 2019
Closed
@adamgauthier adamgauthier mentioned this pull request Dec 3, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dennisdoomen dennisdoomen dennisdoomen approved these changes

@jnyrup jnyrup jnyrup approved these changes

Assignees

@dennisdoomen dennisdoomen

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@krajek @dennisdoomen @jnyrup