fix(ofrep): fix flaky rate limit test with date-based Retry-After#118
fix(ofrep): fix flaky rate limit test with date-based Retry-After#118josecolella merged 1 commit intomainfrom
Conversation
The test used `Time.now + 1` for the Retry-After header, which could expire before the assertion ran on slow CI runners (Ruby 3.4). Changed to `Time.now + 60` to avoid timing sensitivity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical CI flakiness issue affecting the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
🤖 I have created a release *beep* *boop* --- ## [0.1.2](openfeature-ofrep-provider/v0.1.1...openfeature-ofrep-provider/v0.1.2) (2026-03-18) ### 🐛 Bug Fixes * **ofrep:** fix flaky rate limit test with date-based Retry-After ([#118](#118)) ([0ecdf00](0ecdf00)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request fixes a flaky test by increasing the Retry-After duration. While this is a pragmatic solution that will likely resolve the immediate issue, I've added a comment with a suggestion for a more robust fix. By controlling time in the tests, you can eliminate the root cause of the flakiness and improve the overall stability of your test suite.
| it "blocks subsequent calls when rate limited with Retry-After header (date)" do | ||
| stub_request(:post, "http://localhost:8080/ofrep/v1/evaluate/flags/my_flag") | ||
| .to_return(status: 429, headers: {"Retry-After" => (Time.now + 1).httpdate}) | ||
| .to_return(status: 429, headers: {"Retry-After" => (Time.now + 60).httpdate}) |
There was a problem hiding this comment.
While increasing the delay to 60 seconds makes this test less likely to fail, it doesn't fully eliminate the root cause of flakiness, which is the use of Time.now in tests. This makes the test's outcome dependent on execution speed.
A more robust approach is to control time by stubbing Time.now. This makes your time-dependent tests deterministic, faster (by avoiding sleep in other tests), and more reliable.
For example, this test could be rewritten as:
it 'blocks subsequent calls when rate limited with Retry-After header (date)' do
start_time = Time.new(2023, 1, 1, 12, 0, 0)
allow(Time).to receive(:now).and_return(start_time)
stub_request(:post, 'http://localhost:8080/ofrep/v1/evaluate/flags/my_flag')
.to_return(status: 429, headers: { 'Retry-After' => (start_time + 2).httpdate })
expect {
client.evaluate(flag_key: 'my_flag', evaluation_context: default_evaluation_context)
}.to raise_error(OpenFeature::OFREP::RateLimited)
# Advance time, but stay within the retry window
allow(Time).to receive(:now).and_return(start_time + 1)
expect {
client.evaluate(flag_key: 'other_flag', evaluation_context: default_evaluation_context)
}.to raise_error(OpenFeature::OFREP::RateLimited)
endApplying this pattern across your time-sensitive tests would be a significant improvement to the test suite's stability and prevent future flakiness.
Summary
test_ofrep_provider (3.4)CI failure that was blocking PR chore(main): release openfeature-flagsmith-provider 0.1.3 #111 (and other release PRs)Time.now + 1for theRetry-Afterheader, which could expire before the assertion ran on slow CI runnersTime.now + 60to eliminate timing sensitivityTest plan
bundle exec rspec spec/openfeature/ofrep/provider/client_spec.rb:174passes locally🤖 Jose's AI agent