fix: Update OpenFeature SDK dependency lower bound from 1.13.0 to 1.16.0#41
fix: Update OpenFeature SDK dependency lower bound from 1.13.0 to 1.16.0#41kinyoklion wants to merge 5 commits intomainfrom
Conversation
Fixes #40 The dependency range [1.13.0,2.0.0) has two issues: 1. Compilation failure: SDK versions [1.13.0,1.14.1) lack TrackingEventDetails.getValue() (no-arg), causing Provider.java to fail to compile. 2. Binary incompatibility: SDK versions below 1.16.0 define EventProvider.emit*() methods with void return type, but 1.16.0+ changed them to return Awaitable. Code compiled against <1.16.0 throws NoSuchMethodError at runtime with SDK 1.16.0+. Raising the lower bound to 1.16.0 fixes both issues. Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.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:
|
- Add @AfterEach to call OpenFeatureAPI.getInstance().shutdown(), ensuring event handlers are cleared between tests. This prevents handlers from one test leaking into another via the singleton. - Remove incorrect assertion in canShutdownAnOfflineClient: the offline LDClient does not transition its data source status to OFF after close(). The test now verifies shutdown completes without error. - Consolidate shutdown() calls: individual tests no longer need to call OpenFeatureAPI.getInstance().shutdown() since @AfterEach handles cleanup consistently. Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
Add Awaitility.await() guard before asserting on getObjectValue result, matching the pattern used by the other evaluation tests (boolean, string, double). Without this, setProvider (non-blocking) may not have completed by the time getObjectValue is called, returning the default Value() whose asStructure() is null — causing a NullPointerException on Java 11. Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
|
|
||
| OpenFeatureAPI.getInstance().setProvider(ldProvider); | ||
|
|
||
| Awaitility.await().forever().until(() -> { |
There was a problem hiding this comment.
We will need to make sure this test has some timeout associated with it. Otherwise the test run could hang if there was as bug.
There was a problem hiding this comment.
Good catch — updated all four Awaitility waits (boolean, string, double, and value) to use atMost(Duration.ofSeconds(10)) instead of forever(). This ensures the build won't hang if a bug prevents provider initialization.
…erTest Address review feedback: all Awaitility waits now use atMost(10s) instead of forever() to prevent hanging the build if a bug prevents provider initialization. This applies to the existing boolean/string/double tests as well as the new value evaluation test. Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
| OpenFeatureAPI.getInstance().setProvider(ldProvider); | ||
|
|
||
| Awaitility.await().forever().until(() -> { | ||
| Value val = OpenFeatureAPI.getInstance().getClient() |
There was a problem hiding this comment.
Should we be using an open feature provider status API?
Why isn't this available immediately?
There was a problem hiding this comment.
Good question. OpenFeatureAPI.setProvider() is non-blocking — it triggers provider initialization asynchronously, so the provider may not be in READY state when the next line executes. The other three evaluation tests (boolean, string, double) already had Awaitility.await() guards for this reason; this value test was the only one missing it.
An alternative would be to use setProviderAndWait() instead of setProvider() + Awaitility — that's the synchronous variant that blocks until initialization completes. That would be cleaner and avoid the need for Awaitility entirely in these tests. Want me to refactor the evaluation tests to use setProviderAndWait() instead?
There was a problem hiding this comment.
In what open feature version was setProviderAndWait added?
There was a problem hiding this comment.
setProviderAndWait is available since at least SDK 1.13.0 (the previous lower bound) — I confirmed it exists in the bytecode of that version. So it's safe to use across the entire [1.16.0, 2.0.0) range.
Want me to refactor these tests to use setProviderAndWait() and drop the Awaitility waits?
There was a problem hiding this comment.
Yes, lets give that a try.
There was a problem hiding this comment.
Done — refactored all four evaluation tests to use setProviderAndWait() instead of setProvider() + Awaitility. Also added the necessary mock stubs (getFlagTracker(), getDataSourceStatusProvider(), isInitialized()) so the provider's initialize() lifecycle completes successfully. All tests pass locally.
Replace setProvider() + Awaitility polling with setProviderAndWait() in all four evaluation tests (boolean, string, double, value). This is cleaner and avoids the need for async waiting entirely. To support setProviderAndWait() calling initialize(), the mock LDClientInterface now stubs getFlagTracker(), getDataSourceStatusProvider(), and isInitialized() so the provider can complete its initialization lifecycle. Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
BEGIN_COMMIT_OVERRIDE
fix: Update OpenFeature SDK dependency lower bound from 1.13.0 to 1.16.0
ci: Fix flaky tests by awaiting the provider being set.
END_COMMIT_OVERRIDE
Requirements
Related issues
Fixes #40
Describe the solution you've provided
Raises the lower bound of the
dev.openfeature:sdkdependency range from[1.13.0,2.0.0)to[1.16.0,2.0.0).The previous lower bound of 1.13.0 was incorrect for two reasons, both confirmed by inspecting the bytecode of the relevant SDK versions:
Compilation failure (SDK < 1.14.1):
TrackingEventDetails.getValue()(no-arg) does not exist prior to 1.14.1. TheProvider.track()method calls this, so the project cannot compile against SDK versions [1.13.0, 1.14.1).Binary incompatibility (SDK < 1.16.0):
EventProvider.emitProviderReady,emitProviderError,emitProviderStale, andemitProviderConfigurationChangedchanged their return type fromvoidtoAwaitablein SDK 1.16.0. Code compiled against < 1.16.0 throwsNoSuchMethodErrorat runtime when a consumer brings in SDK ≥ 1.16.0.Setting the lower bound to 1.16.0 resolves both issues.
Existing tests already exercise the affected code paths (emit methods in
LifeCycleTest, tracking inProviderTest). No new test coverage is needed since this is a dependency range correction, not a behavior change.Test fixes
This PR also fixes pre-existing flaky/failing tests:
LifeCycleTestitCanHandleClientThatIsNotInitializedImmediately: Event handlers registered viaOpenFeatureAPI.getInstance().on(...)were leaking across tests because the singleton was not consistently cleaned up. Added@AfterEachcallingOpenFeatureAPI.getInstance().shutdown()so handlers are cleared between tests. Removed individualshutdown()calls from test bodies since cleanup is now centralized.canShutdownAnOfflineClient: Removed the assertion that the data source status isOFFafter shutdown. The offlineLDClientdoes not transition its data source status toOFFonclose(), so this assertion was always incorrect. The test now verifies that shutdown completes without error.ProviderTestsetProvider()(non-blocking) +Awaitility.await().forever()polling withsetProviderAndWait()(synchronous). This eliminates the race condition that causeditCanDoAValueEvaluationto throwNullPointerExceptionon Java 11, and removes the risk of tests hanging indefinitely. To supportsetProviderAndWait()callinginitialize(), the mockLDClientInterfacenow stubsgetFlagTracker(),getDataSourceStatusProvider(), andisInitialized().Describe alternatives you've considered
Additional context
Key items for review:
canShutdownAnOfflineClientnow has no assertions beyondassertDoesNotThrow— confirm this is sufficient or whether a different assertion should replace the removed oneOpenFeatureAPI.getInstance().shutdown()in@AfterEachfully clears event handlers (not just providers)isInitialized() → trueandgetStatus() → VALID, soinitialize()returns immediately without waiting on theCompletableFuture— confirm this adequately exercises the provider lifecycle for these evaluation-focused testsLink to Devin session: https://app.devin.ai/sessions/219cccbf82aa4ac6a2372d91eee96ef9
Requested by: @kinyoklion
Note
Medium Risk
Medium risk because it changes the supported OpenFeature SDK version range, which can affect downstream dependency resolution. Code changes are limited to test synchronization/cleanup and should not impact runtime behavior.
Overview
Raises the lower bound of the
dev.openfeature:sdkdependency range from1.13.0to1.16.0.Stabilizes tests by centralizing
OpenFeatureAPIcleanup via@AfterEachinLifeCycleTest, removing an incorrect post-shutdown data source status assertion, and switchingProviderTestevaluations from asyncsetProvider()+ polling to synchronoussetProviderAndWait()(with additionalLDClientInterfacemock stubbing to support initialization).Written by Cursor Bugbot for commit 8a60681. This will update automatically on new commits. Configure here.