feat(provider-tck): emit a machine-readable conformance report - #1841
Draft
aepfli wants to merge 5 commits into
Draft
feat(provider-tck): emit a machine-readable conformance report#1841aepfli wants to merge 5 commits into
aepfli wants to merge 5 commits into
Conversation
Set PROVIDER_TCK_REPORT_DIR and each suite writes <dir>/<configuration>.json, conforming to the report schema in the OpenFeature specification. Unset means no report, which is not an error. Emitting a report is a property of the run rather than of the code, which is why it is an environment variable and not a method on ProviderTckHarness: CI asks for one, a developer running the suite locally does not, and no adopter changes a line to publish one. -Dprovider.tck.report.dir does the same thing for a Maven invocation; the environment variable is the portable spelling every language's TCK reads. The per-scenario list is the load-bearing part. 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 report records the outcome of every scenario exactly once, straight from Cucumber's TestCaseFinished event, so a consumer can verify the rule instead of trusting a runner's headline number. The Go TCK shipped a version of this that recorded every skipped scenario twice, once correctly and once as passed, because its capability-skip signal did not reach the after-hook; one event in, one entry out removes that whole class of bug here, and ConformanceReportPluginTest asserts the totals add up. provider.name is 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. It is derived from the suite class name and overridable with ProviderTckHarness.configuration(). tck.specRevision and tck.assetsTree identify the conformance artifacts that ran. They are baked into the JAR at build time by Maven resource filtering, because the artifacts travel in the JAR and the repository they came from does not. They are pinned in the module POM for now: unlike the Go TCK this module has no spec submodule to read them from, so there is nothing for the build to interrogate. Both are checkable rather than merely asserted, and the vendored artifacts were verified byte for byte against the revision recorded. sdk.version is read from the classpath rather than declared, since the TCK depends on an SDK version range and what a consumer ran against is only knowable at runtime. Verified against the flagd testbed in both resolver modes: 29 scenarios each, 28 passed and 1 not-declared (@strict-numeric-typing), both reports valid against the Draft 2020-12 schema. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
A report entry was identified by feature and name, and every row of a Scenario Outline shares one name. The type-mismatch matrix in errors.feature is eleven rows, so both flagd reports carried eleven entries that nothing distinguished -- 29 entries under 13 distinct names. If one row had failed and ten passed, the report could not have said which, and a consumer building a map from it keeps whichever row it saw last. Each entry now carries `example`, the row's parameters keyed by their Examples column header, as defined by the report schema. Values are the cell contents verbatim, as strings: Gherkin has no types, so "1" stays the string 1 and coercing it would make the report say something the table did not. It is a field rather than a naming convention because the parameters are the identity, and they come from the feature file rather than from any runner. Mandating a mangled name instead would put a separator, an ordering and an escaping rule into normative text that four languages must reproduce byte for byte, with drift invisible until two reports silently fail to line up. The implementations had already diverged on precisely this point before the field existed: Go emitted the bare scenario name for all eleven rows, Python appended its pytest node id, JavaScript its runner's expanded title. Each is a reasonable display name; none of them is a shared identity. Recovering the row takes some care. TestCaseFinished carries a compiled pickle that no longer knows it came from a table, but TestCase.getLocation() resolves the last of the pickle's AST node ids, which the pickle compiler sets to the Examples TableRow -- a plain scenario's last node is the scenario itself, so a line number tells the two apart. ScenarioExamples parses the feature source Cucumber publishes on TestSourceRead, rather than resolving the feature file a second time: Cucumber has already located and decoded it, and re-resolving classpath:features/errors.feature would give a different answer whenever a consumer supplies features from somewhere else. Parsing uses the Gherkin parser Cucumber already depends on, so the report reads the same document the runner executed; io.cucumber:gherkin and io.cucumber:messages were already on the classpath transitively and are now declared, with versions still managed by cucumber-bom. A row skipped for an undeclared capability carries its example too. Eleven skips sharing a name are exactly as ambiguous as eleven failures sharing one. Gherkin also permits a tag on an individual Examples block, so two rows of one outline can differ in whether the capability gate stops them. Nothing here is keyed by scenario name -- one event in, one entry out, and the row lookup is by URI and line -- so a gated row cannot suppress its siblings, and a test covers that case directly. The Go implementation had that bug: its skip bookkeeping was keyed by name, and gating one row dropped every other row of the outline from the report. Verified against the flagd testbed in both resolver modes: 29 entries each, 11 distinct example objects under "Requesting the wrong type returns the code default", (feature, name, example) unique across all 29 where (feature, name) yields only 13, and both reports still valid against the Draft 2020-12 schema. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…assed A declared capability read `passed` whether or not anything had examined it. That is the vacuous pass the capability vocabulary was introduced to eliminate, arriving through the report rather than through the suite, and it reached that state two ways. @targeting is reserved: it exists in the tag vocabulary but no scenario carries it, because asserting that an evaluation context reached the backend needs an echo operation the control API does not have. A provider declaring it got a green result for free. @caching is the same. The second route is subtler. A scenario can carry two capability tags and be skipped for the one the provider did not declare, and counting a capability as exercised because a scenario *carried* its tag counts that skip. events.feature is exactly this shape -- the feature is tagged @events and each of its two scenarios adds @Stale or @configuration-change -- so a provider declaring @events alone ran neither scenario and was told @events passed. Exercising is now counted by execution: a scenario contributes to its capabilities only when its outcome is passed or failed. Such a capability is omitted from `capabilities` entirely. Nothing asked the question, so there is no answer to report, and a consumer sees the tag is absent rather than a pass it cannot rely on. Omitting is preferred to inventing a fifth outcome: the four in the schema describe what the provider did, and "nothing asked this of the provider" is a fact about the run. Everything else about the rollup is unchanged. Undeclared is still not-declared with a reason; declared, exercised and failing is still failed with a reason, now saying how many of how many ran; declared, exercised and passing is still passed. Follows go-sdk-contrib#944, which made the same two changes there. The second was found by the Python implementation, whose in-memory self-test declares @events without @Stale and so hits it directly. flagd declares every capability but @strict-numeric-typing, so its reports now carry seven entries rather than nine. The second route does not change them -- the one undeclared capability is the only tag on its scenario -- so it is latent there, and live for a provider that declares @events without @Stale. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The report used to define its own per-scenario format: a list of scenarios, each with a four-value outcome and the Examples row it came from. Inventing a results format was the wrong call. It has to be maintained, versioned and reimplemented in four languages, and everything it carried is already specified by Cucumber Messages. A run now writes two files. The envelope, <configuration>.json, says what was tested and what the provider claims. The results, <configuration>.ndjson, are a Cucumber Messages stream produced by Cucumber's own MessageFormatter -- the same class the built-in message:<path> plugin instantiates, so the bytes are what --plugin message:... would have written. The envelope's results.location names the stream and results.digest covers it. The plugin stays rather than becoming a @ConfigurationParameter because a plugin option is a compile-time constant, so the built-in plugin's path cannot be derived from PROVIDER_TCK_REPORT_DIR, and flagd's two suites would write to the same file. It now delegates the stream to Cucumber and writes only the envelope. Deleted: ScenarioExamples, which re-parsed the feature source and matched a pickle's reported line number against the Examples tables to recover the row it came from. A pickle's astNodeIds state that outright -- [scenario, table row], resolving in the gherkinDocument message. Reverse-engineering what the standard format already provides is the clearest argument for adopting it. Also deleted: the Outcome enum, the per-scenario and per-capability result lists, and tck.assetsTree, since the stream carries the source of every feature that ran. Added ProviderTckHarness.knownDeviations(), for the one thing neither the stream nor the declaration can express: whether a withheld capability is a limitation or a bug. flagd withholds @strict-numeric-typing because it narrows a float to an integer with no error code, and that reads identically to a provider with no streaming transport declining @configuration-change unless it is stated. The capability gate moves to CapabilityGate.requireDeclared so the gate that produces a skip and the test that proves the skip survives into the results are looking at the same code. ConformanceReportPluginTest now runs a fixture suite through the real Cucumber engine and reads the emitted stream back as a consumer would, because "a gated scenario is never reported as passed" is a property of what Cucumber emits and only a real run can demonstrate it. Verified against both flagd resolvers: 29 scenarios each, all accounted for exactly once, 28 passed and 1 skipped, the skip being @strict-numeric-typing and reported as SKIPPED with the gate's reason on the aborted hook's step result. Both envelopes validate against the reshaped schema and both streams validate against the published Cucumber Messages Envelope schema, 744 messages each. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
github-actions
Bot
requested review from
Kavindu-Dodan,
beeme1mr,
thisthat and
toddbaert
September 10, 2026 18:12
…he stream The envelope named the results format but not its version, and Messages is versioned. cucumber-jvm 7.34.3 emits protocol 30.1.0, while the Go TCK builds against 21.0.1, the JavaScript one 24.1.0 and the Python one 34.2.0. Four implementations, four releases -- 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. The value is read back out of the stream's own meta.protocolVersion rather than from a constant or the io.cucumber:messages artifact version. Cucumber decides what it writes there, and taking it from anywhere else would let the envelope and the stream disagree about which release produced them, which is worse than either being absent. meta is the first envelope cucumber writes, so only the first line is parsed, and a stream whose first line will not parse omits the field rather than failing a run that otherwise succeeded. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1830 (
feat/provider-tck). Part of open-feature/spec#424; the envelope this emits is defined by the schema in open-feature/spec#425.This has been reworked. The earlier revision defined its own per-scenario results format: a list of scenarios, each with a four-value outcome and the
Examplesrow it came from. That was the wrong call, and the review that said so was right. A results format has to be maintained, versioned and reimplemented in four languages, and everything it carried is already specified by Cucumber Messages. The schema in #425 has been reshaped accordingly and so has this.Set
PROVIDER_TCK_REPORT_DIRand each suite now writes two files:<configuration>.json<configuration>.ndjsonThe envelope's
results.locationnames the stream andresults.digestcovers it. Unset means no report, and that is not an error.Real output
Both flagd resolvers, on this branch, against the flagd testbed:
{ "schemaVersion" : "1", "provider" : { "name" : "flagd", "language" : "java", "configuration" : "flagd-rpc" }, "sdk" : { "name" : "dev.openfeature:sdk", "version" : "1.22.0" }, "tck" : { "implementation" : "java-sdk-contrib/tools/provider-tck", "version" : "0.0.1", "specRevision" : "dfa16586d91ca020ef1b3b82a7c972d833ff8f29" }, "backend" : { "description" : "Docker Compose stack docker-compose.yaml, service backend", "controlApi" : "http" }, "declaration" : { "declared" : [ "@lifecycle", "@events", "@stale", "@configuration-change", "@object", "@unavailable", "@targeting", "@caching" ] }, "results" : { "format" : "cucumber-messages", "location" : "flagd-rpc.ndjson", "digest" : "sha256:c390f2bd578fe323d0c2e54fdfce1606df09b1258ed65d74d95f8b3436a9bbd5" }, "knownDeviations" : [ { "capability" : "@strict-numeric-typing", "summary" : "Evaluating float-flag (0.5) through the integer API returns 0 with no error code, rather than TYPE_MISMATCH with the code default: the value is silently narrowed. Both resolvers behave identically, which places the defect in the shared provider layer rather than in either transport." } ] }Reading the outcomes out of the stream, which needs one thing understood — a scenario's outcome is the most severe result among its steps, hooks included, because
testCaseFinishedcarries no status of its own:Identical for
flagd-in-process. 29 pickles, 29testCase, 29testCaseStarted, 29testCaseFinished, nothing started twice, nothing left unexecuted. The one skip in each is A float flag is not silently narrowed to an integer, and it is the aborted@Beforehook that makes it a skip: its step result isSKIPPEDand carries the gate's own message,Skipped: provider does not declare capability STRICT_NUMERIC_TYPING (tag @strict-numeric-typing). That is the rule Appendix F cares about, and it is truthful in the stream.What the stream carries, and what it replaced
Scenario Outline row identity.
pickle.astNodeIdsis[scenario id, table row id], and the second entry resolves in thegherkinDocumentmessage to theExamplesrow the scenario was compiled from. For the eleven rows of Requesting the wrong type returns the code default inerrors.feature:Eleven distinct row ids for eleven scenarios sharing one name.
ScenarioExamplesand the report'sexamplefield are deleted: they re-parsed the feature source Cucumber publishes onTestSourceReadand matched a pickle's reported line number, viaTestCase.getLocation(), back against theExamplestables — an approximation, reverse-engineered fromCucumberQuery.getLocationBy, of exactly the mechanism the format already provides. That is the clearest argument for adopting a standard format rather than defining one, so it is worth saying rather than quietly dropping.Also deleted: the per-scenario list, the per-capability rollup, the four-value
Outcomeenum, andtck.assetsTree— the stream carries thesourceof every feature that executed, which is strictly better than a tree hash asserting which revision it came from.specRevisionstays, because it identifies the two artifacts the stream does not carry,flags/canonical-flags.jsonandopenapi/control-api.yaml.Tags, including per-
Examples-block tags, are onpickle.tagswith the AST node each came from. Gherkin allows a tag on an individualExamplesblock, so two rows of one outline can differ in whether the capability gate stops them; the self-test fixture has exactly that shape and asserts that only the tagged row is skipped.Cucumber's own formatter, at a path the run chooses
The stream is produced by
io.cucumber.core.plugin.MessageFormatter— the same class the built-inmessage:<path>plugin instantiates — so the bytes are what--plugin message:...would have written.ConformanceReportPluginregisters it against the same publisher and writes only the envelope.The built-in plugin is not used directly for one reason: a
@ConfigurationParametervalue is a compile-time constant, socucumber.plugin=message:<path>cannot have a path derived fromPROVIDER_TCK_REPORT_DIR, and flagd's two suites in one module would write to the same file. Delegating gets the standard bytes without giving up per-suite naming or the zero-configuration adoption.Two consequences worth naming. The stream is buffered in memory and written at the end, because the file name comes from the provider configuration, which the suite only reports once its runtime has started — after the first messages have been emitted. It is 330 KB here. And the envelope handler is registered after the formatter's, because Cucumber invokes handlers for one event type in registration order and the formatter closes its writer on the run-finished message; going second is what guarantees the digest covers a complete file.
What stays OpenFeature-specific, and why
The envelope is not a summary of the results. Every field in it answers a question no results format answers, because a Messages stream cannot say what it was a test of:
provider— what the provider calls itself through its own metadata, not the suite name. The suite name reads well in a failure message (flagd-rpc), which makes it the configuration; one provider with two materially different modes produces two reports that are not interchangeable.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 which revision of the artifacts.declaration— the capability set the provider claims. This is the load-bearing one, and it is an input to reading the results rather than 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, which is what let the whole per-scenario list go.knownDeviationsis the one thing neither the stream nor the declaration can express. Withholding a capability reads identically whether it describes a limitation or works around a bug, and the TCK cannot tell the two apart from the outside. SoProviderTckHarness.knownDeviations()lets the provider author say, and flagd says it:@strict-numeric-typingis withheld because flagd narrows a float to an integer with no error code, in both resolvers, which places the defect in the shared provider layer rather than in either transport.Recorded as
untracked, not omitted. There is no issue for this gap — it was found by this suite and has not been filed — and the schema makesissueoptional for that case. Silence was the alternative, and it is worse: a consumer would read flagd declining@strict-numeric-typingexactly as it reads a provider with no streaming transport declining@configuration-change, and one of those is a decision while the other is a bug.KnownDeviation.tracked(...)takes the issue link once there is one.Testing the property rather than the serialisation
ConformanceReportPluginTestno longer asserts over a report this code wrote. It runs a fixture suite through the real Cucumber engine on the JUnit Platform, with the real plugin registered, and reads the emitted stream back the way a consumer would. That change is the point: "a gated scenario is never reported as passed" is now a property of what Cucumber emits, and only a real run can demonstrate it. Asserting over hand-built messages would only have checked Cucumber's serialiser.The fixture is shaped like the suite rather than minimal — a capability tag on the feature, one on a scenario, one on a single
Examplesblock, and an outline whose rows share a name — because those are the shapes the properties depend on. Fourteen tests: every pickle executed exactly once, outcome counts, both gated scenariosSKIPPEDand neitherPASSED, the gate's reason present,Examples-block tags reaching the right row only, eleven distinct row ids resolving to the right cells, the executed source byte-identical to the file, the digest matching the stream, and the envelope carrying what the schema requires and nothing it forbids.The capability gate moved to
CapabilityGate.requireDeclaredso that the gate producing the skip and the test proving the skip survives are looking at the same code. Inlined in the step definitions, the self-test could only have shown that some abort becomes a skip.Verification
mvn -pl tools/provider-tck verify— green: 14 tests, 0 Checkstyle violations, PMD clean, SpotBugsBugInstance size is 0, spotless clean.mvn -pl providers/flagd test -Dtest='Flagd*TckTest'withPROVIDER_TCK_REPORT_DIRset — 58 scenarios, 0 failures, 2 skipped. Four files written, one pair per resolver, no collision.jsonschema4.10.3).results.digestmatchessha256sumof the corresponding.ndjsonfor both.jsonschema/messages.schema.jsonfromcucumber/messages@main— 744 messages each, zero invalid.testCase/ 29testCaseStarted/ 29testCaseFinished, no pickle executed twice or never, 28PASSED+ 1SKIPPED, and the one scenario carrying an undeclared capability tag is theSKIPPEDone.jqrecipes in the README were run against the real output and produce what they claim.Run on JDK 21 with Docker; the flagd suites need a Docker daemon, so they are not part of the module's own test run.
How this interacts with the rest of the stack
ConformanceReport,ConformanceReportPlugin,ScenarioExamples,Outcome— while the additions landed in three of the four files it does.ProviderTckHarnessgrew from 21 to 41 conflicted lines (knownDeviations()sits next tocapabilities(), which feat(provider-tck): in-process control path and in-memory/multi-provider self-tests #1837 also moves) andTckRuntimefrom 76 to 90 (recordRun). Two new files conflict:steps/ProviderSteps(one line, the gate call) andAbstractFlagdTckTest(two lines). All of it is mechanical.pom.xmlandREADME.mdstill merge cleanly despite both branches editing them. ThecontrolApi()follow-up described before still applies: it should become a default method onBackendControlreturning empty, overridden byHttpBackendControlwithhttpandInProcessBackendControlwithin-process.git rev-parse HEADin the same step that copies the artifacts.provider-tck.spec.assets-treeis gone, so there is one fewer thing to generate.