Panic when testing a goroutine with a context parameter

[mihaitodor]

I am using the latest version of Convey (3bd662e) and this code:

package main

import (
	"errors"
	"testing"

	. "github.com/smartystreets/goconvey/convey"
)

var foobarErr = errors.New("foobar error!")

func foobar() error {
	return foobarErr
}

func TestFoobar(t *testing.T) {
	Convey("Verify foobar", t, func(c C) {

		testChan := make(chan bool)

		go func() {
			err := foobar()

			c.So(err, ShouldBeNil)

			testChan <- true
		}()

		_ = <-testChan
	})
}

is producing the following output:

➜ test go test -v . === RUN TestFoobar

Verify foobar ✘panic: FAILURE_HALT

goroutine 6 [running]: github.com/smartystreets/goconvey/convey.(*context).assertionReport(0xc42006ca20, 0xc4200ba310) /Users/mtodor/Projects/Nitro/go/src/github.com/smartystreets/goconvey/convey/context.go:270 +0xd5 github.com/smartystreets/goconvey/convey.(*context).So(0xc42006ca20, 0x1147a20, 0xc420015080, 0x117df98, 0x0, 0x0, 0x0) /Users/mtodor/Projects/Nitro/go/src/github.com/smartystreets/goconvey/convey/context.go:176 +0xc3 test.TestFoobar.func1.1(0x1218840, 0xc42006ca20, 0xc42006cae0) /Users/mtodor/Projects/Nitro/go/src/test/main_test.go:24 +0x7d created by test.TestFoobar.func1 /Users/mtodor/Projects/Nitro/go/src/test/main_test.go:27 +0x75 exit status 2 FAIL test 0.013s

Am I doing something wrong or is this a bug?

[gdamore]

Is this related to #476 ?

[mihaitodor]

I tested a few combinations of different Go versions and older versions of Convey and I can still replicate it. I think it might be some edge case that was missed by @riannucci in #264, but I haven't studied it in detail.

[riannucci]

Hm... this Should Work™️, but clearly it doesn't... I'll try to take a look at this (and add a test), but I don't have an overabundance of extra time outside $DAY_JOB. If someone else has some cycles I'd be happy to help with the code excavation :)

[mihaitodor]

@riannucci I can take a stab at fixing it and adding tests if you can provide some insight into how it should behave. Right now, it simply ends up here, but I'm not sure what it needs to do instead. I am thinking that ctx.failureMode needs to be set to some other value somewhere, but I need to read the code in more detail.

Also, it would be great if you can think of related edge cases that might need to be tested.

[mihaitodor]

@riannucci I'm still able to reproduce this issue. Would you be able to share some insights into what is going wrong? I'll polish it into a fix & PR once I understand it.

[riannucci]

Ack... I keep losing track of this and haven't had a chance to dig into it.

I just took a closer look at this, and I think what's happening is that:

• So expressions normally panic when they fail (the FailureMode). This occurs at github.com/smartystreets/goconvey/convey/context.go:270
• When a panic occurs in a goroutine, and nothing recover()'s it, go terminates the application.

If you change your test program to (Note the FailureContinues):

package main

import (
	"errors"
	"testing"

	. "github.com/smartystreets/goconvey/convey"
)

var foobarErr = errors.New("foobar error!")

func foobar() error {
	return foobarErr
}

func TestFoobar(t *testing.T) {
	Convey("Verify foobar", t, FailureContinues, func(c C) {

		testChan := make(chan bool)

		go func() {
			err := foobar()

			c.So(err, ShouldBeNil)

			testChan <- true
		}()

		_ = <-testChan
	})
}

Then it works.

More generally... I'm not sure what the desired behavior is here :). Some things I can think of:

• Have your goroutine be panic-safe (so recover and the testChan write in a defer)
• Have So return a boolean indicating if the assertion failed or not; not useful normally, but useful in FailureContinues mode.
• Maybe have some helper function that knows how to recover the Convey-generated panic (so the top of the goroutine can specify it).
• ???

[riannucci]

I guess when I added the Context support, I was assuming that the tests all pass ^_^. I'm now seeing that the ergonomics when the tests don't pass is pretty crappy.