-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: Add structure to automatically reset between test runs #249
Conversation
// Historically, tests in this repository have suffered from onion-like | ||
// nesting. The further you have to dig, the more you cry. | ||
// | ||
// In the time honored tradition of every other language on the planet, we are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was @cwaldren-ld 's prompting. Way to be the change you want to see. 😄
h.RequireEventually(t, | ||
checkForUpdatedValue(t, client, "flag-key", context, | ||
ldvalue.String("fallthrough"), ldvalue.String("default"), ldvalue.String("default")), | ||
time.Second, time.Millisecond*20, "flag value was NOT updated after cache TTL") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting mixed messages between the assertion message and the test description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked.
DSN: s.persistentStore.DSN(), | ||
}) | ||
persistence.SetCache(servicedef.SDKConfigPersistentCache{ | ||
Mode: servicedef.CacheModeInfinite, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Infinite mode isn't a uniform capability. I guess we can suppress the test though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I could make it a capability, but it does kind of feel like not supporting infinite is more of a suppression thing. If you can support the other two, seems like you should be able to support this.
No description provided.