Skip to content

Conversation

@vbreuss
Copy link
Contributor

@vbreuss vbreuss commented Oct 12, 2024

As we improved the Throw methods with #870, we should use them consistently. Therefore I would suggest to remove the Throw methods from the static Assert class, as they are redundant.

@thomhurst
Copy link
Owner

I think I'm happy to leave them to be honest. Some people will prefer the shorthand versions.

@vbreuss
Copy link
Contributor Author

vbreuss commented Oct 12, 2024

For me, it would be a benefit to have a consistent assertion API, where I can search for Assert.That and find all assertions in a code base. When there are shortcuts/alternatives for this, it gets harder to distinguish the normal pattern, especially as here the order of the exception type and the subject under test is different.
That was the main difficulty I had, when migrating a previous project from NUnit to xunit, that the order of the arguments was sometimes different.

And it is also not that much shorter 😄

await Assert.Throws<CustomException>(action);
// vs.
await Assert.That(action).Throws<CustomException>();

That's just my thoughts. In the end it is up to you, and I can close the PR...

@thomhurst
Copy link
Owner

Yeah let's close it for now. The API you desire is still there. And it might annoy some people if we remove this overload. If you don't wanna use it then you don't have to 😄

@vbreuss vbreuss closed this Oct 13, 2024
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

Successfully merging this pull request may close these issues.

2 participants