diff --git a/.gitmodules b/.gitmodules index 3994607a6..a2d25a69c 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 = v3.0.1 + branch = feat/add-env-var-tag [submodule "providers/flagd/spec"] path = providers/flagd/spec url = https://github.com/open-feature/spec.git diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java index f031091da..0bb64dd5c 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java @@ -1,6 +1,7 @@ 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.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.PLUGIN_PROPERTY_NAME; import org.junit.jupiter.api.Order; @@ -17,6 +18,11 @@ @Suite @IncludeEngines("cucumber") @SelectFile("test-harness/gherkin/config.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") +@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "summary") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps.config") +// Config scenarios read System env vars in FlagdOptions.build() and some scenarios also +// mutate them. Parallel execution causes env-var races (e.g. FLAGD_PORT=3456 leaking into +// a "Default Config" scenario that expects 8015). Since the entire suite runs in <0.4s, +// parallelism offers no benefit here — run sequentially for correctness. +@ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "false") public class ConfigCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerEntry.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerEntry.java new file mode 100644 index 000000000..820f80869 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerEntry.java @@ -0,0 +1,50 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.RandomStringUtils; +import org.testcontainers.containers.ComposeContainer; +import org.testcontainers.containers.wait.strategy.Wait; + +/** A single pre-warmed Docker Compose stack (flagd + envoy) and its associated temp directory. */ +public class ContainerEntry { + + public static final int FORBIDDEN_PORT = 9212; + + public final ComposeContainer container; + public final Path tempDir; + + private ContainerEntry(ComposeContainer container, Path tempDir) { + this.container = container; + this.tempDir = tempDir; + } + + /** Start a new container entry. Blocks until all services are ready. */ + public static ContainerEntry start() throws IOException { + Path tempDir = Files.createDirectories( + Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); + + ComposeContainer container = new ComposeContainer(new File("test-harness/docker-compose.yaml")) + .withEnv("FLAGS_DIR", tempDir.toAbsolutePath().toString()) + .withExposedService("flagd", 8013, Wait.forListeningPort()) + .withExposedService("flagd", 8015, Wait.forListeningPort()) + .withExposedService("flagd", 8080, Wait.forListeningPort()) + .withExposedService("envoy", 9211, Wait.forListeningPort()) + .withExposedService("envoy", FORBIDDEN_PORT, Wait.forListeningPort()) + .withStartupTimeout(Duration.ofSeconds(45)); + container.start(); + + return new ContainerEntry(container, tempDir); + } + + /** Stop the container and clean up the temp directory. */ + public void stop() throws IOException { + container.stop(); + FileUtils.deleteDirectory(tempDir.toFile()); + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerPool.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerPool.java new file mode 100644 index 000000000..685215d98 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerPool.java @@ -0,0 +1,106 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import lombok.extern.slf4j.Slf4j; + +/** + * A pool of pre-warmed {@link ContainerEntry} instances. + * + *

All containers are started in parallel during {@link #initialize()}, paying the ~45s Docker + * Compose startup cost only once. Scenarios borrow a container via {@link #acquire()} and return + * it via {@link #release(ContainerEntry)} after teardown, allowing the next scenario to reuse it + * immediately without any cold-start overhead. + * + *

Pool size is controlled by the system property {@code flagd.e2e.pool.size} (default: 2). + * + *

Multiple test classes may share the same JVM fork (Surefire {@code reuseForks=true}). Each + * class calls {@link #initialize()} and {@link #shutdown()} once. A reference counter ensures + * that containers are only started on the first {@code initialize()} call and only stopped when + * the last {@code shutdown()} call is made, preventing one class from destroying containers that + * are still in use by another class running concurrently in the same JVM. + */ +@Slf4j +public class ContainerPool { + + private static final int POOL_SIZE = Integer.getInteger( + "flagd.e2e.pool.size", Math.min(Runtime.getRuntime().availableProcessors(), 4)); + + private static final BlockingQueue pool = new LinkedBlockingQueue<>(); + private static final List all = new ArrayList<>(); + private static final java.util.concurrent.atomic.AtomicInteger refCount = + new java.util.concurrent.atomic.AtomicInteger(0); + + public static void initialize() throws Exception { + if (refCount.getAndIncrement() > 0) { + log.info("Container pool already initialized (refCount={}), reusing existing pool.", refCount.get()); + return; + } + log.info("Starting container pool of size {}...", POOL_SIZE); + ExecutorService executor = Executors.newFixedThreadPool(POOL_SIZE); + try { + List> futures = new ArrayList<>(); + for (int i = 0; i < POOL_SIZE; i++) { + futures.add(executor.submit(ContainerEntry::start)); + } + for (Future future : futures) { + ContainerEntry entry = future.get(); + pool.add(entry); + all.add(entry); + } + } catch (Exception e) { + // Stop any containers that started successfully before the failure + all.forEach(entry -> { + try { + entry.stop(); + } catch (IOException suppressed) { + e.addSuppressed(suppressed); + } + }); + pool.clear(); + all.clear(); + refCount.decrementAndGet(); + throw e; + } finally { + executor.shutdown(); + } + log.info("Container pool ready ({} containers).", POOL_SIZE); + } + + public static void shutdown() { + int remaining = refCount.decrementAndGet(); + if (remaining > 0) { + log.info("Container pool still in use by {} class(es), deferring shutdown.", remaining); + return; + } + log.info("Last shutdown call — stopping all containers."); + all.forEach(entry -> { + try { + entry.stop(); + } catch (IOException e) { + log.warn("Error stopping container entry", e); + } + }); + pool.clear(); + all.clear(); + } + + /** + * Borrow a container from the pool, blocking until one becomes available. + * The caller MUST call {@link #release(ContainerEntry)} when done. + */ + public static ContainerEntry acquire() throws InterruptedException { + return pool.take(); + } + + /** Return a container to the pool so the next scenario can use it. */ + public static void release(ContainerEntry entry) { + pool.add(entry); + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java index edea71850..689eb0d41 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java @@ -24,7 +24,7 @@ @SelectDirectories("test-harness/gherkin") // if you want to run just one feature file, use the following line instead of @SelectDirectories // @SelectFile("test-harness/gherkin/connection.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") +@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "summary") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") @ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory") @IncludeTags("file") 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 385d4e83c..098316261 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 @@ -24,7 +24,7 @@ @SelectDirectories("test-harness/gherkin") // if you want to run just one feature file, use the following line instead of @SelectDirectories // @SelectFile("test-harness/gherkin/selector.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") +@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "summary") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") @ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory") @IncludeTags("in-process") diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java index d98fb5986..4e64f79c6 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java @@ -24,7 +24,7 @@ @SelectDirectories("test-harness/gherkin") // if you want to run just one feature file, use the following line instead of @SelectDirectories // @SelectFile("test-harness/gherkin/rpc-caching.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") +@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "summary") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") @ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory") @IncludeTags({"rpc"}) 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 2d3a227a4..15f555e46 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 @@ -16,6 +16,11 @@ public class State { public ProviderType providerType; public Client client; public FeatureProvider provider; + /** The domain name under which this scenario's provider is registered with OpenFeatureAPI. */ + public String providerName; + /** The container borrowed from {@link ContainerPool} for this scenario. */ + public ContainerEntry containerEntry; + public ConcurrentLinkedQueue events = new ConcurrentLinkedQueue<>(); 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 dc11bbb6a..6e8222b19 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 @@ -60,9 +60,18 @@ public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { .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.clear(); + // Drain all events up to and including the first match. This ensures that + // older events (e.g. a READY from before a disconnect) cannot satisfy a + // later assertion that expects a *new* event of the same type, while still + // preserving events that arrived *after* the match for subsequent steps. + Event matched = null; + while (!state.events.isEmpty()) { + Event head = state.events.poll(); + if (head != null && head.type.equals(eventType)) { + matched = head; + break; + } + } + state.lastEvent = java.util.Optional.ofNullable(matched); } } 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 90d082292..b747672cc 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 @@ -6,9 +6,12 @@ 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.ContainerEntry; +import dev.openfeature.contrib.providers.flagd.e2e.ContainerPool; import dev.openfeature.contrib.providers.flagd.e2e.ContainerUtil; import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.NoOpProvider; import dev.openfeature.sdk.OpenFeatureAPI; import dev.openfeature.sdk.ProviderState; import io.cucumber.java.After; @@ -18,66 +21,60 @@ import io.cucumber.java.en.Then; import io.cucumber.java.en.When; import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.time.Duration; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.io.FileUtils; -import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.testcontainers.containers.ComposeContainer; -import org.testcontainers.containers.wait.strategy.Wait; @Slf4j public class ProviderSteps extends AbstractSteps { public static final int UNAVAILABLE_PORT = 9999; - public static final int FORBIDDEN_PORT = 9212; - static ComposeContainer container; - - static Path sharedTempDir; public ProviderSteps(State state) { super(state); } @BeforeAll - public static void beforeAll() throws IOException { - sharedTempDir = Files.createDirectories( - Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); - container = new ComposeContainer(new File("test-harness/docker-compose.yaml")) - .withEnv("FLAGS_DIR", sharedTempDir.toAbsolutePath().toString()) - .withExposedService("flagd", 8013, Wait.forListeningPort()) - .withExposedService("flagd", 8015, Wait.forListeningPort()) - .withExposedService("flagd", 8080, Wait.forListeningPort()) - .withExposedService("envoy", 9211, Wait.forListeningPort()) - .withExposedService("envoy", FORBIDDEN_PORT, Wait.forListeningPort()) - .withStartupTimeout(Duration.ofSeconds(45)); - container.start(); + public static void beforeAll() throws Exception { + ContainerPool.initialize(); } @AfterAll - public static void afterAll() throws IOException { - container.stop(); - FileUtils.deleteDirectory(sharedTempDir.toFile()); + public static void afterAll() { + ContainerPool.shutdown(); } @After public void tearDown() { - if (state.client != null) { - when().post("http://" + ContainerUtil.getLaunchpadUrl(container) + "/stop") - .then() - .statusCode(200); + if (state.containerEntry != null) { + if (state.client != null) { + when().post("http://" + ContainerUtil.getLaunchpadUrl(state.containerEntry.container) + "/stop") + .then() + .statusCode(200); + } + ContainerPool.release(state.containerEntry); + state.containerEntry = null; + } + // Replace the domain provider with a NoOp through the SDK lifecycle so the SDK + // properly calls detachEventProvider (nulls onEmit) and shuts down the emitter + // executor — neither of which happens when calling provider.shutdown() directly. + if (state.providerName != null) { + OpenFeatureAPI.getInstance().setProvider(state.providerName, new NoOpProvider()); } - OpenFeatureAPI.getInstance().shutdown(); } @Given("a {} flagd provider") public void setupProvider(String providerType) throws InterruptedException { + state.containerEntry = ContainerPool.acquire(); + ComposeContainer container = state.containerEntry.container; + String flagdConfig = "default"; - state.builder.deadline(1000).keepAlive(0).retryGracePeriod(2); + state.builder + .deadline(1000) + .keepAlive(0) + .retryGracePeriod(2) + .retryBackoffMs(500) + .retryBackoffMaxMs(2000); boolean wait = true; switch (providerType) { @@ -85,25 +82,26 @@ public void setupProvider(String providerType) throws InterruptedException { this.state.providerType = ProviderType.SOCKET; state.builder.port(UNAVAILABLE_PORT); if (State.resolverType == Config.Resolver.FILE) { - state.builder.offlineFlagSourcePath("not-existing"); } wait = false; break; case "forbidden": - state.builder.port(container.getServicePort("envoy", FORBIDDEN_PORT)); + state.builder.port(container.getServicePort("envoy", ContainerEntry.FORBIDDEN_PORT)); wait = false; break; case "socket": this.state.providerType = ProviderType.SOCKET; - String socketPath = - sharedTempDir.resolve("socket.sock").toAbsolutePath().toString(); + String socketPath = state.containerEntry + .tempDir + .resolve("socket.sock") + .toAbsolutePath() + .toString(); state.builder.socketPath(socketPath); state.builder.port(UNAVAILABLE_PORT); break; case "ssl": String path = "test-harness/ssl/custom-root-cert.crt"; - File file = new File(path); String absolutePath = file.getAbsolutePath(); this.state.providerType = ProviderType.SSL; @@ -115,12 +113,10 @@ public void setupProvider(String providerType) throws InterruptedException { 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()); @@ -135,10 +131,10 @@ public void setupProvider(String providerType) throws InterruptedException { case "stable": this.state.providerType = ProviderType.DEFAULT; if (State.resolverType == Config.Resolver.FILE) { - state.builder .port(UNAVAILABLE_PORT) - .offlineFlagSourcePath(sharedTempDir + .offlineFlagSourcePath(state.containerEntry + .tempDir .resolve("allFlags.json") .toAbsolutePath() .toString()); @@ -174,26 +170,31 @@ public void setupProvider(String providerType) throws InterruptedException { } else { api.setProvider(providerName, provider); } + this.state.provider = provider; + this.state.providerName = providerName; this.state.client = api.getClient(providerName); } @When("the connection is lost") public void the_connection_is_lost() { - when().post("http://" + ContainerUtil.getLaunchpadUrl(container) + "/stop") + when().post("http://" + ContainerUtil.getLaunchpadUrl(state.containerEntry.container) + "/stop") .then() .statusCode(200); } @When("the connection is lost for {int}s") public void the_connection_is_lost_for(int seconds) { - when().post("http://" + ContainerUtil.getLaunchpadUrl(container) + "/restart?seconds={seconds}", seconds) + when().post( + "http://" + ContainerUtil.getLaunchpadUrl(state.containerEntry.container) + + "/restart?seconds={seconds}", + seconds) .then() .statusCode(200); } @When("the flag was modified") public void the_flag_was_modded() { - when().post("http://" + ContainerUtil.getLaunchpadUrl(container) + "/change") + when().post("http://" + ContainerUtil.getLaunchpadUrl(state.containerEntry.container) + "/change") .then() .statusCode(200); } diff --git a/providers/flagd/src/test/resources/junit-platform.properties b/providers/flagd/src/test/resources/junit-platform.properties new file mode 100644 index 000000000..0d0be24ee --- /dev/null +++ b/providers/flagd/src/test/resources/junit-platform.properties @@ -0,0 +1,19 @@ +# Enable parallel scenario execution within each suite runner. +# Each scenario borrows its own ContainerEntry from ContainerPool, so +# concurrent scenarios are fully isolated — no shared flagd process. +cucumber.execution.parallel.enabled=true +# Dynamic strategy scales with available CPUs (factor=1.0 → 1 thread per core). +# ContainerPool caps at min(availableProcessors, 4) containers so Docker isn't +# overwhelmed; extra threads simply block waiting for a free container. +# Override pool size via -Dflagd.e2e.pool.size=N if needed. +cucumber.execution.parallel.config.strategy=dynamic +cucumber.execution.parallel.config.dynamic.factor=1 +# Scenarios tagged @env-var mutate System env vars globally. +# Serialise them behind an exclusive resource lock so concurrent scenarios +# don't clobber each other's environment variable state. +cucumber.execution.exclusive-resources.env-var.read-write=ENV_VARS +# Scenarios tagged @grace involve container restart + reconnection timing. +# Running two concurrent restarts under parallel load can push the +# reconnection past the 12-second EVENT_TIMEOUT_MS threshold. Serialise +# them so each restart has the full machine resources to itself. +cucumber.execution.exclusive-resources.grace.read-write=CONTAINER_RESTART diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index 3bff4b7ea..a2dc5ebbb 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit 3bff4b7eaee0efc8cfe60e0ef6fbd77441b370e6 +Subproject commit a2dc5ebbb45f171e8f4d10031e48a3a7e637a1cf