Support other units #1
Conversation
NelsonVides
force-pushed
the
support_other_units
branch
from
600ff08
to
be2ec8d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1 +/- ##
===========================================
+ Coverage 72.72% 84.37% +11.64%
===========================================
Files 2 2
Lines 77 96 +19
===========================================
+ Hits 56 81 +25
+ Misses 21 15 -6 ☔ View full report in Codecov by Sentry. |
| NewHistory = [{ConsumeNow, DelayMs, NewShaper} | History], | ||
| NewDelay = AccumulatedDelay + DelayMs, | ||
| NewToConsume = TokensLeft - ConsumeNow, | ||
| case is_integer(DelayMs) andalso DelayMs >= 0 of |
There was a problem hiding this comment.
The type of DelayMs should be non_neg_integer(), so why would you expect anything else?
There was a problem hiding this comment.
I'm checking in tests that that is actually the case, because the type definition might end up failing, disadvantage of dynamic typing 🥲
There was a problem hiding this comment.
But it looks like we accept a bad result (not matching the type spec), because the test doesn't fail in the false clause.
There was a problem hiding this comment.
Hmm... but we stop running and will fail later. Actually, I've just noticed here I'm returning a 4-tuple, which will fail with an error. I'll return a value that will always fall outside of the conditioned range instead.
| NewHistory = [{ConsumeNow, DelayMs, NewShaper} | History], | ||
| NewDelay = AccumulatedDelay + DelayMs, | ||
| NewToConsume = TokensLeft - ConsumeNow, | ||
| case is_integer(DelayMs) andalso DelayMs >= 0 of |
There was a problem hiding this comment.
But it looks like we accept a bad result (not matching the type spec), because the test doesn't fail in the false clause.
Also solves an issue where the penalisation added to the
last_updateelement was inmillisecondunits unconverted to the correctly usednative.