-
-
Notifications
You must be signed in to change notification settings - Fork 920
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
test(finance): fix tests for amount #2702
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2702 +/- ##
==========================================
+ Coverage 99.56% 99.57% +0.01%
==========================================
Files 2859 2859
Lines 248915 248878 -37
Branches 986 630 -356
==========================================
- Hits 247837 247827 -10
+ Misses 1078 1022 -56
- Partials 0 29 +29 |
@xDivisionByZerox I would like if you could take a look on this and approve accordingly |
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.
@xDivisionByZerox I would like if you could take a look on this and approve accordingly
Because I'm not fully sure if 0 was an intended result or not
Since the implementation did not change, this looks valid to me.
Additionally, I don't see why we should disallow 0 as return value. WHat would happen if the user provides 0 as min AND max value to the function?
This PR fixes a rare bug in the finance tests, that cause them to fail when amount returns
0
.Previously the tests checked for
0 < amount < 1001
Now the check looks like this:
0 <= amount <= 1000
I also removed the dedicated
it returns a string
test in favor of more checks in the other tests.And I removed some description texts, because they state the exact same as the check itself.