Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Must-like utility #1590

Closed
seiyab opened this issue Apr 17, 2024 · 3 comments
Closed

Proposal: Must-like utility #1590

seiyab opened this issue Apr 17, 2024 · 3 comments

Comments

@seiyab
Copy link

seiyab commented Apr 17, 2024

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
Copy link
Collaborator

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: golang/go#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
Copy link
Author

seiyab commented May 1, 2024

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
Copy link
Collaborator

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

@brackendawson brackendawson closed this as not planned Won't fix, can't repro, duplicate, stale May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants