stretchr / testify

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

Proposal: Must-like utility #1590

Closed seiyab closed 6 months ago

seiyab commented 7 months ago

Description

I want something like Must (examples: html, regexp) for test. For functions that returns value and error, it's common to make test fail if error exists. So I wonder if I can write such code shorter.

// standard
f, err := os.Open(path)
if err != nil {
  t.Fatal(err)
}

// with testify stable
f, err := os.Open(path)
require.NoError(t, err)

// proposal (this is only for illustration. probably this can't compile)
f := require.Must(t, os.Open(path))

I'm sorry but I'm not sure that here is proper project to request it. Anyway I suggest my idea.

Proposed solution

AFAIK, it's not straightforward to receive a tuple of (value, error) and testing.T simultaneously. Following approach should work. But it looks weird a bit.

func [T any]Must(value T, err error) func(*testing.T) T {
  return func(t *testing.T) T {
    if err != nil {
      t.Fatal(err)
    }
    return value
  }
}

// usage
f := require.Must(os.Open(path))(t)

Use case

See example in description.

brackendawson commented 6 months ago

So this is like the well known generic Must(), but rather than converting non-nil errors to a panic, it converts non-nil errors to a fatal test failure.

Would assert.Must exist? Would it be a non-fatal test failure? If it did then it would feel very wrong calling it "must". Probably it would only apply to require.

You only show usage in test setup, which is not the primary purpose of test assertions. How does this look in a test assertion?

actual := require.Must(t, codeundertest.GetData(arg))
require.Equal(t, expected, actual)

Sure, maybe, but do we also need Must2, Must3 etc.. Because type parameters cannot be variadic. It looks like Go is not going to add Must to the standard library: https://github.com/golang/go/issues/32219

In my view this turns 3 lines into 2 for test assertions, 2 lines into 1 for test setup. At best it doesn't make the test more readable. I don't think we should include this.

seiyab commented 6 months ago

Rethinking it, I agree that it doesn't look to suit this project enough.

You only show usage in test setup, which is not the primary purpose of test assertions. How does this look in a test assertion?

Your example looks appropriate. More clear one:

// setup
x := Setup()

// act
x.MethodUnderTest()

// assertion
actual := require.Must(t, x.Get())
require.Equal(t, expected, actual)

Anyway probably it will mostly used for setup.

In my view this turns 3 lines into 2 for test assertions, 2 lines into 1 for test setup. At best it doesn't make the test more readable. I don't think we should include this.

Now I agree with you. I'd thought that it it turns 10 lines into 5 lines for test setup and it makes sense. However, probably we should just write a setup function if setup needs 10 lines.

brackendawson commented 6 months ago

Parking for now. If anyone feels strongly in favour of the feature then feel free to post your reasoning.