Skip to content

chore(flagd): update testharness and add metadata tests #1293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
[submodule "providers/flagd/test-harness"]
path = providers/flagd/test-harness
url = https://github.com/open-feature/test-harness.git
branch = v2.5.0
branch = v2.7.1
[submodule "providers/flagd/spec"]
path = providers/flagd/spec
url = https://github.com/open-feature/spec.git
2 changes: 1 addition & 1 deletion providers/flagd/schemas
2 changes: 1 addition & 1 deletion providers/flagd/spec
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,14 @@ private <ValT, ReqT extends Message, ResT extends Message> ProviderEvaluation<Va
return result;
}

private <T> Boolean isEvaluationCacheable(ProviderEvaluation<T> evaluation) {
private <T> boolean isEvaluationCacheable(ProviderEvaluation<T> evaluation) {
String reason = evaluation.getReason();

return reason != null && reason.equals(Config.STATIC_REASON) && this.cacheAvailable();
}

private Boolean cacheAvailable() {
return this.cache.getEnabled();
private boolean cacheAvailable() {
return this.cache.isEnabled();
}

private static ImmutableMetadata metadataFromResponse(Message response) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ public class Cache {
private Map<String, ProviderEvaluation<?>> store;

@Getter
private final Boolean enabled;
private final boolean enabled;

/**
* Initialize the cache.
*
* @param forType type of the cache.
* @param forType type of the cache.
* @param maxCacheSize max amount of element to keep.
*/
public Cache(final String forType, int maxCacheSize) {
Expand All @@ -36,19 +36,50 @@ public Cache(final String forType, int maxCacheSize) {
}
}

/**
* Adds a provider evaluation to the cache.
*
* @param key the key of the flag
* @param value the provider evaluation
*/
public void put(String key, ProviderEvaluation<?> value) {
if (!enabled) {
return;
}
this.store.put(key, value);
}

/**
* Retrieves a provider evaluation from the cache, or null if the key has not been cached before.
*
* @param key the key of the flag
*/
public ProviderEvaluation<?> get(String key) {
if (!enabled) {
return null;
}
return this.store.get(key);
}

/**
* Removes a provider evaluation from the cache.
*
* @param key the key of the flag
*/
public void remove(String key) {
if (!enabled) {
return;
}
this.store.remove(key);
}

/**
* Clears the cache.
*/
public void clear() {
if (!enabled) {
return;
}
this.store.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,14 @@ void interruptingWaitingThread_isIgnored() throws InterruptedException {
@Test
void callingInitialize_wakesUpWaitingThread() throws InterruptedException {
final AtomicBoolean isWaiting = new AtomicBoolean();
final AtomicBoolean successfulTest = new AtomicBoolean();
Thread waitingThread = new Thread(() -> {
long start = System.currentTimeMillis();
isWaiting.set(true);
flagdProviderSyncResources.waitForInitialization(10000);
long end = System.currentTimeMillis();
long duration = end - start;
Assertions.assertTrue(duration < MAX_TIME_TOLERANCE);
successfulTest.set(duration < MAX_TIME_TOLERANCE * 2);
});
waitingThread.start();

Expand All @@ -98,12 +99,15 @@ void callingInitialize_wakesUpWaitingThread() throws InterruptedException {
flagdProviderSyncResources.initialize();

waitingThread.join();

Assertions.assertTrue(successfulTest.get());
}

@Timeout(2)
@Test
void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedException {
final AtomicBoolean isWaiting = new AtomicBoolean();
final AtomicBoolean successfulTest = new AtomicBoolean();
Thread waitingThread = new Thread(() -> {
long start = System.currentTimeMillis();
isWaiting.set(true);
Expand All @@ -112,7 +116,7 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep

long end = System.currentTimeMillis();
long duration = end - start;
Assertions.assertTrue(duration < MAX_TIME_TOLERANCE);
successfulTest.set(duration < MAX_TIME_TOLERANCE * 2);
});
waitingThread.start();

Expand All @@ -125,6 +129,8 @@ void callingShutdown_wakesUpWaitingThreadWithException() throws InterruptedExcep
flagdProviderSyncResources.shutdown();

waitingThread.join();

Assertions.assertTrue(successfulTest.get());
}

@Timeout(2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public EventSteps(State state) {
@Given("a {} event handler")
public void a_stale_event_handler(String eventType) {
state.client.on(mapEventType(eventType), eventDetails -> {
log.info("event tracked for {} at {} ms ", eventType, System.currentTimeMillis() % 100_000);
log.info("{} event tracked", eventType);
state.events.add(new Event(eventType, eventDetails));
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@

import dev.openfeature.contrib.providers.flagd.e2e.State;
import dev.openfeature.sdk.FlagEvaluationDetails;
import dev.openfeature.sdk.ImmutableMetadata;
import dev.openfeature.sdk.Value;
import io.cucumber.java.en.Given;
import io.cucumber.java.en.Then;
import io.cucumber.java.en.When;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.List;
import java.util.Map;
import lombok.extern.slf4j.Slf4j;
import org.junit.jupiter.api.parallel.Isolated;

@Slf4j
@Isolated()
public class FlagSteps extends AbstractSteps {

Expand Down Expand Up @@ -54,6 +61,9 @@ public void the_flag_was_evaluated_with_details() throws InterruptedException {

@Then("the resolved details value should be \"{}\"")
public void the_resolved_details_value_should_be(String value) throws Throwable {
if (state.evaluation.getErrorCode() != null) {
log.warn(state.evaluation.getErrorMessage());
}
assertThat(state.evaluation.getValue()).isEqualTo(Utils.convert(value, state.flag.type));
}

Expand Down Expand Up @@ -84,4 +94,47 @@ public Flag(String type, String name, Object defaultValue) {
this.type = type;
}
}

@Then("the resolved metadata is empty")
@SuppressWarnings("unchecked")
public void the_resolved_metadata_is_empty() throws NoSuchFieldException, IllegalAccessException {
ImmutableMetadata flagMetadata = state.evaluation.getFlagMetadata();

Field metadataField = flagMetadata.getClass().getDeclaredField("metadata");
metadataField.setAccessible(true);
Map<String, Object> metadataMap = (Map<String, Object>) metadataField.get(flagMetadata);
assertThat(metadataMap).isEmpty();
}

@Then("the resolved metadata should contain")
@SuppressWarnings("unchecked")
public void the_resolved_metadata_should_contain(io.cucumber.datatable.DataTable dataTable)
throws IOException, ClassNotFoundException {

ImmutableMetadata flagMetadata = state.evaluation.getFlagMetadata();

List<Map<String, String>> rows = dataTable.asMaps(String.class, String.class);
for (Map<String, String> row : rows) {
switch (row.get("metadata_type")) {
case "String":
assertThat(flagMetadata.getString(row.get("key")))
.isEqualTo(Utils.convert(row.get("value"), row.get("metadata_type")));
break;
case "Boolean":
assertThat(flagMetadata.getBoolean(row.get("key")))
.isEqualTo(Utils.convert(row.get("value"), row.get("metadata_type")));
break;
case "Float":
assertThat(flagMetadata.getDouble(row.get("key")))
.isEqualTo(Utils.convert(row.get("value"), row.get("metadata_type")));
break;
case "Integer":
assertThat(flagMetadata.getInteger(row.get("key")))
.isEqualTo(Utils.convert(row.get("value"), row.get("metadata_type")));
break;
default:
throw new AssertionError("type not supported");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static io.restassured.RestAssured.when;

import dev.openfeature.contrib.providers.flagd.Config;
import dev.openfeature.contrib.providers.flagd.FlagdOptions;
import dev.openfeature.contrib.providers.flagd.FlagdProvider;
import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer;
import dev.openfeature.contrib.providers.flagd.e2e.State;
Expand Down Expand Up @@ -104,7 +105,21 @@ public void setupProvider(String providerType) throws InterruptedException {
.certPath(absolutePath);
flagdConfig = "ssl";
break;
case "metadata":
flagdConfig = "metadata";

if (State.resolverType == Config.Resolver.FILE) {
FlagdOptions build = state.builder.build();
String selector = build.getSelector();
String replace = selector.replace("rawflags/", "");

state.builder
.port(UNAVAILABLE_PORT)
.offlineFlagSourcePath(new File("test-harness/flags/" + replace).getAbsolutePath());
} else {
state.builder.port(container.getPort(State.resolverType));
}
break;
default:
this.state.providerType = ProviderType.DEFAULT;
if (State.resolverType == Config.Resolver.FILE) {
Expand All @@ -124,12 +139,13 @@ public void setupProvider(String providerType) throws InterruptedException {
.then()
.statusCode(200);

// giving flagd a little time to start
Thread.sleep(300);
Thread.sleep(50);

FeatureProvider provider =
new FlagdProvider(state.builder.resolverType(State.resolverType).build());
String providerName = "Provider " + Math.random();
OpenFeatureAPI api = OpenFeatureAPI.getInstance();

if (wait) {
api.setProviderAndWait(providerName, provider);
} else {
Expand All @@ -151,10 +167,7 @@ public void the_connection_is_lost_for(int seconds) {
}

@When("the flag was modified")
public void the_flag_was_modded() throws InterruptedException {
public void the_flag_was_modded() {
when().post("http://" + container.getLaunchpadUrl() + "/change").then().statusCode(200);

// we might be too fast in the execution
Thread.sleep(1000);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ public void init() throws Exception {
when(stub.withDeadlineAfter(anyLong(), any())).thenReturn(stub);
doAnswer(new Answer<Void>() {
public Void answer(InvocationOnMock invocation) {
latch.countDown();
Object[] args = invocation.getArguments();
if (args[1] != null) {
observer = (QueueingStreamObserver<EventStreamResponse>) args[1];
}
latch.countDown();
return null;
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ void cacheTypeTest() {
final Cache undefined = new Cache("invalid", 10);

// then
assertTrue(lru.getEnabled());
assertFalse(disabled.getEnabled());
assertFalse(undefined.getEnabled());
assertTrue(lru.isEnabled());
assertFalse(disabled.isEnabled());
assertFalse(undefined.isEnabled());
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion providers/flagd/test-harness