diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java index 4c42307d61..d01e3718d9 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java @@ -6,10 +6,10 @@ import dev.openfeature.contrib.tools.providertck.AbstractProviderTckTest; import dev.openfeature.contrib.tools.providertck.BackendEndpoint; import dev.openfeature.contrib.tools.providertck.Capability; +import dev.openfeature.contrib.tools.providertck.KnownDeviation; import dev.openfeature.sdk.FeatureProvider; import java.io.File; import java.util.Collections; -import java.util.EnumSet; import java.util.List; import java.util.Set; @@ -89,9 +89,11 @@ public FeatureProvider createUnavailableProvider() { /** * {@inheritDoc} * - *

Everything except {@link Capability#STRICT_NUMERIC_TYPING}. Evaluating {@code float-flag} - * (0.5) through the integer API returns {@code 0} with no error code rather than - * {@code TYPE_MISMATCH} with the code default — the value is silently truncated. That is a + *

Everything declarable except {@link Capability#NUMERIC_COERCION}. Evaluating + * {@code float-flag} (0.5) through the integer API returns {@code 0} with no error code + * rather than {@code TYPE_MISMATCH} with the code default — the value is silently truncated. + * Coercion as such is permitted, and the capability says so: the rule is that a lossless + * coercion must succeed and a lossy one must fail. It is the lossy case being accepted that is a * defect to fix, not a design choice; this override should be deleted once it is. * *

Declared here rather than per mode because both resolvers behave identically, which places @@ -101,10 +103,45 @@ public FeatureProvider createUnavailableProvider() { *

That includes {@link Capability#LIFECYCLE}, and legitimately so: flagd reaches its backend * during initialisation in both modes — an RPC round trip, or a full ruleset sync — so the * lifecycle scenarios assert something real here rather than passing vacuously. + * + *

{@link Capability#declarableExcept} rather than {@code EnumSet.complementOf}, which is what + * this used to be. The complement of one capability is every other enum constant, + * including {@code @targeting} and {@code @caching} — reserved tags no scenario carries — so a + * report emitted from here claimed two capabilities nothing had examined. */ @Override public Set capabilities() { - return EnumSet.complementOf(EnumSet.of(Capability.STRICT_NUMERIC_TYPING)); + return Capability.declarableExcept(Capability.NUMERIC_COERCION); + } + + /** + * {@inheritDoc} + * + *

The withheld {@link Capability#NUMERIC_COERCION} is a defect, not a limitation, and + * the report has to say so. In the results stream the two are indistinguishable: the scenario is + * skipped either way, and the declaration explains only that the capability was not + * claimed, never whether flagd chose not to claim it. A consumer comparing providers would + * otherwise read this exactly as it reads a provider with no streaming transport declining + * {@code @configuration-change}, which is a decision rather than a bug. + * + *

Tracked against flagd's numeric coercion ADR, which is where the rule this deviates from is + * settled: coercion is permitted when it is lossless and must fail with {@code TYPE_MISMATCH} + * only when information would be lost. The summary says which half is broken, because "flagd + * coerces numbers" on its own reads as a description of intended behaviour. Delete the entry — + * and the {@code capabilities()} override above — once the lossy case reports + * {@code TYPE_MISMATCH}. + */ + @Override + public List knownDeviations() { + return Collections.singletonList(KnownDeviation.tracked( + Capability.NUMERIC_COERCION, + "https://github.com/open-feature/flagd/issues/1996", + "The lossy half of the coercion rule is not enforced: evaluating float-flag (0.5) " + + "through the integer API returns 0 with no error code, rather than " + + "TYPE_MISMATCH with the code default, so the fractional part is discarded " + + "silently. Lossless coercion is permitted and is not the defect. Both " + + "resolvers behave identically, which places it in the shared provider layer " + + "rather than in either transport.")); } private FlagdOptions.FlagdOptionsBuilder baseOptions() { diff --git a/tools/provider-tck/README.md b/tools/provider-tck/README.md index e9d8232c35..42fd587a6e 100644 --- a/tools/provider-tck/README.md +++ b/tools/provider-tck/README.md @@ -208,6 +208,93 @@ and if you register more than one, select between them with +## Adding your own scenarios + +A provider with features of its own — flagd's `fractional` targeting, a vendor's proprietary +evaluation mode — extends the suite rather than maintaining a second one. Two files, no annotations: + +``` +src/test/resources/tck-extensions/fractional.feature +src/test/java/openfeature/tck/extensions/FractionalSteps.java // package openfeature.tck.extensions +``` + +That is the whole extension point. Both are already selected by `AbstractProviderTckTest`, so your +scenarios run **inside** the suite: same Compose stack, same `@BeforeAll`, same control API, same +conformance report. Step classes may take `TckState` as a constructor argument exactly as the +canonical steps do, and reach the control API and the backend endpoint through `TckRuntime.get()`. +Canonical steps are on the glue path too, so an extension scenario can open with `Given a stable +provider` and go on to whatever is specific to your provider. + +The alternative — your own Cucumber runner — is a second backend lifecycle to start and a second copy +of this suite's configuration to keep in step with it. + +**Why `tck-extensions/` and not `features/`.** Two classpath roots holding the same directory are +scanned additively; two holding the same directory *and* the same file name are not — one wins +silently and the other file is never read. A `features/errors.feature` in your test resources would +therefore *replace* the canonical file, and the suite would report success having run yours. The +extension directory has a different name so that collision cannot be reached by accident. `features/` +is the canonical set and belongs to the specification; extensions are yours. If a scenario is +portable across providers, send it to the TCK rather than keeping it as an extension. + +The directory is shipped in this JAR containing only a README, because a classpath resource selector +naming a resource that exists on no classpath root is a hard discovery error rather than an empty +selection. An adopter who extends nothing therefore still resolves it, and pays nothing for the glue +package either — Cucumber tolerates a glue package that does not exist. + +### The suite's configuration as constants + +`ProviderTck` names every value the suite's annotations carry, so that an adopter who does write a +`@ConfigurationParameter` composes rather than copies: + +```java +@ConfigurationParameter(key = Constants.GLUE_PROPERTY_NAME, value = ProviderTck.ALL_GLUE + ",com.vendor.steps") +``` + +| Constant | Value | +|---|---| +| `ProviderTck.FEATURES` | `features` — the canonical set, reserved | +| `ProviderTck.EXTENSIONS` | `tck-extensions` — where yours go | +| `ProviderTck.GLUE` | the canonical step definitions package | +| `ProviderTck.EXTENSION_GLUE` | `openfeature.tck.extensions` | +| `ProviderTck.ALL_GLUE` | both, comma-separated — what the suite runs with | +| `ProviderTck.PLUGINS`, `PARALLEL_EXECUTION_ENABLED`, `FEATURE_EXECUTION_MODE`, `OBJECT_FACTORY` | the rest of the Cucumber configuration | + +An annotation value has to be a compile-time constant, so a method call would not compile there; +constant concatenation does. If you add a glue package this way, keep `ProviderTck.GLUE` in the +value — dropping it makes every canonical step undefined. + +## The canonical set cannot be reduced + +Extending the suite is safe by convention. Shrinking it is what a conformance suite has to prevent, +because a run that asks twenty-seven of the twenty-nine questions and reports success is +indistinguishable, in every artifact it produces, from one that asked all twenty-nine. + +`CanonicalScenarioGuard` is an ordinary JUnit test that the suite selects, and it fails the build if +this run is set up to execute less than the canonical set: + +- a feature file added to `features/`, or shadowing a canonical one — the selected scenarios no + longer match what this artifact ships, which it reads from its own JAR rather than through the + classpath +- `cucumber.filter.tags` or `cucumber.filter.name` — Cucumber applies these by skipping scenarios at + execution, so the run is filtered however the plan looks +- selectors or glue overridden in your `junit-platform.properties` + +It checks the setup rather than counting afterwards: both the discovered plan and the run's filter +configuration are settled before the first scenario, so the check needs no Compose stack and takes no +measurable time. Where its result appears in the run depends on the order the JUnit Platform executes +the suite's two engines in, which is not specified. Extension scenarios are ignored: the check is +defined over `features/` alone. + +Narrowing a run legitimately is what `capabilities()` is for — those scenarios are reported as +skipped with a reason, which a filtered scenario is not. To filter anyway while debugging, set +`-Dprovider.tck.partial=true` (or `PROVIDER_TCK_PARTIAL`). The guard then reports itself as +**skipped** rather than passed, so the run states that its canonical set was not verified. + +What the guard does not establish is that the canonical files contain what they should — a +replacement placing its scenarios on the same lines would satisfy it. That is covered better +elsewhere: the results stream carries the `source` of every feature that executed, and +`tck.specRevision` says which revision it should match. + ## Declaring capabilities Not every provider implements every optional part of the spec. Scenarios that exercise an optional @@ -223,20 +310,31 @@ green on scenarios it did not run is worse than no suite at all. | `CONFIGURATION_CHANGE` | `@configuration-change` | detects config changes, emits `PROVIDER_CONFIGURATION_CHANGED` | | `OBJECT` | `@object` | supports structured flag values | | `UNAVAILABLE_INIT` | `@unavailable` | reports an error state instead of hanging on a dead backend | -| `STRICT_NUMERIC_TYPING` | `@strict-numeric-typing` | does not coerce between integer and float | -| `TARGETING` | `@targeting` | reserved, no scenarios yet | -| `CACHING` | `@caching` | reserved, no scenarios yet | +| `NUMERIC_COERCION` | `@numeric-coercion` | coerces between integer and float only when lossless, else `TYPE_MISMATCH` | +| `TARGETING` | `@targeting` | reserved, **not declarable** — no scenarios yet | +| `CACHING` | `@caching` | reserved, **not declarable** — no scenarios yet | -The default is every capability. **Narrow it, do not widen it**: start from the default, run the -suite, and remove only what your provider genuinely cannot do. +The default is every *declarable* capability. **Narrow it, do not widen it**: start from the +default, run the suite, and remove only what your provider genuinely cannot do. ```java @Override public Set capabilities() { - return EnumSet.complementOf(EnumSet.of(Capability.STALE, Capability.CACHING)); + return Capability.declarableExcept(Capability.STALE); } ``` +The reserved entries are part of the vocabulary so that every language's TCK spells the same +property the same way, but no scenario carries their tag — so declaring one cannot produce a skip, +cannot be contradicted by any result, and tells a reader a capability was verified when nothing +examined it. Declaring one **fails the run**, with a message naming the tag. + +That is a rule about an accident rather than about intent: `EnumSet.complementOf(EnumSet.of(X))` +reads as "everything except X" and in fact means "every other enum constant", reserved tags +included. The flagd suite said exactly that and published `"declared": [..., "@targeting", +"@caching"]` for two capabilities nobody had claimed. `Capability.declarable()` and +`Capability.declarableExcept(...)` are the forms that mean what the first one looks like. + A note on `LIFECYCLE` vs `EVENTS`: they look like the same thing and are not. `EVENTS` says the provider emits events; `LIFECYCLE` says there is a real initialisation behind them. The SDK's `FeatureProviderStateManager` emits `PROVIDER_READY`/`PROVIDER_ERROR` around `initialize` for *any* @@ -247,13 +345,47 @@ while emitting no events of its own, and would have been excluded. Declare `LIFE initialisation actually talks to the backend; a provider with nothing to reach — an in-memory provider, or a facade over other providers — should not declare it however many events it emits. -A note on `STRICT_NUMERIC_TYPING`: unlike the others it is not an optional feature. The spec -requires `TYPE_MISMATCH` when the requested type cannot be satisfied, and narrowing `0.5` to `0` -loses information silently — the worst failure mode for a feature flag, because the application -sees a plausible value and no error. It is a capability only so a provider with this defect can -adopt the TCK today and see the gap reported explicitly. Not declaring it is an admission of a -known bug. **The flagd provider currently does not declare it**, in either RPC or in-process mode — -see [`AbstractFlagdTckTest`](../../providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java). +A note on `NUMERIC_COERCION`: unlike the others it is not an optional feature. The rule is that +coercion between integer and float is permitted **when it is lossless** and must fail with +`TYPE_MISMATCH` **when it is not** — `10.0` requested as an integer must succeed, `0.5` must not. +Narrowing `0.5` to `0` loses information silently, which is the worst failure mode for a feature +flag, because the application sees a plausible value and no error. It is a capability only so a +provider with this defect can adopt the TCK today and see the gap reported explicitly. Not +declaring it is an admission of a known bug. **The flagd provider currently does not declare it**, +in either RPC or in-process mode — see +[`AbstractFlagdTckTest`](../../providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/AbstractFlagdTckTest.java) +and [flagd#1996](https://github.com/open-feature/flagd/issues/1996). + +Two things the tag does not cover, both open in Appendix F rather than fixed here: + +- **The lossless case has no scenario.** Only the lossy half is tested, because the canonical flag + set contains no integral float to ask the other half of, and adding one changes the flag set for + every language at once. A provider that wrongly rejects `10.0` as an integer declares this + capability and passes. +- **Accessor width is unmodelled.** The [numeric coercion + ADR](https://github.com/open-feature/flagd/blob/main/docs/architecture-decisions/numeric-coercion.md) + distinguishes a 64-bit integer accessor from a 32-bit one — flagd's own testbed tags the latter + `@int32-bounded` — and neither Appendix F nor this suite has anything equivalent. + +### Saying that a withheld capability is a defect + +Narrowing `capabilities()` reads the same way in the results whether you did it to describe a +limitation or to work around a bug: the scenarios are skipped either way, and nothing in the run can +tell the two apart. Declare a `KnownDeviation` when it is the latter. + +```java +@Override +public List knownDeviations() { + return List.of(KnownDeviation.tracked( + Capability.NUMERIC_COERCION, + "https://github.com/open-feature/java-sdk-contrib/issues/1234", + "float-flag through the integer API returns 0 with no error code")); +} +``` + +Use `KnownDeviation.untracked(...)` when there is no issue to point at yet. That is still worth +reporting — naming the defect is what separates it from a choice — but an issue link is better. +Empty is the default, and it is silence rather than a claim of having none. ## Tuning timeouts @@ -295,6 +427,129 @@ documented. The Compose stack starts once per suite and is never restarted. Scenario isolation comes from the control API. +## Conformance reports + +Set `PROVIDER_TCK_REPORT_DIR` and each suite writes two files: an envelope conforming to the +[report schema][report-schema] in the specification, and the run's results as a +[Cucumber Messages][messages] stream. + +```console +$ PROVIDER_TCK_REPORT_DIR=./reports mvn test -Dtest='Flagd*TckTest' +$ ls reports/ +flagd-in-process.json flagd-in-process.ndjson flagd-rpc.json flagd-rpc.ndjson +``` + +`-Dprovider.tck.report.dir=...` does the same thing and is often easier to pass through Maven. The +environment variable is the portable spelling — every language's TCK reads it, so one cross-language +CI job can set one thing. + +It is an environment variable rather than a method on `ProviderTckHarness` so that emitting a report +is a property of the run and not of the code: CI sets it, a developer running the suite locally does +not, and no adopter changes a line to publish one. Unset means no report, which is not an error. +Several suites in one JVM each write their own pair, so flagd's two resolvers do not collide. + +### The results are not a format this project defines + +The `.ndjson` is a Cucumber Messages stream, produced by Cucumber's own `MessageFormatter` — the +same class the built-in `message:` plugin instantiates, so the bytes are what +`--plugin message:...` would have written. It already carries everything a per-scenario report would +have had to invent: the outcome of every scenario, its tags including any set on an individual +`Examples` block, an exact Scenario Outline row identity, and the source of every feature that ran. + +The plugin exists rather than the built-in one because a `@ConfigurationParameter` value is a +compile-time constant, so the built-in plugin's path cannot be derived from the directory the run +asked for — and flagd's two suites would write to the same file. + +Reading it needs no special tooling, but it does need one thing understood: **a scenario's outcome +is the most severe result among its steps**, hooks included. `testCaseFinished` carries no status of +its own. That is what makes a capability-gated skip truthful, because the aborted `@Before` hook +contributes a `SKIPPED` result that outranks every step it stopped from running. + +```console +$ jq -c 'select(.testStepFinished) | .testStepFinished + | {c: .testCaseStartedId, s: .testStepResult.status}' reports/flagd-rpc.ndjson \ + | jq -s 'group_by(.c) | map({s: (map(.s) | if any(. == "FAILED") then "FAILED" + elif any(. == "SKIPPED") then "SKIPPED" + else "PASSED" end)}) + | group_by(.s) | map({(.[0].s): length}) | add' +{ + "PASSED": 28, + "SKIPPED": 1 +} +``` + +The [`cucumber-query`](https://github.com/cucumber/messages/tree/main/java) helpers do this properly +and in several languages; the above is only to show that the fact is in the file. + +### What identifies a scenario + +`pickle.astNodeIds`. For a scenario compiled from a Scenario Outline it is +`[scenario id, table row id]`, and the second entry resolves in the `gherkinDocument` message to the +`Examples` row the scenario was built from. Feature and name are not enough — the type-mismatch +matrix in `errors.feature` is eleven rows sharing one name — and this is exact rather than derived: + +```console +$ jq -c 'select(.pickle) | .pickle + | select(.name == "Requesting the wrong type returns the code default") + | {id, row: .astNodeIds[1]}' reports/flagd-rpc.ndjson | head -3 +{"id":"6c8debd2-...","row":"ab8b4a4b-..."} +{"id":"a7c76b0a-...","row":"63d6d6c8-..."} +{"id":"fecd333d-...","row":"bbd7f5ee-..."} +``` + +An earlier version of this module reverse-engineered the same fact by re-parsing the feature source +and matching a pickle's reported line number against the Examples tables. The stream states it +outright, which is the whole argument for a standard format over one we maintain. + +### What the envelope is for + +A Messages stream cannot say what it was a test *of*. The envelope carries the four things no +standard results format identifies: + +- **`provider`** — what the provider calls itself through its own metadata, not the suite name. The + suite name is chosen to read well in a failure message (`flagd-rpc`), which makes it the + *configuration*, and it is reported as such. One provider with two materially different modes + produces two reports that are not interchangeable. Derived from the suite class name + (`FlagdInProcessTckTest` → `flagd-in-process`); override `ProviderTckHarness.configuration()`. +- **`sdk`** — read from the classpath rather than declared, because the TCK depends on an SDK version + *range* so that adopting it can never force an upgrade. What a consumer actually ran against is + only knowable at runtime. +- **`tck`** — which implementation asked the questions, and `specRevision`, the open-feature/spec + commit the packaged artifacts came from. Baked into the JAR at build time from this module's POM: + the artifacts travel in the JAR, the repository they came from does not. The executed Gherkin no + longer rests on that pin alone — the stream carries the `source` of every feature, so it can be + diffed against the revision — but the pin is what identifies the two artifacts the stream does not + carry, `flags/canonical-flags.json` and `openapi/control-api.yaml`. +- **`declaration`** — the capability set the provider claims. This is an **input** to reading the + results, not a summary of them, which is why it cannot be derived from the stream. The stream says + a scenario was skipped; only the declaration says whether that is because the provider declines the + capability it needed. Given the declaration and a scenario's tags — both present — the reason for + each skip follows, so it does not have to be transported per scenario. + +`knownDeviations` is the one thing neither the stream nor the declaration can express: whether a +withheld capability is a limitation or a bug. See +[Saying that a withheld capability is a defect](#saying-that-a-withheld-capability-is-a-defect). + +`results.digest` covers the `.ndjson`, so a consumer that fetched the two separately can tell that +what it has is what the envelope describes. + +### What the report is for + +This suite promises that a scenario skipped for an undeclared capability is reported as skipped with +the reason and *never* as passed — and a promise is not a check. The stream records every scenario +individually, so a consumer can verify the rule instead of trusting a runner's headline number. Go's +runner counts capability-gated skips in its **passed** tally, which is exactly the failure mode this +makes impossible to hide. + +Every scenario appears exactly once, whatever happened to it. A report that quietly omitted the +scenarios it did not run would satisfy every rule above and still mislead, because a reader would +have no way to know how many questions went unasked. `ConformanceReportPluginTest` runs a fixture +suite through the real Cucumber engine and asserts both properties over the emitted stream. + +[report-schema]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/report/conformance-report.schema.json +[messages]: https://github.com/cucumber/messages + + ## Relationship to the flagd test harness The step vocabulary is inherited from the @@ -343,9 +598,18 @@ consumers — the features stay on the classpath and stay inside the JAR. like `GET /last-evaluation` returning the request the backend last received. Until then, a provider that silently drops the context passes. - **Targeting and bucketing.** Out of scope by design: that is backend evaluation logic. The - `@targeting` tag is reserved for context-passthrough scenarios once the gap above is closed. + `@targeting` tag is reserved for context-passthrough scenarios once the gap above is closed, and + is not declarable until they exist. - **Caching.** Whether a stale provider keeps serving last-known values during an outage depends on - whether it holds a local copy of the ruleset. The `@caching` tag is reserved; no scenarios yet. + whether it holds a local copy of the ruleset. The `@caching` tag is reserved; no scenarios yet, + and so not declarable. +- **Lossless numeric coercion.** `@numeric-coercion` tests only the lossy half of its rule. The + canonical flag set holds no integral float, so there is nothing to ask "must `10.0` resolve as an + integer?" of, and a provider that wrongly answers no still passes. Closing it means adding a flag + to the canonical set, which changes it for every language at once. +- **Integer accessor width.** flagd's numeric coercion ADR distinguishes a 64-bit integer accessor + from a 32-bit one, and tags the latter `@int32-bounded` in its own testbed. Neither this suite nor + Appendix F models width at all, and it is a real source of cross-language disagreement. - **Hooks.** Not covered. - **Flag metadata.** The flagd harness has metadata scenarios; they are not yet ported. - **Multi-suite JVMs.** `TckRuntime` is static, so TCK suites run one at a time within a JVM fork. diff --git a/tools/provider-tck/pom.xml b/tools/provider-tck/pom.xml index 3de834808a..79551098b6 100644 --- a/tools/provider-tck/pom.xml +++ b/tools/provider-tck/pom.xml @@ -14,6 +14,39 @@ ${groupId}.providertck + + + dc4d7ae8df1c664f82a4adf46cd43812980c0da3 + 3.27.7 4.3.0 2.22.1 @@ -103,6 +136,43 @@ cucumber-junit-platform-engine + + + io.cucumber + cucumber-core + + + + io.cucumber + messages + + + + + + io.cucumber + gherkin + compile + + io.cucumber @@ -118,6 +188,18 @@ compile + + + org.junit.jupiter + junit-jupiter-api + compile + + @@ -182,4 +264,23 @@ + + + + + src/main/resources + false + + + src/main/resources-filtered + true + + + + diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/AbstractProviderTckTest.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/AbstractProviderTckTest.java index 4f3304d94d..9b413319b4 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/AbstractProviderTckTest.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/AbstractProviderTckTest.java @@ -3,6 +3,7 @@ import io.cucumber.junit.platform.engine.Constants; import org.junit.platform.suite.api.ConfigurationParameter; import org.junit.platform.suite.api.IncludeEngines; +import org.junit.platform.suite.api.SelectClasses; import org.junit.platform.suite.api.SelectClasspathResource; import org.junit.platform.suite.api.Suite; @@ -33,14 +34,68 @@ * {@code dev.openfeature.contrib.tools.providertck.steps}, which reach the harness through * {@link TckRuntime}. * + *

The suite also carries {@link ConformanceReportPlugin}, so an adopter needs no configuration to + * publish a machine-readable conformance report: setting {@code PROVIDER_TCK_REPORT_DIR} on a run is + * enough, and leaving it unset writes nothing. + * + *

That plugin is registered here rather than as Cucumber's built-in {@code message:} plugin + * for one reason: a {@code @ConfigurationParameter} value is a compile-time constant, so the + * built-in plugin's output path cannot be derived from the report directory the run asked for, and + * two suites in one module — flagd's two resolvers — would write to the same file. The plugin + * delegates to Cucumber's own message formatter for the stream itself, so the results are the same + * bytes {@code message:} would have produced, at a path this suite can choose. + * + *

Adding your own scenarios. A provider with features of its own — flagd's + * {@code fractional} targeting, a vendor's proprietary evaluation mode — puts feature files in + * {@code src/test/resources/tck-extensions/} and step definitions in the package + * {@code openfeature.tck.extensions}, and writes no annotations. Both are selected here, so the + * extra scenarios run inside this suite: same Compose stack, same {@code @BeforeAll}, same control + * API, same conformance report. The alternative — a second suite of one's own — is a second backend + * lifecycle to start and a second set of runner configuration to keep in step with this one. + * + *

The extension directory is not {@code features/} and is not a subdirectory of it, for + * a measured reason. Two classpath roots that contain the same directory are scanned additively, but + * two that contain the same directory and the same file name are not: one wins silently and + * the other file is never read. An adopter who put {@code features/errors.feature} in their test + * resources would replace a canonical feature with their own and see the suite pass — a conformance + * suite reporting success for questions it never asked. A distinct directory name removes the + * collision rather than documenting it. + * + *

The directory is shipped inside this JAR holding nothing but a README, because + * {@link SelectClasspathResource} on a resource that exists on no classpath root is a discovery + * error, not an empty selection. An adopter who adds nothing therefore still resolves it. The + * extension glue package costs nothing when unused either: Cucumber tolerates a glue package that + * does not exist. + * + *

One JUnit Jupiter test runs alongside the scenarios: {@link CanonicalScenarioGuard}, which + * fails a suite whose canonical set has been reduced — by a tag filter, a selector override, or a + * feature file shadowing a canonical one. It is why {@code junit-jupiter} is in the engine list. It + * inspects the discovered test plan and the run's filter configuration, both of which are settled + * before the first scenario, so it costs nothing and does not depend on the order the engines happen + * to run in. + * + *

Every value these annotations carry is named in {@link ProviderTck}. An adopter who does write a + * {@code @ConfigurationParameter} of their own composes from those constants — + * {@code ProviderTck.ALL_GLUE + ",com.vendor.steps"} — rather than restating this configuration as a + * string literal that nothing would keep in step. + * * @see ProviderTckHarness + * @see ProviderTck + * @see ConformanceReportPlugin + * @see CanonicalScenarioGuard */ @Suite -@IncludeEngines("cucumber") -@SelectClasspathResource("features") -@ConfigurationParameter(key = Constants.PLUGIN_PROPERTY_NAME, value = "summary") -@ConfigurationParameter(key = Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, value = "false") -@ConfigurationParameter(key = Constants.EXECUTION_MODE_FEATURE_PROPERTY_NAME, value = "same_thread") -@ConfigurationParameter(key = Constants.GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.tools.providertck.steps") -@ConfigurationParameter(key = Constants.OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory") +@IncludeEngines({"cucumber", "junit-jupiter"}) +@SelectClasspathResource(ProviderTck.FEATURES) +@SelectClasspathResource(ProviderTck.EXTENSIONS) +@SelectClasses(CanonicalScenarioGuard.class) +@ConfigurationParameter(key = Constants.PLUGIN_PROPERTY_NAME, value = ProviderTck.PLUGINS) +@ConfigurationParameter( + key = Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, + value = ProviderTck.PARALLEL_EXECUTION_ENABLED) +@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) public abstract class AbstractProviderTckTest implements ProviderTckHarness {} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuard.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuard.java new file mode 100644 index 0000000000..876d998c23 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuard.java @@ -0,0 +1,292 @@ +package dev.openfeature.contrib.tools.providertck; + +import io.cucumber.junit.platform.engine.Constants; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.platform.engine.TestSource; +import org.junit.platform.engine.support.descriptor.ClasspathResourceSource; +import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; + +/** + * Fails a suite that is set up to run less than the whole canonical scenario set. + * + *

Everything else in this module makes the canonical set easy to extend safely. Nothing makes it + * hard to shrink, and shrinking it is the failure that matters: a run that asks twenty-seven of the + * twenty-nine questions and reports success is indistinguishable, in every artifact it produces, from + * one that asked all twenty-nine. The known ways to get there are a feature file dropped into + * {@code features/}, a {@code cucumber.filter.tags} or {@code cucumber.filter.name} expression, and + * selectors or glue overridden in a consuming module's {@code junit-platform.properties}. + * + *

Selected by {@link AbstractProviderTckTest} as an ordinary JUnit Jupiter test, so a reduced set + * fails the build the way any other failing test does. A {@link + * org.junit.platform.launcher.TestExecutionListener} cannot do that job: the JUnit Platform catches + * and logs whatever a listener throws, which is exactly the silent pass being guarded against. + * + *

Two kinds of evidence, both available before a single scenario has run: + * + *

+ * + *

Checking the setup rather than counting afterwards is not a compromise made for convenience. A + * count is only complete once the last scenario has finished, and the only hook that runs there is a + * listener, which cannot fail anything. Both kinds of evidence are settled before the first scenario + * runs, so the check itself takes no measurable time and needs no Compose stack — though where its + * result appears in a run depends on the order the JUnit Platform happens to execute the two engines + * in, which is not specified. + * + *

Extension scenarios are ignored entirely. The check is defined over {@code features/}, so an + * adopter's {@code tck-extensions/} scenarios can neither stand in for a canonical scenario nor look + * like a spurious one. + * + *

Separable from the rest of the extension work by design: it guards a bypass rather than enabling + * anything, and dropping it leaves the extension point unaffected. + */ +@ExtendWith(CanonicalScenarioGuard.CaptureConfiguration.class) +public final class CanonicalScenarioGuard { + + /** + * System property that downgrades this check to a skip. + * + *

An escape hatch is necessary rather than a weakness. Running one scenario with + * {@code -Dcucumber.filter.tags=@events} is routine while debugging a provider, and a check that + * made that impossible would be switched off permanently instead of temporarily. It produces a + * skip rather than a pass, so the run says out loud that its canonical set was not verified. + */ + public static final String PARTIAL_PROPERTY = "provider.tck.partial"; + + /** Environment variable equivalent of {@link #PARTIAL_PROPERTY}. */ + public static final String PARTIAL_ENV = "PROVIDER_TCK_PARTIAL"; + + /** Cucumber configuration keys that stop a discovered scenario from running. */ + private static final String[] FILTER_KEYS = { + Constants.FILTER_TAGS_PROPERTY_NAME, Constants.FILTER_NAME_PROPERTY_NAME + }; + + /** + * What each TCK suite in a discovered plan selected under {@code features/}. + * + *

Keyed by suite class name and accumulated rather than replaced. A suite discovers its + * children through a launcher of its own, so this is called more than once per build with plans + * of differing scope, and a plan containing no TCK suite says nothing about the ones already + * recorded. + */ + private static final Map> DISCOVERED = new ConcurrentHashMap<>(); + + private static final String CANONICAL_PREFIX = ProviderTck.FEATURES + "/"; + + /** + * Records what the suites in a discovered plan will run. + * + * @param plan the plan about to be executed + */ + static void observe(TestPlan plan) { + for (TestIdentifier root : plan.getRoots()) { + collectSuites(plan, root); + } + } + + /** Forgets what has been observed, so a test can drive the guard over a plan of its own. */ + static void forget() { + DISCOVERED.clear(); + } + + /** + * Returns what each suite in the observed plans selected under {@code features/}. + * + * @return canonical scenarios by suite class name + */ + static Map> discovered() { + return Collections.unmodifiableMap(new LinkedHashMap<>(DISCOVERED)); + } + + /** Asserts that this run will execute the whole canonical scenario set. */ + @Test + @DisplayName("the canonical scenario set was not reduced") + void theCanonicalScenarioSetWasNotReduced() { + if (partialRunAllowed()) { + Assumptions.abort("Skipped: " + PARTIAL_PROPERTY + " is set, so the canonical scenario set was not " + + "verified. This run is not a conformance run."); + } + + String problems = check(CanonicalScenarios.shipped(), discovered(), CaptureConfiguration.filters()); + if (problems != null) { + throw new AssertionError(problems); + } + } + + /** + * Reports every way this run falls short of the canonical set. + * + * @param canonical the canonical scenario set, as the TCK ships it + * @param discovered what each suite selected under {@code features/} + * @param filters configured Cucumber filters, by configuration key + * @return a report of every problem found, or {@code null} when there is none + */ + static String check( + Set canonical, + Map> discovered, + Map filters) { + List problems = new ArrayList<>(); + + for (Map.Entry filter : filters.entrySet()) { + problems.add(filter.getKey() + " is set to '" + filter.getValue() + + "'. Cucumber applies it by skipping scenarios that would otherwise have run, so a " + + "conformance run cannot be filtered. Decline capabilities your provider does not have " + + "through capabilities() instead — those scenarios are reported as skipped with a reason, " + + "which a filtered one is not. To filter anyway while debugging, set -D" + PARTIAL_PROPERTY + + "=true and accept that the run is not a conformance run."); + } + + if (discovered.isEmpty()) { + problems.add("No TCK suite was found in the JUnit test plan, so it cannot be shown that the canonical " + + "scenarios will run. This normally means TckSuiteListener was not auto-registered — the same " + + "condition that makes harness discovery fall back to ServiceLoader. Enable JUnit Platform " + + "listener auto-registration, or set -D" + PARTIAL_PROPERTY + "=true to accept a run whose " + + "canonical set is unverified."); + } + + for (Map.Entry> suite : discovered.entrySet()) { + Set missing = new TreeSet<>(canonical); + missing.removeAll(suite.getValue()); + if (!missing.isEmpty()) { + problems.add(suite.getKey() + " left out " + missing.size() + " of " + canonical.size() + + " canonical scenarios: " + missing + + ". A conformance run executes the canonical set in full. Look for a selector or glue " + + "override in junit-platform.properties, a cucumber.features property, or a feature file " + + "of your own in " + CANONICAL_PREFIX + " shadowing a canonical one. Scenarios your " + + "provider cannot support are declined through capabilities(), which reports them as " + + "skipped rather than removing them."); + } + + Set unexpected = new TreeSet<>(suite.getValue()); + unexpected.removeAll(canonical); + if (!unexpected.isEmpty()) { + problems.add(suite.getKey() + " selected " + unexpected.size() + " scenario(s) under " + + CANONICAL_PREFIX + " that this TCK does not ship: " + unexpected + + ". That directory is the canonical set, and adding to it changes what conformance means. " + + "Put your own scenarios in " + ProviderTck.EXTENSIONS + "/ instead, where they run in the " + + "same suite and the same backend lifecycle."); + } + } + + return problems.isEmpty() ? null : String.join(System.lineSeparator() + System.lineSeparator(), problems); + } + + /** + * Returns the Cucumber filters this run is configured with. + * + * @param context the executing test's context, which resolves configuration the way the Cucumber + * engine beside it does + * @return configured filters by configuration key, empty when the run is unfiltered + */ + static Map filtersIn(ExtensionContext context) { + Map configured = new LinkedHashMap<>(); + for (String key : FILTER_KEYS) { + context.getConfigurationParameter(key) + .map(String::trim) + .filter(value -> !value.isEmpty()) + .ifPresent(value -> configured.put(key, value)); + } + return configured; + } + + /** Returns whether the run has declared itself partial. */ + private static boolean partialRunAllowed() { + return isTrue(System.getProperty(PARTIAL_PROPERTY)) || isTrue(System.getenv(PARTIAL_ENV)); + } + + private static boolean isTrue(String value) { + return value != null && ("".equals(value.trim()) || Boolean.parseBoolean(value.trim())); + } + + private static void collectSuites(TestPlan plan, TestIdentifier identifier) { + Optional> suite = TckSuiteListener.harnessClassOf(identifier); + if (suite.isPresent()) { + Set canonical = new LinkedHashSet<>(); + collectCanonical(plan, identifier, canonical); + DISCOVERED.put(suite.get().getName(), Collections.unmodifiableSet(canonical)); + return; + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectSuites(plan, child); + } + } + + private static void collectCanonical(TestPlan plan, TestIdentifier identifier, Set into) { + if (identifier.isTest()) { + canonicalRefOf(identifier).ifPresent(into::add); + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectCanonical(plan, child, into); + } + } + + /** + * Returns the canonical scenario a test identifier stands for, if it is one. + * + *

A Cucumber scenario discovered from a classpath resource carries a + * {@link ClasspathResourceSource} naming the feature resource and the scenario's line — for a + * Scenario Outline, the line of the {@code Examples} row it was compiled from, which is what makes + * each row count separately. + */ + private static Optional canonicalRefOf(TestIdentifier identifier) { + Optional source = identifier.getSource(); + if (!source.isPresent() || !(source.get() instanceof ClasspathResourceSource)) { + return Optional.empty(); + } + ClasspathResourceSource resource = (ClasspathResourceSource) source.get(); + String name = resource.getClasspathResourceName(); + if (!name.startsWith(CANONICAL_PREFIX)) { + return Optional.empty(); + } + return resource.getPosition() + .map(position -> new CanonicalScenarios.Ref(name, position.getLine(), identifier.getDisplayName())); + } + + /** + * Hands the guard the configuration of the engine it is running in. + * + *

Jupiter resolves configuration parameters for an extension, not for a test method, so the + * context is captured here rather than injected. Inside a suite it carries the suite's own + * {@code @ConfigurationParameter} values as well as the system properties and + * {@code junit-platform.properties} the Cucumber engine beside it reads. + */ + static final class CaptureConfiguration implements BeforeEachCallback { + + private static volatile Map filters = Collections.emptyMap(); + + @Override + public void beforeEach(ExtensionContext context) { + filters = Collections.unmodifiableMap(filtersIn(context)); + } + + static Map filters() { + return filters; + } + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarios.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarios.java new file mode 100644 index 0000000000..24532c60ae --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarios.java @@ -0,0 +1,260 @@ +package dev.openfeature.contrib.tools.providertck; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.cucumber.gherkin.GherkinParser; +import io.cucumber.messages.types.Envelope; +import io.cucumber.messages.types.Examples; +import io.cucumber.messages.types.Feature; +import io.cucumber.messages.types.FeatureChild; +import io.cucumber.messages.types.GherkinDocument; +import io.cucumber.messages.types.Rule; +import io.cucumber.messages.types.RuleChild; +import io.cucumber.messages.types.Scenario; +import io.cucumber.messages.types.TableRow; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.CodeSource; +import java.util.Collections; +import java.util.Enumeration; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.stream.Stream; + +/** + * The canonical scenario set, as this artifact ships it. + * + *

Read from this artifact's own code source — the JAR or {@code target/classes} that + * {@link ProviderTck} was loaded from — rather than through the classloader. That is the point of the + * class: a feature file placed in {@code features/} on another classpath root shadows the canonical + * one of the same name, and a check that read the shadowed copy back through + * {@code getResource("features/errors.feature")} would be checking the replacement against itself. + * The code source is the only view of {@code features/} that an adopter's classpath cannot alter. + * + *

A scenario is identified by its resource name and its line, which is what a Cucumber + * {@code ClasspathResourceSource} in the JUnit test plan carries. For a Scenario Outline that line is + * the {@code Examples} row, so each row counts individually — which is necessary, because eleven rows + * of the type-mismatch matrix share one name. + * + *

What this does not establish is that the canonical files contain what they + * should. A replacement file that happened to place its scenarios on the same lines would satisfy the + * comparison. Content is covered elsewhere and better: the results stream carries the {@code source} + * of every feature that executed, and the report's {@code tck.specRevision} says which revision it + * should match. + */ +final class CanonicalScenarios { + + private static final String FEATURE_SUFFIX = ".feature"; + + private static volatile Set shipped; + + private CanonicalScenarios() {} + + /** + * Returns every scenario the canonical feature files in this artifact compile to. + * + * @return the canonical scenario set, never empty + * @throws IllegalStateException if this artifact's own feature files cannot be read + */ + static Set shipped() { + Set cached = shipped; + if (cached == null) { + synchronized (CanonicalScenarios.class) { + if (shipped == null) { + shipped = Collections.unmodifiableSet(read()); + } + cached = shipped; + } + } + return cached; + } + + /** Reads and compiles the canonical feature files out of the code source this class came from. */ + @SuppressFBWarnings( + value = "PATH_TRAVERSAL_IN", + justification = "The path is this class's own code source, reported by the JVM. Nothing outside the " + + "running artifact can influence it, and reading it from anywhere else would defeat the point " + + "of the class") + private static Set read() { + CodeSource codeSource = ProviderTck.class.getProtectionDomain().getCodeSource(); + URL location = codeSource == null ? null : codeSource.getLocation(); + if (location == null) { + throw new IllegalStateException("The provider-tck code source is not visible to this JVM, so the " + + "canonical scenario set cannot be established. Set -D" + CanonicalScenarioGuard.PARTIAL_PROPERTY + + "=true to run without the canonical-set check, understanding that the run is then not a " + + "conformance run."); + } + + Path path; + try { + path = Paths.get(location.toURI()); + } catch (URISyntaxException | IllegalArgumentException e) { + throw new IllegalStateException( + "The provider-tck code source " + location + " is not a file, so the " + + "canonical scenario set cannot be established.", + e); + } + + Set refs = new LinkedHashSet<>(); + try { + if (Files.isDirectory(path)) { + fromDirectory(path, refs); + } else { + fromJar(path, refs); + } + } catch (IOException e) { + throw new UncheckedIOException("Could not read the canonical feature files from " + path, e); + } + + if (refs.isEmpty()) { + throw new IllegalStateException("No canonical scenarios found in " + ProviderTck.FEATURES + "/ of the " + + "provider-tck artifact at " + path + ". The artifact is not intact."); + } + return refs; + } + + private static void fromDirectory(Path root, Set refs) throws IOException { + Path features = root.resolve(ProviderTck.FEATURES); + if (!Files.isDirectory(features)) { + return; + } + try (DirectoryStream entries = Files.newDirectoryStream(features, "*" + FEATURE_SUFFIX)) { + for (Path entry : entries) { + String name = ProviderTck.FEATURES + "/" + entry.getFileName(); + collect(name, Files.readAllBytes(entry), refs); + } + } + } + + private static void fromJar(Path jar, Set refs) throws IOException { + try (JarFile file = new JarFile(jar.toFile())) { + Enumeration entries = file.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + String name = entry.getName(); + if (!entry.isDirectory() + && name.startsWith(ProviderTck.FEATURES + "/") + && name.endsWith(FEATURE_SUFFIX)) { + try (InputStream in = file.getInputStream(entry)) { + collect(name, readAll(in), refs); + } + } + } + } + } + + private static byte[] readAll(InputStream in) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) != -1) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + + /** + * Compiles one feature file to the scenarios Cucumber would run from it. + * + *

The Gherkin document is walked rather than the pickles it produces, because a pickle carries + * no line number — and the line is what the JUnit test plan identifies a scenario by. The walk is + * the same rule Gherkin's own compiler applies: a Scenario Outline yields one scenario per + * {@code Examples} body row, anything else yields one at its own line. + */ + private static void collect(String resource, byte[] content, Set refs) { + GherkinParser parser = GherkinParser.builder() + .includeSource(false) + .includePickles(false) + .includeGherkinDocument(true) + .build(); + + try (Stream envelopes = parser.parse(resource, content)) { + envelopes.forEach(envelope -> envelope.getGherkinDocument() + .flatMap(GherkinDocument::getFeature) + .ifPresent(feature -> collectFeature(resource, feature, refs))); + } + } + + private static void collectFeature(String resource, Feature feature, Set refs) { + for (FeatureChild child : feature.getChildren()) { + child.getScenario().ifPresent(scenario -> collectScenario(resource, scenario, refs)); + child.getRule().ifPresent(rule -> collectRule(resource, rule, refs)); + } + } + + private static void collectRule(String resource, Rule rule, Set refs) { + for (RuleChild child : rule.getChildren()) { + child.getScenario().ifPresent(scenario -> collectScenario(resource, scenario, refs)); + } + } + + private static void collectScenario(String resource, Scenario scenario, Set refs) { + List examples = scenario.getExamples(); + if (examples.isEmpty()) { + refs.add(new Ref(resource, scenario.getLocation().getLine(), scenario.getName())); + return; + } + for (Examples block : examples) { + for (TableRow row : block.getTableBody()) { + refs.add(new Ref(resource, row.getLocation().getLine(), scenario.getName())); + } + } + } + + /** + * One scenario, identified the way the JUnit test plan identifies it. + * + *

The name is carried for the failure message only; two scenarios are the same scenario when + * they are at the same line of the same classpath resource. + */ + static final class Ref implements Comparable { + + private final String resource; + private final long line; + private final String name; + + Ref(String resource, long line, String name) { + this.resource = resource; + this.line = line; + this.name = name; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof Ref)) { + return false; + } + Ref that = (Ref) other; + return line == that.line && resource.equals(that.resource); + } + + @Override + public int hashCode() { + return 31 * resource.hashCode() + Long.hashCode(line); + } + + @Override + public int compareTo(Ref other) { + int byResource = resource.compareTo(other.resource); + return byResource != 0 ? byResource : Long.compare(line, other.line); + } + + @Override + public String toString() { + return resource + ":" + line + (name == null || name.isEmpty() ? "" : " (" + name + ")"); + } + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/Capability.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/Capability.java index 38bcb5898d..b5e1f558e9 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/Capability.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/Capability.java @@ -1,6 +1,10 @@ package dev.openfeature.contrib.tools.providertck; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.EnumSet; +import java.util.List; import java.util.Optional; /** @@ -17,6 +21,14 @@ * never as passed. Silently green scenarios would make a conformance suite worthless. * *

Scenarios with no capability tag are considered mandatory and always run. + * + *

Some entries are {@linkplain #reserved() reserved}: they exist in the vocabulary so that every + * language's TCK spells the same property the same way, but no scenario carries their tag yet. A + * reserved capability must not be declared — there is nothing for it to gate, so + * declaring it cannot produce a skip and cannot be contradicted by any result. Declare + * {@link #declarable()}, or {@link #declarableExcept} for "everything except", rather than + * {@code EnumSet.allOf} or {@code EnumSet.complementOf}: both of the latter sweep up every reserved + * tag on the way past, which is how a report comes to claim a capability nobody examined. */ public enum Capability { @@ -62,7 +74,14 @@ public enum Capability { UNAVAILABLE_INIT("@unavailable"), /** - * Provider keeps the integer and float types distinct instead of coercing between them. + * Provider coerces between the integer and float types only when the coercion is lossless. + * + *

The rule is lossless coercion is permitted; lossy coercion must fail with + * {@code TYPE_MISMATCH}. An integral float such as {@code 10.0} requested as an integer + * must succeed, because nothing is lost by answering it; {@code 0.5} requested as an integer must + * not, because narrowing it to {@code 0} discards the fractional part. The distinction is flagd's + * numeric + * coercion ADR, and this capability is named after it. * *

Unlike the other entries here this is not an optional spec feature. The * specification requires a provider to report {@code TYPE_MISMATCH} when the requested type @@ -74,30 +93,42 @@ public enum Capability { * see the gap reported as an explicit skip, rather than being unable to adopt at all. Not * declaring it is an admission of a known bug, not a design choice. Declare it as soon as the * provider is fixed. + * + *

Only the lossy half is tested. The canonical flag set contains no integral + * float, so there is nothing to ask the lossless half of, and a provider that wrongly rejects + * {@code 10.0} as an integer declares this and passes. Closing that gap means adding a flag to + * the canonical set, which changes it for every language at once; Appendix F records it as open + * rather than pretending it is covered. */ - STRICT_NUMERIC_TYPING("@strict-numeric-typing"), + NUMERIC_COERCION("@numeric-coercion"), /** * Provider supports targeting rules driven by evaluation context. * - *

Reserved. No scenario in the current suite carries this tag — targeting is backend - * evaluation logic, which the TCK deliberately does not test. It exists so the tag vocabulary - * stays aligned with the flagd test harness and so context-passthrough scenarios have a home - * once the control API grows an echo endpoint. + *

{@linkplain #reserved() Reserved}. No scenario in the current suite carries this tag — + * targeting is backend evaluation logic, which the TCK deliberately does not test. It exists so + * the tag vocabulary stays aligned with the flagd test harness and so context-passthrough + * scenarios have a home once the control API grows an echo endpoint. */ - TARGETING("@targeting"), + TARGETING("@targeting", true), /** * Provider caches evaluation results and invalidates them on configuration change. * - *

Reserved; no scenario carries this tag yet. + *

{@linkplain #reserved() Reserved}; no scenario carries this tag yet. */ - CACHING("@caching"); + CACHING("@caching", true); private final String tag; + private final boolean reserved; Capability(String tag) { + this(tag, false); + } + + Capability(String tag, boolean reserved) { this.tag = tag; + this.reserved = reserved; } /** @@ -109,6 +140,20 @@ public String tag() { return tag; } + /** + * Returns whether this capability is reserved, and so must not be declared. + * + *

Reserved means the tag is part of the shared vocabulary but no scenario in the suite + * carries it. Such a capability cannot gate anything: it produces no skip, so it plays no part + * in reading the results, and listing it in a report invites a reader to believe it was verified + * when nothing examined it. + * + * @return {@code true} if no scenario carries this capability's tag + */ + public boolean reserved() { + return reserved; + } + /** * Looks up the capability gated by a Gherkin tag. * @@ -118,4 +163,66 @@ public String tag() { public static Optional fromTag(String tag) { return Arrays.stream(values()).filter(c -> c.tag.equals(tag)).findFirst(); } + + /** + * Returns every capability that may be declared, which is every capability some scenario gates. + * + *

This, not {@code EnumSet.allOf(Capability.class)}, is what "everything" means for a + * declaration. + * + * @return the declarable capabilities, as a fresh mutable set + */ + public static EnumSet declarable() { + EnumSet declarable = EnumSet.allOf(Capability.class); + declarable.removeIf(Capability::reserved); + return declarable; + } + + /** + * Returns every declarable capability except the given ones. + * + *

The counterpart to {@code EnumSet.complementOf}, and the reason it exists: a provider + * saying "everything except the one thing I cannot do" wants everything declarable + * except that thing, whereas {@code complementOf} hands back the reserved tags as well. + * + * @param excluded capabilities to withhold; reserved capabilities are absent regardless + * @return the declarable capabilities minus {@code excluded}, as a fresh mutable set + */ + public static EnumSet declarableExcept(Capability... excluded) { + EnumSet declared = declarable(); + for (Capability capability : excluded) { + declared.remove(capability); + } + return declared; + } + + /** + * Rejects a declaration that names a reserved capability. + * + *

Fails the run rather than warning and dropping it. The declaration is the one part of a + * conformance report that no result can check — everything else in it was observed, this is + * asserted by the provider author — so a claim that cannot possibly be true is worth stopping + * for. There is nothing to lose by refusing, either: no scenario carries a reserved tag, so no + * coverage depends on the claim, and the fix is to call {@link #declarable()} or + * {@link #declarableExcept}. + * + * @param declared the capabilities a harness declares + * @throws IllegalArgumentException if any of them is reserved + */ + public static void requireDeclarable(Collection declared) { + List reservedTags = new ArrayList<>(); + for (Capability capability : declared) { + if (capability.reserved()) { + reservedTags.add(capability.name() + " (" + capability.tag() + ")"); + } + } + if (!reservedTags.isEmpty()) { + throw new IllegalArgumentException("capabilities() declares reserved " + reservedTags + + ", which no scenario in the suite carries. A reserved capability cannot be " + + "verified or contradicted by any result, so it must not be declared. Use " + + "Capability.declarable(), or Capability.declarableExcept(...) for " + + "\"everything except\" — EnumSet.allOf and EnumSet.complementOf pick reserved " + + "capabilities up on the way past."); + } + } } diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CapabilityGate.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CapabilityGate.java new file mode 100644 index 0000000000..8dd177b4e5 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/CapabilityGate.java @@ -0,0 +1,43 @@ +package dev.openfeature.contrib.tools.providertck; + +import java.util.Collection; +import java.util.Optional; +import java.util.Set; +import org.opentest4j.TestAbortedException; + +/** + * Skips a scenario that needs a capability the provider did not declare. + * + *

One implementation, deliberately. This is the rule the conformance report exists to make + * checkable — a scenario skipped for an undeclared capability must be reported as skipped and never + * as passed — so the gate that produces the skip and the tests that prove the skip survives into the + * results have to be looking at the same code. Inlined into the step definitions, a self-test could + * only demonstrate that some abort becomes a skip, not that this abort does. + * + *

Aborting rather than failing is what makes the outcome a skip: {@link TestAbortedException} maps + * to {@code SKIPPED} in Cucumber's step results, which is what reaches the results stream. + */ +public final class CapabilityGate { + + private CapabilityGate() {} + + /** + * Aborts the running scenario if any of its tags gates a capability that was not declared. + * + *

Tags that gate nothing are ignored, so a scenario with no capability tag is mandatory and + * always runs. + * + * @param tags the scenario's Gherkin tags, including the leading at-sign + * @param declared the capabilities the provider declares + * @throws TestAbortedException if a tag gates an undeclared capability + */ + public static void requireDeclared(Collection tags, Set declared) { + for (String tag : tags) { + Optional capability = Capability.fromTag(tag); + if (capability.isPresent() && !declared.contains(capability.get())) { + throw new TestAbortedException("Skipped: provider does not declare capability " + + capability.get().name() + " (tag " + tag + "). Declared capabilities: " + declared); + } + } + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReport.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReport.java new file mode 100644 index 0000000000..c3d2aafd37 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReport.java @@ -0,0 +1,244 @@ +package dev.openfeature.contrib.tools.providertck; + +import com.fasterxml.jackson.annotation.JsonInclude; +import java.util.List; + +/** + * The envelope of one conformance run: what was tested, what the provider claims, and where the + * results are. + * + *

This document deliberately carries no per-scenario outcomes. The results + * themselves are a standard format — a Cucumber Messages stream, referenced by {@link Results} — + * because per-scenario outcomes, tags, Scenario Outline row identity and the executed feature source + * are all already specified there. Defining them a second time here would create a format to + * maintain and version and two places for the same fact to disagree. + * + *

The field names and nesting are fixed by the report schema in the OpenFeature specification + * repository. This class is a transcription of that schema rather than a shape that would be + * convenient in Java, because the point of the format is that every language's TCK emits the same + * document. + * + *

Fields left {@code null} are omitted from the JSON. The schema sets + * {@code additionalProperties: false} throughout, so an unexpected field is a validation failure + * rather than something a consumer ignores. + * + * @see open-feature/spec#424 + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public final class ConformanceReport { + + /** + * The major version of the report schema this document conforms to. + * + *

An integer as a string, so a consumer can reject a report it does not understand rather + * than guessing at it. + */ + public static final String SCHEMA_VERSION = "1"; + + /** The schema version this document claims. */ + public final String schemaVersion; + + /** What was tested. */ + public final Provider provider; + + /** Which OpenFeature SDK the provider was exercised through. */ + public final Sdk sdk; + + /** What asked the questions, and which questions. */ + public final Tck tck; + + /** What the provider was pointed at. */ + public final Backend backend; + + /** The capability set this provider claims. */ + public final Declaration declaration; + + /** Where the executed results live, and in what format. */ + public final Results results; + + /** Deviations the provider acknowledges, or {@code null} to say nothing. */ + public final List knownDeviations; + + ConformanceReport( + Provider provider, + Sdk sdk, + Tck tck, + Backend backend, + Declaration declaration, + Results results, + List knownDeviations) { + this.schemaVersion = SCHEMA_VERSION; + this.provider = provider; + this.sdk = sdk; + this.tck = tck; + this.backend = backend; + this.declaration = declaration; + this.results = results; + this.knownDeviations = knownDeviations; + } + + /** Identifies the provider under test. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Provider { + + /** + * What the provider calls itself, through its own metadata. + * + *

Not the name of the suite. A suite name is chosen to read well in a failure message — + * {@code flagd-rpc} — which makes it the {@link #configuration}, not the identity. + */ + public final String name; + + /** The language the provider is written in, always {@code java} here. */ + public final String language; + + /** + * Which configuration of the provider was tested. + * + *

A provider with more than one materially different mode produces one report per mode, + * and they are not interchangeable: flagd's RPC and in-process resolvers differ in whether + * they emit {@code PROVIDER_STALE}, so a report keyed on the provider name alone would have + * to pick one and misrepresent the other. + */ + public final String configuration; + + Provider(String name, String language, String configuration) { + this.name = name; + this.language = language; + this.configuration = configuration; + } + } + + /** Identifies the OpenFeature SDK the run went through. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Sdk { + + /** Maven coordinates of the SDK, without a version. */ + public final String name; + + /** The resolved SDK version. */ + public final String version; + + Sdk(String name, String version) { + this.name = name; + this.version = version; + } + } + + /** Identifies the TCK implementation and the conformance artifacts it executed. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Tck { + + /** Which TCK implementation produced this report. */ + public final String implementation; + + /** The version of that implementation. */ + public final String version; + + /** + * The open-feature/spec commit the executed artifacts came from. + * + *

The executed Gherkin no longer has to be taken on trust: the results stream carries the + * {@code source} of every feature it ran, so a consumer can diff what executed against what + * this revision contains. The revision still identifies the two artifacts the stream does + * not carry — the canonical flag set and the control API definition. + */ + public final String specRevision; + + Tck(String implementation, String version, String specRevision) { + this.implementation = implementation; + this.version = version; + this.specRevision = specRevision; + } + } + + /** Describes what the provider was pointed at. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Backend { + + /** A short human-readable description of the stack under test. */ + public final String description; + + /** + * How the backend was driven, {@code http} or {@code in-process}. + * + *

{@code in-process} is the narrow allowance made for providers with no backend; a report + * claiming it for a provider that has one should be treated with suspicion. + */ + public final String controlApi; + + Backend(String description, String controlApi) { + this.description = description; + this.controlApi = controlApi; + } + } + + /** The capability set this provider claims, as the Gherkin tags that gate them. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Declaration { + + /** + * Capabilities the provider declares, including the leading at-sign. + * + *

An input to reading the results rather than a summary of them, which is + * why it cannot be derived from the results payload and has to be stated here. A skipped + * scenario in the stream says the question was not put to this provider; only the declaration + * says whether that is because the provider declines the capability. Given the declaration + * and a scenario's tags — both of which the stream carries — the reason for a skip follows + * without being transported per scenario. + */ + public final List declared; + + Declaration(List declared) { + this.declared = declared; + } + } + + /** Where the executed results live, and in what format. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Results { + + /** The ndjson protocol at {@code https://github.com/cucumber/messages}. */ + public static final String CUCUMBER_MESSAGES = "cucumber-messages"; + + /** The results format, always {@link #CUCUMBER_MESSAGES} here. */ + public final String format; + + /** + * The Cucumber Messages release the stream was produced against. + * + *

Messages is versioned and the four TCK implementations pin different releases -- this + * one takes whatever cucumber-jvm bundles, while the Go TCK builds against v21 and the + * Python one against 34.2.0 -- so a consumer holding two reports cannot assume one schema + * validates both. + * + *

Guessing is worse than not validating. A later schema accepts messages this producer + * could not have emitted, and an earlier one rejects messages that are perfectly valid, so a + * check against the wrong version reports a result that has nothing to do with the stream. + * + *

Read back out of the stream's own {@code meta.protocolVersion} rather than from a + * constant or the artifact version, so the envelope and the stream cannot disagree about + * which release produced it. + */ + public final String formatVersion; + + /** + * Where to fetch the results: a path relative to this document. + * + *

Referenced rather than inlined because a Messages stream carries the feature sources and + * so is far larger than this envelope, and because a consumer deciding whether it cares about + * a report should not have to fetch the whole run to find out. + */ + public final String location; + + /** Digest over the results payload as {@code sha256:}. */ + public final String digest; + + Results(String format, String formatVersion, String location, String digest) { + this.format = format; + this.formatVersion = formatVersion; + this.location = location; + this.digest = digest; + } + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPlugin.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPlugin.java new file mode 100644 index 0000000000..ab21cc8401 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPlugin.java @@ -0,0 +1,302 @@ +package dev.openfeature.contrib.tools.providertck; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.cucumber.core.plugin.MessageFormatter; +import io.cucumber.messages.types.Envelope; +import io.cucumber.plugin.ConcurrentEventListener; +import io.cucumber.plugin.event.EventPublisher; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Writes a machine-readable conformance report for the suite that just ran. + * + *

Registered automatically by {@link AbstractProviderTckTest}, so an adopter changes nothing to + * get one. It is opt-in per run: set {@value #REPORT_DIR_ENV} (or the + * {@value #REPORT_DIR_PROPERTY} system property) and each suite writes two files, + * {@code

/.json} and {@code /.ndjson}. Emitting a report is + * a property of the run rather than of the code — CI asks for one, a developer running the suite + * locally does not — and unset means no report, which is not an error. Several suites in one JVM each + * write their own pair of files, so flagd's two resolvers do not collide. + * + *

The results are not a format this project defines. The {@code .ndjson} file is + * a Cucumber Messages stream, produced by + * Cucumber's own {@link MessageFormatter} — the same class the built-in {@code message:} plugin + * instantiates, so the bytes are what {@code --plugin message:...} would have written. It already + * carries everything a per-scenario report would have had to invent: the outcome of every scenario, + * its tags including any on an individual {@code Examples} block, an exact Scenario Outline row + * identity through pickle AST node ids, and the {@code source} of every feature that executed. + * + *

The {@code .json} file is the envelope, and it exists because a Messages stream cannot say what + * it was a test of. No standard format identifies the provider, the SDK it was driven + * through, the TCK build, or — most importantly — the capability set the provider declared. That + * declaration is an input to reading the results rather than a summary of them: the stream says a + * scenario was skipped, and only the declaration says whether that is because the provider declines + * the capability it needed. + * + *

Why the stream is buffered rather than streamed to the file. The file name is + * derived from the provider configuration, which is not known when Cucumber wires plugins up — the + * suite reports it once its runtime has started, which is after the first messages have already been + * emitted. Buffering keeps one file per suite correctly named, and has the side benefit that + * {@code results.digest} is computed over exactly the bytes that were written. The stream for this + * suite is well under a megabyte. + * + * @see open-feature/spec#424 + */ +public final class ConformanceReportPlugin implements ConcurrentEventListener { + + /** Environment variable naming the directory reports are written to. */ + public static final String REPORT_DIR_ENV = "PROVIDER_TCK_REPORT_DIR"; + + /** + * System property naming the directory reports are written to, taking precedence over the + * environment variable. + * + *

Accepted in addition to {@value #REPORT_DIR_ENV} because {@code -D} is how a Maven or + * Gradle invocation is usually parameterised. The environment variable is the portable spelling + * and is what every other language's TCK reads, so a cross-language CI job can set one thing. + */ + public static final String REPORT_DIR_PROPERTY = "provider.tck.report.dir"; + + /** Extension of the envelope, which is what a consumer reads first. */ + static final String ENVELOPE_EXTENSION = ".json"; + + /** Extension of the Cucumber Messages stream the envelope points at. */ + static final String RESULTS_EXTENSION = ".ndjson"; + + private static final Logger log = LoggerFactory.getLogger(ConformanceReportPlugin.class); + + private static final String LANGUAGE = "java"; + + private final Supplier reportDir; + private final Supplier> metadata; + + /** Creates the plugin Cucumber instantiates by name. */ + public ConformanceReportPlugin() { + this(ConformanceReportPlugin::configuredReportDir, TckRuntime::lastRunMetadata); + } + + ConformanceReportPlugin(Supplier reportDir, Supplier> metadata) { + this.reportDir = reportDir; + this.metadata = metadata; + } + + @Override + public void setEventPublisher(EventPublisher publisher) { + String dir = reportDir.get(); + if (dir == null) { + // Nothing asked for a report, so nothing is collected either. Registering the message + // formatter regardless would buffer a stream for every local test run. + return; + } + + ByteArrayOutputStream results = new ByteArrayOutputStream(); + new MessageFormatter(results).setEventPublisher(publisher); + + // Registered *after* the formatter, and deliberately so. Cucumber invokes the handlers for + // one event type in registration order, and the formatter closes its writer when it sees + // the run-finished message; going second is what guarantees the buffer is complete and + // flushed before the digest is taken over it. + publisher.registerHandlerFor(Envelope.class, envelope -> { + if (envelope.getTestRunFinished().isPresent()) { + write(dir, results.toByteArray()); + } + }); + } + + /** + * Returns the directory reports are written to, or {@code null} when none was configured. + * + * @return the configured report directory, trimmed, or {@code null} + */ + static String configuredReportDir() { + String property = System.getProperty(REPORT_DIR_PROPERTY); + if (property != null && !property.trim().isEmpty()) { + return property.trim(); + } + String environment = System.getenv(REPORT_DIR_ENV); + if (environment != null && !environment.trim().isEmpty()) { + return environment.trim(); + } + return null; + } + + /** + * Assembles the envelope for what the run observed. + * + * @param run what the runtime recorded about this suite + * @param location where the results stream sits, relative to the envelope + * @param digest digest over the results stream + * @return the envelope, ready to serialise + */ + ConformanceReport build(TckRunMetadata run, String location, String digest, String formatVersion) { + return new ConformanceReport( + new ConformanceReport.Provider(run.providerName(), LANGUAGE, run.configuration()), + new ConformanceReport.Sdk(TckBuildInfo.SDK_NAME, TckBuildInfo.sdkVersion()), + new ConformanceReport.Tck( + TckBuildInfo.IMPLEMENTATION, TckBuildInfo.tckVersion(), TckBuildInfo.specRevision()), + backendOf(run), + new ConformanceReport.Declaration(declaredTags(run.capabilities())), + new ConformanceReport.Results( + ConformanceReport.Results.CUCUMBER_MESSAGES, formatVersion, location, digest), + run.knownDeviations().isEmpty() ? null : run.knownDeviations()); + } + + /** + * Lists the declared capabilities as Gherkin tags, in the order the vocabulary declares them. + * + *

Ordered by the enum rather than by the set so that two runs of the same configuration + * produce byte-identical declarations, which is what makes the envelopes diffable. + * + *

{@linkplain Capability#reserved() Reserved} capabilities are skipped. A declaration that + * names one is already rejected where it enters the run, in {@link TckRunMetadata}, so this is + * not the thing that tells the adopter; it is what makes "no reserved tag in the emitted + * declaration" a property of the code that writes the document rather than a consequence of a + * check somewhere upstream of it. + */ + private static List declaredTags(Set declared) { + List tags = new ArrayList<>(declared.size()); + for (Capability capability : Capability.values()) { + if (!capability.reserved() && declared.contains(capability)) { + tags.add(capability.tag()); + } + } + return Collections.unmodifiableList(tags); + } + + private static ConformanceReport.Backend backendOf(TckRunMetadata run) { + String description = run.backendDescription().orElse(null); + String controlApi = run.controlApi().orElse(null); + return description == null && controlApi == null + ? null + : new ConformanceReport.Backend(description, controlApi); + } + + /** + * Reads the Messages release out of the stream's own {@code meta} message. + * + *

Taken from the stream rather than from a constant or the {@code io.cucumber:messages} + * artifact version, because the envelope and the stream disagreeing about which release produced + * them would be worse than either being absent. {@code meta} is the first envelope cucumber + * writes, so only the first line is parsed. + * + * @param results the buffered results stream + * @return the protocol version, or {@code null} when the stream carries none + */ + private static String protocolVersionOf(byte[] results) { + String stream = new String(results, StandardCharsets.UTF_8); + int newline = stream.indexOf('\n'); + String first = newline < 0 ? stream : stream.substring(0, newline); + if (first.trim().isEmpty()) { + return null; + } + try { + JsonNode version = new ObjectMapper().readTree(first).path("meta").path("protocolVersion"); + return version.isTextual() ? version.asText() : null; + } catch (JsonProcessingException e) { + // A stream whose first line will not parse is a bug worth surfacing, but not here: + // omitting an optional field is better than failing a run that otherwise succeeded. + log.warn("provider-tck: could not read the Messages protocol version from the stream", e); + return null; + } + } + + @SuppressFBWarnings( + value = "PATH_TRAVERSAL_IN", + justification = "The directory is supplied by whoever started the test run, which is the " + + "whole point of the setting; the file names within it are sanitised by ReportNames") + private void write(String dir, byte[] results) { + Optional observed = metadata.get(); + if (!observed.isPresent()) { + // Nothing observed the suite, which means it never got as far as starting the runtime. + // The run has failed for some other reason by now; adding a report with an invented + // provider name on top of that would only mislead. + log.error( + "{} is set but no TCK suite ran, so there is nothing to report on. " + + "This normally means the suite failed before its backend stack started.", + REPORT_DIR_ENV); + return; + } + + TckRunMetadata run = observed.get(); + String configuration = run.configuration(); + Path directory; + try { + directory = Paths.get(dir); + } catch (InvalidPathException e) { + throw new IllegalStateException( + "provider-tck [" + configuration + "]: " + REPORT_DIR_ENV + " is not a usable path: " + dir, e); + } + + String base = ReportNames.baseNameOf(configuration); + String location = base + RESULTS_EXTENSION; + Path resultsPath = directory.resolve(location); + Path envelopePath = directory.resolve(base + ENVELOPE_EXTENSION); + + String json; + try { + json = new ObjectMapper() + .writerWithDefaultPrettyPrinter() + .writeValueAsString(build(run, location, digestOf(results), protocolVersionOf(results))) + + "\n"; + } catch (JsonProcessingException e) { + throw new IllegalStateException( + "provider-tck [" + configuration + "]: could not encode the conformance report", e); + } + + // A failure to write is raised rather than logged and swallowed. CI that asked for a report + // and silently did not get one is how a publishing pipeline serves a stale result forever. + // The results go first: an envelope naming a stream that is not there is worse than neither. + try { + Files.createDirectories(directory); + Files.write(resultsPath, results); + Files.write(envelopePath, json.getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new UncheckedIOException( + "provider-tck [" + configuration + "]: could not write the conformance report to " + directory, e); + } + + log.info( + "provider-tck [{}]: conformance report written to {}, results to {}", + configuration, + envelopePath, + resultsPath); + } + + /** Digests the results stream in the {@code sha256:} form the schema asks for. */ + private static String digestOf(byte[] results) { + MessageDigest sha256; + try { + sha256 = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + // SHA-256 is required of every Java platform, so this cannot happen on a working JVM. + throw new IllegalStateException("SHA-256 is not available", e); + } + byte[] digest = sha256.digest(results); + StringBuilder hex = new StringBuilder(7 + digest.length * 2); + hex.append("sha256:"); + for (byte b : digest) { + hex.append(Character.forDigit((b >> 4) & 0xf, 16)).append(Character.forDigit(b & 0xf, 16)); + } + return hex.toString(); + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ControlApiClient.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ControlApiClient.java index 6c0a19e57d..527e55876a 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ControlApiClient.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ControlApiClient.java @@ -56,6 +56,19 @@ public String baseUrl() { return baseUrl; } + /** + * Returns how the backend was driven, for the conformance report. + * + *

{@code http} is the normative control API and the only one this TCK implements. The schema + * also allows {@code in-process}, a narrow allowance for providers with no backend at all; a + * report claiming it for a provider that has one should be treated with suspicion. + * + * @return the control API kind, always {@code http} + */ + public String controlApi() { + return "http"; + } + /** * Starts the backend with a named configuration, seeding flag state to that configuration's * baseline. diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/KnownDeviation.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/KnownDeviation.java new file mode 100644 index 0000000000..b38022dc1a --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/KnownDeviation.java @@ -0,0 +1,65 @@ +package dev.openfeature.contrib.tools.providertck; + +import com.fasterxml.jackson.annotation.JsonInclude; + +/** + * A gap the provider is known to have against something the specification does not treat as + * optional. + * + *

Distinct from an undeclared capability, which is a choice. A provider that does not + * declare {@code @configuration-change} has no streaming transport and is not pretending otherwise; + * a provider that does not declare {@code @numeric-coercion} has a bug. Both look identical in + * the results stream — scenarios skipped, reason recoverable from the declaration — so the + * difference has to be stated, or a consumer cannot tell a design decision from a defect. + * + *

Declared by the provider author through {@link ProviderTckHarness#knownDeviations()}, which is + * the only place that knows the difference. The TCK cannot infer it: from the outside, a capability + * the provider chose to withhold and one it withheld because it is broken are the same absence. + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public final class KnownDeviation { + + /** The capability tag the deviation concerns, or {@code null} when it maps to none. */ + public final String capability; + + /** Where the gap is tracked, or {@code null} when it is not tracked anywhere. */ + public final String issue; + + /** What the gap is, in a form someone comparing providers can use. */ + public final String summary; + + private KnownDeviation(String capability, String issue, String summary) { + this.capability = capability; + this.issue = issue; + this.summary = summary; + } + + /** + * Records a deviation that is tracked somewhere. + * + * @param capability the capability withheld because of the gap, or {@code null} when the gap is + * against a mandatory scenario and so belongs to no capability + * @param issue a URI where the gap is tracked + * @param summary what the gap is + * @return the deviation, ready to report + */ + public static KnownDeviation tracked(Capability capability, String issue, String summary) { + return new KnownDeviation(capability == null ? null : capability.tag(), issue, summary); + } + + /** + * Records a deviation that is not tracked anywhere yet. + * + *

Worth reporting even so. Naming the defect is what separates it from a capability the + * provider chose to withhold, and a report that merely omits the tag cannot say which of the two + * happened. Prefer {@link #tracked} as soon as there is an issue to point at. + * + * @param capability the capability withheld because of the gap, or {@code null} when the gap is + * against a mandatory scenario and so belongs to no capability + * @param summary what the gap is + * @return the deviation, ready to report + */ + public static KnownDeviation untracked(Capability capability, String summary) { + return new KnownDeviation(capability == null ? null : capability.tag(), null, summary); + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTck.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTck.java new file mode 100644 index 0000000000..ed97eb875a --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTck.java @@ -0,0 +1,98 @@ +package dev.openfeature.contrib.tools.providertck; + +/** + * The values {@link AbstractProviderTckTest} configures Cucumber with, as compile-time constants. + * + *

Every one of these is already implied by the suite's annotations. They are named here because + * an annotation value has to be a compile-time constant, so an adopter who adds a + * {@code @ConfigurationParameter} of their own cannot compute one — they would otherwise have to + * restate our package name, our resource directory or our object factory as a string literal, and a + * literal copy of someone else's configuration is a copy that goes stale without anything failing. + * Annotation values permit constant concatenation, so {@code ProviderTck.ALL_GLUE + ",com.vendor.steps"} + * is legal where a method call is not. + * + *

A class of its own rather than constants on the suite. The values describe the TCK's classpath + * conventions rather than the behaviour of a suite, and things that are not suites read them: a build + * check, a custom launcher, a test that asserts the extension point still works. Putting them on + * {@link AbstractProviderTckTest} would also inherit the whole namespace into every adopter's suite + * class, where {@code GLUE} would show up as a member of their own type. + * + *

Nothing here is a setting. Changing what the suite passes to Cucumber means changing the + * annotations on {@link AbstractProviderTckTest}; these constants follow that, they do not drive it. + */ +public final class ProviderTck { + + /** + * Classpath directory holding the canonical feature files, packaged inside this JAR. + * + *

Reserved for the conformance suite. A feature file an adopter adds here does not extend the + * canonical set, and one that collides with a canonical file name silently replaces it — see + * {@link #EXTENSIONS}. + */ + public static final String FEATURES = "features"; + + /** + * Classpath directory an adopter puts their own feature files in. + * + *

Deliberately not a subdirectory of {@link #FEATURES}, and deliberately a different name. + * The same directory name in two classpath roots is scanned additively, but the same directory + * and file name is not: one root wins and the other file is never read, with no warning. + * An adopter who dropped {@code features/errors.feature} beside ours would therefore replace a + * canonical file with their own and watch the suite go green having run theirs — the worst + * outcome available to a conformance suite. A separate directory makes that collision impossible + * to reach by accident. + * + *

Shipped in this JAR containing only a README, because + * {@link org.junit.platform.suite.api.SelectClasspathResource} on a resource that exists nowhere + * on the classpath is a discovery error rather than an empty selection. Cucumber ignores files + * that are not {@code .feature}, so the README costs nothing. + */ + public static final String EXTENSIONS = "tck-extensions"; + + /** Package holding the canonical step definitions. */ + public static final String GLUE = "dev.openfeature.contrib.tools.providertck.steps"; + + /** + * Package an adopter puts their own step definitions in. + * + *

Outside this artifact's package namespace on purpose: it is the adopter's package, not + * ours, and it lives in their source tree. A glue package that does not exist is tolerated + * silently by Cucumber, so an adopter who writes no extensions pays nothing for it being on the + * glue path. + */ + public static final String EXTENSION_GLUE = "openfeature.tck.extensions"; + + /** + * The glue path the suite runs with: the canonical steps and the extension package. + * + *

Concatenate to add more, as {@code ProviderTck.ALL_GLUE + ",com.vendor.steps"}. Note that + * an adopter adding a package this way has to keep the canonical one, or every canonical step + * becomes undefined. + */ + public static final String ALL_GLUE = GLUE + "," + EXTENSION_GLUE; + + /** + * The Cucumber plugins the suite registers. + * + *

Spelled out rather than derived from {@code ConformanceReportPlugin.class.getName()}, + * which is not a compile-time constant and so cannot appear in an annotation value. + */ + public static final String PLUGINS = "summary,dev.openfeature.contrib.tools.providertck.ConformanceReportPlugin"; + + /** + * Whether scenarios may run in parallel: never. + * + *

Control API state is global to the Compose stack, so concurrent scenarios corrupt each + * other. The suite pins this where it overrides a consuming module's + * {@code junit-platform.properties}. + */ + public static final String PARALLEL_EXECUTION_ENABLED = "false"; + + /** Execution mode for the scenarios of one feature, matching {@link #PARALLEL_EXECUTION_ENABLED}. */ + public static final String FEATURE_EXECUTION_MODE = "same_thread"; + + /** Object factory that injects {@link TckState} into every step class. */ + public static final String OBJECT_FACTORY = "io.cucumber.picocontainer.PicoFactory"; + + private ProviderTck() {} +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTckHarness.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTckHarness.java index 1cc671faf5..641443ce1f 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTckHarness.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ProviderTckHarness.java @@ -4,7 +4,6 @@ import java.io.File; import java.time.Duration; import java.util.Collections; -import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -116,13 +115,56 @@ public interface ProviderTckHarness { *

Scenarios tagged with a capability that is not in this set are reported as * skipped. They are never silently passed. * - *

Defaults to every capability. Narrow it rather than widening it: start from the default, - * run the suite, and remove only what your provider genuinely cannot do. + *

Defaults to every {@linkplain Capability#declarable() declarable} capability. Narrow it + * rather than widening it: start from the default, run the suite, and remove only what your + * provider genuinely cannot do — {@link Capability#declarableExcept} is the idiomatic way to say + * "everything except". + * + *

Do not build the set with {@code EnumSet.allOf} or {@code EnumSet.complementOf}. Both + * include the {@linkplain Capability#reserved() reserved} capabilities, which no scenario + * carries, and declaring one of those fails the run. * * @return the capabilities this provider supports */ default Set capabilities() { - return EnumSet.allOf(Capability.class); + return Capability.declarable(); + } + + /** + * Declares gaps this provider is known to have against parts of the contract the specification + * does not treat as optional. + * + *

Reported in the conformance report so that a consumer can tell a design decision from a + * defect. Withholding a capability and having a bug look identical in the results — scenarios + * skipped, either way — and the TCK cannot tell them apart from the outside. Only the provider + * author can, so only the provider author can say. + * + *

Empty by default, which is silence rather than a claim. Declare an entry when you have + * narrowed {@link #capabilities()} to work around a defect rather than to describe a limitation, + * and delete it when the defect is fixed. + * + * @return the deviations this provider acknowledges, empty by default + */ + default List knownDeviations() { + return Collections.emptyList(); + } + + /** + * Returns the name of the provider configuration this suite exercises. + * + *

Used to name the conformance report — {@code

/.json} and its + * {@code .ndjson} results — and reported in it as the provider's configuration rather than its + * identity. The identity is what the provider + * says through its own metadata; this is which of its modes was tested, and a provider with two + * materially different modes produces two reports that are not interchangeable. + * + *

Derived from the suite class name by default: {@code FlagdInProcessTckTest} becomes + * {@code flagd-in-process}. Override it when that does not read well. + * + * @return a short name for this configuration + */ + default String configuration() { + return ReportNames.configurationOf(getClass()); } /** diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ReportNames.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ReportNames.java new file mode 100644 index 0000000000..a4fde8f7b5 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/ReportNames.java @@ -0,0 +1,87 @@ +package dev.openfeature.contrib.tools.providertck; + +import java.util.Locale; + +/** + * Derives the names a conformance report is identified and filed under. + * + *

Kept apart from the report itself so that both the default + * {@link ProviderTckHarness#configuration()} and the files the plugin writes agree on one derivation. + */ +final class ReportNames { + + /** Suffixes a suite class name carries for JUnit's benefit rather than the report's. */ + private static final String[] SUITE_SUFFIXES = {"TckTest", "TCKTest", "TckSuite", "Test", "IT"}; + + /** Used when a name sanitises away to nothing, which an anonymous class manages. */ + private static final String FALLBACK = "provider-tck"; + + private ReportNames() {} + + /** + * Derives a configuration name from a suite class. + * + *

{@code FlagdInProcessTckTest} becomes {@code flagd-in-process}: the suffix that exists only + * so JUnit picks the class up is dropped, and the rest is hyphenated. A provider whose modes do + * not read well this way overrides {@link ProviderTckHarness#configuration()} and says so + * directly. + * + * @param suite the concrete suite class + * @return a hyphenated, lower-case configuration name + */ + static String configurationOf(Class suite) { + String simple = suite.getSimpleName(); + for (String suffix : SUITE_SUFFIXES) { + if (simple.length() > suffix.length() && simple.endsWith(suffix)) { + simple = simple.substring(0, simple.length() - suffix.length()); + break; + } + } + String hyphenated = simple.replaceAll("([a-z0-9])([A-Z])", "$1-$2") + .replaceAll("([A-Z]+)([A-Z][a-z])", "$1-$2") + .toLowerCase(Locale.ROOT); + return hyphenated.isEmpty() ? FALLBACK : hyphenated; + } + + /** + * Turns a configuration name into the base name both of a run's files share. + * + *

A run writes an envelope and a results stream, and they are matched by name — the envelope's + * {@code results.location} is this base name plus the stream's extension — so one derivation + * produces both. + * + *

Configuration names are chosen to read well in a failure message rather than to be + * path-safe, so anything that is not obviously safe becomes a hyphen. Without this a + * configuration named {@code flagd/rpc} would silently write outside the directory it was given. + * + * @param configuration the configuration name + * @return a file name stem, with no extension + */ + static String baseNameOf(String configuration) { + StringBuilder safe = new StringBuilder(configuration.length()); + for (int i = 0; i < configuration.length(); i++) { + char c = configuration.charAt(i); + boolean allowed = (c >= 'a' && c <= 'z') + || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') + || c == '-' + || c == '_' + || c == '.'; + safe.append(allowed ? c : '-'); + } + String trimmed = trim(safe.toString()); + return trimmed.isEmpty() ? FALLBACK : trimmed; + } + + private static String trim(String value) { + int start = 0; + int end = value.length(); + while (start < end && (value.charAt(start) == '-' || value.charAt(start) == '.')) { + start++; + } + while (end > start && (value.charAt(end - 1) == '-' || value.charAt(end - 1) == '.')) { + end--; + } + return value.substring(start, end); + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckBuildInfo.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckBuildInfo.java new file mode 100644 index 0000000000..4b69b494bc --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckBuildInfo.java @@ -0,0 +1,124 @@ +package dev.openfeature.contrib.tools.providertck; + +import dev.openfeature.sdk.OpenFeatureAPI; +import java.io.IOException; +import java.io.InputStream; +import java.util.Properties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * What this TCK build is, and which conformance artifacts it carries. + * + *

A conformance report has to say which questions were asked, not only what the answers were, and + * neither fact is available from the running code. The spec revision the packaged Gherkin came from + * is a property of the build — the artifacts are in the JAR, the repository they came from is not — + * so it is baked into a generated resource at build time and read back here. The SDK version is the + * opposite case: the TCK depends on a version range, so what a consumer actually ran + * against is only knowable at runtime. + * + *

Everything here degrades to {@code unknown} rather than throwing. A report that is slightly + * less identifiable is worth more than a test run that fails while tidying up after itself. + */ +final class TckBuildInfo { + + /** Which TCK implementation this is, in the form the report schema asks for. */ + static final String IMPLEMENTATION = "java-sdk-contrib/tools/provider-tck"; + + /** Maven coordinates of the SDK, reported without a version. */ + static final String SDK_NAME = "dev.openfeature:sdk"; + + /** Stands in for anything the build or the classpath could not tell us. */ + static final String UNKNOWN = "unknown"; + + private static final Logger log = LoggerFactory.getLogger(TckBuildInfo.class); + + /** Generated by the build from the module POM; see {@code src/main/resources-filtered}. */ + private static final String BUILD_PROPERTIES = "provider-tck-build.properties"; + + /** Written into every JAR by maven-archiver, and the most direct statement of what resolved. */ + private static final String SDK_POM_PROPERTIES = "META-INF/maven/dev.openfeature/sdk/pom.properties"; + + private static final Properties BUILD = loadBuildProperties(); + + private TckBuildInfo() {} + + /** Returns the version of this TCK module. */ + static String tckVersion() { + return property("tck.version"); + } + + /** Returns the open-feature/spec commit the packaged conformance artifacts came from. */ + static String specRevision() { + return property("spec.revision"); + } + + /** + * Returns the OpenFeature SDK version that was actually on the classpath. + * + *

Read rather than declared. The TCK depends on an SDK version range so that adopting it can + * never force an upgrade, which means the version is a property of the consumer's build; a + * hardcoded one would be a second place to be wrong. + */ + static String sdkVersion() { + Properties pom = load(SDK_POM_PROPERTIES); + if (pom != null) { + String version = pom.getProperty("version", "").trim(); + if (!version.isEmpty()) { + return version; + } + } + + // The Maven descriptor can be stripped, or the SDK can arrive from somewhere that is not a + // JAR at all — an IDE's output directory, say — so fall back to the manifest. + Package sdkPackage = OpenFeatureAPI.class.getPackage(); + String implementation = sdkPackage == null ? null : sdkPackage.getImplementationVersion(); + if (implementation != null && !implementation.trim().isEmpty()) { + return implementation.trim(); + } + + log.warn( + "Could not determine the OpenFeature SDK version from {} or from the package manifest; " + + "the conformance report will say '{}'", + SDK_POM_PROPERTIES, + UNKNOWN); + return UNKNOWN; + } + + private static String property(String key) { + String value = BUILD.getProperty(key, "").trim(); + return value.isEmpty() ? UNKNOWN : value; + } + + private static Properties loadBuildProperties() { + Properties properties = load(packagePath() + BUILD_PROPERTIES); + if (properties == null) { + log.warn( + "{} is missing from the TCK JAR, so a conformance report cannot identify the build " + + "that produced it. This means the resource filtering configured in the " + + "provider-tck POM did not run.", + BUILD_PROPERTIES); + return new Properties(); + } + return properties; + } + + private static String packagePath() { + return TckBuildInfo.class.getPackage().getName().replace('.', '/') + "/"; + } + + private static Properties load(String resource) { + ClassLoader loader = TckBuildInfo.class.getClassLoader(); + try (InputStream in = loader.getResourceAsStream(resource)) { + if (in == null) { + return null; + } + Properties properties = new Properties(); + properties.load(in); + return properties; + } catch (IOException e) { + log.warn("Could not read {} from the classpath", resource, e); + return null; + } + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRunMetadata.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRunMetadata.java new file mode 100644 index 0000000000..0f967e69d4 --- /dev/null +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRunMetadata.java @@ -0,0 +1,101 @@ +package dev.openfeature.contrib.tools.providertck; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +/** + * What a conformance report needs to know about the run, gathered as the run proceeds. + * + *

It exists because the two facts a report needs most are known at different times and are gone + * by the time it is written. The declared capabilities and the backend description come from the + * harness when the suite starts; the provider's own metadata name is only observable once a scenario + * has built a provider. Meanwhile Cucumber emits {@code TestRunFinished} — where the file is written + * — after {@code @AfterAll} has already torn the {@link TckRuntime} down. + * + *

So the runtime fills this in as it goes and keeps it after {@link TckRuntime#stop()}, and the + * report plugin reads it at the end. + * + *

It is also the one place every path to a report passes through, which makes it where the + * declaration is checked: a {@linkplain Capability#reserved() reserved} capability is rejected here + * rather than being carried into a document that would then claim it. + */ +final class TckRunMetadata { + + private final String configuration; + private final Set capabilities; + private final String backendDescription; + private final String controlApi; + private final List knownDeviations; + + private volatile String providerName; + + TckRunMetadata( + String configuration, + Set capabilities, + String backendDescription, + String controlApi, + List knownDeviations) { + Capability.requireDeclarable(capabilities); + this.configuration = configuration; + this.capabilities = capabilities.isEmpty() + ? Collections.emptySet() + : Collections.unmodifiableSet(EnumSet.copyOf(capabilities)); + this.backendDescription = backendDescription; + this.controlApi = controlApi; + this.knownDeviations = Collections.unmodifiableList(new ArrayList<>(knownDeviations)); + } + + /** Returns the suite name, which is the provider configuration under test. */ + String configuration() { + return configuration; + } + + /** Returns the capabilities the harness declared. */ + Set capabilities() { + return capabilities; + } + + /** Returns a short description of the backend stack, or empty when there is none. */ + Optional backendDescription() { + return Optional.ofNullable(backendDescription); + } + + /** Returns how the backend was driven, or empty when there is no control API. */ + Optional controlApi() { + return Optional.ofNullable(controlApi); + } + + /** Returns the deviations the provider author acknowledged, empty when there are none. */ + List knownDeviations() { + return knownDeviations; + } + + /** + * Records what the provider called itself, as observed from a scenario that built one. + * + * @param name the provider's own metadata name + */ + void recordProviderName(String name) { + if (name != null && !name.trim().isEmpty()) { + providerName = name; + } + } + + /** + * Returns the provider's own metadata name, falling back to the configuration name. + * + *

The fallback covers a run in which no scenario ever built a provider — every scenario + * skipped, or the stack never came up. Reporting the suite name there is more useful than the + * empty string the schema would reject. + * + * @return the name to report as the provider's identity + */ + String providerName() { + String observed = providerName; + return observed == null ? configuration : observed; + } +} diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRuntime.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRuntime.java index 82da5aa11d..10542385b5 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRuntime.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckRuntime.java @@ -41,10 +41,19 @@ public final class TckRuntime { private static TckRuntime instance; + /** + * What the most recent suite was, kept after {@link #stop()} for the conformance report. + * + *

Cucumber emits the end-of-run event that writes the report after {@code @AfterAll} + * has stopped the runtime, so the report would otherwise have nothing left to describe. + */ + private static volatile TckRunMetadata lastRunMetadata; + private final ProviderTckHarness harness; private final ComposeContainer compose; private final ControlApiClient controlApi; private final BackendEndpoint endpoint; + private final TckRunMetadata metadata; private TckRuntime(ProviderTckHarness harness, ComposeContainer compose) { this.harness = harness; @@ -53,6 +62,26 @@ private TckRuntime(ProviderTckHarness harness, ComposeContainer compose) { String baseUrl = "http://" + compose.getServiceHost(harness.backendService(), null) + ":" + compose.getServicePort(harness.backendService(), harness.controlPort()); this.controlApi = new ControlApiClient(baseUrl, harness.settleTime()); + this.metadata = new TckRunMetadata( + harness.configuration(), + harness.capabilities(), + "Docker Compose stack " + harness.composeFile().getName() + ", service " + harness.backendService(), + controlApi.controlApi(), + harness.knownDeviations()); + recordRun(this.metadata); + } + + /** + * Records what the current run is, for the conformance report to describe afterwards. + * + *

A method rather than a field assignment because it is also the seam the report tests use: + * they drive a real Cucumber run to check what the results stream says, and that needs a run to + * describe without needing a Compose stack to describe it. + * + * @param metadata what the run is, or {@code null} to forget the last one + */ + static void recordRun(TckRunMetadata metadata) { + lastRunMetadata = metadata; } /** @@ -64,6 +93,10 @@ public static synchronized TckRuntime startIfNeeded() { if (instance == null) { ProviderTckHarness harness = discoverHarness(); log.info("Provider TCK harness: {}", harness.getClass().getName()); + // Checked before the Compose stack goes up, rather than only where the declaration + // reaches the report: an adopter should not wait for Docker to be told about a + // one-line mistake in capabilities(). + Capability.requireDeclarable(harness.capabilities()); instance = new TckRuntime(harness, startCompose(harness)); instance.controlApi.awaitReady(harness.startupTimeout()); log.info("Control API ready at {}", instance.controlApi.baseUrl()); @@ -121,6 +154,27 @@ public BackendEndpoint endpoint() { return endpoint; } + /** + * Records what the provider under test calls itself. + * + *

A conformance report identifies the provider by its own metadata name rather than by the + * suite's, and only a scenario that has built one can say what that is. + * + * @param name the provider's metadata name + */ + public void recordProviderName(String name) { + metadata.recordProviderName(name); + } + + /** + * Returns what was observed about the most recent suite, for the conformance report. + * + * @return the metadata of the last suite to start, or empty if none has + */ + static Optional lastRunMetadata() { + return Optional.ofNullable(lastRunMetadata); + } + private static ComposeContainer startCompose(ProviderTckHarness harness) { File composeFile = harness.composeFile(); if (!composeFile.isFile()) { diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckSuiteListener.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckSuiteListener.java index 31251ed3fc..b2a40e0474 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckSuiteListener.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/TckSuiteListener.java @@ -6,6 +6,7 @@ import org.junit.platform.engine.support.descriptor.ClassSource; import org.junit.platform.launcher.TestExecutionListener; import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,6 +36,21 @@ public class TckSuiteListener implements TestExecutionListener { private static volatile Class current; + /** + * Records what each TCK suite in the plan is about to run, for {@link CanonicalScenarioGuard}. + * + *

Read from the plan before any of it executes, which is as early as the selected scenarios + * can be known: selectors and glue are resolved by then, so a canonical file that was shadowed, + * replaced or added to is already visible. The guard needs nothing from the run itself and no + * Compose stack. + * + * @param testPlan the plan about to be executed + */ + @Override + public void testPlanExecutionStarted(TestPlan testPlan) { + CanonicalScenarioGuard.observe(testPlan); + } + @Override public void executionStarted(TestIdentifier testIdentifier) { harnessClassOf(testIdentifier).ifPresent(suite -> { @@ -62,7 +78,13 @@ static Optional> currentSuite() { return Optional.ofNullable(current); } - private static Optional> harnessClassOf(TestIdentifier testIdentifier) { + /** + * Returns the TCK suite class a test identifier stands for, if it is one. + * + * @param testIdentifier the identifier to inspect + * @return the concrete suite class, or empty when the identifier is not a TCK suite + */ + static Optional> harnessClassOf(TestIdentifier testIdentifier) { return testIdentifier .getSource() .filter(ClassSource.class::isInstance) diff --git a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/steps/ProviderSteps.java b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/steps/ProviderSteps.java index 8fe2394143..06a4f0c65b 100644 --- a/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/steps/ProviderSteps.java +++ b/tools/provider-tck/src/main/java/dev/openfeature/contrib/tools/providertck/steps/ProviderSteps.java @@ -3,7 +3,7 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.awaitility.Awaitility.await; -import dev.openfeature.contrib.tools.providertck.Capability; +import dev.openfeature.contrib.tools.providertck.CapabilityGate; import dev.openfeature.contrib.tools.providertck.ProviderTckHarness; import dev.openfeature.contrib.tools.providertck.TckRuntime; import dev.openfeature.contrib.tools.providertck.TckState; @@ -19,10 +19,7 @@ import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; -import java.util.Optional; -import java.util.Set; import java.util.UUID; -import org.opentest4j.TestAbortedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,14 +67,7 @@ public static void afterAll() { */ @Before(order = 0) public void gateOnCapabilities(Scenario scenario) { - Set supported = harness().capabilities(); - for (String tag : scenario.getSourceTagNames()) { - Optional capability = Capability.fromTag(tag); - if (capability.isPresent() && !supported.contains(capability.get())) { - throw new TestAbortedException("Skipped: provider does not declare capability " - + capability.get().name() + " (tag " + tag + "). Declared capabilities: " + supported); - } - } + CapabilityGate.requireDeclared(scenario.getSourceTagNames(), harness().capabilities()); } /** @@ -149,11 +139,13 @@ public void createProvider(String flavour) { state.provider = provider; state.domain = domain; state.client = api.getClient(domain); - log.info( - "Registered {} provider {} under domain {}", - flavour, - provider.getMetadata().getName(), - domain); + + // A conformance report identifies the provider by what it calls itself, not by the suite + // name, and this is the only place that knows it. + String providerName = provider.getMetadata().getName(); + runtime().recordProviderName(providerName); + + log.info("Registered {} provider {} under domain {}", flavour, providerName, domain); } /** diff --git a/tools/provider-tck/src/main/resources-filtered/dev/openfeature/contrib/tools/providertck/provider-tck-build.properties b/tools/provider-tck/src/main/resources-filtered/dev/openfeature/contrib/tools/providertck/provider-tck-build.properties new file mode 100644 index 0000000000..72b965a640 --- /dev/null +++ b/tools/provider-tck/src/main/resources-filtered/dev/openfeature/contrib/tools/providertck/provider-tck-build.properties @@ -0,0 +1,7 @@ +# Generated at build time by Maven resource filtering. Do not edit the copy in target/. +# +# Identifies the TCK build and the conformance artifacts it carries, for the +# tck section of a conformance report. The values come from the provider-tck +# POM; see the comment there for how the spec revision is maintained. +tck.version=${project.version} +spec.revision=${provider-tck.spec.revision} diff --git a/tools/provider-tck/src/main/resources/features/errors.feature b/tools/provider-tck/src/main/resources/features/errors.feature index 0346df3da1..0efbe57261 100644 --- a/tools/provider-tck/src/main/resources/features/errors.feature +++ b/tools/provider-tck/src/main/resources/features/errors.feature @@ -14,7 +14,7 @@ Feature: Provider error handling Scenario Outline: Requesting the wrong type returns the code default # The full non-numeric mismatch matrix. Numeric coercion is a separate question and is covered - # by the @strict-numeric-typing scenarios below, because "is 0.5 an integer?" has a defensible + # by the @numeric-coercion scenarios below, because "is 0.5 an integer?" has a defensible # wrong answer whereas "is a string a boolean?" does not. Given a -flag with key "" and a default value "" When the flag was evaluated with details @@ -59,10 +59,16 @@ Feature: Provider error handling | Integer | 1 | | Float | 0.1 | - @strict-numeric-typing + @numeric-coercion Scenario: A float flag is not silently narrowed to an integer # 'float-flag' resolves to 0.5. Narrowing that to an integer would lose information # silently, so it must be reported as a type mismatch rather than rounded. + # + # This is the lossy half of the coercion contract. The lossless half -- that an + # integral float such as 10.0 requested as an integer MUST succeed -- has no scenario + # yet, because the canonical flag set has no integral float to ask it of. Adding one + # is a change to the flag set and so to every language at once; see the tag's entry in + # Appendix F. Given a Integer-flag with key "float-flag" and a default value "1" When the flag was evaluated with details Then the resolved details value should be "1" diff --git a/tools/provider-tck/src/main/resources/features/lifecycle.feature b/tools/provider-tck/src/main/resources/features/lifecycle.feature index 338b2c0524..3e168565ab 100644 --- a/tools/provider-tck/src/main/resources/features/lifecycle.feature +++ b/tools/provider-tck/src/main/resources/features/lifecycle.feature @@ -15,7 +15,7 @@ Feature: Provider lifecycle # of provider registration, takes the host application down with it — so the requirement is # not merely that initialisation fails, but that it fails observably and promptly. - Scenario: A provider reaching its backend becomes ready + Scenario: A provider that successfully initializes becomes ready Given a stable provider And a ready event handler Then the ready event handler should have been executed diff --git a/tools/provider-tck/src/main/resources/openapi/control-api.yaml b/tools/provider-tck/src/main/resources/openapi/control-api.yaml index fd9bc7000d..d21191067d 100644 --- a/tools/provider-tck/src/main/resources/openapi/control-api.yaml +++ b/tools/provider-tck/src/main/resources/openapi/control-api.yaml @@ -158,6 +158,26 @@ paths: already running is not an error: the implementation restarts the process (or otherwise ensures it is running) with the requested configuration. + **MUST NOT return until the seeded flag state is actually being served.** + A 200 is a promise that the very next evaluation will resolve against the + new baseline. Returning as soon as the process reports healthy is not + enough: a backend can accept connections and answer a readiness probe + while its flag store is still empty, and an evaluation in that window + gets `FLAG_NOT_FOUND` for a flag the configuration plainly defines. + + This is easy to get wrong and easy to miss. A provider that blocks during + initialisation -- streaming, or syncing a ruleset -- absorbs the window + and never sees it. A **stateless** provider, which evaluates over HTTP + with no initialisation at all, has nothing to hide it behind and fails + essentially every scenario, which reads as a catastrophically broken + provider rather than as a racing testbed. The reference implementation + exhibits this: its `/start` returns roughly 40ms before flagd's file + sources reach the flag store. + + A TCK MAY defensively probe after `/start`, but it should not have to, + and requiring every stateless adopter to reimplement that probe is worse + than stating the requirement here. + Because this operation resets flag state, the TCK uses it as its default scenario-isolation mechanism when `/reset` is not implemented. diff --git a/tools/provider-tck/src/main/resources/tck-extensions/README.md b/tools/provider-tck/src/main/resources/tck-extensions/README.md new file mode 100644 index 0000000000..957870544c --- /dev/null +++ b/tools/provider-tck/src/main/resources/tck-extensions/README.md @@ -0,0 +1,43 @@ +# Provider TCK extension point + +Feature files placed on the classpath under `tck-extensions/` run inside the TCK suite, alongside +the canonical conformance scenarios. + +This file is here so that the directory exists on the classpath even when nobody has extended +anything. `AbstractProviderTckTest` selects `tck-extensions` unconditionally, and a classpath +resource selector naming a resource that exists on no classpath root is a discovery error rather +than an empty selection. Cucumber ignores files that are not `.feature`, so the README itself is +never read as a scenario. + +## Adding scenarios + +Write nothing but the two files: + +``` +src/test/resources/tck-extensions/fractional.feature +src/test/java/openfeature/tck/extensions/FractionalSteps.java // package openfeature.tck.extensions +``` + +No annotations, no second suite, no runner configuration. The scenarios are discovered by the same +suite as the canonical set, so they share its backend lifecycle: one Compose stack, one +`@BeforeAll`, the same control API, the same conformance report. + +Your step classes may take `dev.openfeature.contrib.tools.providertck.TckState` as a constructor +argument to reach the client and the last evaluation, exactly as the canonical steps do, and +`TckRuntime.get()` for the control API and the backend endpoint. + +## Why this directory rather than `features/` + +Two classpath roots containing the same directory are scanned additively. Two containing the same +directory *and* the same file name are not: one silently wins. A feature file added to `features/` +under a canonical name would therefore replace a canonical file, and the suite would report success +having run the replacement. The extension directory has a different name so that collision cannot +be reached by accident. + +`features/` is the canonical set and belongs to the specification. Extensions are yours. + +## What extensions are not + +An extension scenario is not conformance. It does not appear in the canonical set, it cannot make +the canonical set smaller, and a conformance report is not a claim about it. If a scenario is +portable across providers it belongs in `features/` — send it to the TCK. diff --git a/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuardTest.java b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuardTest.java new file mode 100644 index 0000000000..fff4721e52 --- /dev/null +++ b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/CanonicalScenarioGuardTest.java @@ -0,0 +1,229 @@ +package dev.openfeature.contrib.tools.providertck; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.cucumber.junit.platform.engine.Constants; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.EngineFilter; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; +import org.junit.platform.launcher.listeners.SummaryGeneratingListener; +import org.junit.platform.launcher.listeners.TestExecutionSummary; + +/** + * A run that quietly asks fewer questions must not be able to report success. + * + *

Two levels here, deliberately. The comparison itself is checked directly, because a failure + * message is the whole product of a guard and it has to name the scenario that went missing. The + * wiring is checked by running the guard through the JUnit Platform under a real filter + * configuration, because "the guard sees what the Cucumber engine beside it sees" is a claim about + * the platform rather than about this code. + * + *

No scenario is executed anywhere in this file. The guard works from the discovered plan and the + * run's configuration, which is what lets it be checked — and, in a real build, fail — without a + * backend. + */ +class CanonicalScenarioGuardTest { + + private static final Map UNFILTERED = Collections.emptyMap(); + + @BeforeEach + void forgetPreviousPlans() { + CanonicalScenarioGuard.forget(); + } + + @Test + @DisplayName("the canonical set is read from this artifact, outline row by outline row") + void theCanonicalSetIsReadFromTheArtifact() { + Set canonical = CanonicalScenarios.shipped(); + + assertThat(canonical).isNotEmpty(); + assertThat(canonical) + .as("every canonical scenario comes from a packaged feature file") + .allSatisfy(ref -> assertThat(ref.toString()).startsWith(ProviderTck.FEATURES + "/")); + + // The eleven rows of the type-mismatch matrix share one name, so a set that counted scenarios + // by name would see one of them. Each Examples row is its own line and its own entry. + assertThat(canonical) + .filteredOn(ref -> ref.toString().contains("Requesting the wrong type returns the code default")) + .hasSize(11); + } + + @Test + @DisplayName("a suite that selects the whole canonical set, unfiltered, passes") + void anIntactSuitePasses() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + assertThat(CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), CanonicalScenarioGuard.discovered(), UNFILTERED)) + .isNull(); + } + + @Test + @DisplayName("extension scenarios neither count towards the canonical set nor disturb it") + void extensionScenariosAreIgnored() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + Set observed = + CanonicalScenarioGuard.discovered().get(TckSuiteFixture.class.getName()); + + // The fixture suite does select an extension feature — ExtensionPointTest checks that it does + // — and none of it reaches the guard. + assertThat(observed) + .as("the guard is defined over %s/ alone", ProviderTck.FEATURES) + .isEqualTo(CanonicalScenarios.shipped()) + .allSatisfy(ref -> assertThat(ref.toString()).doesNotContain(ProviderTck.EXTENSIONS + "/")); + } + + @Test + @DisplayName("a scenario filter fails the run, whatever it would have excluded") + void aScenarioFilterFails() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + CanonicalScenarioGuard.discovered(), + Collections.singletonMap(Constants.FILTER_TAGS_PROPERTY_NAME, "not @events")); + + // Cucumber applies a tag filter by skipping scenarios during execution, so the plan still + // contains them and only the configured expression shows what will be left out. The guard + // therefore rejects the filter itself rather than trying to predict what it matches. + assertThat(problems) + .isNotNull() + .contains(Constants.FILTER_TAGS_PROPERTY_NAME) + .contains("not @events") + .as("and it points at the mechanism that legitimately narrows a run") + .contains("capabilities()"); + } + + @Test + @DisplayName("the guard reads the filter the Cucumber engine beside it would read") + void theGuardReadsTheRunsFilterConfiguration() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + TestExecutionSummary filtered = runGuard(Constants.FILTER_TAGS_PROPERTY_NAME, "@events"); + + assertThat(filtered.getTestsFailedCount()) + .as("a filtered run fails at the guard, before a Compose stack is worth starting") + .isEqualTo(1); + assertThat(filtered.getFailures().get(0).getException()) + .hasMessageContaining(Constants.FILTER_TAGS_PROPERTY_NAME); + } + + @Test + @DisplayName("an unfiltered run of an intact suite passes the guard as executed") + void anIntactRunPassesTheGuard() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + assertThat(runGuard(null, null).getTestsSucceededCount()).isEqualTo(1); + } + + @Test + @DisplayName("a run that declares itself partial is skipped rather than passed") + void aPartialRunIsSkipped() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + String previous = System.getProperty(CanonicalScenarioGuard.PARTIAL_PROPERTY); + System.setProperty(CanonicalScenarioGuard.PARTIAL_PROPERTY, "true"); + try { + TestExecutionSummary summary = runGuard(Constants.FILTER_TAGS_PROPERTY_NAME, "@events"); + + // Aborted rather than skipped: the test ran and declined to conclude, which is what an + // assumption produces and what Surefire and the JUnit reports show as skipped. + assertThat(summary.getTestsAbortedCount()) + .as("a skip says the canonical set was not verified; a pass would claim it was") + .isEqualTo(1); + assertThat(summary.getTestsSucceededCount()).isZero(); + assertThat(summary.getTestsFailedCount()).isZero(); + } finally { + restore(CanonicalScenarioGuard.PARTIAL_PROPERTY, previous); + } + } + + @Test + @DisplayName("a feature file added to the canonical directory fails the run") + void anAddedCanonicalFeatureFails() { + Set withExtra = new LinkedHashSet<>(CanonicalScenarios.shipped()); + withExtra.add(new CanonicalScenarios.Ref(ProviderTck.FEATURES + "/vendor.feature", 7, "A vendor scenario")); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + Collections.singletonMap(TckSuiteFixture.class.getName(), withExtra), + UNFILTERED); + + assertThat(problems) + .isNotNull() + .contains(ProviderTck.FEATURES + "/vendor.feature") + .as("and it says where the scenario should have gone") + .contains(ProviderTck.EXTENSIONS + "/"); + } + + @Test + @DisplayName("a canonical file replaced by another of the same name fails the run") + void aShadowedCanonicalFileFails() { + // What the separate extension directory exists to prevent, expressed as what the guard sees + // if it happens anyway: the replacement compiles to different scenarios, so entries of the + // canonical set go missing. + Set shadowed = new LinkedHashSet<>(CanonicalScenarios.shipped()); + Iterator entries = shadowed.iterator(); + CanonicalScenarios.Ref removed = entries.next(); + entries.remove(); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + Collections.singletonMap(TckSuiteFixture.class.getName(), shadowed), + UNFILTERED); + + assertThat(problems).isNotNull().contains(removed.toString()).contains("shadowing"); + } + + @Test + @DisplayName("a plan with no TCK suite in it is a failure, not a pass") + void anUnobservedRunFails() { + Map> nothing = Collections.emptyMap(); + + assertThat(CanonicalScenarioGuard.check(CanonicalScenarios.shipped(), nothing, UNFILTERED)) + .as("an unverifiable conformance run is not a conformance run") + .isNotNull() + .contains("TckSuiteListener") + .contains(CanonicalScenarioGuard.PARTIAL_PROPERTY); + } + + /** Discovers the real suite configuration, without executing anything. */ + private static org.junit.platform.launcher.TestPlan discoverFixtureSuite() { + return LauncherFactory.create() + .discover(LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(TckSuiteFixture.class)) + .build()); + } + + /** Runs the guard itself through the JUnit Platform, optionally under a Cucumber filter. */ + private static TestExecutionSummary runGuard(String key, String value) { + LauncherDiscoveryRequestBuilder request = LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(CanonicalScenarioGuard.class)) + .filters(EngineFilter.includeEngines("junit-jupiter")); + if (key != null) { + request.configurationParameter(key, value); + } + + SummaryGeneratingListener summary = new SummaryGeneratingListener(); + LauncherFactory.create().execute(request.build(), summary); + return summary.getSummary(); + } + + private static void restore(String key, String previous) { + if (previous == null) { + System.clearProperty(key); + } else { + System.setProperty(key, previous); + } + } +} diff --git a/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPluginTest.java b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPluginTest.java new file mode 100644 index 0000000000..864865b5fc --- /dev/null +++ b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ConformanceReportPluginTest.java @@ -0,0 +1,622 @@ +package dev.openfeature.contrib.tools.providertck; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.openfeature.contrib.tools.providertck.selftest.ReportSelfTestSteps; +import io.cucumber.junit.platform.engine.Constants; +import io.cucumber.plugin.event.EventHandler; +import io.cucumber.plugin.event.EventPublisher; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeMap; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.EngineFilter; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; + +/** + * The conformance report exists to make one rule checkable, so these tests check it. + * + *

Appendix F requires that a scenario skipped for an undeclared capability is reported as skipped + * with the reason and never as passed. The Go TCK shipped an emitter that broke the rule silently — + * its runner did not deliver the capability-skip signal to the after-hook, so every skipped scenario + * was recorded twice, once correctly and once as passed. + * + *

Since the results are now a Cucumber Messages stream rather than a format this project defines, + * the rule is a property of what Cucumber emits, and only a real Cucumber run can demonstrate it. + * These tests therefore execute a fixture suite through the JUnit Platform — same engine, same gate, + * same plugin — and read the stream back as a consumer would: outcome per scenario derived from the + * most severe step result, exactly as the {@code cucumber-query} helpers do it. + * + *

What is not asserted here is the serialisation of the stream. That is Cucumber's own + * {@code MessageFormatter}, and re-checking it would only be checking Cucumber. + */ +class ConformanceReportPluginTest { + + /** The fixture's {@code @stale} tag is deliberately absent, so two scenarios must be skipped. */ + private static final Set DECLARED = EnumSet.of(Capability.OBJECT, Capability.EVENTS); + + private static final String CONFIGURATION = "my-provider-rpc"; + + private static final String FEATURE_URI = "classpath:report-selftest/report.feature"; + + private static final String OUTLINE = "Requesting the wrong type returns the code default"; + + /** + * Severity order of {@code TestStepResultStatus}, least to most severe. + * + *

A {@code testCaseFinished} message carries no status: a scenario's outcome is the most + * severe result among its steps, hooks included. That is not incidental — it is the mechanism + * that makes a gated skip truthful, because the aborted {@code @Before} hook contributes a + * {@code SKIPPED} result that outranks every {@code PASSED} step it prevented from running. + */ + private static final List SEVERITY = + Arrays.asList("UNKNOWN", "PASSED", "SKIPPED", "PENDING", "UNDEFINED", "AMBIGUOUS", "FAILED"); + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @TempDir + static Path reportDir; + + private static JsonNode envelope; + private static byte[] resultsBytes; + private static Results results; + + @BeforeAll + static void runTheFixtureSuite() throws IOException { + runSuite(reportDir, DECLARED); + + Path envelopePath = reportDir.resolve(CONFIGURATION + ".json"); + Path resultsPath = reportDir.resolve(CONFIGURATION + ".ndjson"); + assertThat(envelopePath).exists(); + assertThat(resultsPath).exists(); + + envelope = MAPPER.readTree(Files.readAllBytes(envelopePath)); + resultsBytes = Files.readAllBytes(resultsPath); + results = Results.parse(new String(resultsBytes, StandardCharsets.UTF_8)); + } + + @Test + @DisplayName("every scenario appears exactly once, whatever happened to it") + void everyScenarioAppearsExactlyOnce() { + // Seven pickles: four plain scenarios and three outline rows. A report that quietly omitted + // the ones it did not run would satisfy every other rule here and still mislead, because a + // reader would have no way to know how many questions went unasked. + assertThat(results.pickles).hasSize(7); + + assertThat(results.pickleOfTestCase.values()) + .as("one test case per pickle, and no pickle executed twice") + .containsExactlyInAnyOrderElementsOf(results.pickles.keySet()); + + assertThat(results.testCaseOfStarted.values()) + .as("one execution per test case; a scenario counted twice is how Go's report went wrong") + .containsExactlyInAnyOrderElementsOf(results.pickleOfTestCase.keySet()); + + assertThat(results.finished) + .as("every started scenario also finished") + .containsExactlyInAnyOrderElementsOf(results.testCaseOfStarted.keySet()); + } + + @Test + @DisplayName("the outcome counts are what the fixture describes") + void theOutcomeCountsAreWhatTheFixtureDescribes() { + assertThat(results.outcomeCounts()) + .containsExactlyInAnyOrderEntriesOf(counts("PASSED", 4L, "FAILED", 1L, "SKIPPED", 2L)); + } + + @Test + @DisplayName("a scenario gated on an undeclared capability is skipped, never passed") + void aGatedScenarioIsSkipped() { + List gated = new ArrayList<>(); + for (Map.Entry pickle : results.pickles.entrySet()) { + if (needsSomethingUndeclared(results.tagsOf(pickle.getValue()))) { + gated.add(pickle.getKey()); + } + } + + assertThat(gated) + .as("the premise: the fixture has a gated plain scenario and a gated outline row") + .hasSize(2); + + for (String pickleId : gated) { + assertThat(results.outcomeOfPickle(pickleId)) + .as("pickle %s carries an undeclared capability tag", pickleId) + .isEqualTo("SKIPPED"); + } + + assertThat(results.outcomeCounts().get("SKIPPED")) + .as("nothing else was skipped, so a skip cannot hide an unrelated one") + .isEqualTo((long) gated.size()); + } + + @Test + @DisplayName("the gate's own reason survives into the stream") + void theGateReasonSurvives() { + // The declaration in the envelope is enough to work out *why* a scenario was skipped, but + // the reason the gate gave is carried too, on the hook result that produced the skip. + assertThat(results.messagesOfSkippedSteps()).isNotEmpty().allSatisfy(message -> assertThat(message) + .contains("does not declare capability") + .contains("STALE")); + } + + @Test + @DisplayName("a scenario's tags include one set on its Examples block alone") + void tagsIncludeExamplesBlockTags() { + List rows = results.picklesNamed(OUTLINE); + + assertThat(rows).hasSize(3); + assertThat(rows).as("the feature tag reaches every row").allSatisfy(row -> assertThat(results.tagsOf(row)) + .contains("@events")); + + // Gherkin allows a tag on an individual Examples block, so two rows of one outline can + // differ in whether the gate stops them. Both rows must appear, and only the tagged one + // may be skipped: a skip that took its siblings with it would be invisible in the totals. + List outcomes = new ArrayList<>(); + List stale = new ArrayList<>(); + for (JsonNode row : rows) { + outcomes.add(results.outcomeOfPickle(row.get("id").asText())); + stale.add(results.tagsOf(row).contains("@stale")); + } + assertThat(stale).containsExactly(false, false, true); + assertThat(outcomes).containsExactly("PASSED", "PASSED", "SKIPPED"); + } + + @Test + @DisplayName("each Scenario Outline row is identified by the Examples row it came from") + void outlineRowsAreDistinguishable() { + List rows = results.picklesNamed(OUTLINE); + + assertThat(rows) + .as("all three rows share one name, which is the premise of this test") + .extracting(row -> row.get("name").asText()) + .containsOnly(OUTLINE); + + // A pickle's astNodeIds are [scenario, table row] for an outline-derived scenario. The + // second is the row's identity, and it resolves in the gherkinDocument to the cells the row + // was compiled from. This is the mechanism the TCK used to reverse-engineer from a pickle's + // reported line number; the stream states it outright. + List rowIds = new ArrayList<>(); + List> cells = new ArrayList<>(); + for (JsonNode row : rows) { + JsonNode astNodeIds = row.get("astNodeIds"); + assertThat(astNodeIds).hasSize(2); + String rowId = astNodeIds.get(1).asText(); + rowIds.add(rowId); + cells.add(results.cellsOfTableRow(rowId)); + } + + assertThat(rowIds).doesNotHaveDuplicates(); + assertThat(cells) + .containsExactly( + Arrays.asList("string-flag", "Boolean"), + Arrays.asList("string-flag", "Integer"), + Arrays.asList("boolean-flag", "String")); + } + + @Test + @DisplayName("the stream carries the source of the feature that executed") + void theStreamCarriesTheExecutedSource() throws IOException { + assertThat(results.sources).containsOnlyKeys(FEATURE_URI); + + // Line endings are normalised on both sides: this repo is checked out with whatever the + // platform does, and the point of the assertion is that the text is the file's, not that + // Cucumber preserves CRLF. + byte[] onDisk = Files.readAllBytes(Paths.get("src/test/resources/report-selftest/report.feature")); + assertThat(results.sources.get(FEATURE_URI).replace("\r\n", "\n")) + .as("what ran, verbatim, rather than a claim about which revision it came from") + .isEqualTo(new String(onDisk, StandardCharsets.UTF_8).replace("\r\n", "\n")); + } + + @Test + @DisplayName("the envelope points at the results and covers them with a digest") + void theEnvelopePointsAtTheResults() throws NoSuchAlgorithmException { + JsonNode reference = envelope.get("results"); + + assertThat(reference.get("format").asText()).isEqualTo("cucumber-messages"); + assertThat(reference.get("location").asText()) + .as("a path relative to the envelope, so a published pair can be moved together") + .isEqualTo(CONFIGURATION + ".ndjson"); + assertThat(reference.get("digest").asText()).isEqualTo(sha256Of(resultsBytes)); + } + + @Test + @DisplayName("the envelope carries everything the schema requires and nothing it forbids") + void theEnvelopeCarriesWhatTheSchemaRequires() { + assertThat(envelope.get("schemaVersion").asText()).isEqualTo("1"); + assertThat(envelope.get("provider").get("name").asText()).isEqualTo("My Provider"); + assertThat(envelope.get("provider").get("language").asText()).isEqualTo("java"); + assertThat(envelope.get("provider").get("configuration").asText()).isEqualTo(CONFIGURATION); + assertThat(envelope.get("sdk").get("name").asText()).isEqualTo("dev.openfeature:sdk"); + assertThat(envelope.get("sdk").get("version").asText()).isNotEmpty(); + assertThat(envelope.get("tck").get("implementation").asText()).isEqualTo("java-sdk-contrib/tools/provider-tck"); + assertThat(envelope.get("tck").get("specRevision").asText()).hasSizeGreaterThanOrEqualTo(7); + assertThat(envelope.get("backend").get("controlApi").asText()).isEqualTo("http"); + + // The schema sets additionalProperties: false throughout, so anything the results payload + // now owns is a validation failure rather than harmless duplication. + List fields = new ArrayList<>(); + envelope.fieldNames().forEachRemaining(fields::add); + assertThat(fields).doesNotContain("scenarios", "capabilities"); + assertThat(envelope.get("tck").has("assetsTree")).isFalse(); + } + + @Test + @DisplayName("the declaration lists the capabilities the provider claims, in vocabulary order") + void theDeclarationListsWhatIsClaimed() { + JsonNode declared = envelope.get("declaration").get("declared"); + + // The declaration is an input to reading the results, not a summary of them: the stream says + // a scenario was skipped, and only this says whether the provider declines what it needed. + assertThat(declared) + .extracting(JsonNode::asText) + .containsExactly(Capability.EVENTS.tag(), Capability.OBJECT.tag()); + } + + @Test + @DisplayName("a reserved capability cannot reach the declaration, even when everything is claimed") + void aReservedCapabilityCannotReachTheDeclaration() { + // The provider that claims the most is the case that used to break the rule: "everything" + // spelt EnumSet.allOf, or "everything except X" spelt EnumSet.complementOf, collected the + // reserved tags along the way and published a claim about two capabilities no scenario + // examines. So this asks the maximal declaration for its report. + JsonNode declared = MAPPER.valueToTree(report(Capability.declarable())) + .get("declaration") + .get("declared"); + + List tags = new ArrayList<>(); + declared.forEach(tag -> tags.add(tag.asText())); + + assertThat(tags) + .as("a reserved tag gates nothing, so declaring it is a claim nothing can contradict") + .doesNotContain(Capability.TARGETING.tag(), Capability.CACHING.tag()); + assertThat(tags) + .as("and every capability some scenario does gate is still there") + .containsExactly( + Capability.LIFECYCLE.tag(), + Capability.EVENTS.tag(), + Capability.STALE.tag(), + Capability.CONFIGURATION_CHANGE.tag(), + Capability.OBJECT.tag(), + Capability.UNAVAILABLE_INIT.tag(), + Capability.NUMERIC_COERCION.tag()); + } + + @Test + @DisplayName("naming a reserved capability fails the run rather than being dropped quietly") + void namingAReservedCapabilityFailsTheRun() { + // Chosen over a warning: the declaration is the one part of the report no result can check, + // and a report is read long after the log it would have been warned in has gone. Nothing is + // lost by refusing, because no scenario carries the tag. + Set overclaimed = EnumSet.of(Capability.OBJECT, Capability.TARGETING); + + assertThatThrownBy(() -> metadata(overclaimed)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(Capability.TARGETING.tag()) + .hasMessageContaining("declarableExcept"); + } + + @Test + @DisplayName("\"everything except X\" means everything declarable except X") + void declarableExceptYieldsOnlyDeclarableCapabilities() { + assertThat(Capability.declarableExcept(Capability.STALE)) + .doesNotContain(Capability.STALE, Capability.TARGETING, Capability.CACHING) + .contains(Capability.OBJECT, Capability.NUMERIC_COERCION); + + // The counterpart it replaces, and why it had to be replaced. + assertThat(EnumSet.complementOf(EnumSet.of(Capability.STALE))) + .as("complementOf is the complement of the enum, not of the declarable vocabulary") + .contains(Capability.TARGETING, Capability.CACHING); + } + + @Test + @DisplayName("a withheld capability that is a defect is reported as a deviation") + void aDefectIsReportedAsADeviation() { + JsonNode deviations = envelope.get("knownDeviations"); + + assertThat(deviations).hasSize(1); + assertThat(deviations.get(0).get("capability").asText()).isEqualTo(Capability.NUMERIC_COERCION.tag()); + assertThat(deviations.get(0).get("summary").asText()).isNotEmpty(); + assertThat(deviations.get(0).has("issue")) + .as("the fixture's deviation is untracked, and the field is omitted rather than empty") + .isFalse(); + } + + @Test + @DisplayName("the SDK version is read from the classpath rather than declared") + void theSdkVersionIsRead() { + assertThat(TckBuildInfo.sdkVersion()) + .as("the OpenFeature SDK is on the test classpath, so its version must be discoverable") + .isNotEqualTo(TckBuildInfo.UNKNOWN) + .matches("\\d+\\.\\d+.*"); + } + + @Test + @DisplayName("nothing is collected or written when no report directory is configured") + void nothingIsWrittenByDefault(@TempDir Path dir) throws IOException { + CountingEventPublisher publisher = new CountingEventPublisher(); + new ConformanceReportPlugin(() -> null, () -> Optional.of(metadata(DECLARED))).setEventPublisher(publisher); + + assertThat(publisher.registered) + .as("not even the message formatter is wired up, so an unasked-for run buffers nothing") + .isZero(); + try (Stream written = Files.list(dir)) { + assertThat(written).isEmpty(); + } + } + + @Test + @DisplayName("both files are named after the configuration, safely") + void theFilesAreNamedAfterTheConfiguration() { + assertThat(ReportNames.configurationOf(MyProviderRpcTckTest.class)).isEqualTo(CONFIGURATION); + assertThat(ReportNames.baseNameOf("flagd-rpc")).isEqualTo("flagd-rpc"); + assertThat(ReportNames.baseNameOf("flagd/rpc")) + .as("a configuration name is chosen to read well, not to be path-safe") + .isEqualTo("flagd-rpc"); + assertThat(ReportNames.baseNameOf("../escape")).isEqualTo("escape"); + } + + /** A suite whose name the default configuration derivation has to cope with. */ + private static final class MyProviderRpcTckTest {} + + /** + * Runs the fixture feature through the real Cucumber engine, with the real plugin registered. + * + *

Driven through the JUnit Platform rather than through Cucumber's CLI because that is how the + * TCK itself runs: {@link AbstractProviderTckTest} is a JUnit Platform Suite, and the plugin's + * position in the event stream is a property of that engine. + */ + private static void runSuite(Path dir, Set declared) { + ReportSelfTestSteps.declare(declared); + TckRuntime.recordRun(metadata(declared)); + String previous = System.getProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY); + System.setProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY, dir.toString()); + try { + LauncherFactory.create() + .execute(LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClasspathResource("report-selftest")) + .filters(EngineFilter.includeEngines("cucumber")) + .configurationParameter( + Constants.GLUE_PROPERTY_NAME, + ReportSelfTestSteps.class.getPackage().getName()) + .configurationParameter( + Constants.PLUGIN_PROPERTY_NAME, ConformanceReportPlugin.class.getName()) + .configurationParameter( + Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, + ProviderTck.PARALLEL_EXECUTION_ENABLED) + .configurationParameter(Constants.OBJECT_FACTORY_PROPERTY_NAME, ProviderTck.OBJECT_FACTORY) + .build()); + } finally { + if (previous == null) { + System.clearProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY); + } else { + System.setProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY, previous); + } + TckRuntime.recordRun(null); + } + } + + /** Builds the envelope a run with this declaration would emit, without running one. */ + private static ConformanceReport report(Set declared) { + return new ConformanceReportPlugin(() -> null, Optional::empty) + .build(metadata(declared), CONFIGURATION + ".ndjson", "sha256:0", "26.1.0"); + } + + private static TckRunMetadata metadata(Set declared) { + TckRunMetadata metadata = new TckRunMetadata( + CONFIGURATION, + declared, + "a test double", + "http", + Collections.singletonList(KnownDeviation.untracked( + Capability.NUMERIC_COERCION, "the fixture provider narrows a float to an integer"))); + metadata.recordProviderName("My Provider"); + return metadata; + } + + private static boolean needsSomethingUndeclared(List tags) { + for (String tag : tags) { + Optional capability = Capability.fromTag(tag); + if (capability.isPresent() && !DECLARED.contains(capability.get())) { + return true; + } + } + return false; + } + + private static Map counts(Object... statusesAndCounts) { + Map expected = new LinkedHashMap<>(); + for (int i = 0; i < statusesAndCounts.length; i += 2) { + expected.put((String) statusesAndCounts[i], (Long) statusesAndCounts[i + 1]); + } + return expected; + } + + private static String sha256Of(byte[] bytes) throws NoSuchAlgorithmException { + byte[] digest = MessageDigest.getInstance("SHA-256").digest(bytes); + StringBuilder hex = new StringBuilder("sha256:"); + for (byte b : digest) { + hex.append(String.format("%02x", b)); + } + return hex.toString(); + } + + /** Counts what a plugin registers, for the case where it should register nothing. */ + private static final class CountingEventPublisher implements EventPublisher { + + private int registered; + + @Override + public void registerHandlerFor(Class eventType, EventHandler handler) { + registered++; + } + + @Override + public void removeHandlerFor(Class eventType, EventHandler handler) { + registered--; + } + } + + /** + * A Cucumber Messages stream, read the way a consumer reads one. + * + *

Deliberately built from the raw ndjson with nothing but Jackson. Using Cucumber's own query + * helpers would make these tests agree with Cucumber by construction; the point is to show that + * the facts the report needs are recoverable from the bytes on disk. + */ + private static final class Results { + + private final Map pickles = new LinkedHashMap<>(); + private final Map pickleOfTestCase = new LinkedHashMap<>(); + private final Map testCaseOfStarted = new LinkedHashMap<>(); + private final Map mostSevereByStarted = new LinkedHashMap<>(); + private final Map tableRows = new LinkedHashMap<>(); + private final Map sources = new TreeMap<>(); + private final List finished = new ArrayList<>(); + private final List skippedStepMessages = new ArrayList<>(); + + static Results parse(String ndjson) throws IOException { + Results results = new Results(); + for (String line : ndjson.split("\n")) { + if (line.trim().isEmpty()) { + continue; + } + results.accept(MAPPER.readTree(line)); + } + return results; + } + + private void accept(JsonNode message) { + if (message.has("source")) { + JsonNode source = message.get("source"); + sources.put(source.get("uri").asText(), source.get("data").asText()); + } + if (message.has("gherkinDocument")) { + collectTableRows(message.get("gherkinDocument")); + } + if (message.has("pickle")) { + JsonNode pickle = message.get("pickle"); + pickles.put(pickle.get("id").asText(), pickle); + } + if (message.has("testCase")) { + JsonNode testCase = message.get("testCase"); + pickleOfTestCase.put( + testCase.get("id").asText(), testCase.get("pickleId").asText()); + } + if (message.has("testCaseStarted")) { + JsonNode started = message.get("testCaseStarted"); + testCaseOfStarted.put( + started.get("id").asText(), started.get("testCaseId").asText()); + } + if (message.has("testCaseFinished")) { + finished.add( + message.get("testCaseFinished").get("testCaseStartedId").asText()); + } + if (message.has("testStepFinished")) { + JsonNode step = message.get("testStepFinished"); + String startedId = step.get("testCaseStartedId").asText(); + JsonNode result = step.get("testStepResult"); + String status = result.get("status").asText(); + mostSevereByStarted.merge(startedId, status, Results::moreSevere); + if ("SKIPPED".equals(status) && result.has("message")) { + skippedStepMessages.add(result.get("message").asText()); + } + } + } + + private void collectTableRows(JsonNode document) { + for (JsonNode child : orEmpty(document.get("feature"), "children")) { + JsonNode scenario = child.get("scenario"); + if (scenario == null) { + continue; + } + for (JsonNode examples : orEmpty(scenario, "examples")) { + for (JsonNode row : orEmpty(examples, "tableBody")) { + tableRows.put(row.get("id").asText(), row); + } + } + } + } + + private static Iterable orEmpty(JsonNode parent, String field) { + JsonNode node = parent == null ? null : parent.get(field); + return node == null ? Collections.emptyList() : node; + } + + private static String moreSevere(String left, String right) { + return SEVERITY.indexOf(left) >= SEVERITY.indexOf(right) ? left : right; + } + + Map outcomeCounts() { + Map counts = new LinkedHashMap<>(); + for (String status : mostSevereByStarted.values()) { + counts.merge(status, 1L, Long::sum); + } + return counts; + } + + String outcomeOfPickle(String pickleId) { + for (Map.Entry started : testCaseOfStarted.entrySet()) { + if (pickleId.equals(pickleOfTestCase.get(started.getValue()))) { + return mostSevereByStarted.get(started.getKey()); + } + } + throw new AssertionError("pickle " + pickleId + " was never executed"); + } + + List tagsOf(JsonNode pickle) { + List tags = new ArrayList<>(); + for (JsonNode tag : orEmpty(pickle, "tags")) { + tags.add(tag.get("name").asText()); + } + return tags; + } + + List picklesNamed(String name) { + List matching = new ArrayList<>(); + for (JsonNode pickle : pickles.values()) { + if (name.equals(pickle.get("name").asText())) { + matching.add(pickle); + } + } + return matching; + } + + List cellsOfTableRow(String rowId) { + JsonNode row = tableRows.get(rowId); + assertThat(row) + .as("ast node %s is a table row in the gherkin document", rowId) + .isNotNull(); + List values = new ArrayList<>(); + for (JsonNode cell : orEmpty(row, "cells")) { + values.add(cell.get("value").asText()); + } + return values; + } + + List messagesOfSkippedSteps() { + return skippedStepMessages; + } + } +} diff --git a/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ExtensionPointTest.java b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ExtensionPointTest.java new file mode 100644 index 0000000000..14011d98e4 --- /dev/null +++ b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/ExtensionPointTest.java @@ -0,0 +1,184 @@ +package dev.openfeature.contrib.tools.providertck; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.cucumber.junit.platform.engine.Constants; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.function.Predicate; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.platform.engine.TestSource; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.engine.support.descriptor.ClasspathResourceSource; +import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; +import org.junit.platform.launcher.listeners.SummaryGeneratingListener; +import org.junit.platform.suite.api.ConfigurationParameter; + +/** + * An adopter extends the suite by convention, writing no annotations. + * + *

What that promise decomposes into, and what each test here checks: + * + *

    + *
  • a feature file under {@code tck-extensions/} on the adopter's test classpath is discovered by + * the suite, into the same Cucumber engine as the canonical set — which is what "the same + * backend lifecycle phase" means, since {@code @BeforeAll} is scoped to exactly that; + *
  • a step class in {@code openfeature.tck.extensions} is resolved from the glue path; + *
  • the extension directory shipped in this artifact keeps the selector resolvable for an adopter + * who extends nothing. + *
+ * + *

The fixture feature and steps are test-scoped, so they are not in the released JAR. They live + * where an adopter's would live, which is the only way to check that the convention holds without + * asserting it about a path that no build actually uses. + */ +class ExtensionPointTest { + + private static final String EXTENSION_FEATURE = ProviderTck.EXTENSIONS + "/extension-selftest.feature"; + + @Test + @DisplayName("an extension feature is discovered into the same suite and engine as the canonical set") + void extensionsJoinTheCanonicalSuite() { + TestPlan plan = discover(LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(TckSuiteFixture.class))); + + TestIdentifier suite = only( + plan, identifier -> TckSuiteListener.harnessClassOf(identifier).isPresent()); + TestIdentifier cucumber = only( + plan, identifier -> "cucumber".equals(engineIdOf(identifier)) && isDescendant(plan, identifier, suite)); + + List resources = new ArrayList<>(); + for (TestIdentifier test : testsUnder(plan, cucumber)) { + resourceOf(test).ifPresent(resources::add); + } + + assertThat(resources) + .as("the extension scenario and the canonical scenarios are children of one Cucumber engine, " + + "under one suite — so they share its @BeforeAll, its Compose stack and its report") + .contains(EXTENSION_FEATURE) + .contains(ProviderTck.FEATURES + "/errors.feature"); + } + + @Test + @DisplayName("the extension glue package is resolved, so an adopter's steps need no registration") + void theExtensionGluePackageIsResolved() { + // Executed rather than discovered, because an unresolved glue package is not a discovery + // failure — it produces undefined steps at execution time, which is what this rules out. + // Only the extension glue is on the path here: the canonical steps would start a Compose + // stack in their @BeforeAll, and Docker is not this module's test dependency. + SummaryGeneratingListener summary = new SummaryGeneratingListener(); + LauncherFactory.create() + .execute( + LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClasspathResource(EXTENSION_FEATURE)) + .configurationParameter(Constants.GLUE_PROPERTY_NAME, ProviderTck.EXTENSION_GLUE) + .configurationParameter( + Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, + ProviderTck.PARALLEL_EXECUTION_ENABLED) + .configurationParameter( + Constants.OBJECT_FACTORY_PROPERTY_NAME, ProviderTck.OBJECT_FACTORY) + .build(), + summary); + + assertThat(summary.getSummary().getTestsSucceededCount()) + .as("the fixture scenario ran with its steps resolved from %s", ProviderTck.EXTENSION_GLUE) + .isEqualTo(1); + assertThat(summary.getSummary().getTotalFailureCount()).isZero(); + } + + @Test + @DisplayName("the extension directory ships in this artifact, so the selector resolves with no adopter files") + void theExtensionDirectoryIsShipped() { + // A @SelectClasspathResource naming a resource on no classpath root is a hard discovery + // error, so an adopter who extends nothing depends on this file existing in the JAR. + assertThat(getClass().getClassLoader().getResource(ProviderTck.EXTENSIONS + "/README.md")) + .as( + "%s/README.md keeps the extension selector resolvable for an adopter who adds nothing", + ProviderTck.EXTENSIONS) + .isNotNull(); + } + + @Test + @DisplayName("the glue constant composes in an annotation value") + void theGlueConstantComposesInAnAnnotationValue() { + // The assertion is a formality; the compilation of VendorGlue is the actual evidence, since + // an annotation value has to be a compile-time constant and a method call would not compile. + ConfigurationParameter parameter = VendorGlue.class.getAnnotation(ConfigurationParameter.class); + + assertThat(parameter.value()) + .isEqualTo("dev.openfeature.contrib.tools.providertck.steps," + + "openfeature.tck.extensions,com.vendor.steps"); + } + + /** An adopter who wants a third glue package writes this, and does not restate our package. */ + @ConfigurationParameter(key = Constants.GLUE_PROPERTY_NAME, value = ProviderTck.ALL_GLUE + ",com.vendor.steps") + private static final class VendorGlue {} + + private static TestPlan discover(LauncherDiscoveryRequestBuilder request) { + return LauncherFactory.create().discover(request.build()); + } + + private static Optional resourceOf(TestIdentifier identifier) { + Optional source = identifier.getSource(); + if (!source.isPresent() || !(source.get() instanceof ClasspathResourceSource)) { + return Optional.empty(); + } + return Optional.of(((ClasspathResourceSource) source.get()).getClasspathResourceName()); + } + + private static String engineIdOf(TestIdentifier identifier) { + return identifier.getUniqueIdObject().getLastSegment().getType().equals("engine") + ? identifier.getUniqueIdObject().getLastSegment().getValue() + : null; + } + + private static boolean isDescendant(TestPlan plan, TestIdentifier identifier, TestIdentifier ancestor) { + Optional parent = plan.getParent(identifier); + while (parent.isPresent()) { + if (parent.get().equals(ancestor)) { + return true; + } + parent = plan.getParent(parent.get()); + } + return false; + } + + private static List testsUnder(TestPlan plan, TestIdentifier root) { + List tests = new ArrayList<>(); + collectTests(plan, root, tests); + return tests; + } + + private static void collectTests(TestPlan plan, TestIdentifier identifier, List into) { + if (identifier.isTest()) { + into.add(identifier); + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectTests(plan, child, into); + } + } + + private static TestIdentifier only(TestPlan plan, Predicate predicate) { + List matching = new ArrayList<>(); + for (TestIdentifier root : plan.getRoots()) { + collectMatching(plan, root, predicate, matching); + } + assertThat(matching).hasSize(1); + return matching.get(0); + } + + private static void collectMatching( + TestPlan plan, TestIdentifier identifier, Predicate predicate, List into) { + if (predicate.test(identifier)) { + into.add(identifier); + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectMatching(plan, child, predicate, into); + } + } +} diff --git a/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/TckSuiteFixture.java b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/TckSuiteFixture.java new file mode 100644 index 0000000000..3e0b4048e4 --- /dev/null +++ b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/TckSuiteFixture.java @@ -0,0 +1,40 @@ +package dev.openfeature.contrib.tools.providertck; + +import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.NoOpProvider; +import java.io.File; +import java.util.Collections; +import java.util.List; + +/** + * A concrete TCK suite, used by these tests for what the JUnit Platform makes of it. + * + *

Only ever discovered, never executed: discovery needs the annotations on + * {@link AbstractProviderTckTest} and nothing else, so these tests exercise the real suite + * configuration — the real selectors, the real glue, the real engines — without Docker. + * + *

Deliberately not named {@code *Test}, so Surefire does not find it and try to run it. Running it + * would start a Compose stack that does not exist. + */ +public class TckSuiteFixture extends AbstractProviderTckTest { + + @Override + public File composeFile() { + return new File("src/test/resources/there-is-no-stack.yaml"); + } + + @Override + public List backendPorts() { + return Collections.singletonList(8013); + } + + @Override + public FeatureProvider createProvider(BackendEndpoint endpoint) { + return new NoOpProvider(); + } + + @Override + public FeatureProvider createUnavailableProvider() { + return new NoOpProvider(); + } +} diff --git a/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/selftest/ReportSelfTestSteps.java b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/selftest/ReportSelfTestSteps.java new file mode 100644 index 0000000000..544c8839dc --- /dev/null +++ b/tools/provider-tck/src/test/java/dev/openfeature/contrib/tools/providertck/selftest/ReportSelfTestSteps.java @@ -0,0 +1,75 @@ +package dev.openfeature.contrib.tools.providertck.selftest; + +import dev.openfeature.contrib.tools.providertck.Capability; +import dev.openfeature.contrib.tools.providertck.CapabilityGate; +import io.cucumber.java.Before; +import io.cucumber.java.Scenario; +import io.cucumber.java.en.Given; +import java.util.EnumSet; +import java.util.Set; + +/** + * Glue for {@code report-selftest/report.feature}, the fixture the conformance report tests run. + * + *

It stands in for a provider, and for nothing else: there is no Compose stack, no control API + * and no provider here, because none of that is what the report tests are about. What it does share + * with the real suite is the part that matters — the capability gate is + * {@link CapabilityGate#requireDeclared}, the same call the real + * {@code ProviderSteps.gateOnCapabilities} makes from the same kind of {@code @Before(order = 0)} + * hook. A fixture that aborted by some other route would prove only that some abort becomes a skip. + */ +public class ReportSelfTestSteps { + + private static volatile Set declared = Capability.declarable(); + + /** + * Sets the capabilities the fixture provider declares, for the run that is about to start. + * + * @param capabilities the declared capabilities + */ + public static void declare(Set capabilities) { + declared = EnumSet.copyOf(capabilities); + } + + /** + * Gates the scenario on the declared capabilities, exactly as the real suite does. + * + * @param scenario the scenario about to run + */ + @Before(order = 0) + public void gateOnCapabilities(Scenario scenario) { + CapabilityGate.requireDeclared(scenario.getSourceTagNames(), declared); + } + + /** A step that does nothing, successfully. */ + @Given("a step that passes") + public void aStepThatPasses() { + // A passing scenario needs a step that passes and nothing more. + } + + /** + * A step that does nothing, unsuccessfully. + * + *

An {@link AssertionError} rather than a checked failure, because that is what a failing + * assertion in a step definition throws, and the point is what the results stream makes of it. + */ + @Given("a step that fails") + public void aStepThatFails() { + throw new AssertionError("this fixture scenario is expected to fail"); + } + + /** + * A passing step that takes its arguments from an Examples row. + * + *

The parameters are unused. They exist so the outline's rows differ in their compiled step + * text as well as in their table row, which is how a consumer that reads only the steps can + * still tell the rows apart. + * + * @param key the flag key from the row + * @param requested the requested type from the row + */ + @Given("a step that passes with {string} as {string}") + public void aStepThatPassesWith(String key, String requested) { + // Interpolated into the pickle's step text by Cucumber; nothing to do here. + } +} diff --git a/tools/provider-tck/src/test/java/openfeature/tck/extensions/ExtensionSelfTestSteps.java b/tools/provider-tck/src/test/java/openfeature/tck/extensions/ExtensionSelfTestSteps.java new file mode 100644 index 0000000000..101a126a93 --- /dev/null +++ b/tools/provider-tck/src/test/java/openfeature/tck/extensions/ExtensionSelfTestSteps.java @@ -0,0 +1,40 @@ +package openfeature.tck.extensions; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.cucumber.java.en.Given; +import io.cucumber.java.en.Then; + +/** + * Step definitions for the TCK's own extension fixture, in the package an adopter would use. + * + *

Test-scoped, so it is not in the released JAR. It exists to demonstrate the claim the extension + * point makes: a step class in {@code openfeature.tck.extensions} is on the suite's glue path + * without an annotation, a runner or a line of configuration written for it. + * + *

The steps deliberately need nothing from {@link + * dev.openfeature.contrib.tools.providertck.TckRuntime}. An extension scenario in a real suite runs + * after the canonical {@code @BeforeAll} and has the started Compose stack and the control API — but + * asserting that here would make the TCK's own unit tests need Docker, which is a worse trade than + * proving the lifecycle structurally: the extension features are discovered into the same suite and + * the same Cucumber engine descriptor as the canonical ones, which is what a shared + * {@code @BeforeAll} is. + */ +public class ExtensionSelfTestSteps { + + private boolean ran; + + /** A step Cucumber can only find if the extension glue package is on the glue path. */ + @Given("a step defined in the extension glue package") + public void aStepDefinedInTheExtensionGluePackage() { + ran = true; + } + + /** Asserts the step above ran, so that a missing glue package fails rather than passes. */ + @Then("the extension step ran") + public void theExtensionStepRan() { + assertThat(ran) + .as("the extension glue package was resolved and its steps executed") + .isTrue(); + } +} diff --git a/tools/provider-tck/src/test/resources/report-selftest/report.feature b/tools/provider-tck/src/test/resources/report-selftest/report.feature new file mode 100644 index 0000000000..cc1cab4791 --- /dev/null +++ b/tools/provider-tck/src/test/resources/report-selftest/report.feature @@ -0,0 +1,37 @@ +# Fixture for ConformanceReportPluginTest, and not part of the conformance suite: it lives under a +# separate classpath root so that nothing selecting "features" can pick it up. +# +# It is shaped like the real suite rather than minimal, because the properties the report has to +# preserve are properties of that shape: a capability tag on the feature, one on a scenario, one on +# a single Examples block, and an outline whose rows all share a name. Running real Cucumber over +# this is the only way to check what a capability-gated abort actually becomes in the results. + +@events +Feature: Report self-test + + Scenario: A mandatory scenario + Given a step that passes + + @object + Scenario: A scenario needing a declared capability + Given a step that passes + + @stale + Scenario: A scenario needing an undeclared capability + Given a step that passes + + Scenario: A scenario that fails + Given a step that fails + + Scenario Outline: Requesting the wrong type returns the code default + Given a step that passes with "" as "" + + Examples: a string flag requested as something else + | key | requested | + | string-flag | Boolean | + | string-flag | Integer | + + @stale + Examples: gated by a tag on this block alone + | key | requested | + | boolean-flag | String | diff --git a/tools/provider-tck/src/test/resources/tck-extensions/extension-selftest.feature b/tools/provider-tck/src/test/resources/tck-extensions/extension-selftest.feature new file mode 100644 index 0000000000..3e4bece949 --- /dev/null +++ b/tools/provider-tck/src/test/resources/tck-extensions/extension-selftest.feature @@ -0,0 +1,14 @@ +@extension-selftest +Feature: An adopter's own scenarios run inside the TCK suite + + This file is the TCK's own proof of its extension point. It sits exactly where an adopter's + extension features sit — on the test classpath under tck-extensions/ — and is picked up with no + annotation, no selector and no runner configuration written anywhere for it. + + It is test-scoped, so it is not packaged in the released JAR and cannot reach an adopter's run or + their conformance report. It carries no canonical scenario and cannot stand in for one: the + canonical set is what features/ contains, and this file is not in it. + + Scenario: A step class in the adopter's own glue package is on the suite's glue path + Given a step defined in the extension glue package + Then the extension step ran