From c84c0de8202987bfc7982b5561709b3414742d79 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 20 Jan 2025 21:44:42 +0100 Subject: [PATCH 1/4] test(flagd): use newest testbed launchpad Signed-off-by: Simon Schrottner --- .gitmodules | 2 +- providers/flagd/pom.xml | 11 +- providers/flagd/spec | 2 +- .../connector/grpc/GrpcStreamConnector.java | 1 - ...umberTest.java => ConfigCucumberTest.java} | 6 +- .../providers/flagd/e2e/FlagdContainer.java | 28 ++-- .../flagd/e2e/steps/AbstractSteps.java | 6 +- .../e2e/steps/EnvironmentVariableUtils.java | 2 +- .../providers/flagd/e2e/steps/EventSteps.java | 6 +- .../providers/flagd/e2e/steps/FlagSteps.java | 12 +- .../flagd/e2e/steps/ProviderSteps.java | 145 +++++------------- .../e2e/steps/{ => config}/ConfigSteps.java | 5 +- providers/flagd/test-harness | 2 +- 13 files changed, 76 insertions(+), 152 deletions(-) rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/{e2e/RunConfigCucumberTest.java => ConfigCucumberTest.java} (85%) rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/{ => config}/ConfigSteps.java (93%) diff --git a/.gitmodules b/.gitmodules index 619d4fff1..acdcde3c2 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,7 +4,7 @@ [submodule "providers/flagd/test-harness"] path = providers/flagd/test-harness url = https://github.com/open-feature/test-harness.git - branch = v1.1.1 + branch = v1.3.2 [submodule "providers/flagd/spec"] path = providers/flagd/spec url = https://github.com/open-feature/spec.git diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 15a9d779f..044f03508 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -150,14 +150,13 @@ test - org.testcontainers - toxiproxy - 1.20.4 + io.rest-assured + rest-assured + 5.5.0 test - - + org.slf4j slf4j-simple 2.0.16 @@ -254,7 +253,7 @@ com.google.protobuf:protoc:3.25.5:exe:${os.detected.classifier} grpc-java - io.grpc:protoc-gen-grpc-java:1.68.2:exe:${os.detected.classifier} + io.grpc:protoc-gen-grpc-java:1.69.1:exe:${os.detected.classifier} ${project.basedir}/schemas/protobuf/ diff --git a/providers/flagd/spec b/providers/flagd/spec index 6c673d771..d261f6833 160000 --- a/providers/flagd/spec +++ b/providers/flagd/spec @@ -1 +1 @@ -Subproject commit 6c673d771618d86042adbcce70ace75f2b91fe76 +Subproject commit d261f68331b94fd8ed10bc72bc0485cfc72a51a8 diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 832c8a487..e76ebfb24 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -118,7 +118,6 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB metadataException = e; } - log.info("stream"); while (!shutdown.get()) { final GrpcResponseModel response = streamReceiver.take(); if (response.isComplete()) { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java similarity index 85% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java index d5dce18fe..f031091da 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java @@ -1,4 +1,4 @@ -package dev.openfeature.contrib.providers.flagd.e2e; +package dev.openfeature.contrib.providers.flagd; import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.PLUGIN_PROPERTY_NAME; @@ -18,5 +18,5 @@ @IncludeEngines("cucumber") @SelectFile("test-harness/gherkin/config.feature") @ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") -public class RunConfigCucumberTest {} +@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps.config") +public class ConfigCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java index 0aa8f50d0..edf8b5f15 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -1,9 +1,9 @@ package dev.openfeature.contrib.providers.flagd.e2e; +import dev.openfeature.contrib.providers.flagd.Config; import java.io.File; import java.nio.file.Files; import java.util.List; -import org.apache.logging.log4j.util.Strings; import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; @@ -28,16 +28,25 @@ public class FlagdContainer extends GenericContainer { private String feature; public FlagdContainer() { - this(""); + super(generateContainerName()); + this.feature = feature; + this.addExposedPorts(8013, 8014, 8015, 8080); } - public FlagdContainer(String feature) { - super(generateContainerName(feature)); - this.withReuse(true); - this.feature = feature; - if (!"socket".equals(this.feature)) this.addExposedPorts(8013, 8014, 8015, 8016); + public int getPort(Config.Resolver resolver) { + switch (resolver) { + case RPC: + return getMappedPort(8013); + case IN_PROCESS: + return getMappedPort(8015); + default: + return 0; + } } + public String getLaunchpadUrl() { + return this.getHost() + ":" + this.getMappedPort(8080); + } /** * @return a {@link org.testcontainers.containers.GenericContainer} instance of envoy container using * flagd sync service as backend expose on port 9211 @@ -52,11 +61,8 @@ public static GenericContainer envoy() { .withNetworkAliases("envoy"); } - public static @NotNull String generateContainerName(String feature) { + public static @NotNull String generateContainerName() { String container = "ghcr.io/open-feature/flagd-testbed"; - if (!Strings.isBlank(feature)) { - container += "-" + feature; - } container += ":v" + version; return container; } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java index 133c1fb49..4a74400c4 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java @@ -2,10 +2,10 @@ import dev.openfeature.contrib.providers.flagd.e2e.State; -abstract class AbstractSteps { - State state; +public abstract class AbstractSteps { + protected State state; - public AbstractSteps(State state) { + protected AbstractSteps(State state) { this.state = state; } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java index b3ef4346e..f7427cfa2 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java @@ -14,7 +14,7 @@ * This class modifies the internals of the environment variables map with reflection. Warning: If * your {@link SecurityManager} does not allow modifications, it fails. */ -class EnvironmentVariableUtils { +public class EnvironmentVariableUtils { private EnvironmentVariableUtils() { // private constructor to prevent instantiation of utility class diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index 6dbd0c9ca..e03bbed23 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -47,14 +47,12 @@ public void a_stale_event_handler(String eventType) { @When("a {} event was fired") public void eventWasFired(String eventType) throws InterruptedException { - eventHandlerShouldBeExecutedWithin(eventType, 10000); - // we might be too fast in the execution - Thread.sleep(500); + eventHandlerShouldBeExecutedWithin(eventType, 8000); } @Then("the {} event handler should have been executed") public void eventHandlerShouldBeExecuted(String eventType) { - eventHandlerShouldBeExecutedWithin(eventType, 30000); + eventHandlerShouldBeExecutedWithin(eventType, 10000); } @Then("the {} event handler should have been executed within {int}ms") diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java index aab14a857..cd0d65afa 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java @@ -1,8 +1,6 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.FlagEvaluationDetails; @@ -70,15 +68,9 @@ public void the_variant_should_be(String variant) { } @Then("the flag should be part of the event payload") - @Then("the flag was modified") public void the_flag_was_modified() { - await().atMost(5000, MILLISECONDS).until(() -> state.events.stream() - .anyMatch(event -> event.type.equals("change") - && event.details.getFlagsChanged().contains(state.flag.name))); - state.lastEvent = state.events.stream() - .filter(event -> event.type.equals("change") - && event.details.getFlagsChanged().contains(state.flag.name)) - .findFirst(); + Event event = state.lastEvent.orElseThrow(AssertionError::new); + assertThat(event.details.getFlagsChanged()).contains(state.flag.name); } public class Flag { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index cf0e5ed0c..10e199047 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -1,16 +1,14 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; +import static io.restassured.RestAssured.when; + import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; -import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdProvider; import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.OpenFeatureAPI; -import eu.rekawek.toxiproxy.Proxy; -import eu.rekawek.toxiproxy.ToxiproxyClient; -import eu.rekawek.toxiproxy.model.ToxicDirection; import io.cucumber.java.After; import io.cucumber.java.AfterAll; import io.cucumber.java.Before; @@ -22,17 +20,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.HashMap; -import java.util.Map; import java.util.Objects; -import java.util.Timer; -import java.util.TimerTask; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.parallel.Isolated; import org.testcontainers.containers.BindMode; -import org.testcontainers.containers.Network; -import org.testcontainers.containers.ToxiproxyContainer; import org.testcontainers.shaded.org.apache.commons.io.FileUtils; @Isolated() @@ -40,11 +32,7 @@ public class ProviderSteps extends AbstractSteps { public static final int UNAVAILABLE_PORT = 9999; - static Map containers = new HashMap<>(); - public static Network network = Network.newNetwork(); - public static ToxiproxyContainer toxiproxy = - new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0").withNetwork(network); - public static ToxiproxyClient toxiproxyClient; + static FlagdContainer container; static Path sharedTempDir; @@ -52,65 +40,27 @@ public ProviderSteps(State state) { super(state); } - static String generateProxyName(Config.Resolver resolver, ProviderType providerType) { - return providerType + "-" + resolver; - } - @BeforeAll public static void beforeAll() throws IOException { - toxiproxy.start(); - toxiproxyClient = new ToxiproxyClient(toxiproxy.getHost(), toxiproxy.getControlPort()); - toxiproxyClient.createProxy( - generateProxyName(Config.Resolver.RPC, ProviderType.DEFAULT), "0.0.0.0:8666", "default:8013"); - - toxiproxyClient.createProxy( - generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.DEFAULT), "0.0.0.0:8667", "default:8015"); - toxiproxyClient.createProxy( - generateProxyName(Config.Resolver.RPC, ProviderType.SSL), "0.0.0.0:8668", "ssl:8013"); - toxiproxyClient.createProxy( - generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.SSL), "0.0.0.0:8669", "ssl:8015"); - - containers.put( - ProviderType.DEFAULT, new FlagdContainer().withNetwork(network).withNetworkAliases("default")); - containers.put( - ProviderType.SSL, new FlagdContainer("ssl").withNetwork(network).withNetworkAliases("ssl")); + sharedTempDir = Files.createDirectories( Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); - containers.put( - ProviderType.SOCKET, - new FlagdContainer("socket") - .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE)); + container = new FlagdContainer() + .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE); + ; } @AfterAll public static void afterAll() throws IOException { - - containers.forEach((name, container) -> container.stop()); + container.stop(); FileUtils.deleteDirectory(sharedTempDir.toFile()); - toxiproxyClient.reset(); - toxiproxy.stop(); } @Before public void before() throws IOException { - - toxiproxyClient.getProxies().forEach(proxy -> { - try { - proxy.toxics().getAll().forEach(toxic -> { - try { - toxic.remove(); - } catch (IOException e) { - log.debug("Failed to remove timout", e); - } - }); - } catch (IOException e) { - log.debug("Failed to remove timout", e); - } - }); - - containers.values().stream() - .filter(containers -> !containers.isRunning()) - .forEach(FlagdContainer::start); + if (!container.isRunning()) { + container.start(); + } } @After @@ -118,30 +68,10 @@ public void tearDown() { OpenFeatureAPI.getInstance().shutdown(); } - public int getPort(Config.Resolver resolver, ProviderType providerType) { - switch (resolver) { - case RPC: - switch (providerType) { - case DEFAULT: - return toxiproxy.getMappedPort(8666); - case SSL: - return toxiproxy.getMappedPort(8668); - } - case IN_PROCESS: - switch (providerType) { - case DEFAULT: - return toxiproxy.getMappedPort(8667); - case SSL: - return toxiproxy.getMappedPort(8669); - } - default: - throw new IllegalArgumentException("Unsupported resolver: " + resolver); - } - } - @Given("a {} flagd provider") - public void setupProvider(String providerType) throws IOException { - state.builder.deadline(500).keepAlive(0).retryGracePeriod(3); + public void setupProvider(String providerType) throws IOException, InterruptedException { + String flagdConfig = "default"; + state.builder.deadline(500).keepAlive(0).retryGracePeriod(1); boolean wait = true; switch (providerType) { case "unavailable": @@ -163,9 +93,10 @@ public void setupProvider(String providerType) throws IOException { String absolutePath = file.getAbsolutePath(); this.state.providerType = ProviderType.SSL; state.builder - .port(getPort(State.resolverType, state.providerType)) + .port(container.getPort(State.resolverType)) .tls(true) .certPath(absolutePath); + flagdConfig = "ssl"; break; case "offline": File flags = new File("test-harness/flags"); @@ -185,9 +116,15 @@ public void setupProvider(String providerType) throws IOException { default: this.state.providerType = ProviderType.DEFAULT; - state.builder.port(getPort(State.resolverType, state.providerType)); + state.builder.port(container.getPort(State.resolverType)); break; } + when().post("http://" + container.getLaunchpadUrl() + "/start?config={config}", flagdConfig) + .then() + .statusCode(200); + + // giving flagd a little time to start + Thread.sleep(100); FeatureProvider provider = new FlagdProvider(state.builder.resolverType(State.resolverType).build()); @@ -201,32 +138,22 @@ public void setupProvider(String providerType) throws IOException { } @When("the connection is lost for {int}s") - public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { + public void the_connection_is_lost_for(int seconds) throws InterruptedException { log.info("Timeout and wait for {} seconds", seconds); - String randomizer = RandomStringUtils.randomAlphanumeric(5); - String timeoutUpName = "restart-up-" + randomizer; - String timeoutDownName = "restart-down-" + randomizer; - Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); - proxy.toxics().timeout(timeoutDownName, ToxicDirection.DOWNSTREAM, seconds); - proxy.toxics().timeout(timeoutUpName, ToxicDirection.UPSTREAM, seconds); - - TimerTask task = new TimerTask() { - public void run() { - try { - proxy.toxics().get(timeoutUpName).remove(); - proxy.toxics().get(timeoutDownName).remove(); - } catch (IOException e) { - log.debug("Failed to remove timeout", e); - } - } - }; - Timer restartTimer = new Timer("Timer" + randomizer); - restartTimer.schedule(task, seconds * 1000L); + when().post("http://" + container.getLaunchpadUrl() + "/restart?seconds={seconds}", seconds) + .then() + .statusCode(200); + // we might be too fast in the execution + Thread.sleep(100); } - static FlagdContainer getContainer(ProviderType providerType) { - log.info("getting container for {}", providerType); - return containers.getOrDefault(providerType, containers.get(ProviderType.DEFAULT)); + @When("the flag was modified") + public void the_flag_was_modded() throws InterruptedException { + + when().post("http://" + container.getLaunchpadUrl() + "/change").then().statusCode(200); + + // we might be too fast in the execution + Thread.sleep(100); } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java similarity index 93% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java index 8e8ee44d6..2824dc12b 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java @@ -1,9 +1,12 @@ -package dev.openfeature.contrib.providers.flagd.e2e.steps; +package dev.openfeature.contrib.providers.flagd.e2e.steps.config; import static org.assertj.core.api.Assertions.assertThat; import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.e2e.State; +import dev.openfeature.contrib.providers.flagd.e2e.steps.AbstractSteps; +import dev.openfeature.contrib.providers.flagd.e2e.steps.EnvironmentVariableUtils; +import dev.openfeature.contrib.providers.flagd.e2e.steps.Utils; import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index fc7867922..7be078990 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit fc786792273b7984911dc3bcb7b47489f261ba57 +Subproject commit 7be07899096bb5cc78b0bd791b7f1701e3db99b5 From a95f61624c837e3d594b431c427d7d0318375dc4 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 4 Feb 2025 11:04:23 +0100 Subject: [PATCH 2/4] Attempt to fix flaky test Signed-off-by: christian.lutnik --- .../flagd/e2e/steps/ProviderSteps.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index 5af0bd429..d80a69a21 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; +import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdProvider; import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; import dev.openfeature.contrib.providers.flagd.e2e.State; @@ -24,6 +25,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.parallel.Isolated; +import org.junit.platform.suite.api.BeforeSuite; import org.testcontainers.containers.BindMode; import org.testcontainers.shaded.org.apache.commons.io.FileUtils; @@ -41,31 +43,27 @@ public ProviderSteps(State state) { } @BeforeAll - public static void beforeAll() throws IOException { + public static void beforeAll() { + State.resolverType = Config.Resolver.RPC; + } + @Before + public void before() throws IOException { + state.events.clear(); sharedTempDir = Files.createDirectories( Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); container = new FlagdContainer() .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE); - ; - } - - @AfterAll - public static void afterAll() throws IOException { - container.stop(); - FileUtils.deleteDirectory(sharedTempDir.toFile()); - } - - @Before - public void before() throws IOException { if (!container.isRunning()) { container.start(); } } @After - public void tearDown() { + public void tearDown() throws IOException { OpenFeatureAPI.getInstance().shutdown(); + container.stop(); + FileUtils.deleteDirectory(sharedTempDir.toFile()); } @Given("a {} flagd provider") From 7fb193ede3ebac98547e0e350af1a868261cbd82 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 5 Feb 2025 10:16:41 +0100 Subject: [PATCH 3/4] Attempt to fix flaky test v2 Signed-off-by: christian.lutnik --- .../providers/flagd/e2e/FlagdContainer.java | 3 -- .../providers/flagd/e2e/steps/EventSteps.java | 3 +- .../flagd/e2e/steps/ProviderSteps.java | 41 +++++++++++++++++-- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java index edf8b5f15..8fb83833a 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -25,11 +25,8 @@ public class FlagdContainer extends GenericContainer { } } - private String feature; - public FlagdContainer() { super(generateContainerName()); - this.feature = feature; this.addExposedPorts(8013, 8014, 8015, 8080); } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index e03bbed23..181d3c439 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -25,7 +25,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 {} ", eventType); + log.info("event tracked for {} at {} ms ", eventType, System.currentTimeMillis()%100_000); state.events.add(new Event(eventType, eventDetails)); }); } @@ -59,6 +59,7 @@ public void eventHandlerShouldBeExecuted(String eventType) { public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { log.info("waiting for eventtype: {}", eventType); await().atMost(ms, MILLISECONDS) + .pollInterval(10, MILLISECONDS) .until(() -> state.events.stream().anyMatch(event -> event.type.equals(eventType))); state.lastEvent = state.events.stream() .filter(event -> event.type.equals(eventType)) diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index d80a69a21..a4fb1edcb 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -42,6 +42,35 @@ public ProviderSteps(State state) { super(state); } + /* + @BeforeAll + public static void beforeAll() throws IOException { + State.resolverType = Config.Resolver.RPC; + sharedTempDir = Files.createDirectories( + Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); + container = new FlagdContainer() + .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE); + } + + @AfterAll + public static void afterAll() throws IOException { + container.stop(); + FileUtils.deleteDirectory(sharedTempDir.toFile()); + } + + @Before + public void before() throws IOException { + if (!container.isRunning()) { + container.start(); + } + } + + @After + public void tearDown() { + OpenFeatureAPI.getInstance().shutdown(); + } + */ + @BeforeAll public static void beforeAll() { State.resolverType = Config.Resolver.RPC; @@ -97,6 +126,7 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx flagdConfig = "ssl"; break; case "offline": + State.resolverType = Config.Resolver.IN_PROCESS; File flags = new File("test-harness/flags"); ObjectMapper objectMapper = new ObjectMapper(); Object merged = new Object(); @@ -127,17 +157,20 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx new FlagdProvider(state.builder.resolverType(State.resolverType).build()); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + String providerName = providerType + Math.random(); if (wait) { - api.setProviderAndWait(providerType, provider); + api.setProviderAndWait(providerName, provider); } else { - api.setProvider(providerType, provider); + api.setProvider(providerName, provider); } - this.state.client = api.getClient(providerType); + log.info("provider name: {}", providerName); + this.state.client = api.getClient(providerName); } @When("the connection is lost for {int}s") public void the_connection_is_lost_for(int seconds) throws InterruptedException { - log.info("Timeout and wait for {} seconds", seconds); + log.info("Timeout and wait for {} seconds starting at {} ms, should resume at {} ms", seconds, + System.currentTimeMillis() % 100_000, System.currentTimeMillis() % 100_000 + seconds * 1000L); when().post("http://" + container.getLaunchpadUrl() + "/restart?seconds={seconds}", seconds) .then() From f520a2924ed59fbe56a628dfb29ac7ffc00fdc64 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 5 Feb 2025 17:18:16 +0100 Subject: [PATCH 4/4] flaky tests attempt, changing it to stop flagd all the time, without stopping the container Signed-off-by: Simon Schrottner --- providers/flagd/spec | 2 +- .../providers/flagd/FlagdProvider.java | 2 +- .../flagd/resolver/common/ChannelMonitor.java | 7 +++- .../resolver/grpc/EventStreamObserver.java | 3 +- .../main/resources/simplelogger.properties | 3 ++ .../providers/flagd/FlagdProviderTest.java | 14 +++---- .../providers/flagd/e2e/RunInProcessTest.java | 2 + .../contrib/providers/flagd/e2e/State.java | 2 + .../providers/flagd/e2e/steps/EventSteps.java | 7 ++-- .../flagd/e2e/steps/ProviderSteps.java | 42 +++++-------------- .../test/resources/simplelogger.properties | 4 ++ providers/flagd/test-harness | 2 +- 12 files changed, 42 insertions(+), 48 deletions(-) create mode 100644 providers/flagd/src/main/resources/simplelogger.properties create mode 100644 providers/flagd/src/test/resources/simplelogger.properties diff --git a/providers/flagd/spec b/providers/flagd/spec index 8d6eeb324..5b0706598 160000 --- a/providers/flagd/spec +++ b/providers/flagd/spec @@ -1 +1 @@ -Subproject commit 8d6eeb3247600f6f66ffc92afa50ebde75b4d3ce +Subproject commit 5b070659853062262e618c74da5b640555f9ae18 diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java index bbf7674f1..e899e7005 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java @@ -209,7 +209,7 @@ EvaluationContext getEnrichedContext() { private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { synchronized (eventsLock) { - log.info("FlagdProviderEvent: {}", flagdProviderEvent); + log.info("FlagdProviderEvent: {}", flagdProviderEvent.getEvent()); eventsLock.syncMetadata = flagdProviderEvent.getSyncMetadata(); if (flagdProviderEvent.getSyncMetadata() != null) { eventsLock.enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java index 1b201d640..7e27da34b 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java @@ -47,8 +47,11 @@ public static void monitorChannelState( log.debug("onConnectionLost is null"); } } - // Re-register the state monitor to watch for the next state transition. - monitorChannelState(currentState, channel, onConnectionReady, onConnectionLost); + if (currentState != ConnectivityState.SHUTDOWN) { + log.debug("shutting down grpc channel"); + // Re-register the state monitor to watch for the next state transition. + monitorChannelState(currentState, channel, onConnectionReady, onConnectionLost); + } }); } diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java index 8b8886bf8..c1ddca3e9 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java @@ -64,6 +64,7 @@ public void onCompleted() {} * @param value the event stream response containing configuration change data */ private void handleConfigurationChangeEvent(EventStreamResponse value) { + log.debug("Received provider change event"); List changedFlags = new ArrayList<>(); Map data = value.getData().getFieldsMap(); @@ -80,7 +81,7 @@ private void handleConfigurationChangeEvent(EventStreamResponse value) { * Handles provider readiness events by clearing the cache (if enabled) and notifying listeners of readiness. */ private void handleProviderReadyEvent() { - log.info("Received provider ready event"); + log.debug("Received provider ready event"); onReady.accept(new FlagdProviderEvent(ProviderEvent.PROVIDER_READY)); } } diff --git a/providers/flagd/src/main/resources/simplelogger.properties b/providers/flagd/src/main/resources/simplelogger.properties new file mode 100644 index 000000000..d9d489e82 --- /dev/null +++ b/providers/flagd/src/main/resources/simplelogger.properties @@ -0,0 +1,3 @@ +org.org.slf4j.simpleLogger.defaultLogLevel=debug + +io.grpc.level=trace diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java index 0aa226fcf..2d94b3a8d 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java @@ -46,7 +46,6 @@ import dev.openfeature.sdk.Reason; import dev.openfeature.sdk.Structure; import dev.openfeature.sdk.Value; -import io.cucumber.java.AfterAll; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.ArrayList; @@ -60,7 +59,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Function; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.MockedConstruction; @@ -90,15 +90,15 @@ class FlagdProviderTest { .build(); private static final String STRING_VALUE = "hi!"; - private static OpenFeatureAPI api; + private OpenFeatureAPI api; - @BeforeAll - public static void init() { + @BeforeEach + public void init() { api = OpenFeatureAPI.getInstance(); } - @AfterAll - public static void cleanUp() { + @AfterEach + public void cleanUp() { api.shutdown(); } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index e0edef240..0c0b32420 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -6,6 +6,7 @@ import dev.openfeature.contrib.providers.flagd.Config; import org.apache.logging.log4j.core.config.Order; +import org.junit.jupiter.api.parallel.Isolated; import org.junit.platform.suite.api.BeforeSuite; import org.junit.platform.suite.api.ConfigurationParameter; import org.junit.platform.suite.api.ExcludeTags; @@ -30,6 +31,7 @@ @IncludeTags("in-process") @ExcludeTags({"unixsocket", "targetURI"}) @Testcontainers +@Isolated public class RunInProcessTest { @BeforeSuite diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java index 4ecab84e5..37507020f 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java @@ -6,6 +6,7 @@ import dev.openfeature.contrib.providers.flagd.e2e.steps.FlagSteps; import dev.openfeature.contrib.providers.flagd.e2e.steps.ProviderType; import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.MutableContext; import java.util.LinkedList; @@ -15,6 +16,7 @@ public class State { public ProviderType providerType; public Client client; + public FeatureProvider provider; public List events = new LinkedList<>(); public Optional lastEvent; public FlagSteps.Flag flag; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index 181d3c439..de2abcb79 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -25,7 +25,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 for {} at {} ms ", eventType, System.currentTimeMillis() % 100_000); state.events.add(new Event(eventType, eventDetails)); }); } @@ -58,12 +58,13 @@ public void eventHandlerShouldBeExecuted(String eventType) { @Then("the {} event handler should have been executed within {int}ms") public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { log.info("waiting for eventtype: {}", eventType); - await().atMost(ms, MILLISECONDS) + await().alias("waiting for eventtype " + eventType) + .atMost(ms, MILLISECONDS) .pollInterval(10, MILLISECONDS) .until(() -> state.events.stream().anyMatch(event -> event.type.equals(eventType))); state.lastEvent = state.events.stream() .filter(event -> event.type.equals(eventType)) .findFirst(); - state.events.removeIf(event -> event.type.equals(eventType)); + state.events.clear(); } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index a4fb1edcb..0f312ff9d 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -25,7 +25,6 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.parallel.Isolated; -import org.junit.platform.suite.api.BeforeSuite; import org.testcontainers.containers.BindMode; import org.testcontainers.shaded.org.apache.commons.io.FileUtils; @@ -42,7 +41,6 @@ public ProviderSteps(State state) { super(state); } - /* @BeforeAll public static void beforeAll() throws IOException { State.resolverType = Config.Resolver.RPC; @@ -66,33 +64,13 @@ public void before() throws IOException { } @After - public void tearDown() { - OpenFeatureAPI.getInstance().shutdown(); - } - */ - - @BeforeAll - public static void beforeAll() { - State.resolverType = Config.Resolver.RPC; - } - - @Before - public void before() throws IOException { - state.events.clear(); - sharedTempDir = Files.createDirectories( - Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); - container = new FlagdContainer() - .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE); - if (!container.isRunning()) { - container.start(); + public void tearDown() throws InterruptedException { + if (state.client != null) { + when().post("http://" + container.getLaunchpadUrl() + "/stop") + .then() + .statusCode(200); } - } - - @After - public void tearDown() throws IOException { OpenFeatureAPI.getInstance().shutdown(); - container.stop(); - FileUtils.deleteDirectory(sharedTempDir.toFile()); } @Given("a {} flagd provider") @@ -167,16 +145,16 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx this.state.client = api.getClient(providerName); } + @When("the connection is lost") + public void the_connection_is_lost() throws InterruptedException { + when().post("http://" + container.getLaunchpadUrl() + "/stop").then().statusCode(200); + } + @When("the connection is lost for {int}s") public void the_connection_is_lost_for(int seconds) throws InterruptedException { - log.info("Timeout and wait for {} seconds starting at {} ms, should resume at {} ms", seconds, - System.currentTimeMillis() % 100_000, System.currentTimeMillis() % 100_000 + seconds * 1000L); - when().post("http://" + container.getLaunchpadUrl() + "/restart?seconds={seconds}", seconds) .then() .statusCode(200); - // we might be too fast in the execution - Thread.sleep(100); } @When("the flag was modified") diff --git a/providers/flagd/src/test/resources/simplelogger.properties b/providers/flagd/src/test/resources/simplelogger.properties new file mode 100644 index 000000000..d2ca1bbdc --- /dev/null +++ b/providers/flagd/src/test/resources/simplelogger.properties @@ -0,0 +1,4 @@ +org.org.slf4j.simpleLogger.defaultLogLevel=debug +org.slf4j.simpleLogger.showDateTime= + +io.grpc.level=trace diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index 4592dbca0..1252f5145 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit 4592dbca0f4de2d32e01ede6092303370e82f992 +Subproject commit 1252f5145324cfd700c4e5dc35130551904a8f05