franela/goblin

basic error handle for incorrect use of the library

rkmax opened this issue · 2 comments

rkmax commented

Today I shame myself by not writing properly testing. when the test succeeds everything is good, but when fails a nasty SIGSEGV: segmentation violation appear and there's no clue why this is happening

These are the two examples I just did the first one, later just checking if happen also with the second

  1. Describe without It
func Test(t *testing.T) {
    g := goblin.Goblin(t)

    g.Describe("GetUploadS3Params", func() {        
        g.Assert("X").Equal("Y")
    })
}

the panic raises when goblin.go#L344 is executed, currentIt is nil

  1. It without Describe
func Test(t *testing.T) {
    g := goblin.Goblin(t)

    g.It("GetUploadS3Params", func() {        
        g.Assert("X").Equal("Y")
    })
}

the panic raises when goblin.go#L279 is execute, parent is nil

I think would be very nice include some internal error handling when either currentIt or parent is nil

@rkmax thx for the issue and the PR. It's something that we definitely need to fix.

Regarding suggestions I've just tried both scenarios in mocha (were we inspired to create goblin) and none of them produce a process.exit output.

  1. Just ignores the assertion and treats the Describe block an empty container. It doesn't print anything to the test output at all

I believe this behavior might be desirable over panicking the test run.

  1. Mocha allows defining it blocks without any parent describe structures. I'll run the test normally without any context.

This one is a bit trickier as we haven't taken this into account when creating goblin. How about if we just print a warning message instead of making the test suite to panic?.

rkmax commented

Oh, that for sure is more desirable behavior. I'll try to replicate it with my current limited experience with golang and goblin source code