-
Notifications
You must be signed in to change notification settings - Fork 48
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] Unit tests for Extension reconciler #690
✨ [Add] Unit tests for Extension reconciler #690
Conversation
Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #690 +/- ##
===========================================
+ Coverage 64.01% 75.76% +11.74%
===========================================
Files 22 22
Lines 1370 1345 -25
===========================================
+ Hits 877 1019 +142
+ Misses 442 265 -177
- Partials 51 61 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
It seems like there may be an opportunity to collapse some of the test code because it looks like a decent chunk is repetitive between different tests.
You might consider a table-driven set of tests, where a test case could contain arbitrary setup and assertion functions that could be implemented per test case.
@@ -92,6 +86,931 @@ func TestExtensionReconcile(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestExtensionNonExistentPackage(t *testing.T) { | |||
defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.EnableExtensionAPI, true)() |
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.
These new tests looked like they had similar enough structure to potentially be done in this kind of format. Seems to me like that might be more maintainable?
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.
Oops, just realized this is a duplicate comment of Joe's here.
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
closing due to inactivity - please re-open if we still need this =D |
Description
This PR:
HasKappAPI
test, since the feature is anyway protected using featureGate. This also helps in running tests easily.Reviewer Checklist