-
Notifications
You must be signed in to change notification settings - Fork 64
🌱 [MONOREPO]: Moving catalogd's metrics test to operator-controller tests #1714
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
🌱 [MONOREPO]: Moving catalogd's metrics test to operator-controller tests #1714
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1714 +/- ##
==========================================
- Coverage 67.66% 67.64% -0.03%
==========================================
Files 59 59
Lines 4991 4991
==========================================
- Hits 3377 3376 -1
- Misses 1367 1368 +1
Partials 247 247
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
93e8a7d
to
a4a456a
Compare
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.
/lgtm
It's just a move of the function.
// 4. Waits for the curl pod to become ready. | ||
// 5. Executes a curl command from the pod to validate the metrics endpoint. | ||
// 6. Cleans up all resources created during the test, such as the ClusterRoleBinding and curl pod. | ||
func TestCatalogdMetricsExportedEndpoint(t *testing.T) { |
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.
With this all put together, is there any common functionality we can extract into helper functions?
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.
We definitely can improve that.
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 will do that in a follow-up so it makes it easier to review.
591c73c
Motivation:
Small step related to #1712