-
-
Notifications
You must be signed in to change notification settings - Fork 282
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
better alternative to BeTrue/False #670
Comments
I'm wondering if we could simply upgrade in an upwards source-compatible fashion to |
@thediveo: I prefer separate functions because it enables compile-time checking and simplifies linting. |
heyo - so finally getting back to this. i agree with @thediveo in that i’d also want to lean towards
WDYT? A few questions for y’all’s thoughts: It could be Negation is tricky here. Obviously we’d prefer the user type
it’s an edge case and its ugly and confusing but short of asking ChatGPT to “negate ” i think it’s the best we can do. Last thought,
I’m kind of leaning towards that, actually. |
@nunnatsa: if we go the route of extending @onsi: accepting format string and args for it is an invitation to write broken code where the format string doesn't match the parameters. Regarding negation: negation with |
sounds good. i’m planning to add the |
@pohly - sound not to hard to implement. My concern is that this is not backward compatible. So yes, the new rule will be off by default, but it's a bit risky. |
How about keeping This is not a pattern used in Gomega, but not unusual elsewhere. The advantage is that the implementation can be marked as a printf-style function using this pattern:
Those variants then would have the behavior outlined by @onsi. I see some advantages with that approach:
|
I'm working on this right now and I'm planning on going with: // BeTrue succeeds if actual is true
//
// If passed an optional reason, the reason will be displayed instead of the default failure message.
func BeTrue(optionalReason ...string) types.GomegaMatcher {
return &matchers.BeTrueMatcher{OptionalReason: optionalReason}
}
// BeFalse succeeds if actual is false
//
// If passed an optional reason, the reason will be displayed instead of the default failure message.
func BeFalse(optionalReason ...string) types.GomegaMatcher {
return &matchers.BeFalseMatcher{OptionalReason: optionalReason}
}
// BeTrueBecause succeeds if actual is true and displays the provided reason if it is false
// If additional arguments are provided fmt.Sprintf is used to render the reason
func BeTrueBecause(format string, args ...any) types.GomegaMatcher {
reason := format
if len(args) > 0 {
reason = fmt.Sprintf(format, args...)
}
return BeTrue(reason)
}
// BeFalseBecause succeeds if actual is false and displays the provided reason if it is true.
// If additional arguments are provided fmt.Sprintf is used to render the reason
func BeFalseBecause(format string, args ...any) types.GomegaMatcher {
reason := format
if len(args) > 0 {
reason = fmt.Sprintf(format, args...)
}
return BeFalse(reason)
} |
Actually as I'm writing the tests I'm learnign that it wil be this: // BeTrueBecause succeeds if actual is true and displays the provided reason if it is false
// fmt.Sprintf is used to render the reason
func BeTrueBecause(format string, args ...any) types.GomegaMatcher {
return BeTrue(fmt.Sprintf(format, args...))
}
// BeFalseBecause succeeds if actual is false and displays the provided reason if it is true.
// fmt.Sprintf is used to render the reason
func BeFalseBecause(format string, args ...any) types.GomegaMatcher {
return BeFalse(fmt.Sprintf(format, args...))
} As the compiler will actually apply checks to the format string (I tried to do |
lol, and now i'm coming back full circle to just keeping |
Alright, the dust has settled. I have a commit up on master now that:
Docs were updated too of course. PTAL and i'll cut a release if folks are happy with this. |
I was about to comment that The commit in master looks good to me. |
great! i’ll cut a release soon |
1.30.0 is out now! |
When using
BeTrue
orBeFalse
, one can annotate theShould
orShouldNot
, but then still gets the rather uselessExpected <bool>: false to be true
. That's noise that makes the failure message harder to read.New matchers could take a message string and then emit that message string as the failure message, without the
Expected: <bool>: false to be true
.The message string could support templating, for those cases where it's unclear whether the matcher is used with
Should
orShouldNot
.Tentative names:
EqualTrue
andEqualFalse
.For details, see https://gophers.slack.com/archives/CQQ50BBNW/p1685525653532099
The text was updated successfully, but these errors were encountered: