From 299691ae0b3535df5526b8a04da9d174d83ada5c Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 21 Oct 2026 16:02:03 +0200 Subject: [PATCH 01/13] feat(tck): select extension scenarios by tag annotations @TckIncludeTags on a harness runs only the extension scenarios carrying one of its tags, so a scenario tagged for several modes runs in each. @TckExcludeTags skips extension scenarios carrying one of its tags. Declarations on superclasses add up. Canonical scenarios are exempt, and capability tags are refused at suite start and in the scenario gate, so capabilities() alone gates them. Signed-off-by: Simon Schrottner --- tools/tck/README.md | 28 ++- .../contrib/tools/tck/AdopterTags.java | 122 +++++++++++++ .../contrib/tools/tck/Capability.java | 4 +- .../contrib/tools/tck/TckExcludeTags.java | 32 ++++ .../contrib/tools/tck/TckIncludeTags.java | 31 ++++ .../tck/internal/steps/ProviderSteps.java | 6 +- .../tck/internal/steps/TckLifecycle.java | 2 + .../tools/tck/ExcludedTagsSuiteFixture.java | 56 ++++++ .../contrib/tools/tck/ExcludedTagsTest.java | 170 ++++++++++++++++++ .../skip-selftest/extensions/adopter.feature | 24 +++ .../skip-selftest/gherkin/canonical.feature | 6 + 11 files changed, 477 insertions(+), 4 deletions(-) create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckExcludeTags.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckIncludeTags.java create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsSuiteFixture.java create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java create mode 100644 tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature create mode 100644 tools/tck/src/test/resources/skip-selftest/gherkin/canonical.feature diff --git a/tools/tck/README.md b/tools/tck/README.md index 36fe7ee389..af301bb64a 100644 --- a/tools/tck/README.md +++ b/tools/tck/README.md @@ -246,11 +246,37 @@ src/test/java/openfeature/tck/extensions/FractionalSteps.java // package openf registered. - Step classes and step methods must be `public`; otherwise Cucumber reports scenarios as undefined. - Don't put files under `gherkin/`; a same-named file would silently shadow a canonical one. -- Unknown tags are tolerated in `extensions/` (they gate nothing) but fail the run in `gherkin/`. +- Unknown tags are tolerated in `extensions/` (select by them with the annotations below) but fail + the run in `gherkin/`. - `ProviderTck` exposes `EXTENSIONS`, `EXTENSION_GLUE` and `ALL_GLUE` for custom `@ConfigurationParameter`s; always use `ALL_GLUE` or canonical steps go undefined. - Portable scenarios belong in the TCK, not in an extension. +### Selecting extension scenarios by tag + +When one extension set serves several configurations, annotate each harness with the tags it runs +and the tags it doesn't: + +```java +@TckIncludeTags("@rpc") +@TckExcludeTags({"@fractional-v1", "@fractional-v2"}) +public class RpcTest extends AbstractResolverTest { ... } + +@TckIncludeTags("@in-process") +@TckExcludeTags("@fractional-v1") +public class InProcessTest extends AbstractResolverTest { ... } +``` + +- With `@TckIncludeTags`, an extension scenario runs only if it carries one of those tags; tag it + `@rpc @in-process` to run it in both. +- `@TckExcludeTags` skips an extension scenario carrying any of its tags, even an included one. +- A skipped scenario is reported with the tag that skipped it. Declarations on a harness and its + superclasses add up. +- Canonical scenarios are exempt. Capability tags are refused, at suite start and per scenario; + narrow `capabilities()` instead. +- `-Dcucumber.filter.tags` still works alongside them; a filtered scenario is absent from the report + rather than skipped. + ## Java notes - **Suite discovery** relies on the JUnit Platform auto-registering the TCK's `TestExecutionListener` diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java new file mode 100644 index 0000000000..f4b7cc0810 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java @@ -0,0 +1,122 @@ +package dev.openfeature.contrib.tools.tck; + +import java.lang.annotation.Annotation; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Function; +import org.opentest4j.TestAbortedException; + +/** + * The gate for {@link TckExcludeTags} and {@link TckIncludeTags} on a harness class. + * + *

Only adopter tags can be named: a {@link Capability} tag is refused, so this cannot skip + * what {@link ProviderTckHarness#capabilities()} gates. Canonical scenarios carry only capability + * tags, and are exempt besides. + */ +public final class AdopterTags { + + private AdopterTags() { + // static gate; harnesses cannot override it + } + + /** + * Rejects a capability tag, a malformed tag, or a tag both excluded and included. + * + * @param harness the harness class + * @throws IllegalArgumentException if the annotations name a capability tag or a malformed + * tag, or overlap + */ + public static void requireValid(Class harness) { + Set excluded = excluded(harness); + Set included = included(harness); + requireAdopterTags("@TckExcludeTags", excluded); + requireAdopterTags("@TckIncludeTags", included); + List both = new ArrayList<>(excluded); + both.retainAll(included); + if (!both.isEmpty()) { + throw new IllegalArgumentException( + harness.getName() + " both excludes and includes " + both + "; a tag is one or the other."); + } + } + + /** + * Skips an extension scenario that carries an excluded tag, or none of the included tags when + * there are any. Canonical scenarios are never skipped here. Validates the annotations first, + * so no path to the gate can skip on a capability tag. + * + * @param source the scenario's feature file, as Cucumber reports it, or {@code null} if unknown + * @param tags the scenario's Gherkin tags, including the leading at-sign + * @param harness the harness class + * @throws IllegalArgumentException if the annotations are invalid; see {@link #requireValid} + * @throws TestAbortedException if the scenario is not selected + */ + public static void requireSelected(URI source, Collection tags, Class harness) { + requireValid(harness); + if (Capability.isCanonical(source)) { + return; + } + Set excluded = excluded(harness); + List carried = new ArrayList<>(); + for (String tag : tags) { + if (excluded.contains(tag)) { + carried.add(tag); + } + } + if (!carried.isEmpty()) { + throw new TestAbortedException( + "Skipped: scenario carries " + carried + ", excluded by " + harness.getSimpleName() + "."); + } + Set included = included(harness); + if (!included.isEmpty() && Collections.disjoint(tags, included)) { + throw new TestAbortedException( + "Skipped: scenario carries none of " + included + ", included by " + harness.getSimpleName() + "."); + } + } + + static Set excluded(Class harness) { + return collect(harness, TckExcludeTags.class, TckExcludeTags::value); + } + + static Set included(Class harness) { + return collect(harness, TckIncludeTags.class, TckIncludeTags::value); + } + + // declarations on the class and its superclasses add up + private static Set collect( + Class harness, Class annotation, Function value) { + Set tags = new LinkedHashSet<>(); + for (Class type = harness; type != null; type = type.getSuperclass()) { + A declared = type.getDeclaredAnnotation(annotation); + if (declared != null) { + tags.addAll(Arrays.asList(value.apply(declared))); + } + } + return tags; + } + + private static void requireAdopterTags(String source, Collection tags) { + List capabilityTags = new ArrayList<>(); + for (String tag : tags) { + if (tag == null + || tag.length() < 2 + || !tag.startsWith("@") + || !tag.trim().equals(tag)) { + throw new IllegalArgumentException( + source + " names '" + tag + "'; a tag starts with '@' and has no surrounding whitespace."); + } + if (Capability.fromTag(tag).isPresent()) { + capabilityTags.add(tag); + } + } + if (!capabilityTags.isEmpty()) { + throw new IllegalArgumentException(source + " names TCK capability tags " + capabilityTags + + "; gate them through capabilities() instead."); + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java index 8a85044f88..2d7e8932af 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java @@ -231,7 +231,7 @@ public static void requireDeclared(URI source, Collection tags, Set found = fromTag(tag); if (!found.isPresent()) { - // adopter tag; gates nothing + // adopter tag; gated by AdopterTags continue; } Capability capability = found.get(); @@ -272,7 +272,7 @@ private static void requireKnownVocabulary(URI source, Collection tags) } // canonical if the parent directory is FEATURES; null is not canonical - private static boolean isCanonical(URI source) { + static boolean isCanonical(URI source) { if (source == null) { return false; } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckExcludeTags.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckExcludeTags.java new file mode 100644 index 0000000000..72d4235ff8 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckExcludeTags.java @@ -0,0 +1,32 @@ +package dev.openfeature.contrib.tools.tck; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Skips extension scenarios carrying any of these tags, even if they carry a + * {@link TckIncludeTags} tag. Canonical scenarios are exempt. Put it on the + * {@link ProviderTckHarness} class; declarations on the harness and its superclasses add up. + * Capability tags are refused. + * + * 

{@code
+ * @TckIncludeTags("@rpc")
+ * @TckExcludeTags({"@fractional-v1", "@fractional-v2"})
+ * public class RpcTest extends AbstractResolverTest { ... }
+ * }
+ */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface TckExcludeTags { + + /** + * Returns the tags to skip. + * + * @return the tags, with the leading at-sign + */ + String[] value(); +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckIncludeTags.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckIncludeTags.java new file mode 100644 index 0000000000..93c67440a6 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckIncludeTags.java @@ -0,0 +1,31 @@ +package dev.openfeature.contrib.tools.tck; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Runs only the extension scenarios carrying at least one of these tags; tag a scenario for + * several modes to run it in each. Canonical scenarios are exempt. Put it on the + * {@link ProviderTckHarness} class; declarations on the harness and its superclasses add up. + * Capability tags are refused. + * + * 
{@code
+ * @TckIncludeTags("@rpc")
+ * public class RpcTest extends AbstractResolverTest { ... }  // runs "@rpc" and "@rpc @in-process"
+ * }
+ */ +@Documented +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface TckIncludeTags { + + /** + * Returns the tags an extension scenario must carry one of. + * + * @return the tags, with the leading at-sign + */ + String[] value(); +} 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 a4101c5cf2..1a436d7b14 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 @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; +import dev.openfeature.contrib.tools.tck.AdopterTags; import dev.openfeature.contrib.tools.tck.BackendControl; import dev.openfeature.contrib.tools.tck.Capability; import dev.openfeature.contrib.tools.tck.ProviderTck; @@ -53,7 +54,8 @@ public static void afterAll() { /** * Skips scenarios needing an undeclared {@link Capability}; see * {@link Capability#requireDeclared(java.net.URI, java.util.Collection, java.util.Set)}. Tags - * under {@link ProviderTck#EXTENSIONS} are exempt from the vocabulary check. + * under {@link ProviderTck#EXTENSIONS} are exempt from the vocabulary check, and only their + * scenarios are selected by {@code @TckIncludeTags} and {@code @TckExcludeTags}; see {@link AdopterTags}. * * @param scenario the scenario about to run */ @@ -61,6 +63,8 @@ public static void afterAll() { public void gateOnCapabilities(Scenario scenario) { Capability.requireDeclared( scenario.getUri(), scenario.getSourceTagNames(), harness().capabilities()); + AdopterTags.requireSelected( + scenario.getUri(), scenario.getSourceTagNames(), harness().getClass()); } /** 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 066a689cac..b3f094f80d 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 @@ -1,5 +1,6 @@ package dev.openfeature.contrib.tools.tck.internal.steps; +import dev.openfeature.contrib.tools.tck.AdopterTags; import dev.openfeature.contrib.tools.tck.BackendControl; import dev.openfeature.contrib.tools.tck.Capability; import dev.openfeature.contrib.tools.tck.ProviderTckHarness; @@ -47,6 +48,7 @@ static void beginSuite() { ProviderTckHarness harness = discoverHarness(); log.info("Provider TCK harness: {}", harness.getClass().getName()); Capability.requireDeclarable(harness.capabilities()); + AdopterTags.requireValid(harness.getClass()); startFailure.set(null); suiteRunning = true; } diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsSuiteFixture.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsSuiteFixture.java new file mode 100644 index 0000000000..fd6ad6f6ad --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsSuiteFixture.java @@ -0,0 +1,56 @@ +package dev.openfeature.contrib.tools.tck; + +import dev.openfeature.sdk.FeatureProvider; +import io.cucumber.junit.platform.engine.Constants; +import java.util.EnumSet; +import java.util.Set; +import org.junit.platform.suite.api.ConfigurationParameter; +import org.junit.platform.suite.api.IncludeEngines; +import org.junit.platform.suite.api.SelectClasspathResource; +import org.junit.platform.suite.api.Suite; + +/** + * A real TCK suite over {@code skip-selftest/}, run by {@link ExcludedTagsTest}. Not named + * {@code *Test} so Surefire skips it; does not extend {@link ProviderTckTest} to avoid inheriting its selectors. + */ +@Suite +@IncludeEngines("cucumber") +@SelectClasspathResource(ExcludedTagsSuiteFixture.FEATURES) +@ConfigurationParameter(key = Constants.PLUGIN_PROPERTY_NAME, value = ProviderTck.PLUGINS) +@ConfigurationParameter(key = Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "false") +@ConfigurationParameter( + key = Constants.EXECUTION_MODE_FEATURE_PROPERTY_NAME, + value = ProviderTck.FEATURE_EXECUTION_MODE) +@ConfigurationParameter(key = Constants.GLUE_PROPERTY_NAME, value = ProviderTck.ALL_GLUE) +@ConfigurationParameter(key = Constants.OBJECT_FACTORY_PROPERTY_NAME, value = ProviderTck.OBJECT_FACTORY) +@TckExcludeTags(ExcludedTagsSuiteFixture.EXCLUDED) +@TckIncludeTags(ExcludedTagsSuiteFixture.INCLUDED) +public class ExcludedTagsSuiteFixture implements ProviderTckHarness { + + /** The classpath directory holding this fixture's feature files. */ + public static final String FEATURES = "skip-selftest"; + + /** The tag this fixture excludes. */ + public static final String EXCLUDED = "@old-version"; + + /** The tag an extension scenario needs to run here. */ + public static final String INCLUDED = "@mode-a"; + + private final InProcessBackendControl control = new InProcessBackendControl(); + + @Override + public BackendControl backendControl() { + return control; + } + + @Override + public FeatureProvider createProvider() { + return control.createProvider(); + } + + /** Declares {@code @events}, so only the exclusion can skip the scenario carrying it. */ + @Override + public Set capabilities() { + return EnumSet.of(Capability.EVENTS); + } +} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java new file mode 100644 index 0000000000..0aee75ab42 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java @@ -0,0 +1,170 @@ +package dev.openfeature.contrib.tools.tck; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.cucumber.junit.platform.engine.Constants; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.platform.engine.TestExecutionResult; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.TestExecutionListener; +import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; +import org.opentest4j.TestAbortedException; + +/** + * {@link TckIncludeTags} and {@link TckExcludeTags} select extension scenarios, leave canonical + * scenarios alone, and cannot name a capability tag. + */ +class ExcludedTagsTest { + + private static final URI EXTENSION = URI.create("classpath:extensions/x.feature"); + + @Test + @DisplayName("an extension scenario runs with an included tag and no excluded one; canonical scenarios always run") + void scenariosAreSelected() { + Results results = run(null); + + assertThat(results.passed) + .containsExactlyInAnyOrder( + "A canonical scenario runs without the included tag", + "An extension scenario carrying the included tag", + "An extension scenario tagged for both modes"); + assertThat(results.aborted) + .as("the other-mode, excluded and to-be-filtered scenarios are skipped, saying why") + .hasSize(3) + .anySatisfy(reason -> assertThat(reason).contains(ExcludedTagsSuiteFixture.INCLUDED)) + .anySatisfy(reason -> assertThat(reason).contains(ExcludedTagsSuiteFixture.EXCLUDED)); + assertThat(results.failed).isEmpty(); + } + + @Test + @DisplayName("a cucumber.filter.tags filter still applies alongside the annotations") + void aTagFilterStillApplies() { + Results results = run("not @filtered"); + + assertThat(results.passed).hasSize(3); + assertThat(results.aborted) + .as("the filtered scenario is gone, not skipped") + .hasSize(2); + assertThat(results.failed).isEmpty(); + } + + @Test + @DisplayName("an excluded tag wins over an included one, and declarations on a superclass add up") + void excludeWinsAndDeclarationsAddUp() { + assertThat(AdopterTags.excluded(RpcMode.class)).containsExactly("@fractional-v2", "@fractional-v1"); + assertThat(AdopterTags.included(RpcMode.class)).containsExactly("@rpc"); + + assertThatCode(() -> + AdopterTags.requireSelected(EXTENSION, Arrays.asList("@rpc", "@in-process"), RpcMode.class)) + .as("a scenario for both modes runs in each") + .doesNotThrowAnyException(); + assertThatThrownBy(() -> + AdopterTags.requireSelected(EXTENSION, Collections.singletonList("@in-process"), RpcMode.class)) + .isInstanceOf(TestAbortedException.class) + .hasMessageContaining("none of [@rpc]") + .hasMessageContaining("RpcMode"); + assertThatThrownBy(() -> + AdopterTags.requireSelected(EXTENSION, Arrays.asList("@rpc", "@fractional-v1"), RpcMode.class)) + .isInstanceOf(TestAbortedException.class) + .hasMessageContaining("[@fractional-v1]"); + assertThatCode(() -> AdopterTags.requireSelected( + URI.create("classpath:gherkin/errors.feature"), Collections.emptyList(), RpcMode.class)) + .as("a canonical scenario needs no included tag") + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("a capability tag cannot be included or excluded, at suite start or in the scenario gate") + void aCapabilityTagIsRefused() { + for (Class harness : Arrays.asList(ExcludesEvents.class, IncludesEvents.class)) { + assertThatThrownBy(() -> AdopterTags.requireValid(harness)) + .as(harness.getSimpleName()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(Capability.EVENTS.tag()) + .hasMessageContaining("capabilities()"); + assertThatThrownBy(() -> AdopterTags.requireSelected( + URI.create("classpath:gherkin/events.feature"), + Collections.singletonList(Capability.EVENTS.tag()), + harness)) + .as("the scenario gate refuses %s too, canonical scenarios included", harness.getSimpleName()) + .isInstanceOf(IllegalArgumentException.class); + } + } + + @Test + @DisplayName("a malformed or contradictory declaration is refused rather than silently matching nothing") + void aMalformedDeclarationIsRefused() { + assertThatThrownBy(() -> AdopterTags.requireValid(NoAtSign.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("'rpc'"); + assertThatThrownBy(() -> AdopterTags.requireValid(BothWays.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("[@rpc]"); + assertThatCode(() -> AdopterTags.requireValid(RpcMode.class)).doesNotThrowAnyException(); + } + + @TckExcludeTags("@fractional-v1") + private static class AnyMode {} + + @TckIncludeTags("@rpc") + @TckExcludeTags("@fractional-v2") + private static final class RpcMode extends AnyMode {} + + @TckExcludeTags("@events") + private static final class ExcludesEvents {} + + @TckIncludeTags("@events") + private static final class IncludesEvents {} + + @TckExcludeTags("rpc") + private static final class NoAtSign {} + + @TckExcludeTags("@rpc") + @TckIncludeTags("@rpc") + private static final class BothWays {} + + private static Results run(String tagFilter) { + LauncherDiscoveryRequestBuilder request = LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(ExcludedTagsSuiteFixture.class)); + if (tagFilter != null) { + request.configurationParameter(Constants.FILTER_TAGS_PROPERTY_NAME, tagFilter); + } + Results results = new Results(); + LauncherFactory.create().execute(request.build(), results); + return results; + } + + /** Scenario outcomes; aborted ones keep their reason. */ + private static final class Results implements TestExecutionListener { + final List passed = Collections.synchronizedList(new ArrayList<>()); + final List aborted = Collections.synchronizedList(new ArrayList<>()); + final List failed = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void executionFinished(TestIdentifier identifier, TestExecutionResult result) { + if (!identifier.isTest()) { + return; + } + switch (result.getStatus()) { + case SUCCESSFUL: + passed.add(identifier.getDisplayName()); + break; + case ABORTED: + aborted.add(result.getThrowable().map(Throwable::getMessage).orElse("")); + break; + default: + failed.add(identifier.getDisplayName() + ": " + result.getThrowable()); + } + } + } +} diff --git a/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature b/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature new file mode 100644 index 0000000000..bc650a61dc --- /dev/null +++ b/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature @@ -0,0 +1,24 @@ +Feature: An adopter selects its own extension scenarios by tag + + Proof of TckIncludeTags and TckExcludeTags, run by ExcludedTagsTest. Kept outside the shared + extensions/ directory so the other suites never pick it up. + + @mode-a + Scenario: An extension scenario carrying the included tag + Given a stable provider + + @mode-a @mode-b + Scenario: An extension scenario tagged for both modes + Given a stable provider + + @mode-b + Scenario: An extension scenario tagged for the other mode only + Given a stable provider + + @mode-a @old-version @events + Scenario: An extension scenario carrying an excluded tag + Given a stable provider + + @mode-b @filtered + Scenario: An extension scenario a tag filter removes first + Given a stable provider diff --git a/tools/tck/src/test/resources/skip-selftest/gherkin/canonical.feature b/tools/tck/src/test/resources/skip-selftest/gherkin/canonical.feature new file mode 100644 index 0000000000..45694af3a5 --- /dev/null +++ b/tools/tck/src/test/resources/skip-selftest/gherkin/canonical.feature @@ -0,0 +1,6 @@ +Feature: A canonical scenario runs whatever the harness includes or excludes + + Proof for ExcludedTagsTest; canonical scenarios are exempt, and carry only capability tags. + + Scenario: A canonical scenario runs without the included tag + Given a stable provider From edeb313f1eb8a2f09d8d25161a50063f2126ee75 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 21 Oct 2026 17:00:16 +0200 Subject: [PATCH 02/13] test(tck): pin that an include skips an untagged extension scenario Every scenario in the self-test fixture carried a tag, so nothing distinguished the two readings of TckIncludeTags: a filter that skips whatever does not carry an included tag, or a hint that spares a scenario carrying no selector at all. The implementation is the first; the fixture could not tell. Adds an untagged extension scenario and asserts it does not run while an include is set, so the next reader does not have to infer the rule from Collections.disjoint. The two existing abort counts move with it. This matters for the flagd adoption in #1951: it tags resolver-specific scenarios @rpc or @in-process and expects untagged ones to run in both, which an include cannot express. Resolver selection wants @TckExcludeTags("@in-process") and @TckExcludeTags("@rpc") instead. 263 tests, 0 failures. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/ExcludedTagsTest.java | 25 ++++++++++++++++--- .../skip-selftest/extensions/adopter.feature | 3 +++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java index 0aee75ab42..e21d8ab93a 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java @@ -28,6 +28,8 @@ class ExcludedTagsTest { private static final URI EXTENSION = URI.create("classpath:extensions/x.feature"); + private static final String UNTAGGED = "An extension scenario with no tags at all"; + @Test @DisplayName("an extension scenario runs with an included tag and no excluded one; canonical scenarios always run") void scenariosAreSelected() { @@ -39,13 +41,30 @@ void scenariosAreSelected() { "An extension scenario carrying the included tag", "An extension scenario tagged for both modes"); assertThat(results.aborted) - .as("the other-mode, excluded and to-be-filtered scenarios are skipped, saying why") - .hasSize(3) + .as("the other-mode, untagged, excluded and to-be-filtered scenarios are skipped, saying why") + .hasSize(4) .anySatisfy(reason -> assertThat(reason).contains(ExcludedTagsSuiteFixture.INCLUDED)) .anySatisfy(reason -> assertThat(reason).contains(ExcludedTagsSuiteFixture.EXCLUDED)); assertThat(results.failed).isEmpty(); } + @Test + @DisplayName("an untagged extension scenario is skipped under an include, so include filters rather than hints") + void anUntaggedExtensionScenarioIsNotSpared() { + Results results = run(null); + + assertThat(results.passed) + .as( + "%s is an include, so an extension scenario carrying no tag at all does not run", + ExcludedTagsSuiteFixture.INCLUDED) + .doesNotContain(UNTAGGED); + assertThat(results.aborted) + .as("it is skipped for lacking the included tag, like the other-mode scenarios") + .filteredOn(reason -> reason.contains(ExcludedTagsSuiteFixture.INCLUDED)) + .hasSize(3); + assertThat(results.failed).isEmpty(); + } + @Test @DisplayName("a cucumber.filter.tags filter still applies alongside the annotations") void aTagFilterStillApplies() { @@ -54,7 +73,7 @@ void aTagFilterStillApplies() { assertThat(results.passed).hasSize(3); assertThat(results.aborted) .as("the filtered scenario is gone, not skipped") - .hasSize(2); + .hasSize(3); assertThat(results.failed).isEmpty(); } diff --git a/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature b/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature index bc650a61dc..e1c2a5321f 100644 --- a/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature +++ b/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature @@ -22,3 +22,6 @@ Feature: An adopter selects its own extension scenarios by tag @mode-b @filtered Scenario: An extension scenario a tag filter removes first Given a stable provider + + Scenario: An extension scenario with no tags at all + Given a stable provider From 228fcdad777d1bf5f63cfe3cd4a8e989b49645ec Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 21 Oct 2026 17:29:44 +0200 Subject: [PATCH 03/13] fix(tck): select scenarios before gating capabilities A scenario this suite deselected is not a capability the provider withheld. Gating capabilities first filed every deselected extension scenario in the conformance skip report as a withheld capability, which overstates what the provider cannot do -- the worse error in a document read away from this repository. The Go binding orders it the other way round for that reason; this matches it. The two hard checks have to stay in front of both gates, or deselecting a scenario could mask a reserved tag on it: an adopter cannot exclude a capability tag, but could exclude some other tag on the same scenario and silence the reservation. They are now Capability.requireValidVocabulary, package-private because AdopterTags shares the package, so nothing new is public and requireDeclared behaves exactly as before. requireKnownVocabulary never needed the guard -- it applies to canonical scenarios only, and AdopterTags exempts those. Order is now vocabulary, selection, capability skip. The new test is mutation-checked: removing the vocabulary call makes it fail. 264 tests, 0 failures. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/AdopterTags.java | 2 ++ .../contrib/tools/tck/Capability.java | 20 +++++++++++++++++-- .../tck/internal/steps/ProviderSteps.java | 6 ++++-- .../contrib/tools/tck/ExcludedTagsTest.java | 10 ++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java index f4b7cc0810..178e7f5980 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java @@ -58,6 +58,8 @@ public static void requireValid(Class harness) { */ public static void requireSelected(URI source, Collection tags, Class harness) { requireValid(harness); + // before any skip, so an exclude cannot mask a reserved tag on the scenario + Capability.requireValidVocabulary(source, tags); if (Capability.isCanonical(source)) { return; } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java index 2d7e8932af..704aa7afeb 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java @@ -226,8 +226,7 @@ static void requireDeclared(Collection tags, Set declared) { * @throws TestAbortedException if a tag gates an inexpressible or an undeclared capability */ public static void requireDeclared(URI source, Collection tags, Set declared) { - requireNoExpiredReservation(tags); - requireKnownVocabulary(source, tags); + requireValidVocabulary(source, tags); for (String tag : tags) { Optional found = fromTag(tag); if (!found.isPresent()) { @@ -247,6 +246,23 @@ public static void requireDeclared(URI source, Collection tags, SetRun ahead of {@link AdopterTags#requireSelected} as well, so deselecting a scenario cannot + * mask a reserved tag on it: a reservation that has expired is a maintenance signal about the + * suite, not a property of the run, and an adopter excluding some other tag on the same scenario + * should not silence it. + * + * @param source the scenario's feature file, as Cucumber reports it, or {@code null} if unknown + * @param tags the scenario's Gherkin tags, including the leading at-sign + * @throws IllegalStateException if a tag is reserved, or unknown on a canonical scenario + */ + static void requireValidVocabulary(URI source, Collection tags) { + requireNoExpiredReservation(tags); + requireKnownVocabulary(source, tags); + } + /** * Fails if a canonical (non-extension) scenario carries a tag unknown to {@link Capability}. * 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 1a436d7b14..3a90b17fca 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 @@ -61,10 +61,12 @@ public static void afterAll() { */ @Before(order = 0) public void gateOnCapabilities(Scenario scenario) { - Capability.requireDeclared( - scenario.getUri(), scenario.getSourceTagNames(), harness().capabilities()); + // Selection first: a scenario this suite deselected is not a capability the provider + // withheld, and gating capabilities first would report it as one. AdopterTags.requireSelected( scenario.getUri(), scenario.getSourceTagNames(), harness().getClass()); + Capability.requireDeclared( + scenario.getUri(), scenario.getSourceTagNames(), harness().capabilities()); } /** diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java index e21d8ab93a..2681756363 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java @@ -102,6 +102,16 @@ void excludeWinsAndDeclarationsAddUp() { .doesNotThrowAnyException(); } + @Test + @DisplayName("deselecting a scenario cannot mask a reserved tag it carries") + void theVocabularyIsCheckedBeforeAnySkip() { + assertThatThrownBy(() -> AdopterTags.requireSelected( + EXTENSION, Arrays.asList("@fractional-v1", "@caching"), RpcMode.class)) + .as("the excluded tag would have skipped it; the reserved one still fails the run") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("CACHING"); + } + @Test @DisplayName("a capability tag cannot be included or excluded, at suite start or in the scenario gate") void aCapabilityTagIsRefused() { From 3607e5bfd8d0bde86665f70e88153baefdbc75a3 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 21 Oct 2026 21:04:25 +0200 Subject: [PATCH 04/13] feat(tck): take over the generic steps an adoption had written locally Four steps in flagd's extension glue were about nothing flagd-specific. Two never touched a backend at all, and two called normative control-API operations through a vendor-named step: the backend serves the {string} configuration POST /start?config= the connection is lost for {int}s POST /restart?seconds= a context with the following keys: the evaluation context the {} event handler should not have been ... the recorded events BackendControl gains serveConfiguration and disconnectFor as optional operations beside disconnect and reconnect, because a control serving one in-process flag set has no second configuration to offer. disconnectFor is specified where the control API deliberately is not: POST /restart may answer when the backend goes down or when it is back, so the step guarantees only that the outage has begun and a scenario asserts the recovery through events or client state. The negative event step gets the window its positive twin always had. At an instant, an event that never fires and one that fires a moment later are the same observation; EventStepsTest pins the difference, and without the window a late event passes. TckValues learns Long and StringList, which the table form needs, and gains convertContextEntry: there an Integer too wide for 32 bits widens rather than failing. A flag's default is bounded by the accessor that reads it back and stays strict, but nothing reads a context entry back, and a targeting operator may hash 4294967295 -- which flagd's fractional scenarios do. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/BackendControl.java | 39 ++++++ .../contrib/tools/tck/HttpBackendControl.java | 36 ++++- .../contrib/tools/tck/internal/TckValues.java | 39 +++++- .../tck/internal/steps/ContextSteps.java | 29 ++++ .../tools/tck/internal/steps/EventSteps.java | 45 ++++++ .../tck/internal/steps/ProviderSteps.java | 28 ++++ .../tools/tck/HttpBackendControlTest.java | 46 ++++++ .../tools/tck/internal/TckValuesTest.java | 50 +++++++ .../tck/internal/steps/EventStepsTest.java | 132 ++++++++++++++++++ 9 files changed, 440 insertions(+), 4 deletions(-) create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java index a16a989c90..11416c6456 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java @@ -1,5 +1,7 @@ package dev.openfeature.contrib.tools.tck; +import java.time.Duration; + /** * The seam between the TCK's step definitions and whatever manipulates the backend. * @@ -54,6 +56,43 @@ default void reconnect() { throw unsupported("reconnect"); } + /** + * Takes the backend down and lets it come back after {@code outage}, without stopping any + * container. + * + *

Returns once the outage has begun, and says nothing about when it ends. + * {@code POST /restart} is specified to answer either when the backend is down or only once it is + * back, so a scenario that wants the recovery asserts it through the provider's own events or + * client state. Assuming this call waited makes a feature file pass against one control API and + * hang against another. + * + * @param outage how long the backend stays unreachable + * @throws UnsupportedOperationException if this backend has no connection to lose + */ + default void disconnectFor(Duration outage) { + throw unsupported("disconnectFor"); + } + + /** + * Restarts the backend on one of its own named flag configurations. + * + *

The names belong to the backend, not to the TCK: the control API takes {@code config} as a + * free string defaulting to {@code default}, so a feature file naming one is naming something + * only that backend's control defines. Optional for the reason {@link #disconnect()} is -- a + * control serving a single in-process flag set has no second configuration to offer -- and + * therefore for extension scenarios only. The canonical scenarios never ask: the baseline every + * control already serves is the configuration they are written against. + * + *

Takes effect before the provider connects, so a scenario states it ahead of + * {@code a stable provider}. The next scenario starts from this suite's own configuration again. + * + * @param configuration the configuration name, as that backend's control defines it + * @throws UnsupportedOperationException if this backend serves one configuration only + */ + default void serveConfiguration(String configuration) { + throw unsupported("serveConfiguration"); + } + /** * Returns a short description of what is being controlled, for logs and failure messages. * 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 1c054408d4..53fcdab219 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 @@ -30,6 +30,9 @@ final class HttpBackendControl implements BackendControl { // whether the backend was last left unreachable, so /reset alone won't suffice private boolean backendStopped; + // a configuration one scenario asked for, which the next scenario has to be taken off again + private String scenarioConfiguration; + /** * Creates a control client for a running backend. No post-command settle; control calls return * once the backend has acted. @@ -71,11 +74,14 @@ public BackendType backendType() { /** * {@inheritDoc} * - *

Uses {@code POST /reset} if the backend is running, else {@code POST /start?config=...}. + *

Uses {@code POST /reset} if the backend is running on this suite's own configuration, else + * {@code POST /start?config=...}: {@code /reset} restores flag state, not which configuration is + * being served, so a scenario that served another one has to be taken off it explicitly. */ @Override public void prepareScenario() { - if (backendStopped) { + if (backendStopped || scenarioConfiguration != null) { + scenarioConfiguration = null; start(); } else { reset(); @@ -113,6 +119,32 @@ public void reconnect() { start(); } + /** + * {@inheritDoc} + * + *

Issues {@code POST /restart?seconds=...}, and does not wait: whether the control API + * answers at the start or the end of the outage is its own choice. + */ + @Override + public void disconnectFor(Duration outage) { + post("/restart?seconds=" + outage.getSeconds()); + // the backend brings itself back, so the next scenario may reset rather than start + backendStopped = false; + } + + /** + * {@inheritDoc} + * + *

Issues {@code POST /start?config=...} with the name given, which also seeds that + * configuration's flag state. + */ + @Override + public void serveConfiguration(String configuration) { + post("/start?config=" + URLEncoder.encode(configuration, StandardCharsets.UTF_8)); + scenarioConfiguration = configuration; + backendStopped = false; + } + /** * Waits until the control API accepts commands, probing optional {@code GET /healthz} (a * {@code 404} counts as ready). diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckValues.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckValues.java index e9b6eb58be..2f721c1476 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckValues.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckValues.java @@ -3,6 +3,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import dev.openfeature.sdk.Value; import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.stream.Collectors; /** * Converts feature-file strings into typed values; {@code Integer} yields {@link Integer} and @@ -20,7 +23,7 @@ private TckValues() {} * @param value the raw string from the feature file; the literal {@code null} yields * {@code null} * @param type the declared type, one of {@code Boolean}, {@code String}, {@code Integer}, - * {@code Float} or {@code Object} + * {@code Long}, {@code Float}, {@code StringList} or {@code Object} * @return the converted value */ public static Object convert(String value, String type) { @@ -41,16 +44,48 @@ public static Object convert(String value, String type) { + "(Client.getIntegerDetails cannot request larger values).", e); } + case "Long": + return Long.parseLong(value); case "Float": return Double.parseDouble(value); + case "StringList": + // a context key an operator reads as a list; empty means the empty list, not [""] + return value.isEmpty() + ? Collections.emptyList() + : Arrays.stream(value.split(",")).map(String::trim).collect(Collectors.toList()); case "Object": return toValue(value); default: throw new IllegalArgumentException("Unknown flag type '" + type - + "'. Supported types are Boolean, String, Integer, Float and Object."); + + "'. Supported types are Boolean, String, Integer, Long, Float, StringList " + + "and Object."); } } + /** + * Converts a feature-file string for an evaluation context entry. + * + *

Differs from {@link #convert} in one place, and deliberately: an {@code Integer} too wide + * for 32 bits widens to a {@code long} instead of failing. A flag's default value is bounded by + * the accessor that reads it back -- {@code getIntegerDetails} cannot ask for more -- but nothing + * reads a context entry back through an accessor, and a targeting operator may hash a 64-bit + * number. Rejecting those would make the TCK refuse context a provider handles correctly. + * + * @param value the raw string from the feature file + * @param type the declared type, as {@link #convert} takes it + * @return the converted value + */ + public static Object convertContextEntry(String value, String type) { + if ("Integer".equals(type) && !"null".equals(value)) { + try { + return Integer.parseInt(value); + } catch (NumberFormatException wider) { + return Long.parseLong(value); + } + } + return convert(value, type); + } + private static Value toValue(String json) { try { return Value.objectToValue(MAPPER.readValue(json, Object.class)); diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java index d4fb1b85a6..31a735d1ff 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java @@ -3,8 +3,14 @@ import dev.openfeature.contrib.tools.tck.BackendControl; import dev.openfeature.contrib.tools.tck.ProviderTckHarness; import dev.openfeature.contrib.tools.tck.TckState; +import dev.openfeature.contrib.tools.tck.internal.TckValues; +import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.MutableStructure; +import dev.openfeature.sdk.Value; +import io.cucumber.datatable.DataTable; import io.cucumber.java.en.Given; +import java.util.HashMap; +import java.util.Map; /** * Steps that build the evaluation context passed to every evaluation. @@ -42,6 +48,29 @@ public void contextContainingKeyWithTypeAndValue(String key, String type, String } } + /** + * Adds several typed entries to the evaluation context at once. + * + *

The table form of the step above, for a scenario whose targeting depends on more than one + * key. Its type vocabulary is {@link TckValues}', which is wider than the single-key step's -- + * {@code Long}, {@code StringList} and {@code Object} included -- because a table is where a + * scenario reaches for the types an operator takes. + * + * @param table rows with {@code key}, {@code type} and {@code value} columns + */ + @Given("a context with the following keys:") + public void aContextWithTheFollowingKeys(DataTable table) { + // Rebuilt rather than added to: MutableContext has typed add() overloads but none taking a + // Value, and the conversion above produces exactly that for an Object or a StringList. + Map entries = new HashMap<>(state.context.asMap()); + for (Map row : table.asMaps(String.class, String.class)) { + entries.put( + row.get("key"), + Value.objectToValue(TckValues.convertContextEntry(row.get("value"), row.get("type")))); + } + state.context = new MutableContext(state.context.getTargetingKey(), entries); + } + /** * Sets the targeting key on the evaluation context. * diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java index ed5a51cf6c..f86a300c21 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java @@ -1,6 +1,7 @@ package dev.openfeature.contrib.tools.tck.internal.steps; import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; import dev.openfeature.contrib.tools.tck.BackendControl; @@ -23,6 +24,9 @@ public class EventSteps extends AbstractSteps { private static final Logger log = LoggerFactory.getLogger(EventSteps.class); + // Awaitility wants a deadline past the poll delay; this only covers the assertion itself. + private static final long SETTLE_SLACK_MS = 250; + public EventSteps(TckState state, ProviderTckHarness harness, BackendControl backend) { super(state, harness, backend); } @@ -71,6 +75,47 @@ public void theEventHandlerShouldHaveBeenExecutedWithin(String eventType, int mi awaitEvent(eventType, milliseconds); } + /** + * Asserts that no event of the given kind arrives, within + * {@link ProviderTckHarness#eventTimeout()}. + * + * @param eventType the event kind + */ + @Then("the {} event handler should not have been executed") + public void theEventHandlerShouldNotHaveBeenExecuted(String eventType) { + awaitNoEvent(eventType, harness().eventTimeout().toMillis()); + } + + /** + * Asserts that no event of the given kind arrives within an explicit window. + * + * @param eventType the event kind + * @param milliseconds how long to watch for it + */ + @Then("the {} event handler should not have been executed within {int}ms") + public void theEventHandlerShouldNotHaveBeenExecutedWithin(String eventType, int milliseconds) { + awaitNoEvent(eventType, milliseconds); + } + + /** + * Watches for the whole window before answering, because at any single instant absence and + * lateness are the same observation: asserting immediately states "no such event yet", + * which a provider one scheduler slice slower satisfies while the scenario means it never fires. + * The positive form awaits, so the negative form has to watch at least as long to be its + * opposite. + */ + private void awaitNoEvent(String eventType, long milliseconds) { + log.info("Watching for no {} event ({}ms)", eventType, milliseconds); + await().alias("no provider event " + eventType) + .pollDelay(milliseconds, MILLISECONDS) + .atMost(milliseconds + SETTLE_SLACK_MS, MILLISECONDS) + .untilAsserted(() -> assertThat(state.events.stream() + .filter(event -> event.type().equals(eventType)) + .findFirst()) + .as("no %s event should have fired within %dms", eventType, milliseconds) + .isEmpty()); + } + private void awaitEvent(String eventType, long milliseconds) { log.info("Awaiting {} event (timeout {}ms)", eventType, milliseconds); await().alias("provider event " + eventType) 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 3a90b17fca..1d922e846f 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 @@ -208,6 +208,34 @@ public void theConnectionIsRestored() { backend().reconnect(); } + /** + * Takes the backend down for a fixed outage and lets it come back by itself. + * + *

What the step guarantees is {@link BackendControl#disconnectFor}'s contract: the outage has + * begun, and nothing more. A scenario that needs the backend back says so -- {@code a ready event + * was fired}, or {@code the client should be in READY state} -- rather than relying on this step + * to have waited. + * + * @param seconds how long the backend stays unreachable + */ + @When("the connection is lost for {int}s") + public void theConnectionIsLostFor(int seconds) { + backend().disconnectFor(Duration.ofSeconds(seconds)); + } + + /** + * Serves one of the backend's own named flag configurations, before the provider connects. + * + *

Extension scenarios only, and the name is the backend's own; see + * {@link BackendControl#serveConfiguration}. + * + * @param configuration the configuration name + */ + @Given("the backend serves the {string} configuration") + public void theBackendServesTheConfiguration(String configuration) { + backend().serveConfiguration(configuration); + } + /** Mutates flag configuration so a conforming provider observes a configuration change. */ @When("the flag was modified") public void theFlagWasModified() { 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 09eda6959d..733f4e6f3f 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 @@ -110,6 +110,52 @@ void aDisconnectForcesStart() throws Exception { } } + @Test + @DisplayName("serving a named configuration starts the backend on that name") + void servingAConfigurationStartsItByName() throws Exception { + try (StubControlApi stub = StubControlApi.start()) { + HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); + control.prepareScenario(); + stub.forget(); + + control.serveConfiguration("metadata"); + + assertThat(stub.requests()).containsExactly("POST /start?config=metadata"); + } + } + + @Test + @DisplayName("the scenario after a served configuration is taken back to the suite's own") + void aServedConfigurationDoesNotLeakIntoTheNextScenario() throws Exception { + try (StubControlApi stub = StubControlApi.start()) { + HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); + control.prepareScenario(); // settles on /reset, which this stub implements + control.serveConfiguration("metadata"); + stub.forget(); + + control.prepareScenario(); + + // /reset would have left the metadata configuration serving the next scenario + assertThat(stub.requests()).containsExactly("POST /start?config=default"); + } + } + + @Test + @DisplayName("a timed outage restarts for that many seconds and leaves the backend expected up") + void aTimedOutageRestarts() throws Exception { + try (StubControlApi stub = StubControlApi.start()) { + HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); + control.prepareScenario(); + stub.forget(); + + control.disconnectFor(Duration.ofSeconds(8)); + control.prepareScenario(); + + // the backend comes back by itself, so the next scenario needs no /start + assertThat(stub.requests()).containsExactly("POST /restart?seconds=8", "POST /reset"); + } + } + @Test @DisplayName("reconnecting clears the disconnect, so the next scenario resets again") void reconnectClearsTheDisconnect() throws Exception { diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java new file mode 100644 index 0000000000..def660ec99 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java @@ -0,0 +1,50 @@ +package dev.openfeature.contrib.tools.tck.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.List; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** The type vocabulary feature files write values in. */ +class TckValuesTest { + + @Test + @DisplayName("Long and StringList are the types a context table reaches for") + void theWiderTypes() { + assertThat(TckValues.convert("4294967296", "Long")).isEqualTo(4294967296L); + assertThat(TckValues.convert("a, b ,c", "StringList")).isEqualTo(List.of("a", "b", "c")); + assertThat(TckValues.convert("", "StringList")).isEqualTo(List.of()); + } + + @Test + @DisplayName("a flag's Integer default stays bounded by the accessor that reads it back") + void aFlagDefaultIsStillRejectedWhenItDoesNotFit() { + assertThatThrownBy(() -> TckValues.convert("4294967295", "Integer")) + .as("getIntegerDetails cannot ask for this, so a feature file asking is a mistake") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not a 32-bit Integer"); + } + + @Test + @DisplayName("the same value in a context entry widens instead, because nothing reads it back") + void aContextEntryWidens() { + assertThat(TckValues.convertContextEntry("4294967295", "Integer")) + .as("a targeting operator may hash a 64-bit number") + .isEqualTo(4294967295L); + assertThat(TckValues.convertContextEntry("7", "Integer")) + .as("what fits is still an Integer, so a provider sees the narrower type") + .isEqualTo(7); + } + + @Test + @DisplayName("every other type converts the same either way") + void theRestIsUnchanged() { + for (String[] pair : new String[][] {{"true", "Boolean"}, {"x", "String"}, {"1.5", "Float"}}) { + assertThat(TckValues.convertContextEntry(pair[0], pair[1])) + .isEqualTo(TckValues.convert(pair[0], pair[1])); + } + assertThat(TckValues.convertContextEntry("null", "Integer")).isNull(); + } +} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java new file mode 100644 index 0000000000..806e65d9f2 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java @@ -0,0 +1,132 @@ +package dev.openfeature.contrib.tools.tck.internal.steps; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import dev.openfeature.contrib.tools.tck.BackendControl; +import dev.openfeature.contrib.tools.tck.InProcessControl; +import dev.openfeature.contrib.tools.tck.ProviderEventRecord; +import dev.openfeature.contrib.tools.tck.ProviderTckHarness; +import dev.openfeature.contrib.tools.tck.TckState; +import dev.openfeature.sdk.FeatureProvider; +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Verifies what {@code should not have been executed} asserts. + * + *

The point of these is the window. A negative assertion taken at one instant cannot tell an + * event that never fires from one that fires a moment later, and the scenarios that use it mean the + * first -- so the step has to watch, and a late event has to fail it. + */ +class EventStepsTest { + + private static final String ERROR = "error"; + + /** A harness that brings nothing up and answers a short event timeout. */ + private static final class StubHarness implements ProviderTckHarness { + + private final InProcessControl control = BackendControl.inProcess(); + + private final Duration eventTimeout; + + private StubHarness(Duration eventTimeout) { + this.eventTimeout = eventTimeout; + } + + @Override + public FeatureProvider createProvider() { + return control.createProvider(); + } + + @Override + public BackendControl backendControl() { + return control; + } + + @Override + public Duration eventTimeout() { + return eventTimeout; + } + } + + private static EventSteps stepsWith(TckState state, Duration eventTimeout) { + StubHarness harness = new StubHarness(eventTimeout); + return new EventSteps(state, harness, harness.backendControl()); + } + + @Test + @DisplayName("an event that never fires passes, and the window is actually waited out") + void anAbsenceIsWatchedForTheWholeWindow() { + TckState state = new TckState(); + EventSteps steps = stepsWith(state, Duration.ofMillis(300)); + + long started = System.nanoTime(); + assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 300)) + .doesNotThrowAnyException(); + + assertThat(Duration.ofNanos(System.nanoTime() - started)) + .as("asserting before the window elapses would only say 'not yet'") + .isGreaterThanOrEqualTo(Duration.ofMillis(300)); + } + + @Test + @DisplayName("an event that fires late inside the window fails the assertion") + void aLateEventIsNotMistakenForAbsence() { + TckState state = new TckState(); + EventSteps steps = stepsWith(state, Duration.ofMillis(600)); + CompletableFuture.runAsync(() -> { + try { + Thread.sleep(150); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + state.events.add(new ProviderEventRecord(ERROR, null)); + }); + + assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 600)) + .as("the instant form passed here, which is the bug this step was given a window for") + .hasMessageContaining("no error event should have fired"); + } + + @Test + @DisplayName("an event already recorded fails, naming the kind and the window") + void anEventAlreadyRecordedFails() { + TckState state = new TckState(); + state.events.add(new ProviderEventRecord(ERROR, null)); + EventSteps steps = stepsWith(state, Duration.ofMillis(200)); + + assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) + .hasMessageContaining("no error event should have fired within 200ms"); + } + + @Test + @DisplayName("another kind of event does not fail it") + void anotherKindOfEventIsIgnored() { + TckState state = new TckState(); + state.events.add(new ProviderEventRecord("ready", null)); + EventSteps steps = stepsWith(state, Duration.ofMillis(200)); + + assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("the step without a window watches for the harness's event timeout") + void theDefaultWindowIsTheHarnessEventTimeout() { + TckState state = new TckState(); + EventSteps steps = stepsWith(state, Duration.ofMillis(400)); + + long started = System.nanoTime(); + assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecuted(ERROR)) + .doesNotThrowAnyException(); + + assertThat(Duration.ofNanos(System.nanoTime() - started)) + .as("the pair has to be symmetrical: the positive form awaits this same timeout") + .isGreaterThanOrEqualTo(Duration.ofMillis(400)); + } +} From bc18c2e1a0f51e205e1d41a97a41e70692d22793 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 09:45:52 +0200 Subject: [PATCH 05/13] fix(tck): keep the configuration marker, and let /restart outlast its outage Two findings from review, both in HttpBackendControl. prepareScenario cleared scenarioConfiguration before the /start that restores this suite's configuration, so a /start that failed left the next scenario on /reset -- which restores flag state but not which configuration is served. It is cleared once the start succeeds now, and the restoration is retried otherwise. disconnectFor sent /restart with the ordinary 30s request timeout, while the step's own contract says the control API may answer only once the backend is back. A 60-second outage against a blocking implementation would fail the request, and the scenario, for behaviour the specification permits; the budget is the outage plus the ordinary timeout. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/HttpBackendControl.java | 35 ++++++++++++++++--- .../tools/tck/HttpBackendControlTest.java | 32 +++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) 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 53fcdab219..4874c157e9 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 @@ -20,6 +20,8 @@ final class HttpBackendControl implements BackendControl { private static final Logger log = LoggerFactory.getLogger(HttpBackendControl.class); + private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(30); + private final HttpClient http; private final String baseUrl; private final String backendConfiguration; @@ -81,8 +83,10 @@ public BackendType backendType() { @Override public void prepareScenario() { if (backendStopped || scenarioConfiguration != null) { - scenarioConfiguration = null; start(); + // cleared only once the restart succeeded: a /start that failed leaves the scenario's + // configuration serving, and /reset would not take the next scenario off it + scenarioConfiguration = null; } else { reset(); } @@ -127,7 +131,7 @@ public void reconnect() { */ @Override public void disconnectFor(Duration outage) { - post("/restart?seconds=" + outage.getSeconds()); + post("/restart?seconds=" + outage.getSeconds(), restartTimeout(outage)); // the backend brings itself back, so the next scenario may reset rather than start backendStopped = false; } @@ -211,13 +215,36 @@ private void reset() { } private void post(String path) { - expectSuccess(path, send(path)); + post(path, REQUEST_TIMEOUT); + } + + private void post(String path, Duration timeout) { + expectSuccess(path, send(path, timeout)); + } + + /** + * How long to wait for {@code POST /restart} to answer. + * + *

The control API may answer when the backend goes down or only once it is back, and a + * blocking implementation of a 60-second outage answers after 60 seconds. Waiting only + * {@link #REQUEST_TIMEOUT} would fail the request -- and the scenario -- for a control API + * behaving exactly as specified, so the budget is the outage plus the ordinary one. + * + * @param outage the outage that was asked for + * @return how long the restart request may take + */ + static Duration restartTimeout(Duration outage) { + return outage.isNegative() ? REQUEST_TIMEOUT : REQUEST_TIMEOUT.plus(outage); } private HttpResponse send(String path) { + return send(path, REQUEST_TIMEOUT); + } + + private HttpResponse send(String path, Duration timeout) { HttpRequest request = HttpRequest.newBuilder(URI.create(baseUrl + path)) .POST(HttpRequest.BodyPublishers.noBody()) - .timeout(Duration.ofSeconds(30)) + .timeout(timeout) .build(); try { return http.send(request, HttpResponse.BodyHandlers.discarding()); 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 733f4e6f3f..ffa66adc2a 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 @@ -156,6 +156,38 @@ void aTimedOutageRestarts() throws Exception { } } + @Test + @DisplayName("a restoration that fails is retried by the next scenario, not downgraded to /reset") + void aFailedRestorationIsRetried() throws Exception { + // the scenario's own /start succeeds; the restoration after it fails once + try (StubControlApi stub = StubControlApi.start().scripting("/start", 200, 500, 200)) { + HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); + control.prepareScenario(); // settles on /reset + control.serveConfiguration("metadata"); + stub.forget(); + + // the scenario whose preparation failed reports it + assertThatThrownBy(control::prepareScenario).isInstanceOf(IllegalStateException.class); + control.prepareScenario(); + + // /reset here would have left the metadata configuration serving + assertThat(stub.requests()) + .containsExactly("POST /start?config=default", "POST /start?config=default"); + } + } + + @Test + @DisplayName("a restart request may take the outage it asked for, plus the ordinary budget") + void theRestartRequestOutlivesTheOutage() { + // a blocking control API answers /restart?seconds=60 after 60 seconds, which is specified + assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(60))) + .isEqualTo(Duration.ofSeconds(90)); + assertThat(HttpBackendControl.restartTimeout(Duration.ZERO)).isEqualTo(Duration.ofSeconds(30)); + assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(-5))) + .as("a nonsense outage still gets a sane budget") + .isEqualTo(Duration.ofSeconds(30)); + } + @Test @DisplayName("reconnecting clears the disconnect, so the next scenario resets again") void reconnectClearsTheDisconnect() throws Exception { From 3caca1cf6aa65ecae33d468d357027674f4368c4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 10:06:55 +0200 Subject: [PATCH 06/13] fix(tck): settle an outage, keep event evidence, let a table set targetingKey Three more findings from review, and the checkstyle name that failed the build. disconnectFor marked the backend up as soon as /restart answered, but an implementation that answers at the start of the outage leaves it down -- so the next scenario's /reset reached nothing. The outage's end is recorded and waited out before the next scenario is prepared; a blocking control API has already spent that time and the wait finds nothing left. A positive event await drained the queue through its match, discarding records of other kinds on the way. An error event arriving before an awaited change event was therefore gone, and "the error event handler should not have been executed" passed because the evidence had been thrown away. The await now takes its own record and leaves the rest. The context table carried the old targeting key over a row that set one, so the evaluation used the wrong key. The map asMap() returns already holds it, which ContextStepsTest pins from both sides: a row may set it, and one set earlier survives a table that does not mention it. checkstyle rejects a method whose second character is uppercase, so aContextWithTheFollowingKeys is contextWithTheFollowingKeys -- the name the other context steps already use. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/HttpBackendControl.java | 27 ++++ .../tck/internal/steps/ContextSteps.java | 7 +- .../tools/tck/internal/steps/EventSteps.java | 19 +-- .../tools/tck/HttpBackendControlTest.java | 28 ++++- .../tck/internal/steps/ContextStepsTest.java | 119 ++++++++++++++++++ .../tck/internal/steps/EventStepsTest.java | 15 +++ 6 files changed, 200 insertions(+), 15 deletions(-) create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextStepsTest.java 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 4874c157e9..406e101e99 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 @@ -8,6 +8,7 @@ import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.time.Instant; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,6 +36,9 @@ final class HttpBackendControl implements BackendControl { // a configuration one scenario asked for, which the next scenario has to be taken off again private String scenarioConfiguration; + // when a timed outage is due to be over, for a control API that answered at its start + private Instant outageUntil; + /** * Creates a control client for a running backend. No post-command settle; control calls return * once the backend has acted. @@ -82,6 +86,7 @@ public BackendType backendType() { */ @Override public void prepareScenario() { + awaitOutageEnd(); if (backendStopped || scenarioConfiguration != null) { start(); // cleared only once the restart succeeded: a /start that failed leaves the scenario's @@ -131,11 +136,33 @@ public void reconnect() { */ @Override public void disconnectFor(Duration outage) { + Instant due = Instant.now().plus(outage); post("/restart?seconds=" + outage.getSeconds(), restartTimeout(outage)); + // The response says nothing about recovery: an implementation that answered at the start of + // the outage leaves the backend down, and the next scenario's /reset would reach nothing. A + // blocking one has already spent this time, and the wait below finds nothing left. + outageUntil = due; // the backend brings itself back, so the next scenario may reset rather than start backendStopped = false; } + /** + * Waits out whatever is left of a timed outage, so the next scenario prepares a backend that is + * up. Costs nothing when the control API answered only once recovery had happened. + */ + private void awaitOutageEnd() { + if (outageUntil == null) { + return; + } + Duration remaining = Duration.between(Instant.now(), outageUntil); + outageUntil = null; + if (remaining.isNegative() || remaining.isZero()) { + return; + } + log.info("waiting {}ms for the backend's timed outage to end", remaining.toMillis()); + sleep(remaining); + } + /** * {@inheritDoc} * diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java index 31a735d1ff..c93923ae20 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextSteps.java @@ -59,16 +59,17 @@ public void contextContainingKeyWithTypeAndValue(String key, String type, String * @param table rows with {@code key}, {@code type} and {@code value} columns */ @Given("a context with the following keys:") - public void aContextWithTheFollowingKeys(DataTable table) { + public void contextWithTheFollowingKeys(DataTable table) { // Rebuilt rather than added to: MutableContext has typed add() overloads but none taking a - // Value, and the conversion above produces exactly that for an Object or a StringList. + // Value, and the conversion above produces exactly that for an Object or a StringList. The + // map carries the targeting key, so a row may set it and the old one survives when none does. Map entries = new HashMap<>(state.context.asMap()); for (Map row : table.asMaps(String.class, String.class)) { entries.put( row.get("key"), Value.objectToValue(TckValues.convertContextEntry(row.get("value"), row.get("type")))); } - state.context = new MutableContext(state.context.getTargetingKey(), entries); + state.context = new MutableContext(entries); } /** diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java index f86a300c21..66fca7fb54 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java @@ -124,14 +124,17 @@ private void awaitEvent(String eventType, long milliseconds) { .until(() -> state.events.stream().anyMatch(event -> event.type().equals(eventType))); - // drain through the first match so stale events can't satisfy later awaits - ProviderEventRecord matched = null; - while (!state.events.isEmpty()) { - ProviderEventRecord head = state.events.poll(); - if (head != null && head.type().equals(eventType)) { - matched = head; - break; - } + // Takes the matched record and nothing else. Draining through the match also discarded + // records of other kinds, which a later assertion is entitled to see: an error event that + // arrived before an awaited change event would be gone, and "the error event handler should + // not have been executed" would then pass because the evidence had been thrown away. + ProviderEventRecord matched = state.events.stream() + .filter(event -> event.type().equals(eventType)) + .findFirst() + .orElse(null); + if (matched != null) { + // removing only this record keeps a stale one of the same kind from a later await + state.events.remove(matched); } state.lastEvent = Optional.ofNullable(matched); } 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 ffa66adc2a..bd4f8f6a59 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 @@ -141,7 +141,7 @@ void aServedConfigurationDoesNotLeakIntoTheNextScenario() throws Exception { } @Test - @DisplayName("a timed outage restarts for that many seconds and leaves the backend expected up") + @DisplayName("a timed outage restarts for that many seconds") void aTimedOutageRestarts() throws Exception { try (StubControlApi stub = StubControlApi.start()) { HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); @@ -149,10 +149,10 @@ void aTimedOutageRestarts() throws Exception { stub.forget(); control.disconnectFor(Duration.ofSeconds(8)); - control.prepareScenario(); - // the backend comes back by itself, so the next scenario needs no /start - assertThat(stub.requests()).containsExactly("POST /restart?seconds=8", "POST /reset"); + // the outage is asked for in seconds; what the next scenario then does is the test below, + // which uses an outage short enough to wait out + assertThat(stub.requests()).containsExactly("POST /restart?seconds=8"); } } @@ -176,6 +176,26 @@ void aFailedRestorationIsRetried() throws Exception { } } + @Test + @DisplayName("the scenario after a timed outage waits for the outage to end before preparing") + void theNextScenarioWaitsOutTheOutage() throws Exception { + try (StubControlApi stub = StubControlApi.start()) { + HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); + control.prepareScenario(); + stub.forget(); + + // this stub answers at once, which is the non-blocking half of what /restart permits + control.disconnectFor(Duration.ofSeconds(1)); + long started = System.nanoTime(); + control.prepareScenario(); + + assertThat(Duration.ofNanos(System.nanoTime() - started)) + .as("/reset against a backend still inside its outage reaches nothing") + .isGreaterThanOrEqualTo(Duration.ofMillis(900)); + assertThat(stub.requests()).containsExactly("POST /restart?seconds=1", "POST /reset"); + } + } + @Test @DisplayName("a restart request may take the outage it asked for, plus the ordinary budget") void theRestartRequestOutlivesTheOutage() { diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextStepsTest.java new file mode 100644 index 0000000000..288a2fe37a --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/ContextStepsTest.java @@ -0,0 +1,119 @@ +package dev.openfeature.contrib.tools.tck.internal.steps; + +import static org.assertj.core.api.Assertions.assertThat; + +import dev.openfeature.contrib.tools.tck.BackendControl; +import dev.openfeature.contrib.tools.tck.InProcessControl; +import dev.openfeature.contrib.tools.tck.ProviderTckHarness; +import dev.openfeature.contrib.tools.tck.TckState; +import dev.openfeature.sdk.FeatureProvider; +import io.cucumber.datatable.DataTable; +import io.cucumber.datatable.DataTableTypeRegistry; +import io.cucumber.datatable.DataTableTypeRegistryTableConverter; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * What a context table does to the evaluation context. + * + *

The targeting key is the part worth pinning: the step rebuilds the context rather than adding + * to it, and rebuilding is where an earlier statement can be lost or a later one overridden. + */ +class ContextStepsTest { + + /** A harness that brings nothing up. */ + private static final class StubHarness implements ProviderTckHarness { + + private final InProcessControl control = BackendControl.inProcess(); + + @Override + public FeatureProvider createProvider() { + return control.createProvider(); + } + + @Override + public BackendControl backendControl() { + return control; + } + } + + private static ContextSteps stepsFor(TckState state) { + StubHarness harness = new StubHarness(); + return new ContextSteps(state, harness, harness.backendControl()); + } + + /** A table carrying the {@code key | type | value} header the step reads. */ + private static DataTable table(String... rows) { + List> cells = new ArrayList<>(); + cells.add(Arrays.asList("key", "type", "value")); + for (String row : rows) { + cells.add(Arrays.asList(row.split("\\s*\\|\\s*"))); + } + // Cucumber's own converter: DataTable.create(cells) alone cannot answer asMaps, and the + // step reads the table the way a scenario hands it over. + return DataTable.create( + cells, new DataTableTypeRegistryTableConverter(new DataTableTypeRegistry(Locale.ENGLISH))); + } + + @Test + @DisplayName("a targeting key set earlier survives a table that does not mention it") + void anEarlierTargetingKeySurvives() { + TckState state = new TckState(); + ContextSteps steps = stepsFor(state); + steps.contextContainingTargetingKey("tim"); + + steps.contextWithTheFollowingKeys(table("region | String | eu")); + + assertThat(state.context.getTargetingKey()) + .as("the table said nothing about it, so it is still the targeting key") + .isEqualTo("tim"); + assertThat(state.context.getValue("region").asString()).isEqualTo("eu"); + } + + @Test + @DisplayName("a table row may set the targeting key") + void theTableMaySetTheTargetingKey() { + TckState state = new TckState(); + ContextSteps steps = stepsFor(state); + steps.contextContainingTargetingKey("tim"); + + steps.contextWithTheFollowingKeys(table("targetingKey | String | ada")); + + assertThat(state.context.getTargetingKey()) + .as("the row is the later statement about it, so it is the one that holds") + .isEqualTo("ada"); + } + + @Test + @DisplayName("a row's type decides what the context carries") + void theTableSetsTypedEntries() { + TckState state = new TckState(); + ContextSteps steps = stepsFor(state); + + steps.contextWithTheFollowingKeys( + table("region | String | eu", "editions | StringList | one, two", "fresh | Boolean | true")); + + assertThat(state.context.getValue("region").asString()).isEqualTo("eu"); + assertThat(state.context.getValue("editions").asList()) + .as("a StringList is a list, not one comma-joined string") + .hasSize(2); + assertThat(state.context.getValue("fresh").asBoolean()).isTrue(); + } + + @Test + @DisplayName("entries from two tables add up") + void twoTablesAddUp() { + TckState state = new TckState(); + ContextSteps steps = stepsFor(state); + + steps.contextWithTheFollowingKeys(table("region | String | eu")); + steps.contextWithTheFollowingKeys(table("tier | String | gold")); + + assertThat(state.context.getValue("region").asString()).isEqualTo("eu"); + assertThat(state.context.getValue("tier").asString()).isEqualTo("gold"); + } +} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java index 806e65d9f2..347f48ea59 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java @@ -104,6 +104,21 @@ void anEventAlreadyRecordedFails() { .hasMessageContaining("no error event should have fired within 200ms"); } + @Test + @DisplayName("awaiting one kind of event does not throw away another kind's record") + void aPositiveAwaitDoesNotEraseTheEvidence() { + TckState state = new TckState(); + EventSteps steps = stepsWith(state, Duration.ofMillis(200)); + state.events.add(new ProviderEventRecord(ERROR, null)); + state.events.add(new ProviderEventRecord("change", null)); + + steps.theEventHandlerShouldHaveBeenExecuted("change"); + + assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) + .as("the error arrived first; awaiting the change must not make it unobservable") + .hasMessageContaining("no error event should have fired"); + } + @Test @DisplayName("another kind of event does not fail it") void anotherKindOfEventIsIgnored() { From 52ebb77e4585157066c0da4c15d0abf6f9a575b7 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 10:13:10 +0200 Subject: [PATCH 07/13] style(tck): join three assertions spotless keeps on one line Formatting only, taken from the diff the failing build printed. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/HttpBackendControlTest.java | 6 ++---- .../contrib/tools/tck/internal/TckValuesTest.java | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) 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 bd4f8f6a59..739e53e98f 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 @@ -171,8 +171,7 @@ void aFailedRestorationIsRetried() throws Exception { control.prepareScenario(); // /reset here would have left the metadata configuration serving - assertThat(stub.requests()) - .containsExactly("POST /start?config=default", "POST /start?config=default"); + assertThat(stub.requests()).containsExactly("POST /start?config=default", "POST /start?config=default"); } } @@ -200,8 +199,7 @@ void theNextScenarioWaitsOutTheOutage() throws Exception { @DisplayName("a restart request may take the outage it asked for, plus the ordinary budget") void theRestartRequestOutlivesTheOutage() { // a blocking control API answers /restart?seconds=60 after 60 seconds, which is specified - assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(60))) - .isEqualTo(Duration.ofSeconds(90)); + assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(60))).isEqualTo(Duration.ofSeconds(90)); assertThat(HttpBackendControl.restartTimeout(Duration.ZERO)).isEqualTo(Duration.ofSeconds(30)); assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(-5))) .as("a nonsense outage still gets a sane budget") diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java index def660ec99..4a60ee9893 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java @@ -42,8 +42,7 @@ void aContextEntryWidens() { @DisplayName("every other type converts the same either way") void theRestIsUnchanged() { for (String[] pair : new String[][] {{"true", "Boolean"}, {"x", "String"}, {"1.5", "Float"}}) { - assertThat(TckValues.convertContextEntry(pair[0], pair[1])) - .isEqualTo(TckValues.convert(pair[0], pair[1])); + assertThat(TckValues.convertContextEntry(pair[0], pair[1])).isEqualTo(TckValues.convert(pair[0], pair[1])); } assertThat(TckValues.convertContextEntry("null", "Integer")).isNull(); } From 694432e7f120dd428c669a59f7d3c31e90906e7f Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 10:20:05 +0200 Subject: [PATCH 08/13] fix(tck): leave the outage's timing in the endpoint, not in the client Reverts the client-side wait and the per-call timeout widening from the two commits before this one. Both moved the outage's timing into the control client, which is what this endpoint exists to avoid: an HTTP client already knows how to wait for a response, while a timer has to be written once per binding. The specification is explicit that the TCK does not rely on the /restart response being delayed and awaits provider events instead, so the control just treats the backend as down afterwards. The next scenario then starts it rather than resetting it, which is correct whether the outage is over or not and needs no clock. An outage is seconds, comfortably inside the 30s request timeout every call already shares; the javadoc says so rather than widening it. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/HttpBackendControl.java | 78 +++++-------------- .../tools/tck/HttpBackendControlTest.java | 29 +++---- 2 files changed, 31 insertions(+), 76 deletions(-) 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 406e101e99..d68208a61c 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 @@ -8,7 +8,6 @@ import java.net.http.HttpResponse; import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.time.Instant; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -21,7 +20,9 @@ final class HttpBackendControl implements BackendControl { private static final Logger log = LoggerFactory.getLogger(HttpBackendControl.class); - private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(30); + private static final int REQUEST_TIMEOUT_SECONDS = 30; + + private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(REQUEST_TIMEOUT_SECONDS); private final HttpClient http; private final String baseUrl; @@ -36,8 +37,6 @@ final class HttpBackendControl implements BackendControl { // a configuration one scenario asked for, which the next scenario has to be taken off again private String scenarioConfiguration; - // when a timed outage is due to be over, for a control API that answered at its start - private Instant outageUntil; /** * Creates a control client for a running backend. No post-command settle; control calls return @@ -86,7 +85,6 @@ public BackendType backendType() { */ @Override public void prepareScenario() { - awaitOutageEnd(); if (backendStopped || scenarioConfiguration != null) { start(); // cleared only once the restart succeeded: a /start that failed leaves the scenario's @@ -131,36 +129,25 @@ public void reconnect() { /** * {@inheritDoc} * - *

Issues {@code POST /restart?seconds=...}, and does not wait: whether the control API - * answers at the start or the end of the outage is its own choice. + *

Issues {@code POST /restart?seconds=...} and waits for the answer like any other call, + * which is the point of the endpoint: a blocking implementation spends the outage inside the + * response, and every language's HTTP client already knows how to wait for that. A timer here + * would have to be written once per binding instead. + * + *

An outage is therefore expected to be seconds, comfortably inside the {@value #REQUEST_TIMEOUT_SECONDS}-second + * request timeout every call shares; one approaching it would fail the request rather than the + * scenario's assertion. */ @Override public void disconnectFor(Duration outage) { - Instant due = Instant.now().plus(outage); - post("/restart?seconds=" + outage.getSeconds(), restartTimeout(outage)); - // The response says nothing about recovery: an implementation that answered at the start of - // the outage leaves the backend down, and the next scenario's /reset would reach nothing. A - // blocking one has already spent this time, and the wait below finds nothing left. - outageUntil = due; - // the backend brings itself back, so the next scenario may reset rather than start - backendStopped = false; - } - - /** - * Waits out whatever is left of a timed outage, so the next scenario prepares a backend that is - * up. Costs nothing when the control API answered only once recovery had happened. - */ - private void awaitOutageEnd() { - if (outageUntil == null) { - return; - } - Duration remaining = Duration.between(Instant.now(), outageUntil); - outageUntil = null; - if (remaining.isNegative() || remaining.isZero()) { - return; - } - log.info("waiting {}ms for the backend's timed outage to end", remaining.toMillis()); - sleep(remaining); + post("/restart?seconds=" + outage.getSeconds()); + // Treated as still down, which costs the next scenario a /start instead of a /reset and + // nothing else. The alternative is a timer: the response may arrive as soon as the outage + // begins, and the specification is explicit that the TCK does not rely on it being delayed + // -- a scenario awaits the provider's own events instead, and sleeping here to find out + // whether the backend is back would put that timing in the control client, where this + // endpoint exists precisely to avoid it. + backendStopped = true; } /** @@ -242,36 +229,13 @@ private void reset() { } private void post(String path) { - post(path, REQUEST_TIMEOUT); - } - - private void post(String path, Duration timeout) { - expectSuccess(path, send(path, timeout)); - } - - /** - * How long to wait for {@code POST /restart} to answer. - * - *

The control API may answer when the backend goes down or only once it is back, and a - * blocking implementation of a 60-second outage answers after 60 seconds. Waiting only - * {@link #REQUEST_TIMEOUT} would fail the request -- and the scenario -- for a control API - * behaving exactly as specified, so the budget is the outage plus the ordinary one. - * - * @param outage the outage that was asked for - * @return how long the restart request may take - */ - static Duration restartTimeout(Duration outage) { - return outage.isNegative() ? REQUEST_TIMEOUT : REQUEST_TIMEOUT.plus(outage); + expectSuccess(path, send(path)); } private HttpResponse send(String path) { - return send(path, REQUEST_TIMEOUT); - } - - private HttpResponse send(String path, Duration timeout) { HttpRequest request = HttpRequest.newBuilder(URI.create(baseUrl + path)) .POST(HttpRequest.BodyPublishers.noBody()) - .timeout(timeout) + .timeout(REQUEST_TIMEOUT) .build(); try { return http.send(request, HttpResponse.BodyHandlers.discarding()); 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 739e53e98f..bf7a1a9521 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 @@ -176,36 +176,27 @@ void aFailedRestorationIsRetried() throws Exception { } @Test - @DisplayName("the scenario after a timed outage waits for the outage to end before preparing") - void theNextScenarioWaitsOutTheOutage() throws Exception { + @DisplayName("the scenario after a timed outage starts the backend instead of waiting for it") + void theNextScenarioStartsRatherThanWaiting() throws Exception { try (StubControlApi stub = StubControlApi.start()) { HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); - control.prepareScenario(); + control.prepareScenario(); // settles on /reset, which this stub implements stub.forget(); - // this stub answers at once, which is the non-blocking half of what /restart permits - control.disconnectFor(Duration.ofSeconds(1)); + // this stub answers at once, which is the half of /restart the specification permits and + // tells the TCK not to rely on: the backend may still be down when the scenario ends + control.disconnectFor(Duration.ofSeconds(5)); long started = System.nanoTime(); control.prepareScenario(); + // /start works whether the outage is over or not, and /reset is not specified to start + assertThat(stub.requests()).containsExactly("POST /restart?seconds=5", "POST /start?config=default"); assertThat(Duration.ofNanos(System.nanoTime() - started)) - .as("/reset against a backend still inside its outage reaches nothing") - .isGreaterThanOrEqualTo(Duration.ofMillis(900)); - assertThat(stub.requests()).containsExactly("POST /restart?seconds=1", "POST /reset"); + .as("no timer: the outage's length is the backend's business, not this client's") + .isLessThan(Duration.ofSeconds(1)); } } - @Test - @DisplayName("a restart request may take the outage it asked for, plus the ordinary budget") - void theRestartRequestOutlivesTheOutage() { - // a blocking control API answers /restart?seconds=60 after 60 seconds, which is specified - assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(60))).isEqualTo(Duration.ofSeconds(90)); - assertThat(HttpBackendControl.restartTimeout(Duration.ZERO)).isEqualTo(Duration.ofSeconds(30)); - assertThat(HttpBackendControl.restartTimeout(Duration.ofSeconds(-5))) - .as("a nonsense outage still gets a sane budget") - .isEqualTo(Duration.ofSeconds(30)); - } - @Test @DisplayName("reconnecting clears the disconnect, so the next scenario resets again") void reconnectClearsTheDisconnect() throws Exception { From 6e4394e9ccc401d2cf1220dde8de9e4e616fc301 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 10:44:20 +0200 Subject: [PATCH 09/13] feat(tck): drop the timed outage, and name the configuration on the provider Two steps go, and neither was pulling its weight. `the connection is lost for {int}s` is an antipattern: a scenario does not care how long the backend was away, it cares that the provider went stale, then errored, then came back ready. Awaiting those events *is* the wait, and it is the wait the provider's own retry timing decides -- so the chains now read disconnect, stale, error, restore, ready, with no clock in them. The specification already said no shipped scenario reaches `/restart`; now nothing in the TCK does either, and `disconnectFor` goes with the step. `the backend serves the {string} configuration` becomes `a {} provider with {string} configuration`. The ordering was the whole difficulty: the backend had to be serving that configuration before the provider connected, and a separate `Given` could be written after `a stable provider` and quietly connect to the wrong flags. Said on the provider step, it cannot be ordered wrongly. `reconnect()` now starts the configuration in effect rather than the suite's, because losing the connection is not how a scenario changes what is served -- which matters now that a scenario restores the connection itself. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/BackendControl.java | 23 +------- .../contrib/tools/tck/HttpBackendControl.java | 51 ++++++----------- .../tck/internal/steps/ProviderSteps.java | 55 +++++++++---------- .../tools/tck/HttpBackendControlTest.java | 38 ------------- 4 files changed, 47 insertions(+), 120 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java index 11416c6456..94491923c7 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java @@ -1,7 +1,5 @@ package dev.openfeature.contrib.tools.tck; -import java.time.Duration; - /** * The seam between the TCK's step definitions and whatever manipulates the backend. * @@ -56,23 +54,6 @@ default void reconnect() { throw unsupported("reconnect"); } - /** - * Takes the backend down and lets it come back after {@code outage}, without stopping any - * container. - * - *

Returns once the outage has begun, and says nothing about when it ends. - * {@code POST /restart} is specified to answer either when the backend is down or only once it is - * back, so a scenario that wants the recovery asserts it through the provider's own events or - * client state. Assuming this call waited makes a feature file pass against one control API and - * hang against another. - * - * @param outage how long the backend stays unreachable - * @throws UnsupportedOperationException if this backend has no connection to lose - */ - default void disconnectFor(Duration outage) { - throw unsupported("disconnectFor"); - } - /** * Restarts the backend on one of its own named flag configurations. * @@ -84,7 +65,9 @@ default void disconnectFor(Duration outage) { * control already serves is the configuration they are written against. * *

Takes effect before the provider connects, so a scenario states it ahead of - * {@code a stable provider}. The next scenario starts from this suite's own configuration again. + * {@code a stable provider}, and {@link #reconnect()} brings this configuration back rather than + * the suite's -- losing the connection is not how a scenario changes what is served. The next + * scenario starts from this suite's own configuration again. * * @param configuration the configuration name, as that backend's control defines it * @throws UnsupportedOperationException if this backend serves one configuration only 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 d68208a61c..d8882dbe5f 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 @@ -20,9 +20,7 @@ final class HttpBackendControl implements BackendControl { private static final Logger log = LoggerFactory.getLogger(HttpBackendControl.class); - private static final int REQUEST_TIMEOUT_SECONDS = 30; - - private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(REQUEST_TIMEOUT_SECONDS); + private static final Duration REQUEST_TIMEOUT = Duration.ofSeconds(30); private final HttpClient http; private final String baseUrl; @@ -86,9 +84,10 @@ public BackendType backendType() { @Override public void prepareScenario() { if (backendStopped || scenarioConfiguration != null) { - start(); - // cleared only once the restart succeeded: a /start that failed leaves the scenario's - // configuration serving, and /reset would not take the next scenario off it + // this suite's configuration, not the last scenario's: that is what preparing means + start(backendConfiguration); + // cleared only once the start succeeded, so a failure is retried next scenario rather + // than downgraded to /reset, which does not change which configuration is served scenarioConfiguration = null; } else { reset(); @@ -119,37 +118,17 @@ public void disconnect() { /** * {@inheritDoc} * - *

Issues {@code POST /start?config=...}, which also restores the baseline flag state. + *

Issues {@code POST /start?config=...} on the configuration in effect -- the one a scenario + * served, if it served one -- because losing the connection is not how a scenario changes what + * the backend serves. It also restores that configuration's baseline flag state, which is why a + * scenario asserting a value across an outage asserts it from the provider's cache rather than + * from a mutation it made. */ @Override public void reconnect() { start(); } - /** - * {@inheritDoc} - * - *

Issues {@code POST /restart?seconds=...} and waits for the answer like any other call, - * which is the point of the endpoint: a blocking implementation spends the outage inside the - * response, and every language's HTTP client already knows how to wait for that. A timer here - * would have to be written once per binding instead. - * - *

An outage is therefore expected to be seconds, comfortably inside the {@value #REQUEST_TIMEOUT_SECONDS}-second - * request timeout every call shares; one approaching it would fail the request rather than the - * scenario's assertion. - */ - @Override - public void disconnectFor(Duration outage) { - post("/restart?seconds=" + outage.getSeconds()); - // Treated as still down, which costs the next scenario a /start instead of a /reset and - // nothing else. The alternative is a timer: the response may arrive as soon as the outage - // begins, and the specification is explicit that the TCK does not rely on it being delayed - // -- a scenario awaits the provider's own events instead, and sleeping here to find out - // whether the backend is back would put that timing in the control client, where this - // endpoint exists precisely to avoid it. - backendStopped = true; - } - /** * {@inheritDoc} * @@ -195,12 +174,16 @@ public void awaitReady(Duration timeout) { throw new IllegalStateException("control API at " + baseUrl + " did not become ready within " + timeout, last); } - /** Starts the backend, seeding the baseline flag state. */ + /** Starts the backend on the configuration in effect: the scenario's, else this suite's. */ private void start() { + start(scenarioConfiguration == null ? backendConfiguration : scenarioConfiguration); + } + + private void start(String configuration) { post( - backendConfiguration == null + configuration == null ? "/start" - : "/start?config=" + URLEncoder.encode(backendConfiguration, StandardCharsets.UTF_8)); + : "/start?config=" + URLEncoder.encode(configuration, StandardCharsets.UTF_8)); backendStopped = false; } 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 1d922e846f..8bae27b35c 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 @@ -95,6 +95,13 @@ public void tearDown() { */ @Given("a {} provider") public void createProvider(String flavour) { + createProvider(flavour, null); + } + + private void createProvider(String flavour, String configuration) { + if (configuration != null) { + backend().serveConfiguration(configuration); + } ProviderTckHarness harness = harness(); FeatureProvider provider; boolean waitForReady; @@ -131,6 +138,26 @@ public void createProvider(String flavour) { domain); } + /** + * Creates the provider under test against one of the backend's own named flag configurations. + * + *

One step rather than two, because the order was the whole difficulty: the backend has to be + * serving that configuration before the provider connects to it, and a scenario that said so in + * a separate {@code Given} could put it after {@code a stable provider} and watch the provider + * connect to the wrong flags. Here it cannot. + * + *

The name is the backend's own -- see {@link BackendControl#serveConfiguration} -- so this is + * for extension scenarios. A canonical scenario names no configuration: the baseline every + * control serves is what it is written against. + * + * @param flavour either {@code stable} or {@code unavailable} + * @param configuration the configuration the backend should serve, as its control defines it + */ + @Given("a {} provider with {string} configuration") + public void createProviderWithConfiguration(String flavour, String configuration) { + createProvider(flavour, configuration); + } + /** * Asserts that the provider metadata name is not empty (requirement 2.1.1). */ @@ -208,34 +235,6 @@ public void theConnectionIsRestored() { backend().reconnect(); } - /** - * Takes the backend down for a fixed outage and lets it come back by itself. - * - *

What the step guarantees is {@link BackendControl#disconnectFor}'s contract: the outage has - * begun, and nothing more. A scenario that needs the backend back says so -- {@code a ready event - * was fired}, or {@code the client should be in READY state} -- rather than relying on this step - * to have waited. - * - * @param seconds how long the backend stays unreachable - */ - @When("the connection is lost for {int}s") - public void theConnectionIsLostFor(int seconds) { - backend().disconnectFor(Duration.ofSeconds(seconds)); - } - - /** - * Serves one of the backend's own named flag configurations, before the provider connects. - * - *

Extension scenarios only, and the name is the backend's own; see - * {@link BackendControl#serveConfiguration}. - * - * @param configuration the configuration name - */ - @Given("the backend serves the {string} configuration") - public void theBackendServesTheConfiguration(String configuration) { - backend().serveConfiguration(configuration); - } - /** Mutates flag configuration so a conforming provider observes a configuration change. */ @When("the flag was modified") public void theFlagWasModified() { 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 bf7a1a9521..395687ae5c 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 @@ -140,22 +140,6 @@ void aServedConfigurationDoesNotLeakIntoTheNextScenario() throws Exception { } } - @Test - @DisplayName("a timed outage restarts for that many seconds") - void aTimedOutageRestarts() throws Exception { - try (StubControlApi stub = StubControlApi.start()) { - HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); - control.prepareScenario(); - stub.forget(); - - control.disconnectFor(Duration.ofSeconds(8)); - - // the outage is asked for in seconds; what the next scenario then does is the test below, - // which uses an outage short enough to wait out - assertThat(stub.requests()).containsExactly("POST /restart?seconds=8"); - } - } - @Test @DisplayName("a restoration that fails is retried by the next scenario, not downgraded to /reset") void aFailedRestorationIsRetried() throws Exception { @@ -175,28 +159,6 @@ void aFailedRestorationIsRetried() throws Exception { } } - @Test - @DisplayName("the scenario after a timed outage starts the backend instead of waiting for it") - void theNextScenarioStartsRatherThanWaiting() throws Exception { - try (StubControlApi stub = StubControlApi.start()) { - HttpBackendControl control = new HttpBackendControl(stub.baseUrl(), "default"); - control.prepareScenario(); // settles on /reset, which this stub implements - stub.forget(); - - // this stub answers at once, which is the half of /restart the specification permits and - // tells the TCK not to rely on: the backend may still be down when the scenario ends - control.disconnectFor(Duration.ofSeconds(5)); - long started = System.nanoTime(); - control.prepareScenario(); - - // /start works whether the outage is over or not, and /reset is not specified to start - assertThat(stub.requests()).containsExactly("POST /restart?seconds=5", "POST /start?config=default"); - assertThat(Duration.ofNanos(System.nanoTime() - started)) - .as("no timer: the outage's length is the backend's business, not this client's") - .isLessThan(Duration.ofSeconds(1)); - } - } - @Test @DisplayName("reconnecting clears the disconnect, so the next scenario resets again") void reconnectClearsTheDisconnect() throws Exception { From 67559f3b540907c06478e2efa3ac31d7cc3edfd0 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 10:52:26 +0200 Subject: [PATCH 10/13] feat(tck): ask about event order instead of watching for an absence "the {} event handler should not have been executed" is the same antipattern as the timed outage, one layer up: an absence can only be asserted over some stretch of time, so the step either slows every run down by waiting or passes because nothing has arrived yet. Both forms go. What the scenarios using it actually mean is an order -- no error *before* the provider came back ready -- so that is the step: the {} event handler should have been executed with no {} event before it It needs no clock. The flagd adoption had written it locally for exactly this reason, and its javadoc said why: the TCK's await drained the records ahead of its match, so an error recorded before an awaited ready was already gone and the negative step passed vacuously. The await keeps those records now, which is what makes the question answerable here. Signed-off-by: Simon Schrottner --- .../tools/tck/internal/steps/EventSteps.java | 83 ++++++------ .../tck/internal/steps/EventStepsTest.java | 119 ++++++++---------- 2 files changed, 90 insertions(+), 112 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java index 66fca7fb54..dd96e4d424 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java @@ -12,6 +12,8 @@ import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,9 +26,6 @@ public class EventSteps extends AbstractSteps { private static final Logger log = LoggerFactory.getLogger(EventSteps.class); - // Awaitility wants a deadline past the poll delay; this only covers the assertion itself. - private static final long SETTLE_SLACK_MS = 250; - public EventSteps(TckState state, ProviderTckHarness harness, BackendControl backend) { super(state, harness, backend); } @@ -76,58 +75,58 @@ public void theEventHandlerShouldHaveBeenExecutedWithin(String eventType, int mi } /** - * Asserts that no event of the given kind arrives, within - * {@link ProviderTckHarness#eventTimeout()}. + * Awaits an event of one kind and asserts that no event of another kind was recorded before it. * - * @param eventType the event kind - */ - @Then("the {} event handler should not have been executed") - public void theEventHandlerShouldNotHaveBeenExecuted(String eventType) { - awaitNoEvent(eventType, harness().eventTimeout().toMillis()); - } - - /** - * Asserts that no event of the given kind arrives within an explicit window. + *

This is how a scenario says "and no error": as an order, not as a window. "No + * error event fired" can only be answered for some stretch of time, and at any instant an event + * that never fires and one that fires a moment later are the same observation -- so a step + * asking it that way either slows every run down or passes because nothing has arrived yet. + * "No error before the provider came back ready" is a question about what was recorded, and the + * recording answers it with no clock involved. * - * @param eventType the event kind - * @param milliseconds how long to watch for it - */ - @Then("the {} event handler should not have been executed within {int}ms") - public void theEventHandlerShouldNotHaveBeenExecutedWithin(String eventType, int milliseconds) { - awaitNoEvent(eventType, milliseconds); - } - - /** - * Watches for the whole window before answering, because at any single instant absence and - * lateness are the same observation: asserting immediately states "no such event yet", - * which a provider one scheduler slice slower satisfies while the scenario means it never fires. - * The positive form awaits, so the negative form has to watch at least as long to be its - * opposite. + * @param eventType the event kind to await + * @param forbiddenType the event kind that must not have been recorded before it */ - private void awaitNoEvent(String eventType, long milliseconds) { - log.info("Watching for no {} event ({}ms)", eventType, milliseconds); - await().alias("no provider event " + eventType) - .pollDelay(milliseconds, MILLISECONDS) - .atMost(milliseconds + SETTLE_SLACK_MS, MILLISECONDS) - .untilAsserted(() -> assertThat(state.events.stream() - .filter(event -> event.type().equals(eventType)) - .findFirst()) - .as("no %s event should have fired within %dms", eventType, milliseconds) - .isEmpty()); + @Then("the {} event handler should have been executed with no {} event before it") + public void theEventHandlerShouldHaveBeenExecutedWithNoneBefore(String eventType, String forbiddenType) { + log.info("Awaiting {} event with no {} event before it", eventType, forbiddenType); + awaitRecorded(eventType, harness().eventTimeout().toMillis()); + + List before = new ArrayList<>(); + for (ProviderEventRecord record : state.events) { + if (record.type().equals(eventType)) { + break; + } + before.add(record.type()); + } + assertThat(before) + .as("events recorded before the %s event", eventType) + .doesNotContain(forbiddenType); + takeEvent(eventType); } private void awaitEvent(String eventType, long milliseconds) { log.info("Awaiting {} event (timeout {}ms)", eventType, milliseconds); + awaitRecorded(eventType, milliseconds); + takeEvent(eventType); + } + + private void awaitRecorded(String eventType, long milliseconds) { await().alias("provider event " + eventType) .atMost(milliseconds, MILLISECONDS) .pollInterval(10, MILLISECONDS) .until(() -> state.events.stream().anyMatch(event -> event.type().equals(eventType))); + } - // Takes the matched record and nothing else. Draining through the match also discarded - // records of other kinds, which a later assertion is entitled to see: an error event that - // arrived before an awaited change event would be gone, and "the error event handler should - // not have been executed" would then pass because the evidence had been thrown away. + /** + * Takes the matched record and nothing else. + * + *

Draining through the match would also discard records of other kinds, which a later + * assertion is entitled to see: an error event recorded before an awaited ready event would be + * gone, and the order assertion above would find nothing left to object to. + */ + private void takeEvent(String eventType) { ProviderEventRecord matched = state.events.stream() .filter(event -> event.type().equals(eventType)) .findFirst() diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java index 347f48ea59..336f276f26 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java @@ -11,19 +11,19 @@ import dev.openfeature.contrib.tools.tck.TckState; import dev.openfeature.sdk.FeatureProvider; import java.time.Duration; -import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; /** - * Verifies what {@code should not have been executed} asserts. + * What the event assertions read, and what they leave behind. * - *

The point of these is the window. A negative assertion taken at one instant cannot tell an - * event that never fires from one that fires a moment later, and the scenarios that use it mean the - * first -- so the step has to watch, and a late event has to fail it. + *

Both halves matter to a chain like stale, then error, then ready: each assertion has to see the + * records an earlier one did not claim, or an ordering question is answered from an emptied queue. */ class EventStepsTest { + private static final String READY = "ready"; + private static final String ERROR = "error"; /** A harness that brings nothing up and answers a short event timeout. */ @@ -31,12 +31,6 @@ private static final class StubHarness implements ProviderTckHarness { private final InProcessControl control = BackendControl.inProcess(); - private final Duration eventTimeout; - - private StubHarness(Duration eventTimeout) { - this.eventTimeout = eventTimeout; - } - @Override public FeatureProvider createProvider() { return control.createProvider(); @@ -49,99 +43,84 @@ public BackendControl backendControl() { @Override public Duration eventTimeout() { - return eventTimeout; + return Duration.ofMillis(300); } } - private static EventSteps stepsWith(TckState state, Duration eventTimeout) { - StubHarness harness = new StubHarness(eventTimeout); + private static EventSteps stepsWith(TckState state) { + StubHarness harness = new StubHarness(); return new EventSteps(state, harness, harness.backendControl()); } @Test - @DisplayName("an event that never fires passes, and the window is actually waited out") - void anAbsenceIsWatchedForTheWholeWindow() { + @DisplayName("an event of the forbidden kind recorded first fails the ordering") + void anEarlierForbiddenEventFailsTheOrdering() { TckState state = new TckState(); - EventSteps steps = stepsWith(state, Duration.ofMillis(300)); - - long started = System.nanoTime(); - assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 300)) - .doesNotThrowAnyException(); + state.events.add(new ProviderEventRecord(ERROR, null)); + state.events.add(new ProviderEventRecord(READY, null)); + EventSteps steps = stepsWith(state); - assertThat(Duration.ofNanos(System.nanoTime() - started)) - .as("asserting before the window elapses would only say 'not yet'") - .isGreaterThanOrEqualTo(Duration.ofMillis(300)); + assertThatThrownBy(() -> steps.theEventHandlerShouldHaveBeenExecutedWithNoneBefore(READY, ERROR)) + .as("the error was recorded before the ready, which is what the scenario forbids") + .hasMessageContaining("events recorded before the ready event"); } @Test - @DisplayName("an event that fires late inside the window fails the assertion") - void aLateEventIsNotMistakenForAbsence() { + @DisplayName("the same records in the other order pass, and the ready is consumed") + void theOrderingPassesWhenTheForbiddenEventComesAfter() { TckState state = new TckState(); - EventSteps steps = stepsWith(state, Duration.ofMillis(600)); - CompletableFuture.runAsync(() -> { - try { - Thread.sleep(150); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return; - } - state.events.add(new ProviderEventRecord(ERROR, null)); - }); - - assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 600)) - .as("the instant form passed here, which is the bug this step was given a window for") - .hasMessageContaining("no error event should have fired"); + state.events.add(new ProviderEventRecord(READY, null)); + state.events.add(new ProviderEventRecord(ERROR, null)); + EventSteps steps = stepsWith(state); + + assertThatCode(() -> steps.theEventHandlerShouldHaveBeenExecutedWithNoneBefore(READY, ERROR)) + .doesNotThrowAnyException(); + + assertThat(state.lastEvent).get().extracting(ProviderEventRecord::type).isEqualTo(READY); + assertThat(state.events) + .as("the ready was claimed; the error is still there for whatever asks next") + .extracting(ProviderEventRecord::type) + .containsExactly(ERROR); } @Test - @DisplayName("an event already recorded fails, naming the kind and the window") - void anEventAlreadyRecordedFails() { + @DisplayName("an ordering question with nothing recorded at all times out rather than passing") + void nothingRecordedIsNotAPass() { TckState state = new TckState(); - state.events.add(new ProviderEventRecord(ERROR, null)); - EventSteps steps = stepsWith(state, Duration.ofMillis(200)); + EventSteps steps = stepsWith(state); - assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) - .hasMessageContaining("no error event should have fired within 200ms"); + assertThatThrownBy(() -> steps.theEventHandlerShouldHaveBeenExecutedWithNoneBefore(READY, ERROR)) + .as("it awaits the event it is about; absence is a failure, not a silent pass") + .hasMessageContaining("provider event ready"); } @Test @DisplayName("awaiting one kind of event does not throw away another kind's record") void aPositiveAwaitDoesNotEraseTheEvidence() { TckState state = new TckState(); - EventSteps steps = stepsWith(state, Duration.ofMillis(200)); state.events.add(new ProviderEventRecord(ERROR, null)); state.events.add(new ProviderEventRecord("change", null)); + EventSteps steps = stepsWith(state); steps.theEventHandlerShouldHaveBeenExecuted("change"); - assertThatThrownBy(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) - .as("the error arrived first; awaiting the change must not make it unobservable") - .hasMessageContaining("no error event should have fired"); + assertThat(state.events) + .as("the error arrived first; claiming the change must not make it unobservable") + .extracting(ProviderEventRecord::type) + .containsExactly(ERROR); } @Test - @DisplayName("another kind of event does not fail it") - void anotherKindOfEventIsIgnored() { + @DisplayName("a second await of the same kind waits for a newer record") + void anAwaitClaimsItsOwnRecord() { TckState state = new TckState(); - state.events.add(new ProviderEventRecord("ready", null)); - EventSteps steps = stepsWith(state, Duration.ofMillis(200)); - - assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecutedWithin(ERROR, 200)) - .doesNotThrowAnyException(); - } + state.events.add(new ProviderEventRecord(READY, null)); + EventSteps steps = stepsWith(state); - @Test - @DisplayName("the step without a window watches for the harness's event timeout") - void theDefaultWindowIsTheHarnessEventTimeout() { - TckState state = new TckState(); - EventSteps steps = stepsWith(state, Duration.ofMillis(400)); - - long started = System.nanoTime(); - assertThatCode(() -> steps.theEventHandlerShouldNotHaveBeenExecuted(ERROR)) - .doesNotThrowAnyException(); + steps.theEventHandlerShouldHaveBeenExecuted(READY); - assertThat(Duration.ofNanos(System.nanoTime() - started)) - .as("the pair has to be symmetrical: the positive form awaits this same timeout") - .isGreaterThanOrEqualTo(Duration.ofMillis(400)); + assertThatThrownBy(() -> steps.theEventHandlerShouldHaveBeenExecutedWithin(READY, 150)) + .as("the one record was claimed, so a chain cannot satisfy two awaits with it") + .hasMessageContaining("provider event ready"); } } From d4124dfe4f7454945ec0012b5c3395cbf893b4ce Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 11:11:17 +0200 Subject: [PATCH 11/13] feat(tck): ask about flag metadata, and fix two formatting slips Flag metadata is specification behaviour, and the specification already has the wording for it: `the resolved metadata should contain`, with a `key | metadata_type | value` table, and `the resolved metadata is empty`. Those are copied verbatim from assets/gherkin/metadata.feature, so an adoption that ports metadata scenarios should find them here rather than write them again -- the flagd adoption had written exactly these two, down to the column names. The canonical suite still only asks a provider for its own name, so nothing here exercises the steps yet; FlagMetadataStepsTest does, each case written so the opposite implementation fails it. Also joins an assertion and drops a blank line left by the timed outage's removal, both of which spotless rejected. Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/HttpBackendControl.java | 1 - .../tools/tck/internal/steps/EventSteps.java | 4 +- .../tools/tck/internal/steps/FlagSteps.java | 57 +++++++ .../internal/steps/FlagMetadataStepsTest.java | 142 ++++++++++++++++++ 4 files changed, 200 insertions(+), 4 deletions(-) create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java 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 d8882dbe5f..8669a32b70 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 @@ -35,7 +35,6 @@ final class HttpBackendControl implements BackendControl { // a configuration one scenario asked for, which the next scenario has to be taken off again private String scenarioConfiguration; - /** * Creates a control client for a running backend. No post-command settle; control calls return * once the backend has acted. diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java index dd96e4d424..9901df22a7 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/EventSteps.java @@ -99,9 +99,7 @@ public void theEventHandlerShouldHaveBeenExecutedWithNoneBefore(String eventType } before.add(record.type()); } - assertThat(before) - .as("events recorded before the %s event", eventType) - .doesNotContain(forbiddenType); + assertThat(before).as("events recorded before the %s event", eventType).doesNotContain(forbiddenType); takeEvent(eventType); } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java index d7726727df..2a05bd0927 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java @@ -9,6 +9,7 @@ import dev.openfeature.contrib.tools.tck.TckState; import dev.openfeature.contrib.tools.tck.internal.TckValues; import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.Structure; import dev.openfeature.sdk.Value; import io.cucumber.datatable.DataTable; @@ -222,6 +223,62 @@ public void theFlagShouldBePartOfTheEventPayload() { assertThat(event.details().getFlagsChanged()).contains(state.flag.key()); } + /** + * Asserts typed flag-metadata entries on the last evaluation; an entry the table does not list + * is not checked. + * + *

The wording and the columns are the specification's own, from + * {@code assets/gherkin/metadata.feature}: flag metadata is spec behaviour, not a vendor's, so + * the question belongs here rather than in each adoption's glue. + * + * @param table rows with {@code key}, {@code metadata_type} and {@code value} columns + */ + @Then("the resolved metadata should contain") + public void theResolvedMetadataShouldContain(DataTable table) { + ImmutableMetadata metadata = requireMetadata(); + for (Map row : table.asMaps(String.class, String.class)) { + String key = row.get("key"); + String type = row.get("metadata_type"); + Object expected = TckValues.convert(row.get("value"), type); + Object actual; + switch (type) { + case "String": + actual = metadata.getString(key); + break; + case "Boolean": + actual = metadata.getBoolean(key); + break; + case "Integer": + actual = metadata.getInteger(key); + break; + case "Float": + actual = metadata.getDouble(key); + break; + default: + throw new IllegalArgumentException("Unknown metadata type '" + type + + "'. Flag metadata is String, Boolean, Integer or Float."); + } + assertThat(actual) + .as("metadata '%s' (%s) in %s", key, type, metadata.asUnmodifiableMap()) + .isEqualTo(expected); + } + } + + /** Asserts that the last evaluation carries no flag metadata. */ + @Then("the resolved metadata is empty") + public void theResolvedMetadataIsEmpty() { + assertThat(requireMetadata().asUnmodifiableMap()) + .as("flag metadata") + .isEmpty(); + } + + private ImmutableMetadata requireMetadata() { + requireEvaluation(); + ImmutableMetadata metadata = state.evaluation.getFlagMetadata(); + assertThat(metadata).as("flag metadata").isNotNull(); + return metadata; + } + private void requireEvaluation() { if (state.evaluation == null) { throw new AssertionError("No evaluation has been performed. " diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java new file mode 100644 index 0000000000..836c7c96b0 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java @@ -0,0 +1,142 @@ +package dev.openfeature.contrib.tools.tck.internal.steps; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import dev.openfeature.contrib.tools.tck.BackendControl; +import dev.openfeature.contrib.tools.tck.InProcessControl; +import dev.openfeature.contrib.tools.tck.ProviderTckHarness; +import dev.openfeature.contrib.tools.tck.TckState; +import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.ImmutableMetadata; +import io.cucumber.datatable.DataTable; +import io.cucumber.datatable.DataTableTypeRegistry; +import io.cucumber.datatable.DataTableTypeRegistryTableConverter; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * The flag-metadata assertions, whose wording and columns are the specification's own. + * + *

Each case is written so the opposite implementation would fail it: a value that matches and one + * that does not, metadata that is empty and metadata that is not. + */ +class FlagMetadataStepsTest { + + /** A harness that brings nothing up. */ + private static final class StubHarness implements ProviderTckHarness { + + private final InProcessControl control = BackendControl.inProcess(); + + @Override + public FeatureProvider createProvider() { + return control.createProvider(); + } + + @Override + public BackendControl backendControl() { + return control; + } + } + + private static FlagSteps stepsFor(TckState state) { + StubHarness harness = new StubHarness(); + return new FlagSteps(state, harness, harness.backendControl()); + } + + private static TckState stateWith(ImmutableMetadata metadata) { + TckState state = new TckState(); + state.evaluation = FlagEvaluationDetails.builder() + .flagKey("metadata-flag") + .value(true) + .flagMetadata(metadata) + .build(); + return state; + } + + /** A table carrying the specification's {@code key | metadata_type | value} header. */ + private static DataTable table(String... rows) { + List> cells = new ArrayList<>(); + cells.add(Arrays.asList("key", "metadata_type", "value")); + for (String row : rows) { + cells.add(Arrays.asList(row.split("\\s*\\|\\s*"))); + } + return DataTable.create( + cells, new DataTableTypeRegistryTableConverter(new DataTableTypeRegistry(Locale.ENGLISH))); + } + + @Test + @DisplayName("every type the specification's table uses is compared by that type") + void typedEntriesAreCompared() { + TckState state = stateWith(ImmutableMetadata.builder() + .addString("string", "1.0.2") + .addInteger("integer", 2) + .addDouble("float", 0.1) + .addBoolean("boolean", true) + .build()); + + assertThatCode(() -> stepsFor(state) + .theResolvedMetadataShouldContain(table( + "string | String | 1.0.2", + "integer | Integer | 2", + "float | Float | 0.1", + "boolean | Boolean | true"))) + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("a value that differs fails, naming the key and what the provider sent") + void aWrongValueFails() { + TckState state = stateWith(ImmutableMetadata.builder().addInteger("integer", 2).build()); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("integer | Integer | 3"))) + .hasMessageContaining("metadata 'integer' (Integer)"); + } + + @Test + @DisplayName("an entry the table does not list is not checked") + void unlistedEntriesAreIgnored() { + TckState state = stateWith(ImmutableMetadata.builder() + .addString("listed", "yes") + .addString("unlisted", "whatever") + .build()); + + assertThatCode(() -> stepsFor(state).theResolvedMetadataShouldContain(table("listed | String | yes"))) + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("empty metadata satisfies the empty assertion, and anything else does not") + void theEmptyAssertion() { + assertThatCode(() -> stepsFor(stateWith(ImmutableMetadata.builder().build())).theResolvedMetadataIsEmpty()) + .doesNotThrowAnyException(); + + TckState withEntry = stateWith(ImmutableMetadata.builder().addString("a", "b").build()); + assertThatThrownBy(() -> stepsFor(withEntry).theResolvedMetadataIsEmpty()) + .hasMessageContaining("flag metadata"); + } + + @Test + @DisplayName("a metadata type flag metadata cannot carry is refused, not silently compared") + void anUnknownTypeIsRefused() { + TckState state = stateWith(ImmutableMetadata.builder().addString("a", "b").build()); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("a | Object | {}"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Flag metadata is String, Boolean, Integer or Float"); + } + + @Test + @DisplayName("asserting metadata before any evaluation says which step is missing") + void noEvaluationIsAClearFailure() { + TckState state = new TckState(); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataIsEmpty()) + .hasMessageContaining("the flag was evaluated with details"); + } +} From 7e1da008903c54d82c9232346e055d951e511c95 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 11:19:19 +0200 Subject: [PATCH 12/13] style(tck): format the metadata step and its test as spotless wants them Signed-off-by: Simon Schrottner --- .../contrib/tools/tck/internal/steps/FlagSteps.java | 4 +--- .../tck/internal/steps/FlagMetadataStepsTest.java | 12 ++++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java index 2a05bd0927..06475e48c1 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java @@ -267,9 +267,7 @@ public void theResolvedMetadataShouldContain(DataTable table) { /** Asserts that the last evaluation carries no flag metadata. */ @Then("the resolved metadata is empty") public void theResolvedMetadataIsEmpty() { - assertThat(requireMetadata().asUnmodifiableMap()) - .as("flag metadata") - .isEmpty(); + assertThat(requireMetadata().asUnmodifiableMap()).as("flag metadata").isEmpty(); } private ImmutableMetadata requireMetadata() { diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java index 836c7c96b0..c0d2ce503a 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java @@ -92,7 +92,8 @@ void typedEntriesAreCompared() { @Test @DisplayName("a value that differs fails, naming the key and what the provider sent") void aWrongValueFails() { - TckState state = stateWith(ImmutableMetadata.builder().addInteger("integer", 2).build()); + TckState state = + stateWith(ImmutableMetadata.builder().addInteger("integer", 2).build()); assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("integer | Integer | 3"))) .hasMessageContaining("metadata 'integer' (Integer)"); @@ -113,10 +114,12 @@ void unlistedEntriesAreIgnored() { @Test @DisplayName("empty metadata satisfies the empty assertion, and anything else does not") void theEmptyAssertion() { - assertThatCode(() -> stepsFor(stateWith(ImmutableMetadata.builder().build())).theResolvedMetadataIsEmpty()) + assertThatCode(() -> + stepsFor(stateWith(ImmutableMetadata.builder().build())).theResolvedMetadataIsEmpty()) .doesNotThrowAnyException(); - TckState withEntry = stateWith(ImmutableMetadata.builder().addString("a", "b").build()); + TckState withEntry = + stateWith(ImmutableMetadata.builder().addString("a", "b").build()); assertThatThrownBy(() -> stepsFor(withEntry).theResolvedMetadataIsEmpty()) .hasMessageContaining("flag metadata"); } @@ -124,7 +127,8 @@ void theEmptyAssertion() { @Test @DisplayName("a metadata type flag metadata cannot carry is refused, not silently compared") void anUnknownTypeIsRefused() { - TckState state = stateWith(ImmutableMetadata.builder().addString("a", "b").build()); + TckState state = + stateWith(ImmutableMetadata.builder().addString("a", "b").build()); assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("a | Object | {}"))) .isInstanceOf(IllegalArgumentException.class) From 81111898add2be0b419ab940cc3b51b677e8d534 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 22 Oct 2026 11:53:20 +0200 Subject: [PATCH 13/13] fix(tck): compare metadata by value, and require the key to be there Two findings from review on the metadata assertions, both of which made the step pass or fail for the wrong reason. ImmutableMetadata's getters are exact. getDouble answers null for a value stored through addFloat, and getInteger null for one stored as a long, so a provider whose value was right and whose box was not failed the assertion. The entry is read from the map and compared by the type the row declares: whole numbers as longs, so a provider carrying more than 32 bits of metadata is not refused, and Float within a tolerance, because a 32-bit float widened to a double is not the double the feature file's decimal parses to -- 0.1f is 0.10000000149011612 while the table says 0.1. The other direction: a row whose value is "null" passed against a provider that omitted the entry entirely, because the conversion yields null and so does a lookup of a key nobody set. The key has to be there before its value is worth comparing. Four tests, each written so the previous implementation fails it. Signed-off-by: Simon Schrottner --- .../tools/tck/internal/steps/FlagSteps.java | 79 +++++++++++++------ .../internal/steps/FlagMetadataStepsTest.java | 47 +++++++++++ 2 files changed, 103 insertions(+), 23 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java index 06475e48c1..67b0ace46a 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagSteps.java @@ -1,6 +1,7 @@ package dev.openfeature.contrib.tools.tck.internal.steps; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.withinPercentage; import dev.openfeature.contrib.tools.tck.BackendControl; import dev.openfeature.contrib.tools.tck.FlagUnderTest; @@ -27,6 +28,9 @@ public class FlagSteps extends AbstractSteps { private static final Logger log = LoggerFactory.getLogger(FlagSteps.class); + // float carries about seven significant digits, so this covers its widening and nothing else + private static final double FLOAT_TOLERANCE_PERCENT = 0.001; + public FlagSteps(TckState state, ProviderTckHarness harness, BackendControl backend) { super(state, harness, backend); } @@ -235,35 +239,64 @@ public void theFlagShouldBePartOfTheEventPayload() { */ @Then("the resolved metadata should contain") public void theResolvedMetadataShouldContain(DataTable table) { - ImmutableMetadata metadata = requireMetadata(); + Map metadata = requireMetadata().asUnmodifiableMap(); for (Map row : table.asMaps(String.class, String.class)) { String key = row.get("key"); String type = row.get("metadata_type"); - Object expected = TckValues.convert(row.get("value"), type); - Object actual; - switch (type) { - case "String": - actual = metadata.getString(key); - break; - case "Boolean": - actual = metadata.getBoolean(key); - break; - case "Integer": - actual = metadata.getInteger(key); - break; - case "Float": - actual = metadata.getDouble(key); - break; - default: - throw new IllegalArgumentException("Unknown metadata type '" + type - + "'. Flag metadata is String, Boolean, Integer or Float."); - } - assertThat(actual) - .as("metadata '%s' (%s) in %s", key, type, metadata.asUnmodifiableMap()) - .isEqualTo(expected); + // Asserted before the value, because a row whose value is "null" would otherwise pass + // against a provider that omitted the entry: the conversion yields null, and so does a + // lookup of a key nobody set. + assertThat(metadata).as("flag metadata").containsKey(key); + assertMetadataValue(key, type, row.get("value"), metadata); } } + /** + * Compares one metadata entry by its declared type rather than by the Java type it arrived in. + * + *

{@link ImmutableMetadata}'s getters are exact: {@code getDouble} answers null for a value + * stored through {@code addFloat}, and {@code getInteger} null for one stored as a long. Reading + * through them would fail a provider whose value is right and whose box is not, and a scenario + * asks about the value. + * + *

{@code Float} is compared with a tolerance for the same reason -- a 32-bit float widened to + * a double is not the double the feature file's decimal parses to: {@code 0.1f} widens to + * 0.10000000149011612 while the table says 0.1. + * + * @param key the metadata key the row names + * @param type the declared metadata type + * @param value the expected value, as the feature file writes it + * @param metadata every entry the provider sent, for the failure message + */ + private void assertMetadataValue(String key, String type, String value, Map metadata) { + Object actual = metadata.get(key); + String where = String.format("metadata '%s' (%s) in %s", key, type, metadata); + switch (type) { + case "String": + case "Boolean": + assertThat(actual).as(where).isEqualTo(TckValues.convert(value, type)); + break; + case "Integer": + // parsed as a long, because metadata is not read back through getIntegerDetails and + // a provider may carry a number wider than 32 bits + assertThat(numberOf(actual, where).longValue()).as(where).isEqualTo(Long.parseLong(value)); + break; + case "Float": + assertThat(numberOf(actual, where).doubleValue()) + .as(where) + .isCloseTo(Double.parseDouble(value), withinPercentage(FLOAT_TOLERANCE_PERCENT)); + break; + default: + throw new IllegalArgumentException( + "Unknown metadata type '" + type + "'. Flag metadata is String, Boolean, Integer or Float."); + } + } + + private static Number numberOf(Object actual, String where) { + assertThat(actual).as("%s, which this row declares numeric", where).isInstanceOf(Number.class); + return (Number) actual; + } + /** Asserts that the last evaluation carries no flag metadata. */ @Then("the resolved metadata is empty") public void theResolvedMetadataIsEmpty() { diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java index c0d2ce503a..35ca2e8ef7 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java @@ -124,6 +124,53 @@ void theEmptyAssertion() { .hasMessageContaining("flag metadata"); } + @Test + @DisplayName("a number is compared by value, whichever numeric type the provider stored it in") + void aNumberIsComparedByValueNotByBox() { + // ImmutableMetadata's getters are exact: getDouble answers null for an addFloat value, and + // getInteger null for a long, so reading through them failed a provider that was right. + TckState state = stateWith(ImmutableMetadata.builder() + .addFloat("float", 0.1f) + .addLong("integer", 2L) + .build()); + + assertThatCode(() -> stepsFor(state) + .theResolvedMetadataShouldContain(table("float | Float | 0.1", "integer | Integer | 2"))) + .as("0.1f widens to 0.10000000149011612, and 2L is not an Integer") + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("a number that differs still fails, tolerance notwithstanding") + void aNumberThatDiffersStillFails() { + TckState state = + stateWith(ImmutableMetadata.builder().addFloat("float", 0.1f).build()); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("float | Float | 0.2"))) + .hasMessageContaining("metadata 'float' (Float)"); + } + + @Test + @DisplayName("a row whose value is null does not pass against a key the provider omitted") + void anAbsentKeyIsNotANullValue() { + TckState state = stateWith( + ImmutableMetadata.builder().addString("present", "yes").build()); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("missing | String | null"))) + .as("the conversion yields null and so does the lookup, which used to agree") + .hasMessageContaining("flag metadata"); + } + + @Test + @DisplayName("a numeric row against a value that is not a number says so") + void aNumericRowAgainstAStringSaysSo() { + TckState state = + stateWith(ImmutableMetadata.builder().addString("count", "2").build()); + + assertThatThrownBy(() -> stepsFor(state).theResolvedMetadataShouldContain(table("count | Integer | 2"))) + .hasMessageContaining("declares numeric"); + } + @Test @DisplayName("a metadata type flag metadata cannot carry is refused, not silently compared") void anUnknownTypeIsRefused() {