franela / goblin

Minimal and Beautiful Go testing framework
MIT License
884 stars 79 forks source link

Fix assertion IsNil() failing when passing a nil slice #94

Closed Neirpyc closed 2 years ago

Neirpyc commented 3 years ago

Previously, such code would fail:

package main

import (
    . "github.com/franela/goblin"
    "testing"
)

func Test(t *testing.T){
    g := Goblin(t)
    g.Describe("Assert(nilSlice).IsNil()", func(){
        g.It("Should pass assertion", func(){
            var slice []interface{} = nil
            g.Assert(slice).IsNil() //fails
        })
    })
}

This is because a nil slice is not a nil pointer. Therefore, to know if an interface{} is a nil slice, we can check wether it is a slice or not with reflect.TypeOf(a.src).Kind() == reflect.Slice. In case it is a slice, we can use reflect.ValueOf(a.src).IsNil() to know if a slice is nil or not.

This PR also adds two test functions TestIsNilWithSlice and TestIsNotNilWithSlice.

Neirpyc commented 3 years ago

Okay, hum I made ugly commits, tried to make it better, made it worse xD Gotta learn a bit more how git works x)

Neirpyc commented 2 years ago

Could we get any feedback on this? I believe this is a bug fix as the current behavior seems unconsistent, and that's a less than 20 lines fix...

marcosnils commented 2 years ago

thx for the contribution @Neirpyc!

I've checked how testify does this and it seems like we could be comprehensive and also add other types like chan and map(ref https://github.com/stretchr/testify/blob/master/assert/assertions.go#L525-L536)

WDYT?

Neirpyc commented 2 years ago

Yes you're right, I didn't think about that.

This should be fixed now :)