-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix: TOOLS-2971 fix in existing failing test cases #56
Conversation
a-spiker
commented
Sep 11, 2024
•
edited
Loading
edited
- Fixing the existing test cases to ensure any further changes in the repo are properly tested.
- Removing the suppress check on the go test command
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #56 +/- ##
==========================================
- Coverage 36.34% 36.27% -0.08%
==========================================
Files 23 23
Lines 5610 5610
==========================================
- Hits 2039 2035 -4
- Misses 3435 3436 +1
- Partials 136 139 +3 ☔ View full report in Codecov by Sentry. |
@a-spiker For our understanding could you lay out why these changes were made? What issues were they causing?
|
There is an existing test case which was failing, which expects dis to be of lib.Stats. Ideally all the sub configs should be instance of lib.Stats for consistent parsing.. It was changed here https://github.com/aerospike/aerospike-management-lib/pull/40/files#diff-3e8a272f186a1070104d40c5f775480b3f913489b147098e3662f7eda1cf8f1cR1149 . if the dc values is empty it was still going to have Len > 0 for the split.
As per the review, I have changed the expected values rather than test generated values. |
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