-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add a featuregatetest package and module to share featuregate testing support #12377
base: main
Are you sure you want to change the base?
Conversation
0e048eb
to
d880b42
Compare
… support Signed-off-by: Bogdan Drutu <[email protected]>
d880b42
to
e83c262
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12377 +/- ##
==========================================
- Coverage 91.49% 91.49% -0.01%
==========================================
Files 466 465 -1
Lines 25612 25610 -2
==========================================
- Hits 23434 23432 -2
Misses 1774 1774
Partials 404 404 ☔ View full report in Codecov by Sentry. |
func setFeatureGateForTest(tb testing.TB, gate *featuregate.Gate, enabled bool) { | ||
// SetGate sets the value to the given gate. Also, it installs a cleanup function to restore | ||
// the gate to the initial value when the test is done. | ||
func SetGate(tb testing.TB, gate *featuregate.Gate, enabled bool) { |
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.
Why do this instead of using a local registry for testing? I am a bit worried about encouraging this pattern, it modifies global state and it can be hard to reason about
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.
Any example where I can see what you envision?
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 was thinking about some old PR but the idea is not directly applicable here (we no longer have IsEnabled
).
I guess the equivalent here for the batch sender tests modified on this PR would be to pass a boolean flag directly to newQueueBatchExporter
and set that flag based on the feature gate on the code
No description provided.