Skip to content

Commit 22824c8

Browse files
aepfliaepfli
committed
test: Reduce usage of singelton withint our tests
Our tests are great, but often we rely on our own Singelton for testing purposes. This can create concurrency issues or make testing really hard. By instantiating a own API object for each test we ensure that we are not messing with each other. Furthermore we should not use `.getInstance()` within our own implementation. Signed-off-by: Simon Schrottner <[email protected]>
1 parent 4ba5695 commit 22824c8

22 files changed

+257
-234
lines changed

‎pom.xml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,13 @@
6767
</dependency>
6868

6969
<!-- test -->
70+
<dependency>
71+
<groupId>com.tngtech.archunit</groupId>
72+
<artifactId>archunit-junit5</artifactId>
73+
<version>1.4.0</version>
74+
<scope>test</scope>
75+
</dependency>
76+
7077
<dependency>
7178
<groupId>org.mockito</groupId>
7279
<artifactId>mockito-core</artifactId>
@@ -242,12 +249,14 @@
242249
<ignoredUnusedDeclaredDependencies>
243250
<ignoredUnusedDeclaredDependency>com.github.spotbugs:*</ignoredUnusedDeclaredDependency>
244251
<ignoredUnusedDeclaredDependency>org.junit*</ignoredUnusedDeclaredDependency>
252+
<ignoredUnusedDeclaredDependency>com.tngtech.archunit*</ignoredUnusedDeclaredDependency>
245253
<ignoredUnusedDeclaredDependency>org.simplify4u:slf4j2-mock*</ignoredUnusedDeclaredDependency>
246254
</ignoredUnusedDeclaredDependencies>
247255
<ignoredDependencies>
248256
<ignoredDependency>com.google.guava*</ignoredDependency>
249257
<ignoredDependency>io.cucumber*</ignoredDependency>
250258
<ignoredDependency>org.junit*</ignoredDependency>
259+
<ignoredDependency>com.tngtech.archunit*</ignoredDependency>
251260
<ignoredDependency>com.google.code.findbugs*</ignoredDependency>
252261
<ignoredDependency>com.github.spotbugs*</ignoredDependency>
253262
<ignoredDependency>org.simplify4u:slf4j-mock-common:*</ignoredDependency>

‎src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
2929

3030
protected OpenFeatureAPI() {
3131
apiHooks = new ArrayList<>();
32-
providerRepository = new ProviderRepository();
32+
providerRepository = new ProviderRepository(this);
3333
eventSupport = new EventSupport();
3434
transactionContextPropagator = new NoOpTransactionContextPropagator();
3535
}
@@ -333,7 +333,7 @@ public void shutdown() {
333333
providerRepository.shutdown();
334334
eventSupport.shutdown();
335335

336-
providerRepository = new ProviderRepository();
336+
providerRepository = new ProviderRepository(this);
337337
eventSupport = new EventSupport();
338338
}
339339
}

‎src/main/java/dev/openfeature/sdk/OpenFeatureClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ public Client onProviderStale(Consumer<EventDetails> handler) {
507507
*/
508508
@Override
509509
public Client on(ProviderEvent event, Consumer<EventDetails> handler) {
510-
OpenFeatureAPI.getInstance().addHandler(domain, event, handler);
510+
openfeatureApi.addHandler(domain, event, handler);
511511
return this;
512512
}
513513

@@ -516,7 +516,7 @@ public Client on(ProviderEvent event, Consumer<EventDetails> handler) {
516516
*/
517517
@Override
518518
public Client removeHandler(ProviderEvent event, Consumer<EventDetails> handler) {
519-
OpenFeatureAPI.getInstance().removeHandler(domain, event, handler);
519+
openfeatureApi.removeHandler(domain, event, handler);
520520
return this;
521521
}
522522
}

‎src/main/java/dev/openfeature/sdk/ProviderRepository.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ class ProviderRepository {
2828
return thread;
2929
});
3030
private final Object registerStateManagerLock = new Object();
31+
private final OpenFeatureAPI openFeatureAPI;
32+
33+
public ProviderRepository(OpenFeatureAPI openFeatureAPI) {
34+
this.openFeatureAPI = openFeatureAPI;
35+
}
3136

3237
FeatureProviderStateManager getFeatureProviderStateManager() {
3338
return defaultStateManger.get();
@@ -205,7 +210,7 @@ private void initializeProvider(
205210
FeatureProviderStateManager oldManager) {
206211
try {
207212
if (ProviderState.NOT_READY.equals(newManager.getState())) {
208-
newManager.initialize(OpenFeatureAPI.getInstance().getEvaluationContext());
213+
newManager.initialize(openFeatureAPI.getEvaluationContext());
209214
afterInit.accept(newManager.getProvider());
210215
}
211216
shutDownOld(oldManager, afterShutdown);

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@
22

33
import static org.junit.jupiter.api.Assertions.*;
44

5-
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
65
import org.junit.jupiter.api.Test;
76

87
class ClientProviderMappingTest {
98

109
@Test
1110
void clientProviderTest() {
12-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
11+
OpenFeatureAPI api = new OpenFeatureAPI();
1312

14-
FeatureProviderTestUtils.setFeatureProvider("client1", new DoSomethingProvider());
15-
FeatureProviderTestUtils.setFeatureProvider("client2", new NoOpProvider());
13+
api.setProviderAndWait("client1", new DoSomethingProvider());
14+
api.setProviderAndWait("client2", new NoOpProvider());
1615

1716
Client c1 = api.getClient("client1");
1817
Client c2 = api.getClient("client2");

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import static org.mockito.Mockito.verify;
99

1010
import dev.openfeature.sdk.fixtures.HookFixtures;
11-
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
1211
import dev.openfeature.sdk.testutils.TestEventsProvider;
1312
import java.util.Arrays;
1413
import java.util.HashMap;
@@ -23,7 +22,7 @@ class DeveloperExperienceTest implements HookFixtures {
2322

2423
@Test
2524
void simpleBooleanFlag() {
26-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
25+
OpenFeatureAPI api = new OpenFeatureAPI();
2726
api.setProviderAndWait(new TestEventsProvider());
2827
Client client = api.getClient();
2928
Boolean retval = client.getBooleanValue(flagKey, false);
@@ -34,7 +33,7 @@ void simpleBooleanFlag() {
3433
void clientHooks() {
3534
Hook<Boolean> exampleHook = mockBooleanHook();
3635

37-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
36+
OpenFeatureAPI api = new OpenFeatureAPI();
3837
api.setProviderAndWait(new TestEventsProvider());
3938
Client client = api.getClient();
4039
client.addHooks(exampleHook);
@@ -48,7 +47,7 @@ void evalHooks() {
4847
Hook<Boolean> clientHook = mockBooleanHook();
4948
Hook<Boolean> evalHook = mockBooleanHook();
5049

51-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
50+
OpenFeatureAPI api = new OpenFeatureAPI();
5251
api.setProviderAndWait(new TestEventsProvider());
5352
Client client = api.getClient();
5453
client.addHooks(clientHook);
@@ -69,7 +68,7 @@ void evalHooks() {
6968
@Test
7069
void providingContext() {
7170

72-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
71+
OpenFeatureAPI api = new OpenFeatureAPI();
7372
api.setProviderAndWait(new TestEventsProvider());
7473
Client client = api.getClient();
7574
Map<String, Value> attributes = new HashMap<>();
@@ -86,8 +85,8 @@ void providingContext() {
8685

8786
@Test
8887
void brokenProvider() {
89-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
90-
FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider());
88+
OpenFeatureAPI api = new OpenFeatureAPI();
89+
api.setProviderAndWait(new AlwaysBrokenProvider());
9190
Client client = api.getClient();
9291
FlagEvaluationDetails<Boolean> retval = client.getBooleanDetails(flagKey, false);
9392
assertEquals(ErrorCode.FLAG_NOT_FOUND, retval.getErrorCode());
@@ -99,23 +98,24 @@ void brokenProvider() {
9998
@Test
10099
void providerLockedPerTransaction() {
101100

101+
final String defaultValue = "string-value";
102+
final OpenFeatureAPI api = new OpenFeatureAPI();
103+
102104
class MutatingHook implements Hook {
103105

104106
@Override
105107
@SneakyThrows
106108
// change the provider during a before hook - this should not impact the evaluation in progress
107109
public Optional before(HookContext ctx, Map hints) {
108110

109-
FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.newInitializedTestEventsProvider());
111+
api.setProviderAndWait(TestEventsProvider.newInitializedTestEventsProvider());
110112

111113
return Optional.empty();
112114
}
113115
}
114116

115-
final String defaultValue = "string-value";
116-
final OpenFeatureAPI api = OpenFeatureAPI.getInstance();
117117
final Client client = api.getClient();
118-
FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider());
118+
api.setProviderAndWait(new DoSomethingProvider());
119119
api.addHooks(new MutatingHook());
120120

121121
// if provider is changed during an evaluation transaction it should proceed with the original provider
@@ -132,7 +132,7 @@ public Optional before(HookContext ctx, Map hints) {
132132
@Test
133133
void setProviderAndWaitShouldPutTheProviderInReadyState() {
134134
String domain = "domain";
135-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
135+
OpenFeatureAPI api = new OpenFeatureAPI();
136136
api.setProviderAndWait(domain, new TestEventsProvider());
137137
Client client = api.getClient(domain);
138138
assertThat(client.getProviderState()).isEqualTo(ProviderState.READY);
@@ -145,7 +145,7 @@ void setProviderAndWaitShouldPutTheProviderInReadyState() {
145145
@Test
146146
void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() {
147147
String domain = "domain";
148-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
148+
OpenFeatureAPI api = new OpenFeatureAPI();
149149
TestEventsProvider provider = new TestEventsProvider();
150150
api.setProviderAndWait(domain, provider);
151151
Client client = api.getClient(domain);
@@ -161,7 +161,7 @@ void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() {
161161
@Test
162162
void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() {
163163
String domain = "domain";
164-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
164+
OpenFeatureAPI api = new OpenFeatureAPI();
165165
TestEventsProvider provider = new TestEventsProvider();
166166
api.setProviderAndWait(domain, provider);
167167
Client client = api.getClient(domain);
@@ -177,7 +177,7 @@ void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() {
177177
@Test
178178
void shouldPutTheProviderInStateReadyAfterEmittingReadyEvent() {
179179
String domain = "domain";
180-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
180+
OpenFeatureAPI api = new OpenFeatureAPI();
181181
TestEventsProvider provider = new TestEventsProvider();
182182
api.setProviderAndWait(domain, provider);
183183
Client client = api.getClient(domain);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ void setup() {
2828

2929
@AfterAll
3030
public static void resetDefaultProvider() {
31-
OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider());
31+
new OpenFeatureAPI().setProviderAndWait(new NoOpProvider());
3232
}
3333

3434
@Test
@@ -91,7 +91,7 @@ void doesNotThrowWhenOnEmitSame() {
9191
@DisplayName("should not deadlock on emit called during emit")
9292
void doesNotDeadlockOnEmitStackedCalls() {
9393
TestStackedEmitCallsProvider provider = new TestStackedEmitCallsProvider();
94-
OpenFeatureAPI.getInstance().setProviderAndWait(provider);
94+
new OpenFeatureAPI().setProviderAndWait(provider);
9595
}
9696

9797
static class TestEventProvider extends EventProvider {

0 commit comments

Comments
 (0)