logtest: add WithCmpOpts#6774
Conversation
| // WithCmpOpts adds additional [cmp.Option] used when comparing values. | ||
| func WithCmpOpts(opts ...cmp.Option) AssertOption { | ||
| return fnOption(func(cfg assertConfig) assertConfig { | ||
| cfg.cmpOpts = append(cfg.cmpOpts, opts...) | ||
| return cfg | ||
| }) | ||
| } |
There was a problem hiding this comment.
Initially I though, about adding something like
// Transform applies a transformation f function that
// converts values of a certain type into that of another.
// f must not mutate A in any way.
func Transform[A, B any](f func(A) B) AssertOption {
return fnOption(func(cfg assertConfig) assertConfig {
cfg.cmpOpts = append(cfg.cmpOpts, cmp.Transformer("", f))
return cfg
})
}Usage:
logtest.AssertEqual(t, want, got,
// Ignore Timestamps.
logtest.Transform(func(time.Time) time.Time {
return time.Time{}
}),
)But after some thoughts I think that allowing to use any cmp.Option gives more flexibility.
There is a cost - tight coupling to cmp. However, I do not think we would want to to use anything different for comparing. This is also a "helper" module for tests and we do not plan making a release anytime soon (at least before cmp has a v1.
I am open to suggestions and different opinions.
There was a problem hiding this comment.
I'm in favor of not tightly coupling to cmp. If a user wants to use that package for testing won't they just use it instead of this package?
I like the transform function provided above.
There was a problem hiding this comment.
@dmathieu, do you have any preference? Are you ok with Transform proposal?
pellared
requested review from
MrAlias,
XSAM,
dashpole and
dmathieu
as code owners
|
@khushijain21, @scorpionknifes, @dmathieu, @MrAlias, PTAL |
Fixes #6491 Fixes #6489 Towards #6341 Per #6774 (comment)
3 participants
Fixes #6491
Fixes #6489
Towards #6341