feat(provider-tck): let an adopter add their own scenarios to the suite - #416
Closed
aepfli wants to merge 11 commits into
Closed
feat(provider-tck): let an adopter add their own scenarios to the suite#416aepfli wants to merge 11 commits into
aepfli wants to merge 11 commits into
Conversation
Setting PROVIDER_TCK_REPORT_DIR makes each suite write its run to <dir>/<name>.json against the report schema in the specification repository (open-feature/spec#425, part of open-feature/spec#424). Unset means no report, which is the default and is not an error. An environment variable rather than a TckConfig field, so that emitting a report is a property of the run and not of the code: CI sets it, a local run does not, and no adopter changes a line to publish one. Several suites in one pytest session each write their own file, so flagd's two resolvers would not collide. The load-bearing part is the per-scenario list. Appendix F requires that a scenario skipped for an undeclared capability is reported as skipped with the reason and never as passed, and nothing downstream can check that against a summary line. Recording every scenario's outcome individually makes the rule checkable by the consumer instead of dependent on the runner. It is also required to be complete, because a document that quietly dropped what it skipped would satisfy the letter of the rule and still mislead whoever read it. pytest, unlike godog, reports a skip honestly -- so the interesting divergence here is elsewhere. The one scenario the Python SDK cannot satisfy is marked xfail, so the run finishes green; the provider still did not satisfy it, and the document says failed with the reason. An expected failure is a recorded deviation, not an excused one. Scenarios are therefore enumerated at collection and resolved at the end of the session rather than as fixtures run, which is also what keeps a scenario skipped by a marker -- whose fixtures never run at all -- from vanishing from the document. Identity comes from spec_revision.json, generated by hatch_build_sync.py beside the copied assets and force-included into the wheel. It has to be captured at build time: the submodule that knows the answer is not in the distribution, so an installed copy has nothing left to ask. A build that cannot reach git -- an unpacked sdist -- warns and records "unknown" rather than inventing a commit. Both the commit and the tree hash are recorded, the tree because it identifies the assets alone: unchanged by unrelated edits elsewhere in the specification, so two runs of identical assets agree even when pinned to different commits, and checkable because `git rev-parse <commit>:specification/assets/provider-tck` reproduces it. Two smaller decisions. The provider is identified by the name it reports through its own metadata, with TckConfig.name recorded as the configuration, because TckConfig.name is chosen to read well in a failure message -- "flagd-rpc" -- and a provider with two materially different modes produces two reports that are not interchangeable. And how the backend was driven is read off an optional control_api property rather than added to the BackendControl protocol, so that adding it leaves every existing control complete and one that stays quiet simply omits the field. The tests assert the two properties a consumer is entitled to assume -- that no scenario the capability gate stopped is ever reported as passed, and that every collected scenario appears exactly once, counted against pytest's own collection rather than against a number written down beside it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A report entry was identified by feature and name. Every row of a Scenario Outline shares one name, so the eleven rows of the type-mismatch matrix in errors.feature produced eleven entries nothing could tell apart -- and in the Python run one of the eleven fails while ten pass, which is exactly the case the report could not express. A consumer keying on feature and name kept whichever row it happened to see last. Each entry from an outline now carries the row it came from, as the Examples parameters keyed by column header, matching the "example" property added to the schema. Values are the cell contents verbatim as strings: Gherkin has no types, so "1" stays "1" rather than becoming a number the table never mentioned. pytest-bdd parametrizes the generated test over one dict per row, keyed by the header, so the row is read back off the node's callspec -- available at collection, which is what lets a row the capability gate skipped be identified as precisely as one that ran. This removes the workaround that appended pytest's own id for the row to the scenario name. It was the wrong shape twice over. The name is the feature file's name, and qualifying it made Python disagree with Go and JavaScript about a scenario all three ran, which defeats the cross-language comparison the report exists for. And a name format would be normative text -- a separator, an ordering, an escaping rule -- that four languages have to reproduce byte for byte, where drift is invisible until two reports silently fail to line up. The parameters are the identity, and they come from the feature file rather than from any runner. The uniqueness test now keys on feature, name and example together, which is the property this change exists to establish. The examples the report emits are checked against the Examples tables read out of the Gherkin by hand, rather than against pytest-bdd's parser, which is what produced them. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…ng untested ones
Two defects in the capability rollup, mirroring the fix already made in Go
(go-sdk-contrib#944).
A failed capability was emitted as {"state": "failed"} with no reason. The schema
now requires a reason for any outcome other than passed, so that entry does not
validate -- and it appears only when a provider is actually failing, which is
precisely when the report matters. It now says how many of how many scenarios
carrying the tag failed, and points at the per-scenario results for which and
why.
No test caught it because every self-test suite passes, so nothing that runs end
to end ever reaches that branch. The test now drives the report builder directly
with synthetic records, which is the only way to exercise a failure without
breaking a provider on purpose.
A declared capability that no scenario carries was reported as passed. @targeting
is reserved -- it exists in the vocabulary but nothing tests it, because asserting
that an evaluation context reached the backend needs an echo operation the
control API does not have -- so a provider declaring it got a green result for a
claim nothing had examined. That is the vacuous pass the capability vocabulary
was introduced to eliminate, arriving through the report rather than through the
suite.
Such a capability is now omitted. The suite asked no question, so it has 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 are about what the provider did, and "the suite does not test this" is
a fact about the suite.
An undeclared capability is still reported with its reason whether or not any
scenario carries it, because that is a fact about the provider rather than about
the suite.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Gherkin lets an Examples block carry its own tags, so two rows of one Scenario Outline can differ in which capability gates them. The capability gate already handled that correctly -- pytest-bdd attaches an Examples block's tags as marks on that block's parameter sets, and the gate reads the node's markers -- but the report did not. A scenario's tags were read from the scenario, the feature and the rule, which is everywhere those tags are not. The consequence was a misreport of exactly the kind the format exists to rule out. A row skipped because its Examples block was tagged with an undeclared capability appeared with no tags at all, so it was classified not-applicable rather than not-declared -- the run had a reason not to execute it, said the report, when the reason was a capability the provider does not have. The capability rollup did not count it either. The row's tags are now resolved by intersecting the tags the scenario's Examples blocks declare with the markers pytest put on the node. That names this row's blocks without having to work out which block a row came from, and admits nothing that is not a Gherkin tag of this scenario. No canonical feature file uses per-Examples tags today, so this is latent. It was found while checking a defect the Go implementation hit in the same area, where per-scenario bookkeeping keyed by scenario name let one gated row suppress the accounting for every row of its outline. Nothing here is keyed by name -- the collector, the durations and the records are all keyed by pytest node id, which is unique per row -- and the test added here confirms that every row of an outline is still reported when one of them is gated. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The emitter defined its own per-scenario result list: a four-value outcome enum, a tag list, a reason, and a field naming which Scenario Outline row an entry came from. All of it already exists in Cucumber Messages, which is maintained, cross-language, schema'd, and emitted natively by cucumber-jvm. The report schema was reshaped to reference a Messages payload rather than define one (open-feature/spec#425); this follows it. A run now writes two files per suite: <name>.json, the envelope, and <name>.ndjson, the results it points at, with results.digest over the exact bytes written. Deleted, because Messages carries them: scenarios[] - now TestCase/TestCaseStarted/TestStepFinished/TestCaseFinished. the outcome enum - Cucumber's own seven statuses. The declared/not-applicable distinction was never a property of the run: it follows from the declaration and the scenario's tags, so it is stated once in the envelope instead of once per scenario. example - a pickle's astNodeIds are [scenario id, table row id], and the row id resolves in the GherkinDocument to the cells the feature file wrote. Four implementations were each reinventing this field by hand. tck.assetsTree - the payload carries the executed feature Source verbatim, which answers "did two runs ask the same questions" directly rather than by proxy. Two things Messages cannot carry, so they stay. The declaration is an input to reading the results, not a summary of them. And no standard results format has a slot for the tested subject: Messages records the runtime and the OS, not what was being asked about. pytest-bdd emits no Messages -- it ships the legacy Cucumber JSON format -- so messages.py assembles the stream. Two dependencies, each doing the half it owns: cucumber-messages, the official Python types from the protocol's own repository, for the execution messages; gherkin-official, already a transitive dependency of pytest-bdd, for the gherkinDocument and pickle payloads, which are used as it produces them rather than round-tripped through another representation. The feature files are parsed again because pytest-bdd's own dataclasses drop the AST node ids a pickle refers to. Step results come from pytest-bdd's step hooks rather than from the scenario's verdict, because a stream that marked all eight steps of a scenario failed would be saying something untrue about the seven that passed and the ones never reached. Each test case also carries a before- and after-hook TestStep: pytest runs three phases and only the middle one executes steps, so that is where a capability skip's reason and a teardown failure belong. A verdict no step accounts for -- a strict xfail that passes -- is attached to the after-hook, so it survives a consumer computing the test case's status as the worst of its steps. An expected failure is still a failure in the payload. The acknowledgement moved to the envelope's knownDeviations, declared by TckConfig.known_deviations, where it records the gap without softening the result. TckConfig also gains not_applicable, for a capability that cannot hold rather than one the provider declines. Verified locally; CI does not run on this branch, which targets the report branch rather than main. Both suites' envelopes validate against the reshaped schema with a Draft 2020-12 validator and their digests match; both streams validate clean against the Cucumber Messages JSON schema at v34.2.0 (661 messages each, zero errors). The stream accounts for all 29 collected scenarios; the five the capability gate stopped are SKIPPED for every step, none PASSED, and the one row the SDK fails is FAILED while pytest exits zero. 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. This implementation is on 34.2.0 while the Go TCK builds against v21 and cucumber-jvm ships a different release again, 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. It reuses the function that already computes the stream's own Meta protocolVersion rather than adding a second source, so the envelope and the stream cannot disagree about which release produced it. That function reads the version from the installed distribution rather than declaring it, so a dependency bump cannot leave the report claiming the old one. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…he backend Two reports of the same kind of provider disagreed about whether they described an in-process backend: the JavaScript in-memory suite said so, the Python one stayed silent. Not because the backends differ, but because the self-test control never offered the optional attribute that reports it. The field is optional in the report and the attribute is optional here, both so that introducing it left no existing control incomplete. Together they make omission invisible: the suite passes, the report validates, and the field is simply absent. It surfaced only when reports from four languages were compared side by side. PlainMemoryControl now reports in-process, which is what it is -- the in-memory provider is rebuilt in this process for every scenario and there is no backend to drive. More usefully, a control that reports nothing now says so in the run output. Every control either drives a real backend over the normative HTTP API or manipulates one in process, so there is no third case an absent value legitimately describes, and an adopter had no way to discover their report had a hole in it. Written to the terminal rather than failing the run, because a missing optional field is not a conformance problem -- it is a gap in what the report can say. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…rcion The tag was named for a stricter rule than the specification asks for, and it 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 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, and flagd's own testbed is gaining @numeric-coercion scenarios -- two names for one property is the drift a shared vocabulary exists to prevent. The specification renamed the tag and corrected the rule in open-feature/spec dc4d7ae8; this follows it. So Capability.STRICT_NUMERIC_TYPING becomes Capability.NUMERIC_COERCION, the marker and tag become numeric-coercion and @numeric-coercion, and the docstring states the rule that now holds rather than the one it was named for. The pytest marker registration needs no change: it iterates the enum. The submodule bump also carries two unrelated spec changes into the executed assets -- a lifecycle scenario renamed, and POST /start required not to return until the seeded flag state is being served. Neither is referenced by name here. Two gaps are recorded rather than closed, in the capability docstring and the README, because closing either is a change to every language at once. The lossless half of the contract has no scenario: the canonical flag set contains no integral float to ask it of, so a provider that wrongly rejects 10.0 as an integer still passes. And accessor width is not modelled at all -- the ADR distinguishes a 64-bit integer accessor from a 32-bit one, and this suite is silent about it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
@targeting and @caching exist in the vocabulary and no scenario carries either. The enum said so in a docstring and left it there, which reads as documentation rather than as the rule it is: a capability nothing carries cannot be verified, cannot produce a skip, and tells a reader of a conformance report only that something was claimed and nothing examined. It is a live defect rather than a hypothetical one. A real Java report asserts both tags as declared -- not by anyone's decision, but because that adoption declares "every capability except X" and picks up every reserved tag on the way past. The report schema now forbids it: see the declaration.declared description in open-feature/spec. So the set is written down once, as RESERVED_CAPABILITIES, and read everywhere else -- by Capability.reserved, by the declare-everything helper, and by the validation in TckConfig -- so the list cannot drift from the rule. ALL_CAPABILITIES becomes DECLARABLE_CAPABILITIES: the vocabulary minus the reserved tags, and named for what it is rather than for "all", because the declare-everything convenience is precisely the route a reserved tag takes into a report by accident. It is also TckConfig.capabilities' default, so a suite that does not narrow its capabilities no longer declares a tag nothing tests. An adopter who names a reserved capability explicitly gets a ValueError from TckConfig rather than a warning or a silent drop. TckConfig already refuses a capability claimed as both declared and not-applicable, and this is the same class of error -- a claim that cannot be true -- caught in the same place, where the adopter's own code is still on the stack. A silent drop would make a rejected configuration look like an accepted one; a warning is a line of CI output nobody reads while an untested capability goes on being asserted in a published report, which is how it got into one. Naming one in not_applicable is refused too: an impossibility recorded about a question never asked reaches the same declaration block. Nothing filters the declaration at emission time, and report.py says why: by the time an envelope is built, a reserved tag cannot be in the TckConfig at all. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A provider is rarely only a provider. flagd has `fractional` targeting, another
vendor has a proprietary rollout rule, and pinning those used to mean a second
harness beside the conformance suite: a second backend lifecycle, a second set
of fixtures, a second thing to keep working.
An adopter's scenarios now run inside the canonical suite instead -- same
session, same provider registration, same backend control. Almost nothing was
needed to make that happen, because pytest already scans: it collects
`conftest.py` on its own and pytest-bdd resolves step definitions through the
fixture system, so a step an adopter writes beside their test module is already
in scope for the scenarios generated into it. The only thing pytest cannot find
by itself is the feature files, because the canonical ones live inside the
installed distribution. `feature_paths()` returns both -- the packaged assets,
and a `tck-extensions` directory beside the calling module if there is one --
so an adoption gains one call and no configuration:
scenarios(*feature_paths())
An extension must never be able to stand in for a canonical scenario. Java's
suite found that a same-named feature file in a second classpath root replaced
the canonical one outright and the run went green having asked the adopter's
questions; Python has a narrower route to the same place, because pytest-bdd
names a feature file by its parent directory joined to its own name and
`tck-extensions/features/errors.feature` therefore arrives under the uri the
canonical `errors.feature` already occupies.
So the uri a feature file reaches the results payload under is derived from
where the file is: `features/` for the packaged assets and nothing else,
`extensions/` for anything below a `tck-extensions` directory -- the same prefix
the Go and JavaScript suites mount extensions under, so a consumer holding
reports from several languages applies one rule. The two cases the derivation
cannot rule out are refused rather than documented, and no report is written for
either: a file of the adopter's own that would reach the reserved `features/`
prefix, and two feature files that would share one uri, which a Messages stream
cannot carry because it holds one source per uri.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The capability gate rules out the loud way a conformance suite can go green on scenarios it did not run: an undeclared capability is reported as skipped, with its reason, never as passed. Nothing ruled out the quiet way, where the scenarios were never collected at all. `-k`, `-m`, `--deselect`, or a test module that stopped calling `scenarios()` on the canonical path each run less of the suite, and none of them is an error to pytest. Go measured the consequence: `-run` on a single scenario passed green and emitted a well-formed report covering one of twenty-nine canonical scenarios, with nothing in the document saying so. So every run is now checked against the scenarios this distribution ships. The expectation is compiled from the packaged feature files with the same Gherkin compiler that produces the results payload, which makes it one entry per Scenario Outline row -- the granularity the runner generates, and therefore the only one a comparison can be made at. A suite that did not execute all of them fails the run and writes no report, naming the scenarios that are missing. Two things may not close a gap. A scenario the capability gate skipped counts as having run, because it was asked and the report accounts for it with a reason. An adopter's own scenarios do not count at all: they are matched by path against the packaged assets rather than by the uri the emitter derives, so the check does not rest on the same derivation it exists to corroborate. `PROVIDER_TCK_PARTIAL=1` buys a green run for someone working on one scenario, and nothing else -- an incomplete suite writes no report either way. Java's TCK spells the same escape hatch the same way. Two adjustments fall out of it. Scenarios are enumerated `trylast` so that pytest's own deselection has already happened, or a filtered run reports every deselected scenario as collected but never run and drowns the one message that matters. And the self-test for per-Examples tags now runs its feature file as an extension beside the canonical set, because a suite that leaves the canonical set out no longer produces a report to read back. Separable from the extension work by design: it guards a bypass rather than enabling anything, and dropping it leaves the extension point unaffected. 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
from
September 11, 2026 10:09
7ec7e43 to
51cbcb3
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.
Stacked on #413. Answers @toddbaert's review request on open-feature/spec#423: an adopter with provider-specific behaviour — flagd's
fractional, a vendor rollout rule — must be able to run their own scenarios in the same backend lifecycle as the canonical suite instead of maintaining a parallel harness. Tracking: open-feature/spec#417.What an adopter writes
A
tck-extensions/directory beside the test module, and step definitions in their ownconftest.py:No registration, no option, no new argument. pytest collects
conftest.pyitself and pytest-bdd resolves steps through the fixture system, so a step written beside the test module is already in scope for the scenarios generated into it, with the canonical step vocabulary available alongside it;tck_statehands it the provider the suite registered. The only thing pytest cannot find on its own is the canonical feature files, which live inside the installed distribution —feature_paths()returns those plus the extension directory. The call is the same with or without extensions, andscenarios(features_path())still works and still sees only the canonical set.Go and JavaScript have no runtime scanning and took explicit options; Java scans the classpath for the same
tck-extensionsname. Python needs neither.Shadowing
A scenario's uri in the results payload is derived from where its file is:
features/for the packaged assets and nothing else,extensions/for anything below atck-extensionsdirectory — the same prefix Go and JavaScript use. This is load-bearing rather than cosmetic. pytest-bdd names a feature file by its parent directory joined to its own name, sotck-extensions/features/errors.featurearrives asfeatures/errors.feature, the uri the canonical file already occupies, and a Messages stream carries one source per uri.tests/test_extensions.pyruns that exact layout and checks both files reach the stream under distinct uris with their own sources.Two cases the derivation cannot rule out are refused, and no report is written for either: a file of the adopter's own that would reach the reserved
features/prefix, and two feature files that would share one uri.Second commit: canonical-set guard
Separable — it guards a bypass rather than enabling anything, and dropping it leaves the extension point unaffected.
A run that executed less than the canonical set fails and writes no report, naming what is missing. A capability-gated skip counts as having run; extension scenarios do not count and cannot close a gap.
PROVIDER_TCK_PARTIAL=1buys a green run for someone working on one scenario with-k, never a report — the same escape hatch Java has. It also makes scenario enumerationtrylastso a filtered run no longer reports every deselected scenario as collected but never run.Verification
Local only:
build.ymltriggers on PRs targetingmain, so a stacked PR gets no CI.pytest tests: 132 passed, 9 skipped, 2 xfailed.ruff check,ruff format --checkandmypyclean.conformance-report.schema.json(Draft 2020-12) on the spec'sfeat/provider-tck-report-schemabranch.scenarios(features_path())did before.