Skip to content

ThatAsync and MultipleAsync #4322

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

Merged
merged 1 commit into from
Apr 6, 2023
Merged

ThatAsync and MultipleAsync #4322

merged 1 commit into from
Apr 6, 2023

Conversation

uecasm
Copy link
Contributor

@uecasm uecasm commented Mar 31, 2023

Because #2843 and #3432 made me sad.

This introduces Assert.ThatAsync (which accepts Is or Throws constraints as usual) and Assert.MultipleAsync (which can contain both ThatAsync and synchronous assertions), and does all the right things with them (at least as far as the test method boundary), provided that you await them inside an async Task test.

Not included, but I was pondering whether it might be a good idea: constraint Is.Canceled and/or Throws.Canceled which is equivalent to Throws.InstanceOf<OperationCanceledException>().

@uecasm
Copy link
Contributor Author

uecasm commented Mar 31, 2023

@dotnet-policy-service agree

@OsirisTerje OsirisTerje requested review from stevenaw and jnm2 March 31, 2023 10:05
@OsirisTerje
Copy link
Member

@stevenaw I have read through the earlier discussions on this issue, and the implementation here looks very straightforward. I can't see any side-effects nor breaking changes here. Do you agree?

@jnm2 You had a long thread on this topic many years ago. Do you agree this PR looks ok?

Copy link
Member

@OsirisTerje OsirisTerje left a comment

Choose a reason for hiding this comment

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

LGTM
And this is additions, nothing existing is changed, afaic see.

@OsirisTerje OsirisTerje merged commit efb6392 into nunit:master Apr 6, 2023
@uecasm uecasm deleted the that-async branch May 11, 2023 04:14
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