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>
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>
…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>
…rcion The capability was named for a stricter rule than the specification wants, and the name was about to collide with a second vocabulary for the same property. flagd is implementing an accepted numeric coercion ADR (open-feature/flagd#1996) whose rule is that coercion is permitted when it is lossless and must fail with TYPE_MISMATCH only when information would be lost: 10.0 requested as an integer succeeds, 0.5 does not. Appendix F said "does not coerce between integer and float", which forbids the case the ADR requires to work; the one scenario survives the difference only because it asks about 0.5, which does have a fractional part. That work also introduces @numeric-coercion scenarios into flagd's own testbed, so keeping the old name would have left the reference implementation and the specification disagreeing about what a rule is called. The spec side is open-feature/spec@dc4d7ae8 on feat/provider-tck-appendix. This brings the vendored assets to that revision -- the renamed tag and its comment in errors.feature, and two unrelated changes that landed in the same range: the lifecycle readiness scenario's name, and the /start requirement that a 200 means the seeded flag state is already being served. src/main/resources/{features,flags, openapi} are byte-identical to specification/assets/provider-tck at dc4d7ae8 modulo line endings, and provider-tck.spec.revision records it. flagd's deviation becomes tracked rather than untracked, against flagd#1996, and its summary now says which half of the rule is broken: the defect is the lossy case being silently accepted, not coercion as such. No scenario is added. The lossless half of the contract has none, because the canonical flag set contains no integral float to ask it of and adding one changes the flag set for every language at once, so a provider that wrongly rejects 10.0 as an integer still passes. Accessor width -- the ADR's 64-bit versus 32-bit integer accessor, which flagd tags @int32-bounded -- is not modelled either. Both are recorded as open gaps in the README rather than closed here. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…laration @targeting and @caching are reserved: they exist in the vocabulary so every language's TCK spells the same property the same way, but no scenario carries either tag. A reserved capability therefore cannot gate anything -- it produces no skip, so nothing in a run can confirm or contradict it -- and the report schema now says it must not be declared and must not appear in declaration.declared. It was appearing. Before: "declared": ["@lifecycle","@events","@Stale","@configuration-change", "@object","@unavailable","@targeting","@caching"] After: "declared": ["@lifecycle","@events","@Stale","@configuration-change", "@object","@unavailable"] Nobody decided to claim the last two. AbstractFlagdTckTest said EnumSet.complementOf(EnumSet.of(NUMERIC_COERCION)), which reads as "everything except the one thing flagd cannot do" and in fact means "every other enum constant", collecting both reserved tags on the way past. Two published reports claimed capabilities nothing had examined, which is the vacuous conformance claim the vocabulary exists to prevent. Capability now carries the reserved flag itself, so the list cannot drift from the rule, and offers the two forms that mean what complementOf looks like: declarable() and declarableExcept(...). The harness default becomes declarable() rather than EnumSet.allOf, which had the same defect for any adopter who never overrode it. Naming a reserved capability explicitly fails the run rather than being dropped with a warning. The declaration is the one part of the report no result can check -- everything else in it was observed, this is asserted by the author -- and a report is read long after the log a warning would have gone to. Nothing is lost by refusing, because no scenario carries the tag, and the check runs before the Compose stack starts, so the cost of the mistake is seconds rather than a suite. It is enforced in TckRunMetadata, the one place every path to a report passes through, and the emitted list skips reserved capabilities as well, so "no reserved tag in a declaration" is a property of the code that writes the document and not only of a check upstream of it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A vendor with provider-specific features -- flagd's fractional targeting, a proprietary evaluation mode -- had no way to test them inside this suite. The only option was a second Cucumber runner of their own, which means a second backend lifecycle to start and a second copy of this suite's configuration to keep in step with it. Answers @toddbaert's review request on open-feature/spec#423. The suite now also selects the classpath directory tck-extensions/ and the glue package openfeature.tck.extensions. An adopter writes two files and no annotations: src/test/resources/tck-extensions/fractional.feature src/test/java/openfeature/tck/extensions/FractionalSteps.java Their scenarios are discovered into the same suite, the same Cucumber engine and therefore the same @BeforeAll -- one Compose stack, one control API, one conformance report. The canonical steps are on the glue path too, so an extension scenario can open with `Given a stable provider` and continue with whatever is specific to that provider. The extension directory is deliberately not features/ and not a subdirectory of it. Measured on this module: two classpath roots holding the same directory are scanned additively, but two holding the same directory *and* the same file name are not -- one wins silently and the other file is never read, with test-classes beating the jar. An adopter who put features/errors.feature in their test resources would replace a canonical file with their own and watch the suite report success having run theirs. A distinct name makes that collision unreachable rather than documented. The directory ships inside the jar holding nothing but a README, because a @SelectClasspathResource naming a resource that exists on no classpath root is a hard discovery error rather than an empty selection -- so an adopter who extends nothing must still resolve it. Cucumber ignores files that are not .feature, and tolerates a glue package that does not exist, so the unused extension point costs an adopter nothing. Also adds ProviderTck, which names every value the suite's annotations carry. An annotation value has to be a compile-time constant, so an adopter who writes a @ConfigurationParameter of their own cannot compute one; without the constants they would restate our package name or our object factory as a string literal that nothing keeps in step. Constant concatenation is legal in an annotation value, so ProviderTck.ALL_GLUE + ",com.vendor.steps" is what they write instead. The TCK's own fixture -- tck-extensions/extension-selftest.feature and a step class in openfeature.tck.extensions -- is test-scoped, so it is not in the released jar and cannot reach an adopter's report. It sits exactly where an adopter's would, which is the only way to check the convention rather than assert it about a path no build uses. Verified end to end against providers/flagd, which is an adopter with no extension directory and no extension glue package: that suite still runs 29 canonical scenarios green. With a throwaway fixture added to flagd's test resources, the extension scenario ran inside the same suite after `Given a stable provider`, resolved boolean-flag against the started Compose backend, and appeared alongside the canonical scenarios in one run. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Extending the suite is now safe by convention. Nothing made it hard to shrink, and shrinking 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 features/, a cucumber.filter.tags or cucumber.filter.name expression, and selectors or glue overridden in a consuming module's junit-platform.properties. CanonicalScenarioGuard is an ordinary JUnit Jupiter test the suite selects, so a reduced set fails the build the way any other failing test does. A TestExecutionListener cannot do that job: the JUnit Platform catches and logs whatever a listener throws, which is exactly the silent pass being guarded against. That is why junit-jupiter joins cucumber in the engine list. It works from two pieces of evidence, both settled before the first scenario runs. The discovered test plan, captured by TckSuiteListener, says which scenarios the suite selected. The run's cucumber.filter.* configuration, read through the guard's own ExtensionContext, says what will be skipped at execution -- Cucumber applies a tag filter as a skip rather than as a discovery filter, so a filtered scenario is in the plan and the plan cannot show it. The canonical set itself is compiled with Gherkin out of this artifact's own code source -- the jar or target/classes the class was loaded from -- rather than through the classloader. A feature file placed in features/ on another classpath root shadows the canonical one of the same name, so reading it back through getResource would be checking the replacement against itself. A scenario is identified by resource and line, which is what a ClasspathResourceSource in the test plan carries; for a Scenario Outline that is the Examples row, so the eleven rows of the type-mismatch matrix count as eleven rather than as one name. Extension scenarios are ignored entirely: the check is defined over features/ alone, so an adopter's tck-extensions/ scenarios can neither stand in for a canonical scenario nor look like a spurious one. -Dprovider.tck.partial=true (or PROVIDER_TCK_PARTIAL) downgrades the check to a skip. Running one scenario under a tag filter is routine while debugging a provider, and a check that made that impossible would be switched off permanently instead of temporarily. A skip rather than a pass keeps the run honest about not having verified its canonical set. What this does not establish is that the canonical files contain what they should: a replacement placing its scenarios on the same lines would satisfy the comparison. The results stream already carries the source of every feature that executed, and tck.specRevision says which revision it should match. Separable from the extension work by design -- it guards a bypass rather than enabling anything, and dropping this commit leaves the extension point unaffected. Verified against providers/flagd: the guard passes on an intact run of FlagdRpcTckTest, and fails the run when cucumber.filter.tags is set. 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 |
aepfli
force-pushed
the
feat/provider-tck-report
branch
2 times, most recently
from
September 11, 2026 10:33
e905d57 to
24cc3e9
Compare
Member
Author
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
1 participant
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.
Lets an adopter run their own scenarios and steps inside the TCK's suite, in the same backend lifecycle phase, and exposes the suite's annotation values as constants.
Stacked on the report PR. Answers @toddbaert's review request on open-feature/spec#423: a vendor with provider-specific behaviour (flagd's
fractional, say) should extend the TCK rather than maintain a parallel harness.What an adopter writes
Nothing. Drop a feature file in
src/test/resources/tck-extensions/and a step class inopenfeature.tck.extensions; both are picked up by the base suite. Java can scan, so this is convention rather than configuration — unlike Go and JavaScript, which need explicit options.Why the extension directory is separate
Measured on this module, not assumed:
@SelectClasspathResourcenaming a resource that does not existtest-classesbeats the jar).featurefileThe third is why
tck-extensions/is a separate name: had extensions sharedfeatures/, a vendor'sfeatures/errors.featurewould silently replace the canonical file and the suite would go green having run theirs. The first is why the jar shipstck-extensions/README.md— the selector must resolve even when no adopter uses it.An absent glue package is tolerated silently, so the extension glue package costs nothing unused.
Constants
The features path, extension path, glue packages and Cucumber configuration values are now
public static final StringonProviderTck. An adopter who does write their own@ConfigurationParametercan useProviderTck.GLUE + ",com.vendor.steps"rather than restating our package as a magic string; annotation values permit compile-time constant concatenation.Second commit is separable
feat(provider-tck): fail a run that executes less than the canonical setcan be dropped independently. It exists because a partial run otherwise produces a well-formed, valid report: in Go,-runon a single scenario passed green and emitted a report covering 1 of 29 canonical scenarios; the same hazard was live in Python. Extension scenarios cannot close a gap in the canonical set.Verified
mvn -pl tools/provider-tck test: 31 tests, 0 failures, includingExtensionPointTestandCanonicalScenarioGuardTest.core.autocrlf=true, so the working tree is CRLF and spotless reads the working tree. The committed blobs are LF (checked directly), so CI's Linux checkout should be clean — but that is inference, not a local green run.The implementing agent was stopped before it finished its own verification pass; the commits are its work, the test run above is mine. Treat the end-to-end extension demonstration as claimed-but-unconfirmed until CI runs.