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..178e7f5980 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/AdopterTags.java @@ -0,0 +1,124 @@ +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); + // before any skip, so an exclude cannot mask a reserved tag on the scenario + Capability.requireValidVocabulary(source, tags); + 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/BackendControl.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/BackendControl.java index a16a989c90..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 @@ -54,6 +54,28 @@ default void reconnect() { throw unsupported("reconnect"); } + /** + * 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}, 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 + */ + 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/Capability.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/Capability.java index 8a85044f88..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,12 +226,11 @@ 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()) { - // adopter tag; gates nothing + // adopter tag; gated by AdopterTags continue; } Capability capability = found.get(); @@ -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}. * @@ -272,7 +288,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/HttpBackendControl.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/HttpBackendControl.java index 1c054408d4..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 @@ -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; @@ -30,6 +32,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,12 +76,18 @@ 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) { - start(); + if (backendStopped || scenarioConfiguration != null) { + // 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(); } @@ -106,13 +117,30 @@ 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 /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). @@ -145,12 +173,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; } @@ -185,7 +217,7 @@ private void post(String path) { private HttpResponse send(String path) { HttpRequest request = HttpRequest.newBuilder(URI.create(baseUrl + path)) .POST(HttpRequest.BodyPublishers.noBody()) - .timeout(Duration.ofSeconds(30)) + .timeout(REQUEST_TIMEOUT) .build(); try { return http.send(request, HttpResponse.BodyHandlers.discarding()); 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/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..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 @@ -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,30 @@ 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 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. 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(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..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 @@ -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; @@ -11,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; @@ -71,22 +74,64 @@ public void theEventHandlerShouldHaveBeenExecutedWithin(String eventType, int mi awaitEvent(eventType, milliseconds); } + /** + * Awaits an event of one kind and asserts that no event of another kind was recorded before it. + * + *

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 to await + * @param forbiddenType the event kind that must not have been recorded before it + */ + @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))); + } - // 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 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() + .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/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..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; @@ -9,6 +10,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; @@ -26,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); } @@ -222,6 +227,89 @@ 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) { + Map metadata = requireMetadata().asUnmodifiableMap(); + for (Map row : table.asMaps(String.class, String.class)) { + String key = row.get("key"); + String type = row.get("metadata_type"); + // 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() { + 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/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..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 @@ -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,12 +54,17 @@ 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 */ @Before(order = 0) public void gateOnCapabilities(Scenario scenario) { + // 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()); } @@ -89,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; @@ -125,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). */ 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..2681756363 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ExcludedTagsTest.java @@ -0,0 +1,199 @@ +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"); + + 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() { + 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, 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() { + Results results = run("not @filtered"); + + assertThat(results.passed).hasSize(3); + assertThat(results.aborted) + .as("the filtered scenario is gone, not skipped") + .hasSize(3); + 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("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() { + 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/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java index 09eda6959d..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 @@ -110,6 +110,55 @@ 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 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("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..4a60ee9893 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/TckValuesTest.java @@ -0,0 +1,49 @@ +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/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 new file mode 100644 index 0000000000..336f276f26 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/EventStepsTest.java @@ -0,0 +1,126 @@ +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 org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * What the event assertions read, and what they leave behind. + * + *

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. */ + 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; + } + + @Override + public Duration eventTimeout() { + return Duration.ofMillis(300); + } + } + + private static EventSteps stepsWith(TckState state) { + StubHarness harness = new StubHarness(); + return new EventSteps(state, harness, harness.backendControl()); + } + + @Test + @DisplayName("an event of the forbidden kind recorded first fails the ordering") + void anEarlierForbiddenEventFailsTheOrdering() { + TckState state = new TckState(); + state.events.add(new ProviderEventRecord(ERROR, null)); + state.events.add(new ProviderEventRecord(READY, null)); + EventSteps steps = stepsWith(state); + + 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("the same records in the other order pass, and the ready is consumed") + void theOrderingPassesWhenTheForbiddenEventComesAfter() { + TckState state = new TckState(); + 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 ordering question with nothing recorded at all times out rather than passing") + void nothingRecordedIsNotAPass() { + TckState state = new TckState(); + EventSteps steps = stepsWith(state); + + 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(); + state.events.add(new ProviderEventRecord(ERROR, null)); + state.events.add(new ProviderEventRecord("change", null)); + EventSteps steps = stepsWith(state); + + steps.theEventHandlerShouldHaveBeenExecuted("change"); + + assertThat(state.events) + .as("the error arrived first; claiming the change must not make it unobservable") + .extracting(ProviderEventRecord::type) + .containsExactly(ERROR); + } + + @Test + @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); + + steps.theEventHandlerShouldHaveBeenExecuted(READY); + + assertThatThrownBy(() -> steps.theEventHandlerShouldHaveBeenExecutedWithin(READY, 150)) + .as("the one record was claimed, so a chain cannot satisfy two awaits with it") + .hasMessageContaining("provider event ready"); + } +} 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..35ca2e8ef7 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/internal/steps/FlagMetadataStepsTest.java @@ -0,0 +1,193 @@ +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 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() { + 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"); + } +} 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..e1c2a5321f --- /dev/null +++ b/tools/tck/src/test/resources/skip-selftest/extensions/adopter.feature @@ -0,0 +1,27 @@ +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 + + Scenario: An extension scenario with no tags at all + 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