Skip to content

Commit f165522

Browse files
committed
refactor(provider-tck): carry results in Cucumber Messages
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>
1 parent b9588b0 commit f165522

13 files changed

Lines changed: 1979 additions & 687 deletions

File tree

tools/openfeature-provider-tck/README.md

Lines changed: 82 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ of, so a provider that wrongly rejects `10.0` as an integer still passes; adding
128128
set for every language at once. Appendix F records that as an open gap, together with a second one:
129129
the width of a language's integer accessor — 64-bit against 32-bit — is not modelled at all.
130130

131+
For a capability that *cannot* hold rather than one you chose not to declare, use
132+
`not_applicable={Capability.X: "why"}`. The suite treats it identically — the scenarios are skipped
133+
either way — but the report keeps the two apart, because collapsing them misrepresents a provider:
134+
declining an optional feature is a choice, and an impossibility is not.
135+
131136
## Controlling the backend
132137

133138
`BackendControl` is the single seam between the scenarios and whatever manipulates the backend. Step
@@ -177,8 +182,10 @@ This is **Python-specific** — the identical scenario passes in every other lan
177182
is a fair advertisement for having more than one implementation. Tracked as
178183
[open-feature/python-sdk#619](https://github.com/open-feature/python-sdk/issues/619).
179184

180-
The self-test marks that one row `xfail(strict=True)` with a pointer to the issue, so it stays
181-
visible in the report and un-hides itself automatically once the SDK is fixed.
185+
The self-test marks that one row `xfail(strict=True)` with a pointer to the issue, so the run
186+
un-hides itself automatically once the SDK is fixed, and declares it in
187+
`TckConfig.known_deviations`, so the report acknowledges it. The results payload still reports the
188+
scenario as `FAILED`: the acknowledgement records the gap, it does not soften it.
182189

183190
### 2. The in-memory provider cannot update its flag set
184191

@@ -220,59 +227,90 @@ This mirrors what `openfeature-flagd-api-testkit` already does for the flagd tes
220227

221228
## Conformance reports
222229

223-
Set `PROVIDER_TCK_REPORT_DIR` and each suite writes a machine-readable record of its run to
224-
`<dir>/<name>.json`, conforming to the [report schema][report-schema] in the specification.
230+
Set `PROVIDER_TCK_REPORT_DIR` and each suite writes **two** files: an envelope at `<dir>/<name>.json`,
231+
conforming to the [report schema][report-schema] in the specification, and the results it points at
232+
at `<dir>/<name>.ndjson`, which is a [Cucumber Messages][messages] stream.
225233

226234
```console
227235
$ PROVIDER_TCK_REPORT_DIR=./reports pytest
228-
provider-tck [in-memory]: report written to reports/in-memory.json (1 failed, 5 not-declared, 23 passed)
229-
230-
$ jq '.scenarios | group_by(.outcome) | map({(.[0].outcome): length}) | add' reports/in-memory.json
231-
{
232-
"failed": 1,
233-
"not-declared": 5,
234-
"passed": 23
235-
}
236+
provider-tck [in-memory]: report written to reports/in-memory.json with results in in-memory.ndjson (1 failed, 23 passed, 5 skipped)
237+
238+
$ jq -c .results reports/in-memory.json
239+
{"format":"cucumber-messages","location":"in-memory.ndjson","digest":"sha256:c7e12a…"}
240+
241+
$ jq -r 'select(.testStepFinished) | .testStepFinished.testStepResult.status' \
242+
reports/in-memory.ndjson | sort | uniq -c
243+
1 FAILED
244+
220 PASSED
245+
45 SKIPPED
236246
```
237247

248+
Statuses are per step, not per scenario. Of the 45 skipped, 42 belong to the five scenarios the
249+
capability gate stopped — their before-hooks included, which is where the reason is — and three are
250+
the steps of the failing scenario that were never reached.
251+
238252
It is an environment variable rather than a `TckConfig` field so that emitting a report is a property
239253
of the *run* and not of the code: CI sets it, a developer running the suite locally does not, and no
240254
adopter changes a line to publish one. Unset means no report, which is not an error. Several suites
241-
in one pytest session each write their own file, so flagd's two resolvers would not collide.
255+
in one pytest session each write their own pair, so flagd's two resolvers would not collide.
256+
257+
### Why the results are not our format
242258

243-
### Why every scenario is listed
259+
Per-scenario outcomes, tags, Scenario Outline row identity and the executed feature source are all
260+
already specified by Cucumber Messages, which is maintained, cross-language, schema'd, and emitted
261+
natively by cucumber-jvm. Defining them again in the report schema created a second format to
262+
maintain and version, and two places for the same fact to disagree. So the envelope says what was
263+
tested and what the provider claims; the payload says what happened.
264+
265+
The results are referenced rather than inlined because the stream carries the feature sources and is
266+
far larger than the envelope, and a consumer deciding whether it cares about a report should not have
267+
to fetch a whole run to find out. `results.digest` is a SHA-256 over the exact bytes written, so a
268+
consumer can tell that what it fetched is what the envelope described.
269+
270+
Two things Messages cannot carry, so they stay in the envelope. `declaration` is an *input* to
271+
reading the results rather than a summary of them: a skipped scenario says the question was not put
272+
to this provider, and only the declaration says whether that is because the provider declines the
273+
capability. And no standard results format has a slot for the tested subject — Messages records the
274+
runtime and the OS, not what was being asked about.
275+
276+
### Reading the payload
244277

245278
Appendix F requires that a scenario skipped for an undeclared capability is reported as skipped
246279
**with the reason** and never as passed. A consumer cannot check that against a summary line, so the
247-
report records the outcome of *every* scenario individually — and is required to be complete, because
248-
a document that quietly dropped what it skipped would satisfy the letter of the rule and still
249-
mislead whoever read it.
280+
stream carries every scenario the run collected, including the ones the capability gate skipped
281+
before their first step, and Cucumber's own `SKIPPED` is what it reports them as.
282+
283+
Each scenario is a `TestCase` referring to a `Pickle`, and a test case is as bad as its worst step,
284+
which is Cucumber's rule. Every test case carries two hook steps as well as its Gherkin steps: pytest
285+
runs a scenario in three phases and only the middle one executes steps, so the before-hook is where a
286+
capability skip's reason lands and the after-hook is where a teardown failure does.
287+
288+
Given a scenario's tags — in its pickle — and the envelope's `declaration`, the capability
289+
responsible for a skip follows, which is why it is no longer transported once per scenario.
250290

251-
Which also means the report is not a transcription of pytest's summary. The run above finishes green:
252-
the one scenario the Python SDK cannot satisfy is marked `xfail` (finding 1), so pytest counts it as
253-
expected and exits zero. The provider still did not satisfy it, and the document says `failed` with
254-
the reason — an expected failure is a recorded deviation, not an excused one.
291+
Which also means the payload is not a transcription of pytest's summary. The run above finishes
292+
green: the one scenario the Python SDK cannot satisfy is marked `xfail` (finding 1), so pytest counts
293+
it as expected and exits zero. The provider still did not satisfy it, and the stream says `FAILED`.
294+
The acknowledgement goes in the envelope's `knownDeviations` instead — an expected failure is a
295+
recorded deviation, not an excused one — which an adoption declares with `TckConfig.known_deviations`.
255296

256-
Four outcomes rather than two, because "did not run" is not one thing:
297+
### Which row of a Scenario Outline
257298

258-
| Outcome | Means |
259-
| --- | --- |
260-
| `passed` | the scenario ran and passed |
261-
| `failed` | the scenario ran and failed, including a known deviation marked `xfail` |
262-
| `not-declared` | skipped because the provider did not declare a capability the scenario is tagged with |
263-
| `not-applicable` | skipped for any other reason — a marker an adopter applied, a step calling `pytest.skip` |
299+
A pickle's `astNodeIds` are `[scenario id, table row id]`, and the row id resolves in the
300+
`GherkinDocument` to exactly the cells the feature file wrote. That is what tells the eleven rows of
301+
the type-mismatch matrix apart — one of which differs in outcome from its ten siblings — and it is
302+
exact rather than a naming convention every implementation has to reproduce byte-for-byte.
264303

265304
### What identifies a report
266305

267-
`tck.specRevision` and `tck.assetsTree` come from `spec_revision.json`, which `hatch_build_sync.py`
268-
generates from the submodule alongside the copied assets. It has to be captured at build time: the
269-
submodule is not in the wheel, so an installed copy has nothing left to ask. A build that cannot
270-
reach git — an unpacked sdist, say — warns and records `unknown` rather than inventing a commit.
306+
`tck.specRevision` comes from `spec_revision.json`, which `hatch_build_sync.py` generates from the
307+
submodule alongside the copied assets. It has to be captured at build time: the submodule is not in
308+
the wheel, so an installed copy has nothing left to ask. A build that cannot reach git — an unpacked
309+
sdist, say — warns and records `unknown` rather than inventing a commit.
271310

272-
The tree hash is carried as well as the commit because it identifies the assets alone. It is
273-
unchanged by unrelated edits elsewhere in the specification, so two runs that executed identical
274-
assets report the same value even when pinned to different commits — and it is checkable, since
275-
`git rev-parse <specRevision>:specification/assets/provider-tck` must reproduce it.
311+
No asset tree hash. It was carried so a consumer could tell whether two runs executed the same
312+
questions; the payload's `Source` messages carry the executed feature files verbatim, which answers
313+
that directly rather than by proxy.
276314

277315
`provider.name` is what the provider reports through its own metadata, not `TckConfig.name`.
278316
`TckConfig.name` is chosen to read well in a failure message — `flagd-rpc` — which makes it the
@@ -291,10 +329,10 @@ the field.
291329
| `test_in_memory_conformance` | the SDK's `InMemoryProvider` | reference adoption for a backend-less provider |
292330
| `test_controllable_conformance` | `ControllableInMemoryProvider` | the only suite that exercises the configuration-change path — see finding 2 |
293331
| `test_in_process_control` | `InProcessControl` | pins what the Gherkin cannot assert about itself |
294-
| `test_report` | the conformance report | checks the two properties a consumer is entitled to assume |
332+
| `test_report` | the conformance report | checks the two properties a consumer is entitled to assume, against the emitted Messages stream |
295333

296334
```
297-
78 passed, 9 skipped, 2 xfailed
335+
94 passed, 9 skipped, 2 xfailed
298336
```
299337

300338
No Docker and no network. The conformance suites take under a second; `test_report` takes most of a
@@ -311,14 +349,15 @@ what they did while the feature was gated on `@events`.
311349
cannot assert one *reached* the backend. That needs an echo operation on the control API.
312350
- **No HTTP control client yet.** It arrives with the first containerised adopter.
313351
- **Caching, hooks and flag metadata** are not covered.
314-
- **A report cannot name a Scenario Outline row portably.** Every row of an outline shares one
315-
scenario name, and the report schema has nowhere to put the row, so several entries would be
316-
indistinguishable — including, here, one that differs in outcome from its siblings. This
317-
implementation qualifies the name with pytest's example id (`... [boolean-flag-Integer-1]`), which
318-
is unambiguous but is not what another language would produce for the same row. Raised on
352+
- **The results payload is assembled here.** pytest-bdd emits no Cucumber Messages — it ships the
353+
legacy Cucumber JSON format and nothing for the ndjson protocol — so `messages.py` builds the
354+
stream from the official types and re-parses the feature files to get the AST node ids a pickle
355+
refers to. If pytest-bdd ever emits Messages itself, that module should shrink to a shim. Whether
356+
a report belongs inside a provider's released artifact is still open on
319357
[open-feature/spec#424](https://github.com/open-feature/spec/issues/424).
320358

321359
[report-schema]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/report/conformance-report.schema.json
360+
[messages]: https://github.com/cucumber/messages
322361
[appendix-a]: https://github.com/open-feature/spec/blob/main/specification/appendix-a-included-utilities.md
323362
[appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md
324363
[spec]: https://github.com/open-feature/spec

tools/openfeature-provider-tck/hatch_build_sync.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,17 @@ def sync() -> None:
8181

8282

8383
def write_revision() -> None:
84-
"""Record the spec commit and the asset tree these copies came from.
85-
86-
The tree hash is carried as well as the commit because it identifies the
87-
assets alone: it does not change when an unrelated part of the specification
88-
does, so two runs that executed identical assets report the same value even
89-
when pinned to different commits. It is also checkable rather than merely
90-
asserted, since ``git rev-parse <commit>:specification/assets/provider-tck``
91-
must reproduce it.
84+
"""Record the spec commit these copies came from.
85+
86+
The asset tree hash that used to accompany it is gone. It was carried so a
87+
consumer could tell whether two runs executed the same questions; the
88+
conformance report's results are now a Cucumber Messages stream, which
89+
carries the executed feature source itself and answers that directly rather
90+
than by proxy.
9291
"""
9392
commit = _git("rev-parse", "HEAD") or UNKNOWN_REVISION
94-
tree = _git("rev-parse", f"HEAD:{ASSETS_PATH_IN_SPEC}") or ""
9593
(DEST_BASE / REVISION_FILE).write_text(
96-
json.dumps({"specRevision": commit, "assetsTree": tree}, indent=2) + "\n",
94+
json.dumps({"specRevision": commit}, indent=2) + "\n",
9795
encoding="utf-8",
9896
)
9997

tools/openfeature-provider-tck/pyproject.toml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ dependencies = [
2222
# Same runner the flagd provider and the flagd testkit already use, so an
2323
# adopting module gains no new test framework.
2424
"pytest-bdd>=8.1.0,<9.0.0",
25+
# The conformance report's results are a Cucumber Messages stream rather
26+
# than a format this package defines. These two are the reference
27+
# implementations of the halves of that protocol: cucumber-messages is the
28+
# official Python types, published from the same repository as the protocol
29+
# itself, and gherkin-official is the parser that produces the
30+
# gherkinDocument and pickle messages. pytest-bdd already depends on
31+
# gherkin-official, so only the first is genuinely new -- and it has no
32+
# dependencies of its own.
33+
#
34+
# pytest-bdd ships no Messages emitter (its cucumber_json.py is the legacy
35+
# JSON format), so the stream is assembled here; assembling it from typed
36+
# messages rather than hand-written dicts is what keeps it from drifting
37+
# away from the protocol.
38+
"cucumber-messages>=34.0.0,<35.0.0",
39+
"gherkin-official>=29.0.0",
2540
]
2641
requires-python = ">=3.10"
2742

@@ -78,6 +93,21 @@ fixed_format_cache = true
7893
pretty = true
7994
strict = true
8095
disallow_any_generics = false
96+
# cucumber-messages and gherkin-official ship no py.typed. Both are annotated
97+
# internally, so following them gives real types for the messages this package
98+
# builds rather than the Any a plain `ignore_missing_imports` would hand back --
99+
# which is the point of using the typed library at all.
100+
follow_untyped_imports = true
101+
102+
[[tool.mypy.overrides]]
103+
# gherkin-official has no annotations at all, so following it turns every call
104+
# into a `no-untyped-call` error rather than into a type. pytest-bdd silences
105+
# the same import the same way. cucumber-messages is the opposite case -- fully
106+
# annotated, only missing py.typed -- and is followed, which is where the value
107+
# of using it rather than hand-written dicts actually lands.
108+
module = ["gherkin.*"]
109+
follow_untyped_imports = false
110+
ignore_missing_imports = true
81111

82112
[tool.coverage.run]
83113
omit = ["tests/**"]

tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,31 +50,33 @@ def tck_config():
5050
import importlib.resources
5151

5252
from .capability import ALL_CAPABILITIES, Capability
53-
from .config import TckConfig
53+
from .config import KnownDeviation, TckConfig
5454
from .control import (
5555
BackendControl,
5656
ConnectionControl,
5757
UnsupportedControlError,
5858
)
5959
from .inprocess import InProcessControl
60+
from .messages import MESSAGES_FORMAT
6061
from .provider import (
6162
CHANGING_FLAG_KEY,
6263
ControllableInMemoryProvider,
6364
canonical_flag_set,
6465
)
65-
from .report import REPORT_DIR_ENV, SCHEMA_VERSION, Outcome
66+
from .report import REPORT_DIR_ENV, SCHEMA_VERSION
6667

6768
__all__ = [
6869
"ALL_CAPABILITIES",
6970
"CHANGING_FLAG_KEY",
71+
"MESSAGES_FORMAT",
7072
"REPORT_DIR_ENV",
7173
"SCHEMA_VERSION",
7274
"BackendControl",
7375
"Capability",
7476
"ConnectionControl",
7577
"ControllableInMemoryProvider",
7678
"InProcessControl",
77-
"Outcome",
79+
"KnownDeviation",
7880
"TckConfig",
7981
"UnsupportedControlError",
8082
"canonical_flag_set",

0 commit comments

Comments
 (0)