Skip to content

Commit 88baa65

Browse files
aepflitoddbaert
aepfli
and
toddbaert
authored
test: Reduce usage of singelton within our tests and implementations (#1331)
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 <simon.schrottner@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
1 parent e163ce1 commit 88baa65

24 files changed

+259
-247
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/AlwaysBrokenWithDetailsProvider.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
package dev.openfeature.sdk;
22

3-
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
4-
53
public class AlwaysBrokenWithDetailsProvider implements FeatureProvider {
64

5+
private final String name = "always broken with details";
6+
77
@Override
88
public Metadata getMetadata() {
9-
return () -> {
10-
throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE);
11-
};
9+
return () -> name;
1210
}
1311

1412
@Override

src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java renamed to src/test/java/dev/openfeature/sdk/AlwaysBrokenWithExceptionProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
44

5-
public class AlwaysBrokenProvider implements FeatureProvider {
5+
public class AlwaysBrokenWithExceptionProvider implements FeatureProvider {
66

77
private final String name = "always broken";
88

‎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: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,27 @@
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;
1514
import java.util.List;
1615
import java.util.Map;
1716
import java.util.Optional;
1817
import lombok.SneakyThrows;
18+
import org.junit.jupiter.api.BeforeEach;
1919
import org.junit.jupiter.api.Test;
2020

2121
class DeveloperExperienceTest implements HookFixtures {
2222
transient String flagKey = "mykey";
23+
private OpenFeatureAPI api;
24+
25+
@BeforeEach
26+
public void setUp() throws Exception {
27+
api = new OpenFeatureAPI();
28+
}
2329

2430
@Test
2531
void simpleBooleanFlag() {
26-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
2732
api.setProviderAndWait(new TestEventsProvider());
2833
Client client = api.getClient();
2934
Boolean retval = client.getBooleanValue(flagKey, false);
@@ -34,7 +39,6 @@ void simpleBooleanFlag() {
3439
void clientHooks() {
3540
Hook<Boolean> exampleHook = mockBooleanHook();
3641

37-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
3842
api.setProviderAndWait(new TestEventsProvider());
3943
Client client = api.getClient();
4044
client.addHooks(exampleHook);
@@ -48,7 +52,6 @@ void evalHooks() {
4852
Hook<Boolean> clientHook = mockBooleanHook();
4953
Hook<Boolean> evalHook = mockBooleanHook();
5054

51-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
5255
api.setProviderAndWait(new TestEventsProvider());
5356
Client client = api.getClient();
5457
client.addHooks(clientHook);
@@ -69,7 +72,6 @@ void evalHooks() {
6972
@Test
7073
void providingContext() {
7174

72-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
7375
api.setProviderAndWait(new TestEventsProvider());
7476
Client client = api.getClient();
7577
Map<String, Value> attributes = new HashMap<>();
@@ -86,8 +88,7 @@ void providingContext() {
8688

8789
@Test
8890
void brokenProvider() {
89-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
90-
FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider());
91+
api.setProviderAndWait(new AlwaysBrokenWithExceptionProvider());
9192
Client client = api.getClient();
9293
FlagEvaluationDetails<Boolean> retval = client.getBooleanDetails(flagKey, false);
9394
assertEquals(ErrorCode.FLAG_NOT_FOUND, retval.getErrorCode());
@@ -99,23 +100,24 @@ void brokenProvider() {
99100
@Test
100101
void providerLockedPerTransaction() {
101102

103+
final String defaultValue = "string-value";
104+
final OpenFeatureAPI api = new OpenFeatureAPI();
105+
102106
class MutatingHook implements Hook {
103107

104108
@Override
105109
@SneakyThrows
106110
// change the provider during a before hook - this should not impact the evaluation in progress
107111
public Optional before(HookContext ctx, Map hints) {
108112

109-
FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.newInitializedTestEventsProvider());
113+
api.setProviderAndWait(TestEventsProvider.newInitializedTestEventsProvider());
110114

111115
return Optional.empty();
112116
}
113117
}
114118

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

121123
// if provider is changed during an evaluation transaction it should proceed with the original provider
@@ -132,7 +134,6 @@ public Optional before(HookContext ctx, Map hints) {
132134
@Test
133135
void setProviderAndWaitShouldPutTheProviderInReadyState() {
134136
String domain = "domain";
135-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
136137
api.setProviderAndWait(domain, new TestEventsProvider());
137138
Client client = api.getClient(domain);
138139
assertThat(client.getProviderState()).isEqualTo(ProviderState.READY);
@@ -145,7 +146,6 @@ void setProviderAndWaitShouldPutTheProviderInReadyState() {
145146
@Test
146147
void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() {
147148
String domain = "domain";
148-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
149149
TestEventsProvider provider = new TestEventsProvider();
150150
api.setProviderAndWait(domain, provider);
151151
Client client = api.getClient(domain);
@@ -161,7 +161,6 @@ void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() {
161161
@Test
162162
void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() {
163163
String domain = "domain";
164-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
165164
TestEventsProvider provider = new TestEventsProvider();
166165
api.setProviderAndWait(domain, provider);
167166
Client client = api.getClient(domain);
@@ -177,7 +176,6 @@ void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() {
177176
@Test
178177
void shouldPutTheProviderInStateReadyAfterEmittingReadyEvent() {
179178
String domain = "domain";
180-
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
181179
TestEventsProvider provider = new TestEventsProvider();
182180
api.setProviderAndWait(domain, provider);
183181
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)