From f520a2924ed59fbe56a628dfb29ac7ffc00fdc64 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 5 Feb 2025 17:18:16 +0100 Subject: [PATCH] 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