haf / expecto

A smooth testing lib for F#. APIs made for humans! Strong testing methodologies for everyone!
Apache License 2.0
663 stars 96 forks source link

Tests are still started after cancellation #332

Open otto-gebb opened 5 years ago

otto-gebb commented 5 years ago

I have a bunch of test suites created using testSequencedGroup. I run them using runTestsInAssemblyWithCancel and expect that after the cancellation token is cancelled all the pending groups don't even start. But it is not what happens, all tests in the project are attempted. afterRunTests is executed immediately after I press Ctrl-C.

Here is an excerpt from my main function:

  [<EntryPoint>]
  let main argv =
    // ...
    use cts = new Threading.CancellationTokenSource()
    Console.CancelKeyPress.Add <| fun arg ->
      cts.Cancel()
      arg.Cancel <- true
    Tests.afterRunTests BrowserPool.cleanup
    Tests.runTestsInAssemblyWithCancel cts.Token testConfig argv
jzabroski commented 5 years ago

Handling control c requires different logic

jzabroski commented 5 years ago

@haf My bad. I did not see the Console.CancelKeyPress.Add in the code. That's what I get for waking up and reading code before coffee.

AnthonyLloyd commented 4 years ago

Sorry, I didn't see this one. I think arg.Cancel may not be doing what you think it does. It seems to keep the process running per the docs.

jzabroski commented 4 years ago

@AnthonyLloyd Source? I actually thought he coded it correctly, based on sample code I had discussed with an engineer at Microsoft, Andrew Arnott.

After digging a bit... it seems Andrew's solution is not portable. In particular, if you're running inside docker, it won't work (my brain is a bit stumped as to why at the moment - I am sure why will occur to me after I click "Comment"):

https://gist.github.com/kuznero/73acdadd8328383ea7d5

open System
open System.Threading
open System.Threading.Tasks

let rec loop () =
  Console.WriteLine (DateTime.Now.ToString())
  Thread.Sleep (1000)
  loop ()

[<EntryPoint>]
let main argv =
  Task.Factory.StartNew(fun () -> loop ()) |> ignore
  let closing = new AutoResetEvent(false)
  let onExit = new ConsoleCancelEventHandler(fun _ args -> Console.WriteLine("Exit"); closing.Set() |> ignore)
  Console.CancelKeyPress.AddHandler onExit
  closing.WaitOne() |> ignore
  0

There seems to be other solutions on StackOverflow that are nastier: They hook into kernel32 dll

AnthonyLloyd commented 4 years ago

https://docs.microsoft.com/en-us/dotnet/api/system.console.cancelkeypress?view=netframework-4.8

We have 2 cancel handlers in the codebase. I'm not sure how they will run together.

jzabroski commented 4 years ago

Got it. So the issue is composing two together, plus apparently doing Control+C in docker doens't work well on top of all this.

For what it's worth, that example is rather confusing / lacks explanation as to what they're trying to do: They print a message telling the user to press Control+C, but then the same demo code doesn't actually let you terminate on Control+C. That probably explains why StackOverflow has so many additional answers regarding how to do this.