search

boinkor-net / governor

A rate-limiting library for Rust (f.k.a. ratelimit_meter)
https://github.com/boinkor-net/governor
MIT License
550 stars 43 forks source link

Fix Reversed Logic for QuantaUpkeepInstant.now() #223

Closed waynerobinson closed 6 months ago

waynerobinson commented 6 months ago

Logic for comparing self.reference and self.clock.recent() was reversed resulting in now() always returning Nanos(0).

antifuchs commented 6 months ago

Whooooops! Good catch, thanks!

antifuchs commented 6 months ago

OK, this is truly weird: running the tests in "coverage" mode, it sounds like the clock isn't advancing! https://github.com/boinkor-net/governor/actions/runs/7919482513/job/21626813726?pr=223#step:5:246

waynerobinson commented 6 months ago

I just decreased the frequencies to see if its an issue with the background task not running fast enough. I did have the interval set to 10µs, which is relatively fast.

codecov[bot] commented 6 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f0cb09c) 98.25% compared to head (b60cc4a) 98.18%. Report is 2 commits behind head on master.

Files Patch % Lines
governor/src/clock/quanta.rs 91.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #223 +/- ## ========================================== - Coverage 98.25% 98.18% -0.08% ========================================== Files 31 31 Lines 2182 2148 -34 ========================================== - Hits 2144 2109 -35 - Misses 38 39 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

antifuchs commented 6 months ago

OK, that looks better, now there's just one tiny amount of formatting to fix up ((:

waynerobinson commented 6 months ago

I really need to add auto-formatting on save to my IDE! 😅

waynerobinson commented 6 months ago

I don't understand that codecov failure… it's pointing at the actual error message in the test!

antifuchs commented 6 months ago

Oh yeah, that's a problem in rcov; it counts line-broken expressions as separate lines, and since this is a macro in which the formatter arguments aren't evaluated unless the tests fail, they count as "uncovered". It's silly & unless we redo the macro as a single line, it's unavoidable. The change merges regardless! (: