After is not called if test is aborted by SIGINT

[benmoss]

I know Ginkgo does this, and I'm sure it comes with some complexity, but is this a known/intentional feature that spec doesn't run any After code? Can you think of how I might be able to setup a signal handler in my own tests to call the After?

[sclevine]

You could catch SIGINT (and perhaps SIGTERM) and then panic. The After blocks are deferred and would get called before the tests terminate. I'd be open to adding a helper to spec that does this.

[sclevine]

On second thought, the panic would need to be in the same goroutine as the deferred After blocks, which would make the suggestion above difficult.

Parallelism makes this tough to solve cleanly, but maybe we could introduce an option that stops testing on SIGINT/SIGTERM and executes any deferred After blocks?

[sclevine]

It looks like Go recently introduced a *T.Cleanup function that is similar to After. There's an upstream issue discussing the interactions between Cleanup, SIGINT, and parallelism: golang/go#41891 [edit: linked fixed, thanks for providing the correct link below!]

As a path forward, we could introduce an option that just stops new subtests from running on SIGINT/SIGTERM.

[benmoss]

golang/go#41891

[sclevine]

Here's what I'm thinking now:

1. Replace spec.S.After's defer-based implementation with *testing.T.Cleanup.
2. On first SIGINT/SIGTERM, *T.Skip all subtests that haven't run yet.
3. Introduce an spec.S.Context() that is canceled on the first SIGINT/SIGTERM.
4. If three SIGINT/SIGTERM are observed within any five second period, os.Exit(1)

To allow users to enable this, we could add:

1. A spec.Interrupt() function, returning a spec.Option, that provides a context that's cancelled on SIGINT/SIGTERM
2. A spec.Context(ctx) function, returning a spec.Option, that allows you to provide a custom context

This would also allow you to specify a test timeout via ctx.Deadline.

[sclevine]

@benmoss would you mind testing out this branch? https://github.com/sclevine/spec/tree/spec-1.5-rc

Just pass spec.Interrupt() as an option to spec.Run() to enable the new behavior.
If you have a long running individual spec, make sure it exits when it.Context() is canceled.

[benmoss]

Seems good to me, still doesn't fix my exact problem of the test cleanup, but I guess that's just the upstream issue

[sclevine]

If you only hit Ctrl-C once, all of the After blocks should still execute. It may take a while for the tests to stop running, but you can use it.Context() to wind them down more quickly.

[benmoss]

ctrl-c once doesn't seem to do anything: https://asciinema.org/a/EAbKe6nHvexc4F2rquiD3HFLg

[sclevine]

Ctrl-C once should:

1. Stop new tests from running
2. Cancel the context returned by it.Context()

If you have individual long-running specs, you could end them early when it.Context() is canceled.

Unfortunately, I think this is the best we can do while using Go's testing package for parallelism.

[benmoss]

Got it, that seems to do the trick:

func TestHelloWorld(t *testing.T) {
  spec.Run(t, "object", func(t *testing.T, when spec.G, it spec.S) {

    it.After(func() {
      fmt.Println("after")
    })

    it("should have some default", func() {
      fmt.Println("1")
      <-it.Context().Done()
      fmt.Println("2")
    })

    it("runs other tests", func() {
      fmt.Println("3")
    })

  }, spec.Interrupt())
}

$ go test -v ./foo/
=== RUN TestHelloWorld
=== RUN TestHelloWorld/object
=== RUN TestHelloWorld/object/should_have_some_default
1
^C2
after
=== RUN TestHelloWorld/object/runs_other_tests
--- PASS: TestHelloWorld (2.06s)
--- PASS: TestHelloWorld/object (2.06s)
--- PASS: TestHelloWorld/object/should_have_some_default (2.06s)
--- SKIP: TestHelloWorld/object/runs_other_tests (0.00s)
PASS
ok github.com/sclevine/spec/foo 2.062s