Skip to content

Commit d0a2027

Browse files
chrfwowchrfwow
committed
implement feedback from codereview
Signed-off-by: christian.lutnik <[email protected]>
1 parent 98c620f commit d0a2027

File tree

7 files changed

+229
-150
lines changed

7 files changed

+229
-150
lines changed

‎src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import dev.openfeature.sdk.fixtures.HookFixtures;
44
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
55
import dev.openfeature.sdk.testutils.TestEventsProvider;
6+
import lombok.SneakyThrows;
67
import org.junit.jupiter.api.Test;
78

89
import java.util.*;
@@ -16,32 +17,35 @@
1617
class DeveloperExperienceTest implements HookFixtures {
1718
transient String flagKey = "mykey";
1819

19-
@Test void simpleBooleanFlag() throws Exception {
20+
@Test
21+
void simpleBooleanFlag() {
2022
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
21-
api.setProvider(TestEventsProvider.initialized());
23+
api.setProviderAndWait(new TestEventsProvider());
2224
Client client = api.getClient();
2325
Boolean retval = client.getBooleanValue(flagKey, false);
2426
assertFalse(retval);
2527
}
2628

27-
@Test void clientHooks() throws Exception {
29+
@Test
30+
void clientHooks() {
2831
Hook<Boolean> exampleHook = mockBooleanHook();
2932

3033
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
31-
api.setProvider(TestEventsProvider.initialized());
34+
api.setProviderAndWait(new TestEventsProvider());
3235
Client client = api.getClient();
3336
client.addHooks(exampleHook);
3437
Boolean retval = client.getBooleanValue(flagKey, false);
3538
verify(exampleHook, times(1)).finallyAfter(any(), any());
3639
assertFalse(retval);
3740
}
3841

39-
@Test void evalHooks() throws Exception {
42+
@Test
43+
void evalHooks() {
4044
Hook<Boolean> clientHook = mockBooleanHook();
4145
Hook<Boolean> evalHook = mockBooleanHook();
4246

4347
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
44-
api.setProvider(TestEventsProvider.initialized());
48+
api.setProviderAndWait(new TestEventsProvider());
4549
Client client = api.getClient();
4650
client.addHooks(clientHook);
4751
Boolean retval = client.getBooleanValue(flagKey, false, null,
@@ -55,10 +59,11 @@ class DeveloperExperienceTest implements HookFixtures {
5559
* As an application author, you probably know special things about your users. You can communicate these to the
5660
* provider via {@link MutableContext}
5761
*/
58-
@Test void providingContext() throws Exception {
62+
@Test
63+
void providingContext() {
5964

6065
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
61-
api.setProvider(TestEventsProvider.initialized());
66+
api.setProviderAndWait(new TestEventsProvider());
6267
Client client = api.getClient();
6368
Map<String, Value> attributes = new HashMap<>();
6469
List<Value> values = Arrays.asList(new Value(2), new Value(4));
@@ -72,7 +77,8 @@ class DeveloperExperienceTest implements HookFixtures {
7277
assertFalse(retval);
7378
}
7479

75-
@Test void brokenProvider() {
80+
@Test
81+
void brokenProvider() {
7682
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
7783
FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider());
7884
Client client = api.getClient();
@@ -89,17 +95,16 @@ void providerLockedPerTransaction() {
8995
class MutatingHook implements Hook {
9096

9197
@Override
98+
@SneakyThrows
9299
// change the provider during a before hook - this should not impact the evaluation in progress
93-
public Optional before(HookContext ctx, Map hints) {
94-
try {
95-
FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.initialized());
96-
} catch (Exception e) {
97-
throw new RuntimeException(e);
98-
}
100+
public Optional before(HookContext ctx, Map hints) {
101+
102+
FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.newInitializedTestEventsProvider());
103+
99104
return Optional.empty();
100105
}
101106
}
102-
107+
103108
final String defaultValue = "string-value";
104109
final OpenFeatureAPI api = OpenFeatureAPI.getInstance();
105110
final Client client = api.getClient();
@@ -111,7 +116,7 @@ public Optional before(HookContext ctx, Map hints) {
111116
assertEquals(new StringBuilder(defaultValue).reverse().toString(), doSomethingValue);
112117

113118
api.clearHooks();
114-
119+
115120
// subsequent evaluations should now use new provider set by hook
116121
String noOpValue = client.getStringValue("val", defaultValue);
117122
assertEquals(noOpValue, defaultValue);

‎src/test/java/dev/openfeature/sdk/EventProviderTest.java‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import dev.openfeature.sdk.exceptions.FatalError;
44
import dev.openfeature.sdk.exceptions.GeneralError;
55
import dev.openfeature.sdk.internal.TriConsumer;
6+
import lombok.SneakyThrows;
67
import org.junit.jupiter.api.BeforeEach;
78
import org.junit.jupiter.api.DisplayName;
89
import org.junit.jupiter.api.Test;
@@ -19,7 +20,8 @@ class EventProviderTest {
1920
private TestEventProvider eventProvider;
2021

2122
@BeforeEach
22-
void setup() throws Exception {
23+
@SneakyThrows
24+
void setup() {
2325
eventProvider = new TestEventProvider();
2426
eventProvider.initialize(null);
2527
}
@@ -95,8 +97,9 @@ protected void doInitialization(EvaluationContext evaluationContext) {
9597
}
9698

9799
@Test
100+
@SneakyThrows
98101
@DisplayName("sets the state to READY after running the initialize method")
99-
void setsStateToReadyAfterInit() throws Exception {
102+
void setsStateToReadyAfterInit() {
100103
AtomicBoolean doInitializationCalled = new AtomicBoolean();
101104
EventProvider provider = new TestEventProvider() {
102105
@Override
@@ -141,6 +144,49 @@ protected void doInitialization(EvaluationContext evaluationContext) {
141144
assertThat(doInitializationCalled).isTrue();
142145
}
143146

147+
@Test
148+
@Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.")
149+
@DisplayName("sets the state to ERROR when an error event is emitted")
150+
void setsStateToErrorWhenErrorEventIsEmitted() {
151+
EventProvider provider = new TestEventProvider() {
152+
@Override
153+
protected void doInitialization(EvaluationContext evaluationContext) {
154+
}
155+
};
156+
assertThat(provider.getState()).isNotEqualTo(ProviderState.ERROR);
157+
provider.emitProviderError(ProviderEventDetails.builder().build());
158+
assertThat(provider.getState()).isEqualTo(ProviderState.ERROR);
159+
}
160+
161+
@Test
162+
@Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.")
163+
@DisplayName("sets the state to STALE when a stale event is emitted")
164+
void setsStateToStaleWhenStaleEventIsEmitted() {
165+
EventProvider provider = new TestEventProvider() {
166+
@Override
167+
protected void doInitialization(EvaluationContext evaluationContext) {
168+
}
169+
};
170+
assertThat(provider.getState()).isNotEqualTo(ProviderState.STALE);
171+
provider.emitProviderStale(ProviderEventDetails.builder().build());
172+
assertThat(provider.getState()).isEqualTo(ProviderState.STALE);
173+
}
174+
175+
@Test
176+
@Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.")
177+
@DisplayName("sets the state to READY when a ready event is emitted")
178+
void setsStateToReadyWhenReadyEventIsEmitted() {
179+
EventProvider provider = new TestEventProvider() {
180+
@Override
181+
protected void doInitialization(EvaluationContext evaluationContext) {
182+
}
183+
};
184+
provider.emitProviderStale(ProviderEventDetails.builder().build());
185+
assertThat(provider.getState()).isNotEqualTo(ProviderState.READY);
186+
provider.emitProviderReady(ProviderEventDetails.builder().build());
187+
assertThat(provider.getState()).isEqualTo(ProviderState.READY);
188+
}
189+
144190
static class TestEventProvider extends EventProvider {
145191

146192
private static final String NAME = "TestEventProvider";

‎src/test/java/dev/openfeature/sdk/EventsTest.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import dev.openfeature.sdk.testutils.TestEventsProvider;
44
import io.cucumber.java.AfterAll;
5+
import lombok.SneakyThrows;
56
import org.junit.jupiter.api.DisplayName;
67
import org.junit.jupiter.api.Nested;
78
import org.junit.jupiter.api.Test;
@@ -332,7 +333,7 @@ void shouldSupportAllEventTypes() {
332333

333334
@Test
334335
@DisplayName("shutdown provider should not run handlers")
335-
void shouldNotRunHandlers() {
336+
void shouldNotRunHandlers() {
336337
final Consumer<EventDetails> handler1 = mockHandler();
337338
final Consumer<EventDetails> handler2 = mockHandler();
338339
final String name = "shouldNotRunHandlers";
@@ -510,13 +511,13 @@ void shouldHaveAllProperties() {
510511
@Test
511512
@DisplayName("if the provider is ready handlers must run immediately")
512513
@Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
513-
void matchingReadyEventsMustRunImmediately() {
514+
void matchingReadyEventsMustRunImmediately() {
514515
final String name = "matchingEventsMustRunImmediately";
515516
final Consumer<EventDetails> handler = mockHandler();
516517

517518
// provider which is already ready
518519
TestEventsProvider provider = new TestEventsProvider();
519-
OpenFeatureAPI.getInstance().setProvider(name, provider);
520+
OpenFeatureAPI.getInstance().setProviderAndWait(name, provider);
520521

521522
// should run even thought handler was added after ready
522523
Client client = OpenFeatureAPI.getInstance().getClient(name);
@@ -527,12 +528,12 @@ void matchingReadyEventsMustRunImmediately() {
527528
@Test
528529
@DisplayName("if the provider is ready handlers must run immediately")
529530
@Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
530-
void matchingStaleEventsMustRunImmediately() throws Exception {
531+
void matchingStaleEventsMustRunImmediately() {
531532
final String name = "matchingEventsMustRunImmediately";
532533
final Consumer<EventDetails> handler = mockHandler();
533534

534535
// provider which is already stale
535-
TestEventsProvider provider =TestEventsProvider.initialized();
536+
TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider();
536537
provider.emitProviderStale(null);
537538
assertThat(provider.getState()).isEqualTo(ProviderState.STALE);
538539
OpenFeatureAPI.getInstance().setProvider(name, provider);
@@ -546,12 +547,12 @@ void matchingStaleEventsMustRunImmediately() throws Exception {
546547
@Test
547548
@DisplayName("if the provider is ready handlers must run immediately")
548549
@Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
549-
void matchingErrorEventsMustRunImmediately() throws Exception {
550+
void matchingErrorEventsMustRunImmediately() {
550551
final String name = "matchingEventsMustRunImmediately";
551552
final Consumer<EventDetails> handler = mockHandler();
552553

553554
// provider which is already in error
554-
TestEventsProvider provider = TestEventsProvider.initialized();
555+
TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider();
555556
provider.emitProviderError(null);
556557
assertThat(provider.getState()).isEqualTo(ProviderState.ERROR);
557558
OpenFeatureAPI.getInstance().setProvider(name, provider);
@@ -593,10 +594,11 @@ void mustPersistAcrossChanges() {
593594

594595
@Nested
595596
class HandlerRemoval {
596-
@Specification(number="5.2.7", text="The API and client MUST provide a function allowing the removal of event handlers.")
597+
@Specification(number = "5.2.7", text = "The API and client MUST provide a function allowing the removal of event handlers.")
597598
@Test
598599
@DisplayName("should not run removed events")
599-
void removedEventsShouldNotRun() throws Exception {
600+
@SneakyThrows
601+
void removedEventsShouldNotRun() {
600602
final String name = "removedEventsShouldNotRun";
601603
final Consumer<EventDetails> handler1 = mockHandler();
602604
final Consumer<EventDetails> handler2 = mockHandler();

‎src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
77
import dev.openfeature.sdk.testutils.TestEventsProvider;
88
import lombok.SneakyThrows;
9+
import org.awaitility.Awaitility;
910
import org.junit.jupiter.api.AfterEach;
1011
import org.junit.jupiter.api.BeforeEach;
1112
import org.junit.jupiter.api.Test;
@@ -35,7 +36,8 @@ private Client _client() {
3536
return api.getClient();
3637
}
3738

38-
private Client _initializedClient() throws Exception {
39+
@SneakyThrows
40+
private Client _initializedClient() {
3941
TestEventsProvider provider = new TestEventsProvider();
4042
provider.initialize(null);
4143
FeatureProviderTestUtils.setFeatureProvider(provider);
@@ -101,7 +103,7 @@ void getApiInstance() {
101103
FeatureProvider provider = new InMemoryProvider(new HashMap<>()) {
102104
@Override
103105
protected void doInitialization(EvaluationContext evaluationContext) throws Exception {
104-
Thread.sleep(10000);
106+
Awaitility.await().wait(3000);
105107
}
106108
};
107109
String providerName = "shouldReturnNotReadyIfNotInitialized";
@@ -235,7 +237,8 @@ protected void doInitialization(EvaluationContext evaluationContext) throws Exce
235237
}
236238

237239
@Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.")
238-
@Test void hooks() throws Exception {
240+
@SneakyThrows
241+
@Test void hooks() {
239242
Client c = _initializedClient();
240243
Hook<Boolean> clientHook = mockBooleanHook();
241244
Hook<Boolean> invocationHook = mockBooleanHook();

0 commit comments

Comments
 (0)