Fix flaky rate limiter test

[LawnGnome]

thread 'krate::publish::publish_new_crate_rate_limited' panicked at 'assertion failed: `(left == right)`
  left: `429`,
 right: `200`', src/tests/krate/publish.rs:1024:10

it looks like one of the tests is a little flaky :-/

Originally posted by @Turbo87 in #6875 (comment)

[nic-hartley]

Running this test alone, it seems to always work. (Hard to verify a negative, but just anecdotally it hasn't failed on me yet.) I feel pretty confident diagnosing the issue as, predictably, the hardcoded time limit sometimes naturally expiring. This is supported by seeing things like service=737ms in the logs of failing runs but service=9ms when it succeeds. I know, I'm just as shocked as you are that the root cause of the flaky ratelimit test appears to be a timing issue. Who could have predicted this.

My pitch for fixing this is to split this test into two separate ones to cover the same functionality:

• Insert a row into publish_rate_overrides for a ratelimit that expires_at sometime far into the future (like, now + 10 years). Then hit the endpoint and assert it 429s with the correct try again time.
• Insert a row into publish_rate_overrides that "just expired" (e.g. now - 1 second). Then hit the endpoint and assert it 200s.

The upside is this is basically impossible to fail spuriously, since chrono::NaiveDateTime doesn't account for DST/leap seconds/etc. The downside is the test stops being a straightforward API hit and starts reaching into the guts of the implementation instead. Right now it's actually getting ratelimited, and then waiting for that ratelimit to actually expire. This change would make assumptions about functionality. That said, the odds of subtle changes that don't break the test but do break the behavior seems... unlikely?

If that sounds good to y'all I can put in a PR.

[LawnGnome]

That approach sounds reasonable to me — I'd gotten about as far as your analysis (sorry, I obviously forgot to write it up), but hadn't really figured out how to deal with it yet. I think those two tests would, as you said, basically exercise the actual behaviour we want to test in a way that should prevent regressions.

I'd be happy to review a PR from you!

[nic-hartley]

Hm, I'm realizing now I misread the code the first time, I thought it referenced the same table twice but it doesn't, whoops. Correct me if I'm wrong but it looks like the code is:

• Checking publish_rate_overrides to see if the that uploader has a special ratelimit for that action (which I could turn down to "you are limited to zero rate", it looks like)
• Updating publish_limit_buckets with and returning the new token count; if you have >1 then you've passed the ratelimit.

So I'm gonna actually add three tests; two setting publish_limit_buckets appropriately to simulate "ratelimit just expired" and "ratelimit expires in fifty kajillion years", plus one more testing that publish_rate_overrides allows you to publish a crate even when you'd otherwise be ratelimited, since I don't see any tests for that functionality.

[LawnGnome]

Sorry, you are quite correct.

That sounds great, thanks!