test: fix race condition in delayed failure event test#47
Conversation
The test_provider_emits_error_event_delayed_failure test was checking the error count immediately after set_provider without waiting for the delayed event timer (0.1s) to fire. This caused a consistent test failure. Use a threading.Event with a timeout, matching the pattern used by test_provider_emits_stale_event and test_provider_emits_configuration_event. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Apply the same event-based wait fix to test_provider_emits_ready_event_when_immediately_ready and test_provider_emits_error_event_immediately_failed. The OpenFeature SDK now dispatches provider events asynchronously, so checking counts immediately after set_provider is unreliable. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
|
|
||
| with lock: | ||
| assert ld_provider_ready_count == 1 | ||
| assert thread_event.wait(timeout=5) |
There was a problem hiding this comment.
Is there some way we could make this more robust against duplicate emissions?
There was a problem hiding this comment.
Good call. Updated all three tests to track an emission_count alongside the threading.Event. After the event fires, a short time.sleep(0.1) allows any spurious duplicate emissions to arrive, then we assert the count is exactly 1.
There was a problem hiding this comment.
aside: Typically we would avoid sleeps, but I think in this arrangement they are fine. We aren't using the sleep to try to catch the event we care about, but just as some extra time to check we aren't getting extra events. Which there isn't an amazing way to test for, but I have seen it be a problem with open feature, and hence the original test shape.
Each event test now tracks emission_count alongside the threading.Event. After the event fires, a short sleep allows any duplicate emissions to arrive before asserting the count is exactly 1. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
2 participants
Requirements
Related issues
None.
Describe the solution you've provided
Three event emission tests (
test_provider_emits_ready_event_when_immediately_ready,test_provider_emits_error_event_immediately_failed,test_provider_emits_error_event_delayed_failure) check a counter immediately afterset_providerwithout waiting for the event to be dispatched. The OpenFeature SDK now dispatches provider events asynchronously, so these assertions fail.The fix uses
threading.Eventwithwait(timeout=5), matching the pattern already used bytest_provider_emits_stale_eventandtest_provider_emits_configuration_event.Describe alternatives you've considered
Could add
time.sleep()calls, but event-based waiting is more robust and consistent with existing tests.Additional context
All 66 tests pass consistently. Lint (mypy) also passes.
Link to Devin session: https://app.devin.ai/sessions/385a2c260fe8433ea7e6af74fcde903c
Requested by: @kinyoklion
Note
Low Risk
Test-only changes with no runtime or library behavior modified.
Overview
Fixes flaky provider lifecycle tests that assumed OpenFeature dispatches
PROVIDER_READY/PROVIDER_ERRORsynchronously onset_provider.test_provider_emits_ready_event_when_immediately_ready,test_provider_emits_error_event_immediately_failed, andtest_provider_emits_error_event_delayed_failurenow wait on athreading.Event(withwait(timeout=5)) set from the handler before asserting the emission count, plus a shorttime.sleep(0.1)so the counter update is visible. Handlers were simplified (sharedemission_count,thread_event.set()). This matchestest_provider_emits_stale_eventandtest_provider_emits_configuration_event.No production/provider code changes—tests only.
Reviewed by Cursor Bugbot for commit 0340a25. Bugbot is set up for automated code reviews on this repo. Configure here.