tests: Add unit tests to ping service#15439
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test all |
|
Images are ready for the commit at 2e9ba9b. To use with deploy scripts, first |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15439 +/- ##
==========================================
- Coverage 49.24% 49.23% -0.01%
==========================================
Files 2578 2581 +3
Lines 189155 189298 +143
==========================================
+ Hits 93149 93206 +57
- Misses 88674 88752 +78
- Partials 7332 7340 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey @guzalv - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
rhybrillou
left a comment
There was a problem hiding this comment.
LGTM. Thank you for adding these tests.
stehessel
left a comment
There was a problem hiding this comment.
In general please include the context behind and a short summary of your changes in the PR description :).
Co-authored-by: Stephan Hesselmann <shesselm@redhat.com>
I was doing that and I deliberately removed what I was writing because it didn't seem to provide any value (it seems to me that the PR title is explanatory enough), and according to the PR template no description is needed in those cases:
On the other hand you pointed this out so I should have kept a description in this case. I will add it, let me know what you think. |
stehessel
left a comment
There was a problem hiding this comment.
I agree that in this case the description was not really needed and the change and intention were clear without it. Just to point this out as a general guideline for the future.
Got it. Thanks!! |
Description
While working with pull/15430 I noticed that the Ping service lacks unit tests. Even if it's a simple service, it can benefit from basic checks that verify that the contract is not broken.
This PR adds unit tests that check that the response is what's expected ("ok") and that the right level of access (anonymous in this case) is allowed.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
CI, manual verification.