diff --git a/tools/tck/pom.xml b/tools/tck/pom.xml index 70d6531289..700c66a1de 100644 --- a/tools/tck/pom.xml +++ b/tools/tck/pom.xml @@ -20,7 +20,7 @@ 3.27.7 4.3.0 - 2.22.1 + 2.22.3 2.0.17 2.0.4 1.3.0 @@ -66,10 +66,16 @@ cucumber-junit-platform-engine - + - io.cucumber - cucumber-picocontainer + org.picocontainer + picocontainer + 2.15.2 compile diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java index cd2004b726..7e7bf0dfbd 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java @@ -186,7 +186,17 @@ private ComposeContainer startCompose() { stack.withStartupTimeout(startupTimeout()); log.info("Starting Compose stack {} (started once per suite, never restarted)", composeFile.getAbsolutePath()); - stack.start(); + try { + stack.start(); + } catch (RuntimeException e) { + // stopSuite() reaches the stack through a field this method has not returned into yet + try { + stack.stop(); + } catch (RuntimeException cleanup) { + e.addSuppressed(cleanup); + } + throw e; + } return stack; } } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/HttpBackendControl.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/HttpBackendControl.java index a8229e3f80..1c054408d4 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/HttpBackendControl.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/HttpBackendControl.java @@ -2,9 +2,11 @@ import java.io.IOException; import java.net.URI; +import java.net.URLEncoder; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; import java.time.Duration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -145,7 +147,10 @@ public void awaitReady(Duration timeout) { /** Starts the backend, seeding the baseline flag state. */ private void start() { - post("/start?config=" + backendConfiguration); + post( + backendConfiguration == null + ? "/start" + : "/start?config=" + URLEncoder.encode(backendConfiguration, StandardCharsets.UTF_8)); backendStopped = 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 18b3b74aef..066a689cac 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 @@ -11,6 +11,7 @@ import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +32,8 @@ final class TckLifecycle { private static volatile boolean suiteRunning; + private static final AtomicReference startFailure = new AtomicReference<>(); + private final ProviderTckHarness harness; private final BackendControl backendControl; @@ -44,12 +47,14 @@ static void beginSuite() { ProviderTckHarness harness = discoverHarness(); log.info("Provider TCK harness: {}", harness.getClass().getName()); Capability.requireDeclarable(harness.capabilities()); + startFailure.set(null); suiteRunning = true; } /** Stops every lifecycle this suite started. */ static void endSuite() { suiteRunning = false; + startFailure.set(null); List toStop; synchronized (started) { toStop = new ArrayList<>(started); @@ -82,7 +87,21 @@ static Optional startForExecutingSuite() { if (!suiteRunning) { return Optional.empty(); } - return Optional.of(start(discoverHarness())); + // Deliberately not synchronized: making a second thread wait for the first thread's + // Compose startup is the serialisation per-thread backends exist to avoid. Threads that race + // before a failure is cached each pay one attempt, in parallel; everyone after fails fast, + // and compareAndSet keeps the first failure rather than whichever landed last. + RuntimeException failed = startFailure.get(); + if (failed != null) { + // one Compose timeout per suite, not one per scenario + throw new IllegalStateException("the suite's backend already failed to start", failed); + } + try { + return Optional.of(start(discoverHarness())); + } catch (RuntimeException e) { + startFailure.compareAndSet(null, e); + throw e; + } } /** Starts the given harness and registers the result for {@link #endSuite()}. */ @@ -99,8 +118,12 @@ static TckLifecycle start(ProviderTckHarness harness) { started.add(lifecycle); return lifecycle; } catch (RuntimeException e) { - // release anything a partial start allocated, e.g. containers - harness.stopSuite(); + // release anything a partial start allocated, e.g. containers, without losing why + try { + harness.stopSuite(); + } catch (RuntimeException cleanup) { + e.addSuppressed(cleanup); + } throw e; } } diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java index 98284ce171..09eda6959d 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java @@ -62,6 +62,26 @@ void fallsBackToStartAndCachesTheAnswer() throws Exception { } } + @Test + @DisplayName("a null configuration posts /start with no query, rather than config=null") + void aNullConfigurationOmitsTheQuery() throws Exception { + try (StubControlApi stub = StubControlApi.start().answering("/reset", 404)) { + new HttpBackendControl(stub.baseUrl(), null).prepareScenario(); + + assertThat(stub.requests()).containsExactly("POST /reset", "POST /start"); + } + } + + @Test + @DisplayName("a configuration name is encoded, so a space cannot break the URI") + void theConfigurationNameIsEncoded() throws Exception { + try (StubControlApi stub = StubControlApi.start().answering("/reset", 404)) { + new HttpBackendControl(stub.baseUrl(), "two words").prepareScenario(); + + assertThat(stub.requests()).containsExactly("POST /reset", "POST /start?config=two+words"); + } + } + @ParameterizedTest @ValueSource(ints = {404, 501}) @DisplayName("both documented not-implemented statuses trigger the fallback") diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycleTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycleTest.java index eeb047ca9e..d88b8d0aa5 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycleTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycleTest.java @@ -119,6 +119,34 @@ public void stopSuite() { .isZero(); } + @Test + @DisplayName("a teardown failure is attached to the startup failure, not substituted for it") + void cleanupFailureDoesNotReplaceTheCause() { + class FailsBothWays implements ProviderTckHarness { + @Override + public FeatureProvider createProvider() { + return new NoOpProvider(); + } + + @Override + public BackendControl backendControl() { + return null; + } + + @Override + public void stopSuite() { + throw new IllegalStateException("teardown also failed"); + } + } + + assertThatThrownBy(() -> TckLifecycle.start(new FailsBothWays())) + .as("the adopter needs the reason the suite would not start") + .hasMessageContaining("backendControl() returned null") + .satisfies(thrown -> assertThat(thrown.getSuppressed()) + .as("and the teardown failure is kept, not dropped") + .hasSize(1)); + } + @Test @DisplayName("no suite running means no lifecycle, so extension-only glue registers nothing") void noSuiteMeansNoLifecycle() {