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.

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

The test StartAsTaskCancellation contains a Debugger.Break(), can it be removed safely? #3605

Closed
abelbraaksma opened this issue Sep 19, 2017 · 4 comments · Fixed by #3627
Closed

Comments

@abelbraaksma
Copy link
Contributor

My test runner hiccups on Debugger.Break() in tests (not surprisingly, a normal executable would do too). I was wondering if this break is intentional and serves a purpose, or was left their as a remnant of some temporary debugging situation.

Also, the |> ignore is redundant.

The test:

    [<Test>]
    member this.StartAsTaskCancellation () =
        let cts = new CancellationTokenSource()
        let tcs = TaskCompletionSource<unit>()
        let a = async {
            cts.CancelAfter (100)
            do! tcs.Task |> Async.AwaitTask }
#if FSCORE_PORTABLE_NEW || coreclr
        let t : Task<unit> =
#else
        use t : Task<unit> =
#endif
            Async.StartAsTask(a, cancellationToken = cts.Token)

        // Should not finish
        try
            let result = t.Wait(300)
            Assert.IsFalse (result)
        with :? AggregateException -> Assert.Fail "Task should not finish, jet"

        tcs.SetCanceled()
        
        try
            this.WaitASec t
        with :? AggregateException as a ->
            match a.InnerException with
            | :? TaskCanceledException as t -> ()
            | _ -> reraise()
        System.Diagnostics.Debugger.Break() |> ignore
        Assert.IsTrue (t.IsCompleted, "Task is not completed")

I can remove this line if nobody objects.

@abelbraaksma abelbraaksma changed the title The test StartAsTaskCancellation contains a Debugger.Break(), should this be there or can it be removed? The test StartAsTaskCancellation contains a Debugger.Break(), can it be removed safely? Sep 19, 2017
@dsyme
Copy link
Contributor

dsyme commented Sep 19, 2017

Yes, please do

@dsyme
Copy link
Contributor

dsyme commented Sep 20, 2017

I'll close since the answer to the question in the issue is simply "yes" :) thanks!

@abelbraaksma
Copy link
Contributor Author

abelbraaksma commented Sep 22, 2017

@dsyme, you closed it already, but it wasn't fixed yet. It is now (or at least if it gets accepted) ;)., see PR #3627.

@dsyme
Copy link
Contributor

dsyme commented Sep 25, 2017

@abelbraaksma OK, thanks

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 a pull request may close this issue.

2 participants