stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
22.52k stars 1.56k forks source link

`assert.Subset` doesn't fail tests #1270

Closed joshprzybyszewski-wf closed 4 months ago

joshprzybyszewski-wf commented 1 year ago

Story

We were writing some unit tests, and I was surprised that they passed. Here is a re-creation:

func TestSubsetSlices(t *testing.T) {
    a := []string{`a`}
    ab := []string{`a`, `b`}

    assert.Subset(t, ab, a)
    assert.NotSubset(t, a, ab)

    // The following asserts should fail this test
    assert.Subset(t, a, ab)
    assert.NotSubset(t, ab, a)
}

func TestSubsetMapsWithValues(t *testing.T) {
    a := map[string]int{
        `a`: 1,
    }
    ab := map[string]int{
        `a`: 1,
        `b`: 2,
    }

    assert.Subset(t, ab, a)
    assert.NotSubset(t, a, ab)

    // The following asserts should fail this test
    assert.Subset(t, a, ab)
    assert.NotSubset(t, ab, a)
}

func TestSubsetMapsWithEmptyStructs(t *testing.T) {
    a := map[string]struct{}{
        `a`: {},
    }
    ab := map[string]struct{}{
        `a`: {},
        `b`: {},
    }

    assert.Subset(t, ab, a)
    assert.NotSubset(t, a, ab)

    // The following asserts should fail this test
    assert.Subset(t, a, ab)
    assert.NotSubset(t, ab, a)
}

Problem

I think the problem is here:

https://github.com/stretchr/testify/blob/181cea6eab8b2de7071383eca4be32a424db38dd/assert/assertions.go#L821-L825

https://github.com/stretchr/testify/blob/181cea6eab8b2de7071383eca4be32a424db38dd/assert/assertions.go#L882-L886

It returns a false value, but it doesn't fail the *testing.T.

glesica commented 1 year ago

Hey Josh, how's stuff?! This looks pretty straightforward, if you can put up a PR I'll merge it.

antichris commented 1 year ago

Hey, @glesica, mind giving #1290 a glance to see if it is decent enough to resolve this?

st3penta commented 4 months ago

Solved by https://github.com/stretchr/testify/pull/1261

brackendawson commented 4 months ago

Dup of #1261