Proposal: changes for next major release (v4)

[dnephin]

With the opportunity to use generics to improve the assert package (#255), we may want to release a new major version. This issue is to discuss and track changes that would require a major version bump, so that we can include all of them when the time is right.

1. Change the signature of the assert, and assert/cmp functions to use generics instead of any as the argument types (#255)

2. Combine the skip, env, and poll packages into a single new package. These packages are very small, and combining them into a single package should make it easier to use gotest.tools by requiring fewer imports. I'm not sure what to name that package yet. They could be included as part of the assert package, but the names don't read as well.

3. Replace the golden package with a GoldenFile(t, filename) function in the assert package. #237 added support for golden-like behaviour using expected values stored in a Go variable. We should be able to do something similar:

   func GoldenFile(t TestingT, filename string) string { ... }
   

   Similar to golden variables, when called with -update, assert.Assert should be able to detect that the expected argument is a GoldenFile and use the actual value to update the specified filename.

4. Maybe icmd could change to use context.Context to set the timeout instead of a time.Duration passed to WaitOnCommand ? Some way to pass a deadline/timeout using context.Context might be good.

Are there other breaking changes we should consider? Anything in fs that could change now that the stdlib has an fs package or because of generics?

[fgimian]

Just my 2 cents below 😄

1. Change the signature of the `assert`, and `assert/cmp` functions to use generics instead of `any` as the argument types ([assert: use type constraints (generics) #255](https://github.com/gotestyourself/gotest.tools/pull/255))

👍

2. Combine the `skip`, `env`, and `poll`  packages into a single new package. These packages are very small, and combining them into a single package should make it easier to use `gotest.tools` by requiring fewer imports. I'm not sure what to name that package yet.  They could be included as part of the `assert` package, but the names don't read as well.

I would suggest not including them in the assert package, one thing I like about this library is how simple and focused the assert package is. It's easy to see everything you can assert at a glance. Adding other utilities in that package will probably ruin that a bit.

Perhaps the package could just be called test? Not sure about a better name TBH😄

3. Replace the `golden` package with a `GoldenFile(t, filename)` function in the `assert` package. [assert: golden variables #237](https://github.com/gotestyourself/gotest.tools/pull/237) added support for golden-like behaviour using expected values stored in a Go variable.

👍

4. Maybe `icmd` could change to use `context.Context` to set the timeout instead of a `time.Duration` passed to `WaitOnCommand` ? Some way to pass a deadline/timeout using `context.Context` might be good.

👍

Are there other breaking changes we should consider? Anything in fs that could change now that the stdlib has an fs package or because of generics?

It may be nice to have colored output as an option, similar to got. This looks really nice when running with gotestsum.

e.g.

Similarly with the struct diffs too 😄

I do also wonder if the assert package should offer a few more asserts, but I realise that's a slippery slope.

One more little thing I'd change (I found this confusing), is not to alias cmp to is anywhere in the code or documentation. I think this just causes more confusion as there is another test framework called is.

[dnephin]

Thank you for the feedback! I like the idea of using color to improve the visibility of the failure messages. With type constraints I think we'll be able to remove the type from the failure message text (Ex: int, vs int64), which should also help make the failure messages more concise.

Removing the aliasing of the cmp package is also a good idea. I'll open a separate issue for that. I think we can make that change without it being a breaking change. #260

I do also wonder if the assert package should offer a few more asserts

I've been trying to reduce the number of assert functions as much as possible. I think the ones we have now cover at least 95% of assertions. For the remaining few percent I think documenting patterns for using assert.Assert will be better than adding more assert functions.

[dolmen]

See also #266 (define a separate Go module for assert/cmd/gty-migrate-from-testify) (however I think that this doesn't need to wait for v4)

[milas]

4. Maybe icmd could change to use context.Context to set the timeout instead of a time.Duration passed to WaitOnCommand ? Some way to pass a deadline/timeout using context.Context might be good.

Having the ability to pass in your own context would also be helpful (a la https://pkg.go.dev/os/exec#CommandContext).

As a use case, it's handy to do the following:

ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

...

That way, any icmd instances running when the test ends would be killed.