From 1b00e23c59464e69d00ae99d0fcf23905289e7c3 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 15:12:11 +0200 Subject: [PATCH 01/10] perf(tck): lease a warmed backend per scenario Off by default. With no openfeature.tck.pool.size set, every object factory starts one backend and keeps it for as long as its thread lives, exactly as before. Set it and the suite warms that many backends up front, hands one to each scenario, and takes it back afterwards. The object factory is where this fits with no indirection: Cucumber calls start() and stop() around every scenario, so the lease brackets the scenario already, and the leased harness and control go straight into the container the steps resolve from. prepareScenario then runs on the leased backend in @Before(order = 10), which keeps the contract that a scenario starts from the baseline -- it only widens who that protects, since a backend now moves between features and suites rather than staying on one thread. The size is a cap on how many backends stand at once, which is the reason to prefer it over a thread count: fixed.parallelism is a target rather than a limit, and a suite whose steps block has been seen growing to eighteen threads, each of which would otherwise start a backend. Surplus threads queue on acquire() instead. Backends warm concurrently, because four Compose startups in sequence would be spent before the first scenario. A backend whose own preparation fails is dropped rather than passed on: the pool shrinks loudly instead of quietly becoming a set of broken backends that fail everything that draws them. Signed-off-by: Simon Schrottner --- .../tck/internal/steps/ProviderSteps.java | 10 +- .../tools/tck/internal/steps/StackPool.java | 199 ++++++++++++++++++ .../tck/internal/steps/TckLifecycle.java | 7 +- .../tck/internal/steps/TckObjectFactory.java | 16 +- 4 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java index 8bae27b35c..e7354aba77 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java @@ -74,7 +74,15 @@ public void gateOnCapabilities(Scenario scenario) { */ @Before(order = 10) public void prepareBackend() { - backend().prepareScenario(); + try { + backend().prepareScenario(); + } catch (RuntimeException e) { + // Said here because this is where the breakage is visible: a backend that cannot be + // brought to the baseline must not be handed to the next scenario, and the object + // factory only learns that a scenario ended, never why. + StackPool.markLeaseBroken(); + throw e; + } } /** diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java new file mode 100644 index 0000000000..0edc665da8 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java @@ -0,0 +1,199 @@ +package dev.openfeature.contrib.tools.tck.internal.steps; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Backends warmed once and leased to one scenario at a time. + * + *

Off by default: with no size set, every {@link TckObjectFactory} starts one backend and keeps + * it for as long as its thread lives, which is the behaviour every adoption has had so far. Set + * {@value #SIZE_PROPERTY} and the suite instead starts that many backends up front, hands one to + * each scenario, and takes it back afterwards. + * + *

The size is the cap on how many backends stand at once. That is the reason to + * prefer it over a thread count: {@code cucumber.execution.parallel.config.fixed.parallelism} is a + * target rather than a limit, and a suite whose steps block has been observed growing to eighteen + * threads -- each of which would otherwise start a backend of its own. Here the surplus threads + * queue on {@link #acquire()} instead, and the number of containers is whatever was declared. + * + *

A scenario receives a backend that the next {@code prepareScenario} has cleaned rather than one + * nobody has touched, so the pool does not weaken the contract that every scenario starts from the + * baseline -- it widens who it protects, because a backend now moves between scenarios of different + * features and suites rather than staying on one thread. + */ +final class StackPool { + + /** How many backends to warm; absent or zero keeps one backend per thread. */ + static final String SIZE_PROPERTY = "openfeature.tck.pool.size"; + + /** + * How long {@link #acquire()} waits before calling a lease leak a leak. + * + *

Every lease is returned by {@link TckObjectFactory#stop()}, which Cucumber calls after the + * scenario's own hooks, so a scenario waiting this long means a lease was lost rather than that + * the suite is busy. Blocking forever would hang the build with no explanation. + */ + private static final long ACQUIRE_TIMEOUT_MINUTES = 10; + + private static final Logger log = LoggerFactory.getLogger(StackPool.class); + + private static final BlockingQueue idle = new LinkedBlockingQueue<>(); + + /** The leases handed out but not yet returned, so a broken one can be named in a message. */ + private static final ThreadLocal leaseBroken = ThreadLocal.withInitial(() -> false); + + private static volatile int size; + + private StackPool() { + // warmed and drained by TckLifecycle, leased by TckObjectFactory + } + + /** + * Whether scenarios lease backends from this pool. + * + * @return true once a suite has warmed a pool of at least one backend + */ + static boolean enabled() { + return size > 0; + } + + /** + * Starts the requested number of backends, concurrently, or does nothing when none was asked + * for. + * + *

Concurrently because the startups are the cost the pool exists to pay once: starting four + * backends one after another would spend four times a Compose startup before the first scenario + * runs. + * + * @param start supplies one started backend, as the per-thread path does + * @throws IllegalStateException if any backend fails to start, after stopping the ones that did + */ + static void warm(Supplier start) { + int requested = requestedSize(); + if (requested <= 0) { + size = 0; + return; + } + + log.info("Warming {} backends ({}={})", requested, SIZE_PROPERTY, requested); + ExecutorService starters = Executors.newFixedThreadPool(requested); + List> futures = new ArrayList<>(); + try { + for (int i = 0; i < requested; i++) { + futures.add(starters.submit(start::get)); + } + List warmed = new ArrayList<>(); + RuntimeException failure = null; + for (Future future : futures) { + try { + warmed.add(future.get()); + } catch (ExecutionException e) { + RuntimeException cause = e.getCause() instanceof RuntimeException + ? (RuntimeException) e.getCause() + : new IllegalStateException(e.getCause()); + failure = failure == null ? cause : failure; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("interrupted while warming the backend pool", e); + } + } + if (failure != null) { + // the ones that came up are registered for endSuite(), which stops them; the suite + // fails here rather than letting scenarios queue for backends that will never arrive + throw new IllegalStateException( + "only " + warmed.size() + " of " + requested + " backends started", failure); + } + idle.addAll(warmed); + size = requested; + } finally { + starters.shutdownNow(); + } + } + + /** + * Takes a backend for one scenario, waiting while every one of them is in use. + * + * @return the leased backend + */ + static TckLifecycle acquire() { + leaseBroken.set(false); + try { + TckLifecycle leased = idle.poll(ACQUIRE_TIMEOUT_MINUTES, TimeUnit.MINUTES); + if (leased == null) { + throw new IllegalStateException("no backend became free within " + ACQUIRE_TIMEOUT_MINUTES + + " minutes, with " + size + " in the pool (" + SIZE_PROPERTY + + "); a lease was not returned"); + } + return leased; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("interrupted while waiting for a backend", e); + } + } + + /** + * Returns a leased backend, unless the scenario reported it broken. + * + *

A backend whose own preparation failed is dropped rather than handed to the next scenario: + * returning it would spread one failure across every scenario that drew it, and the pool would + * quietly become a set of broken backends that fail everything. Shrinking is loud and the + * remaining backends still work; the suite fails when the last one goes. + * + * @param lifecycle the backend to return + */ + static void release(TckLifecycle lifecycle) { + if (Boolean.TRUE.equals(leaseBroken.get())) { + leaseBroken.set(false); + log.warn("Dropping a backend whose preparation failed; {} of {} left in the pool", idle.size(), size); + return; + } + idle.add(lifecycle); + } + + /** + * Marks the backend this thread holds as unfit to pass on. + * + *

Called where the breakage is visible -- the step that prepares the backend -- because + * {@link TckObjectFactory#stop()} only knows that a scenario ended, not why. + */ + static void markLeaseBroken() { + if (enabled()) { + leaseBroken.set(true); + } + } + + /** Forgets every lease; the backends themselves are stopped by {@code endSuite()}. */ + static void drain() { + idle.clear(); + size = 0; + } + + private static int requestedSize() { + String raw = System.getProperty(SIZE_PROPERTY); + if (raw == null || raw.trim().isEmpty()) { + return 0; + } + try { + int parsed = Integer.parseInt(raw.trim()); + if (parsed < 0) { + throw new IllegalArgumentException( + SIZE_PROPERTY + " is " + raw + "; a pool cannot hold fewer than no backends."); + } + return parsed; + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + SIZE_PROPERTY + " is '" + raw + "', which is not a number of backends.", e); + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java index b3f094f80d..e650098b46 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java @@ -51,12 +51,17 @@ static void beginSuite() { AdopterTags.requireValid(harness.getClass()); startFailure.set(null); suiteRunning = true; + // after the declarations are validated, so a suite that cannot run does not spend four + // Compose startups finding that out + StackPool.warm(() -> start(discoverHarness())); } - /** Stops every lifecycle this suite started. */ + /** Stops every lifecycle this suite started, pooled or per-thread. */ static void endSuite() { suiteRunning = false; startFailure.set(null); + // the pooled backends are in `started` too, so dropping the leases is all this needs + StackPool.drain(); List toStop; synchronized (started) { toStop = new ArrayList<>(started); diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java index 046e31037d..da01718b60 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java @@ -23,6 +23,9 @@ public final class TckObjectFactory implements ObjectFactory { private TckLifecycle lifecycle; + /** The pooled backend this scenario holds, or null when backends are per-thread. */ + private TckLifecycle leased; + @Override public boolean addClass(Class glueClass) { if (isInstantiable(glueClass) && glue.add(glueClass)) { @@ -60,7 +63,12 @@ private static boolean isInstantiable(Class candidate) { @Override public void start() { pico = new PicoBuilder().withCaching().build(); - if (lifecycle == null) { + if (StackPool.enabled()) { + // one backend per scenario, taken from the warmed pool and returned in stop(); the + // pool's size, not this thread's existence, decides how many stand at once + leased = StackPool.acquire(); + lifecycle = leased; + } else if (lifecycle == null) { // empty when a launcher selects only the extension glue and never reaches @BeforeAll lifecycle = TckLifecycle.startForExecutingSuite().orElse(null); } @@ -76,6 +84,12 @@ public void start() { @Override public void stop() { + if (leased != null) { + // after the scenario's own hooks, so a backend is only returned once nothing is using it + StackPool.release(leased); + leased = null; + lifecycle = null; + } if (pico != null) { pico.stop(); pico.dispose(); From e1422cd5c5de5e08207bf068be301b8881257d59 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 17:21:37 +0200 Subject: [PATCH 02/10] test: scope flagd's parallel settings to the suites that want them junit-platform.properties sat in test resources and so applied to every suite in the module. Every setting in it was about the legacy e2e suites -- their ContainerPool, their @env-var and @grace tags -- but the Provider TCK suites inherited it too, which is why ProviderTckTest had to pin parallelism off to defend itself. A @ConfigurationParameter is part of the discovery request and outranks both -D and the properties file, so that pin also made parallelism unreachable for anyone running a TCK suite. Moving the settings onto RunRpcTest, RunInProcessTest and RunFileTest gives each suite the configuration it asked for and lets the TCK suites be driven from the command line. Signed-off-by: Simon Schrottner --- .../providers/flagd/e2e/RunFileTest.java | 13 ++++++++++++ .../providers/flagd/e2e/RunInProcessTest.java | 13 ++++++++++++ .../providers/flagd/e2e/RunRpcTest.java | 13 ++++++++++++ .../test/resources/junit-platform.properties | 21 ------------------- .../contrib/tools/tck/ProviderTckTest.java | 1 - 5 files changed, 39 insertions(+), 22 deletions(-) delete mode 100644 providers/flagd/src/test/resources/junit-platform.properties 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 c7387b6e33..5d5c116eda 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 @@ -2,6 +2,9 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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 dev.openfeature.contrib.providers.flagd.Config; @@ -41,6 +44,16 @@ // reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 "large-integers" }) +// Parallel scenario execution, declared here rather than in a module-wide +// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, +// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, +// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +@ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") +@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") +@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") +// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push +// that past the event timeout. +@ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunFileTest { 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 05eb219db1..0584c7977b 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 @@ -2,6 +2,9 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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 dev.openfeature.contrib.providers.flagd.Config; @@ -30,6 +33,16 @@ @IncludeTags("in-process") // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) +// Parallel scenario execution, declared here rather than in a module-wide +// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, +// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, +// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +@ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") +@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") +@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") +// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push +// that past the event timeout. +@ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunInProcessTest { 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 4fe721d093..2ab9fe2a89 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 @@ -2,6 +2,9 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; +import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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 dev.openfeature.contrib.providers.flagd.Config; @@ -30,6 +33,16 @@ @IncludeTags({"rpc"}) // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) +// Parallel scenario execution, declared here rather than in a module-wide +// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, +// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, +// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +@ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") +@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") +@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") +// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push +// that past the event timeout. +@ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunRpcTest { diff --git a/providers/flagd/src/test/resources/junit-platform.properties b/providers/flagd/src/test/resources/junit-platform.properties deleted file mode 100644 index cc6bbf8f62..0000000000 --- a/providers/flagd/src/test/resources/junit-platform.properties +++ /dev/null @@ -1,21 +0,0 @@ -# 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, which every config -# scenario reads when building FlagdOptions defaults. A plain ENV_VARS lock only -# serialises @env-var scenarios against each other, so a concurrent default-reading -# scenario could still observe a mutated var. Acquire the JUnit global lock so -# @env-var scenarios run in full isolation (no other scenario runs concurrently). -cucumber.execution.exclusive-resources.env-var.read-write=org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY -# 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/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java index 31da2bda55..2f9e12d0d5 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java @@ -27,7 +27,6 @@ @ConfigurationParameter( key = Constants.EXECUTION_MODE_FEATURE_PROPERTY_NAME, value = ProviderTck.FEATURE_EXECUTION_MODE) -@ConfigurationParameter(key = Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = ProviderTck.PARALLEL) @ConfigurationParameter(key = Constants.GLUE_PROPERTY_NAME, value = ProviderTck.ALL_GLUE) @ConfigurationParameter(key = Constants.OBJECT_FACTORY_PROPERTY_NAME, value = ProviderTck.OBJECT_FACTORY) public abstract class ProviderTckTest implements ProviderTckHarness {} From ded590b7b88680316c65e23c6c428acf4accf780 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 17:21:38 +0200 Subject: [PATCH 03/10] fix(tck): await the provider swap when a scenario ends The teardown hook swapped in a NoOpProvider without waiting, so the provider under test was still shutting down against its backend after the hook returned. That is harmless while every scenario keeps its own backend for the life of a thread, and wrong as soon as a backend is handed to the next scenario, which then prepares a backend someone else is still talking to. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/internal/steps/ProviderSteps.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java index e7354aba77..a8b994503a 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java @@ -91,7 +91,12 @@ public void prepareBackend() { @After public void tearDown() { if (state.domain != null) { - OpenFeatureAPI.getInstance().setProvider(state.domain, new NoOpProvider()); + // Awaited, not fired and forgotten: the provider under test is still holding whatever it + // opened against the backend, and the backend is handed to the next scenario as soon as + // this hook returns. Swapping without waiting leaves a provider shutting down against a + // backend another scenario is already preparing, which costs nothing while every + // scenario shares one backend and breaks an event assertion as soon as they do not. + OpenFeatureAPI.getInstance().setProviderAndWait(state.domain, new NoOpProvider()); } } From aa8b50245e6132d286756edacd005497578fdf9b Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 17:26:53 +0200 Subject: [PATCH 04/10] style(tck): cut the comments back to what is not evident from the code The rationale for the pool and for the moved parallel settings belongs in the pull request, not in every file it touches. Signed-off-by: Simon Schrottner --- .../providers/flagd/e2e/RunFileTest.java | 9 ++-- .../providers/flagd/e2e/RunInProcessTest.java | 9 ++-- .../providers/flagd/e2e/RunRpcTest.java | 9 ++-- .../tck/internal/steps/ProviderSteps.java | 10 +--- .../tools/tck/internal/steps/StackPool.java | 52 ++++--------------- .../tck/internal/steps/TckObjectFactory.java | 3 +- 6 files changed, 23 insertions(+), 69 deletions(-) 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 5d5c116eda..6ee5d76afa 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 @@ -44,15 +44,12 @@ // reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 "large-integers" }) -// Parallel scenario execution, declared here rather than in a module-wide -// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, -// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, -// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +// Declared here rather than in a module-wide junit-platform.properties, which also reached the +// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") -// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push -// that past the event timeout. +// Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunFileTest { 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 0584c7977b..127a55d612 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 @@ -33,15 +33,12 @@ @IncludeTags("in-process") // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) -// Parallel scenario execution, declared here rather than in a module-wide -// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, -// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, -// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +// Declared here rather than in a module-wide junit-platform.properties, which also reached the +// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") -// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push -// that past the event timeout. +// Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunInProcessTest { 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 2ab9fe2a89..4d7f8962c3 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 @@ -33,15 +33,12 @@ @IncludeTags({"rpc"}) // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) -// Parallel scenario execution, declared here rather than in a module-wide -// junit-platform.properties: every one of these settings is about *this* suite -- its ContainerPool, -// its @env-var and @grace tags -- and a file in test resources applies to every suite in the module, -// including the Provider TCK ones, which then had to pin parallelism off to defend themselves. +// Declared here rather than in a module-wide junit-platform.properties, which also reached the +// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") -// @grace scenarios restart a container and wait for reconnection; two concurrent restarts can push -// that past the event timeout. +// Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") @Testcontainers public class RunRpcTest { diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java index a8b994503a..2ab74f4b1a 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java @@ -77,9 +77,7 @@ public void prepareBackend() { try { backend().prepareScenario(); } catch (RuntimeException e) { - // Said here because this is where the breakage is visible: a backend that cannot be - // brought to the baseline must not be handed to the next scenario, and the object - // factory only learns that a scenario ended, never why. + // the object factory only learns that a scenario ended, never why StackPool.markLeaseBroken(); throw e; } @@ -91,11 +89,7 @@ public void prepareBackend() { @After public void tearDown() { if (state.domain != null) { - // Awaited, not fired and forgotten: the provider under test is still holding whatever it - // opened against the backend, and the backend is handed to the next scenario as soon as - // this hook returns. Swapping without waiting leaves a provider shutting down against a - // backend another scenario is already preparing, which costs nothing while every - // scenario shares one backend and breaks an event assertion as soon as they do not. + // awaited: the backend goes to the next scenario as soon as this returns OpenFeatureAPI.getInstance().setProviderAndWait(state.domain, new NoOpProvider()); } } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java index 0edc665da8..1554c008a3 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java @@ -17,40 +17,23 @@ * Backends warmed once and leased to one scenario at a time. * *

Off by default: with no size set, every {@link TckObjectFactory} starts one backend and keeps - * it for as long as its thread lives, which is the behaviour every adoption has had so far. Set - * {@value #SIZE_PROPERTY} and the suite instead starts that many backends up front, hands one to - * each scenario, and takes it back afterwards. - * - *

The size is the cap on how many backends stand at once. That is the reason to - * prefer it over a thread count: {@code cucumber.execution.parallel.config.fixed.parallelism} is a - * target rather than a limit, and a suite whose steps block has been observed growing to eighteen - * threads -- each of which would otherwise start a backend of its own. Here the surplus threads - * queue on {@link #acquire()} instead, and the number of containers is whatever was declared. - * - *

A scenario receives a backend that the next {@code prepareScenario} has cleaned rather than one - * nobody has touched, so the pool does not weaken the contract that every scenario starts from the - * baseline -- it widens who it protects, because a backend now moves between scenarios of different - * features and suites rather than staying on one thread. + * it for as long as its thread lives. The size caps how many backends stand at once, which a thread + * count cannot do -- {@code cucumber.execution.parallel.config.fixed.parallelism} is a target, and a + * suite whose steps block has been seen growing to eighteen threads. Surplus threads queue on + * {@link #acquire()}. */ final class StackPool { /** How many backends to warm; absent or zero keeps one backend per thread. */ static final String SIZE_PROPERTY = "openfeature.tck.pool.size"; - /** - * How long {@link #acquire()} waits before calling a lease leak a leak. - * - *

Every lease is returned by {@link TckObjectFactory#stop()}, which Cucumber calls after the - * scenario's own hooks, so a scenario waiting this long means a lease was lost rather than that - * the suite is busy. Blocking forever would hang the build with no explanation. - */ + /** Every lease is returned by {@code stop()}, so waiting this long means one was lost. */ private static final long ACQUIRE_TIMEOUT_MINUTES = 10; private static final Logger log = LoggerFactory.getLogger(StackPool.class); private static final BlockingQueue idle = new LinkedBlockingQueue<>(); - /** The leases handed out but not yet returned, so a broken one can be named in a message. */ private static final ThreadLocal leaseBroken = ThreadLocal.withInitial(() -> false); private static volatile int size; @@ -72,12 +55,8 @@ static boolean enabled() { * Starts the requested number of backends, concurrently, or does nothing when none was asked * for. * - *

Concurrently because the startups are the cost the pool exists to pay once: starting four - * backends one after another would spend four times a Compose startup before the first scenario - * runs. - * * @param start supplies one started backend, as the per-thread path does - * @throws IllegalStateException if any backend fails to start, after stopping the ones that did + * @throws IllegalStateException if any backend fails to start */ static void warm(Supplier start) { int requested = requestedSize(); @@ -109,8 +88,8 @@ static void warm(Supplier start) { } } if (failure != null) { - // the ones that came up are registered for endSuite(), which stops them; the suite - // fails here rather than letting scenarios queue for backends that will never arrive + // the ones that came up are stopped by endSuite(); failing here beats letting + // scenarios queue for backends that will never arrive throw new IllegalStateException( "only " + warmed.size() + " of " + requested + " backends started", failure); } @@ -143,12 +122,8 @@ static TckLifecycle acquire() { } /** - * Returns a leased backend, unless the scenario reported it broken. - * - *

A backend whose own preparation failed is dropped rather than handed to the next scenario: - * returning it would spread one failure across every scenario that drew it, and the pool would - * quietly become a set of broken backends that fail everything. Shrinking is loud and the - * remaining backends still work; the suite fails when the last one goes. + * Returns a leased backend, dropping it if the scenario reported it broken rather than spreading + * one failure across every scenario that would draw it next. * * @param lifecycle the backend to return */ @@ -161,12 +136,7 @@ static void release(TckLifecycle lifecycle) { idle.add(lifecycle); } - /** - * Marks the backend this thread holds as unfit to pass on. - * - *

Called where the breakage is visible -- the step that prepares the backend -- because - * {@link TckObjectFactory#stop()} only knows that a scenario ended, not why. - */ + /** Marks the backend this thread holds as unfit to pass on; called where the breakage shows. */ static void markLeaseBroken() { if (enabled()) { leaseBroken.set(true); diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java index da01718b60..9c0949f0b9 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java @@ -64,8 +64,7 @@ private static boolean isInstantiable(Class candidate) { public void start() { pico = new PicoBuilder().withCaching().build(); if (StackPool.enabled()) { - // one backend per scenario, taken from the warmed pool and returned in stop(); the - // pool's size, not this thread's existence, decides how many stand at once + // the pool's size, not this thread's existence, decides how many stand at once leased = StackPool.acquire(); lifecycle = leased; } else if (lifecycle == null) { From 77be6a339f56661030c0a6d3529bfe9d29294575 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 12:37:35 +0100 Subject: [PATCH 05/10] fix(flagd): keep the @env-var lock on the suites that select those scenarios Scoping the parallel settings carried @grace onto the three e2e suites but left @env-var behind, reasoning that only ConfigCucumberTest selects config.feature. It is not the only one: three of that file's env-var scenarios also carry @in-process or @file, and RunInProcessTest and RunFileTest select by exactly those tags and exclude nothing that would filter them out. They mutate process-wide environment variables that every config scenario reads while building FlagdOptions defaults, so under the parallelism these suites now declare they need the global lock the properties file used to give them. On all three rather than the two that need it today: an exclusive-resources entry binds only the scenarios carrying its tag, so it is inert on RunRpcTest until the test-harness tags an env-var scenario @rpc, and then it is already right. Signed-off-by: Simon Schrottner --- .../openfeature/contrib/providers/flagd/e2e/RunFileTest.java | 4 ++++ .../contrib/providers/flagd/e2e/RunInProcessTest.java | 4 ++++ .../openfeature/contrib/providers/flagd/e2e/RunRpcTest.java | 4 ++++ 3 files changed, 12 insertions(+) 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 6ee5d76afa..41a1124dd3 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 @@ -51,6 +51,10 @@ @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") +// @env-var scenarios mutate process-wide variables that every config scenario reads. +@ConfigurationParameter( + key = "cucumber.execution.exclusive-resources.env-var.read-write", + value = "org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY") @Testcontainers public class RunFileTest { 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 127a55d612..9daa64b6b8 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 @@ -40,6 +40,10 @@ @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") +// @env-var scenarios mutate process-wide variables that every config scenario reads. +@ConfigurationParameter( + key = "cucumber.execution.exclusive-resources.env-var.read-write", + value = "org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY") @Testcontainers public class RunInProcessTest { 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 4d7f8962c3..192685ee13 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 @@ -40,6 +40,10 @@ @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") +// @env-var scenarios mutate process-wide variables that every config scenario reads. +@ConfigurationParameter( + key = "cucumber.execution.exclusive-resources.env-var.read-write", + value = "org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_KEY") @Testcontainers public class RunRpcTest { From 82497e9d6ee094534993934f4cfd5d6efab61a39 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 12:37:47 +0100 Subject: [PATCH 06/10] style(tck): drop the parallel constant nothing feeds any more ProviderTckTest no longer pins parallelism off, so the constant behind that pin has no reader. Signed-off-by: Simon Schrottner --- .../java/dev/openfeature/contrib/tools/tck/ProviderTck.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java index 1c14985cd2..4b832ad3af 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java @@ -29,9 +29,6 @@ public final class ProviderTck { /** The Cucumber plugins the suite registers. */ static final String PLUGINS = "summary"; - /** Parallel execution off; control API state is global to the backend. */ - static final String PARALLEL = "false"; - /** Scenarios of one feature stay on one thread. */ static final String FEATURE_EXECUTION_MODE = "same_thread"; From 518ce4b477637d6d9d3e48512480157e5c6ea16f Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 15:46:29 +0100 Subject: [PATCH 07/10] docs(tck): trim the pool comments to what the code needs The pool's class javadoc argued for the size the way the PR description does, and three methods carried an @param and a @return for a one-line fact. The e2e suites no longer explain why the parallelism settings moved; that the code sits on the suite says it. Signed-off-by: Simon Schrottner --- .../providers/flagd/e2e/RunFileTest.java | 2 -- .../providers/flagd/e2e/RunInProcessTest.java | 2 -- .../providers/flagd/e2e/RunRpcTest.java | 2 -- .../tools/tck/internal/steps/StackPool.java | 29 +++++-------------- .../tck/internal/steps/TckLifecycle.java | 3 +- 5 files changed, 8 insertions(+), 30 deletions(-) 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 41a1124dd3..b152949564 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 @@ -44,8 +44,6 @@ // reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 "large-integers" }) -// Declared here rather than in a module-wide junit-platform.properties, which also reached the -// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") 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 9daa64b6b8..fcee4826dc 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 @@ -33,8 +33,6 @@ @IncludeTags("in-process") // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) -// Declared here rather than in a module-wide junit-platform.properties, which also reached the -// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") 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 192685ee13..be74688a92 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 @@ -33,8 +33,6 @@ @IncludeTags({"rpc"}) // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) -// Declared here rather than in a module-wide junit-platform.properties, which also reached the -// Provider TCK suites and left them unable to turn parallelism back on. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") @ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") @ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java index 1554c008a3..352aa563b5 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java @@ -17,10 +17,9 @@ * Backends warmed once and leased to one scenario at a time. * *

Off by default: with no size set, every {@link TckObjectFactory} starts one backend and keeps - * it for as long as its thread lives. The size caps how many backends stand at once, which a thread - * count cannot do -- {@code cucumber.execution.parallel.config.fixed.parallelism} is a target, and a - * suite whose steps block has been seen growing to eighteen threads. Surplus threads queue on - * {@link #acquire()}. + * it for as long as its thread lives. The size caps how many stand at once, which a thread count + * cannot do: {@code cucumber.execution.parallel.config.fixed.parallelism} is a target, not a limit. + * Surplus threads queue on {@link #acquire()}. */ final class StackPool { @@ -42,11 +41,7 @@ private StackPool() { // warmed and drained by TckLifecycle, leased by TckObjectFactory } - /** - * Whether scenarios lease backends from this pool. - * - * @return true once a suite has warmed a pool of at least one backend - */ + /** Whether scenarios lease backends from this pool. */ static boolean enabled() { return size > 0; } @@ -88,8 +83,7 @@ static void warm(Supplier start) { } } if (failure != null) { - // the ones that came up are stopped by endSuite(); failing here beats letting - // scenarios queue for backends that will never arrive + // failing beats letting scenarios queue for backends that will never arrive throw new IllegalStateException( "only " + warmed.size() + " of " + requested + " backends started", failure); } @@ -100,11 +94,7 @@ static void warm(Supplier start) { } } - /** - * Takes a backend for one scenario, waiting while every one of them is in use. - * - * @return the leased backend - */ + /** Takes a backend for one scenario, waiting while every one of them is in use. */ static TckLifecycle acquire() { leaseBroken.set(false); try { @@ -121,12 +111,7 @@ static TckLifecycle acquire() { } } - /** - * Returns a leased backend, dropping it if the scenario reported it broken rather than spreading - * one failure across every scenario that would draw it next. - * - * @param lifecycle the backend to return - */ + /** Returns a leased backend, dropping it if the scenario reported it broken. */ static void release(TckLifecycle lifecycle) { if (Boolean.TRUE.equals(leaseBroken.get())) { leaseBroken.set(false); diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java index e650098b46..dbdbdcedcb 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java @@ -51,8 +51,7 @@ static void beginSuite() { AdopterTags.requireValid(harness.getClass()); startFailure.set(null); suiteRunning = true; - // after the declarations are validated, so a suite that cannot run does not spend four - // Compose startups finding that out + // after the declarations are validated: a suite that cannot run should start no stacks StackPool.warm(() -> start(discoverHarness())); } From 5658a29937a5449d2f5a8b2959b5262acc0e2ce7 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 18:14:42 +0100 Subject: [PATCH 08/10] fix(tck): drop a pooled backend from the count, not just from the queue release() left `size` alone when it dropped a backend whose preparation had failed, so with a pool of one the queue stayed empty for the rest of the run and every later scenario waited out the ten-minute acquire deadline before failing -- with a message blaming an unreturned lease, which was not what happened. A `live` count now falls with the backend, and acquire() polls in steps so it can fail as soon as nothing is left, naming the drop. Signed-off-by: Simon Schrottner --- .../tools/tck/internal/steps/StackPool.java | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java index 352aa563b5..477da6fd8a 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/StackPool.java @@ -9,6 +9,7 @@ import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,6 +38,9 @@ final class StackPool { private static volatile int size; + /** Backends still fit to lease; drops lower it, so waiting scenarios can fail at zero. */ + private static final AtomicInteger live = new AtomicInteger(); + private StackPool() { // warmed and drained by TckLifecycle, leased by TckObjectFactory } @@ -89,6 +93,7 @@ static void warm(Supplier start) { } idle.addAll(warmed); size = requested; + live.set(requested); } finally { starters.shutdownNow(); } @@ -97,14 +102,22 @@ static void warm(Supplier start) { /** Takes a backend for one scenario, waiting while every one of them is in use. */ static TckLifecycle acquire() { leaseBroken.set(false); + long deadline = System.nanoTime() + TimeUnit.MINUTES.toNanos(ACQUIRE_TIMEOUT_MINUTES); try { - TckLifecycle leased = idle.poll(ACQUIRE_TIMEOUT_MINUTES, TimeUnit.MINUTES); - if (leased == null) { - throw new IllegalStateException("no backend became free within " + ACQUIRE_TIMEOUT_MINUTES - + " minutes, with " + size + " in the pool (" + SIZE_PROPERTY - + "); a lease was not returned"); + while (System.nanoTime() < deadline) { + if (live.get() <= 0) { + throw new IllegalStateException("every backend was dropped after its preparation failed (" + + SIZE_PROPERTY + "=" + size + "); there is nothing left to lease"); + } + // in steps, so a scenario does not wait out the whole deadline for a pool that emptied + TckLifecycle leased = idle.poll(1, TimeUnit.SECONDS); + if (leased != null) { + return leased; + } } - return leased; + throw new IllegalStateException("no backend became free within " + ACQUIRE_TIMEOUT_MINUTES + + " minutes, with " + live.get() + " of " + size + " in the pool (" + SIZE_PROPERTY + + "); a lease was not returned"); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IllegalStateException("interrupted while waiting for a backend", e); @@ -115,7 +128,10 @@ static TckLifecycle acquire() { static void release(TckLifecycle lifecycle) { if (Boolean.TRUE.equals(leaseBroken.get())) { leaseBroken.set(false); - log.warn("Dropping a backend whose preparation failed; {} of {} left in the pool", idle.size(), size); + log.warn( + "Dropping a backend whose preparation failed; {} of {} left in the pool", + live.decrementAndGet(), + size); return; } idle.add(lifecycle); @@ -132,6 +148,7 @@ static void markLeaseBroken() { static void drain() { idle.clear(); size = 0; + live.set(0); } private static int requestedSize() { From 8e713974784be2b37ff851be78ce56206ff069d4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 18:14:49 +0100 Subject: [PATCH 09/10] fix(tck): release a pooled backend after the container is disposed stop() returned the backend before disposing the Pico container, so anything the container tears down ran after the backend had been handed to the next scenario. Nothing in the suite implements Startable or Disposable today, but extension glue is adopter code and a step class that implements either would hit it. The release moves into a finally, so a throwing teardown cannot leak the lease. Signed-off-by: Simon Schrottner --- .../tck/internal/steps/TckObjectFactory.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java index 9c0949f0b9..a44d80f65d 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckObjectFactory.java @@ -83,16 +83,19 @@ public void start() { @Override public void stop() { - if (leased != null) { - // after the scenario's own hooks, so a backend is only returned once nothing is using it - StackPool.release(leased); - leased = null; - lifecycle = null; - } - if (pico != null) { - pico.stop(); - pico.dispose(); - pico = null; + try { + if (pico != null) { + pico.stop(); + pico.dispose(); + pico = null; + } + } finally { + if (leased != null) { + // after the container is disposed, so nothing can still be using the backend + StackPool.release(leased); + leased = null; + lifecycle = null; + } } } From ee0f8a781bf381d6f294ba60c1b56de372b64b75 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 31 Oct 2026 18:14:58 +0100 Subject: [PATCH 10/10] fix(flagd): keep the e2e parallelism tunable from the command line Scoping the settings onto the three suites pinned the strategy and the dynamic factor in a @ConfigurationParameter, which outranks -D -- the same trap this branch removes from the Provider TCK suites. junit-platform.properties did not outrank it, so the move quietly took the override away. Only the switch stays. JUnit falls back to the dynamic strategy with a factor of 1 when neither is set, which is what the annotations said, so nothing changes except that -Dcucumber.execution.parallel.config.* works again. Signed-off-by: Simon Schrottner --- .../contrib/providers/flagd/e2e/RunFileTest.java | 6 ++---- .../contrib/providers/flagd/e2e/RunInProcessTest.java | 6 ++---- .../openfeature/contrib/providers/flagd/e2e/RunRpcTest.java | 6 ++---- 3 files changed, 6 insertions(+), 12 deletions(-) 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 b152949564..551812c0bc 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 @@ -2,8 +2,6 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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; @@ -44,9 +42,9 @@ // reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 "large-integers" }) +// Only the switch: the strategy and factor stay at Cucumber's defaults, so +// -Dcucumber.execution.parallel.config.* can still tune them. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") -@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") -@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") // @env-var scenarios mutate process-wide variables that every config scenario reads. 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 fcee4826dc..10bfa75ae6 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 @@ -2,8 +2,6 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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; @@ -33,9 +31,9 @@ @IncludeTags("in-process") // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) +// Only the switch: the strategy and factor stay at Cucumber's defaults, so +// -Dcucumber.execution.parallel.config.* can still tune them. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") -@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") -@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") // @env-var scenarios mutate process-wide variables that every config scenario reads. 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 be74688a92..d2eb856516 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 @@ -2,8 +2,6 @@ import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME; import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME; -import static io.cucumber.junit.platform.engine.Constants.PARALLEL_CONFIG_STRATEGY_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; @@ -33,9 +31,9 @@ @IncludeTags({"rpc"}) // large-integers reads 2^53-1 via 32-bit getInteger; Long scenarios: open-feature/flagd-testbed#379 @ExcludeTags({"unixsocket", "fractional-v1", "fractional-v2", "deprecated", "large-integers"}) +// Only the switch: the strategy and factor stay at Cucumber's defaults, so +// -Dcucumber.execution.parallel.config.* can still tune them. @ConfigurationParameter(key = PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "true") -@ConfigurationParameter(key = PARALLEL_CONFIG_STRATEGY_PROPERTY_NAME, value = "dynamic") -@ConfigurationParameter(key = PARALLEL_CONFIG_DYNAMIC_FACTOR_PROPERTY_NAME, value = "1") // Concurrent container restarts can outlast the reconnection timeout. @ConfigurationParameter(key = "cucumber.execution.exclusive-resources.grace.read-write", value = "CONTAINER_RESTART") // @env-var scenarios mutate process-wide variables that every config scenario reads.