Skip to content

Improve StartWithContinuations test in AsyncType tests - #17767

Merged
T-Gro merged 7 commits into
dotnet:mainfrom
majocha:asynctype-1
Sep 30, 2024
Merged

Improve StartWithContinuations test in AsyncType tests#17767
T-Gro merged 7 commits into
dotnet:mainfrom
majocha:asynctype-1

Conversation

@majocha

@majochamajocha commented Sep 20, 2024

Copy link
Copy Markdown
Contributor

Use a local cancellation token instead of Async.CancelDefaultToken() to cancel just one computation.

When running tests in parallel I've seen OperationCancelledException raised in unrelated tests of the same assembly.
It seems despite the fact this test collection has DisableParallelization = true, xUnit sometimes can't perfectly isolate the test cases.

ETA: It was me misconfiguring xUnit exclusions.
Still, I think this looks a bit cleaner.

@majocha
majocha requested a review from a team as a code ownerSeptember 20, 2024 06:59
@github-actions

Copy link
Copy Markdown
Contributor

✅ No release notes required

@majocha

majocha commented Sep 20, 2024

Copy link
Copy Markdown
ContributorAuthor

New version goes like this:

moduleAsyncType =typeExpectedContinuation= Success | Exception | Cancellation
[<Fact>]letstartWithContinuations()=letcont actual expected _ =if expected <> actual then
failwith $"expected {expected} continuation, but ran {actual}"letonSuccess= cont Success
letonExeption= cont Exception
letonCancellation= cont Cancellation
letexpect expected computation =
Async.StartWithContinuations(computation, onSuccess expected, onExeption expected, onCancellation expected)async{
Async.CancelDefaultToken()return()}|> expect Cancellation
async{ failwith "computation failed"}|> expect Exception
async{return()}|> expect Success

@psfinakipsfinaki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay I think I've verified the equivalence, thanks for the refactoring, it definitely looks clearer now.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants

@majocha@psfinaki@T-Gro