Skip to content

Commit 3fce71e

Browse files
committed
feat(provider-tck): refuse a report that did not run the canonical set
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>
1 parent 7e39639 commit 3fce71e

7 files changed

Lines changed: 652 additions & 32 deletions

File tree

‎tools/openfeature-provider-tck/README.md‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,34 @@ of the *run* and not of the code: CI sets it, a developer running the suite loca
384384
adopter changes a line to publish one. Unset means no report, which is not an error. Several suites
385385
in one pytest session each write their own pair, so flagd's two resolvers would not collide.
386386

387+
### A partial run is not a conformance run
388+
389+
The canonical scenario set is fixed by the specification, and a run that executed less of it cannot
390+
support a conformance claim. `-k`, `-m`, `--deselect`, or a test module that stopped calling
391+
`scenarios()` on the canonical path each run fewer scenarios, and none of them is an error to
392+
pytest. Go measured the consequence: `-run` on a single scenario passed green and emitted a
393+
well-formed report covering 1 of 29 canonical scenarios, with nothing in the document saying so.
394+
395+
So every run is checked against the scenarios this distribution ships, and a suite that did not
396+
execute all of them writes no report:
397+
398+
```console
399+
$ PROVIDER_TCK_REPORT_DIR=./reports pytest -k "unknown_flag_key"
400+
provider-tck [in-memory]: 28 of 29 canonical scenarios did not run, so this run cannot support a
401+
conformance claim and no report is written for it. …
402+
- features/errors.feature: A float flag is not silently narrowed to an integer
403+
- features/errors.feature: Requesting the wrong type returns the code default [key=float-flag requested=Boolean default=false]
404+
… and 18 more
405+
```
406+
407+
A scenario the capability gate skipped **has** run: it was asked, and the report accounts for it
408+
with its reason, so declining a capability never trips this. Your own scenarios are yours — they are
409+
not counted towards the canonical set and cannot close a gap in it.
410+
411+
Set `PROVIDER_TCK_PARTIAL=1` to work on a single scenario without the guard failing the run. It buys
412+
a green run and nothing else: no report is written for an incomplete suite either way. Java's TCK
413+
spells the same escape hatch the same way.
414+
387415
### Why the results are not our format
388416

389417
Per-scenario outcomes, tags, Scenario Outline row identity and the executed feature source are all
@@ -464,14 +492,16 @@ the field.
464492
| `test_extensions` | an adopter's own scenarios | an extension runs inside the canonical suite, changes nothing for an adopter who has none, and cannot take a canonical scenario's identity |
465493
| `test_report` | the conformance report | checks the two properties a consumer is entitled to assume, against the emitted Messages stream |
466494
| `test_extension_reporting` | an extension in a report | it is reported under its own prefix, it changes no other field, and the two uris the derivation cannot rule out produce no report at all |
495+
| `test_canonical_set` | the canonical-set guard | a run that executed less than the canonical set fails and publishes nothing |
467496

468497
```
469-
135 passed, 21 skipped, 2 xfailed
498+
155 passed, 21 skipped, 2 xfailed
470499
```
471500

472-
No Docker and no network. The conformance suites take under a second; `test_report` and
473-
`test_extensions` take most of the rest, because the properties they check are properties of a whole
474-
pytest session and they run generated adoptions in subprocesses to check them.
501+
No Docker and no network. The conformance suites take under a second; `test_report`,
502+
`test_extension_reporting` and `test_canonical_set` take most of the time, because the properties
503+
they check are properties of a whole pytest session and they run generated adoptions in subprocesses
504+
to check them.
475505

476506
Neither in-memory suite declares `@lifecycle`, so the six lifecycle scenarios — three about
477507
initialisation, three about shutdown — are skipped in both. That is the point: with no backend to

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ def tck_config():
5252

5353
import importlib.resources
5454

55+
from .canonical import PARTIAL_ENV
5556
from .capability import DECLARABLE_CAPABILITIES, RESERVED_CAPABILITIES, Capability
5657
from .config import KnownDeviation, TckConfig
5758
from .control import (
@@ -79,6 +80,7 @@ def tck_config():
7980
"DECLARABLE_CAPABILITIES",
8081
"EXTENSIONS_DIRECTORY",
8182
"MESSAGES_FORMAT",
83+
"PARTIAL_ENV",
8284
"REPORT_DIR_ENV",
8385
"RESERVED_CAPABILITIES",
8486
"SCHEMA_VERSION",
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
"""The canonical scenario set, and whether a run actually executed it.
2+
3+
A conformance suite that goes green on scenarios it did not run is worse than no
4+
suite. The capability gate already rules out the loud version of that -- an
5+
undeclared capability is reported as skipped, with its reason, never as passed --
6+
but it says nothing about the quiet version, where the scenarios were never asked
7+
for in the first place. A ``-k`` expression, a ``-m`` filter, a ``--deselect``, a
8+
test module that stopped calling ``scenarios()`` on the canonical path: each of
9+
those runs less of the suite and none of them is an error. The Go implementation
10+
measured it. ``-run`` on a single scenario passed green and emitted a well-formed
11+
report covering one of twenty-nine canonical scenarios, and nothing in the
12+
document said so.
13+
14+
So the run is checked against what this distribution *ships* rather than against
15+
what it was asked to run. The expectation is compiled from the packaged feature
16+
files with the same Gherkin compiler that produces the results payload, which
17+
means it is one entry per Scenario Outline row -- the granularity the runner
18+
generates and therefore the only one a comparison can be made at.
19+
20+
Two properties this has to have, and both are about what may close a gap.
21+
22+
**A skip counts; an absence does not.** A scenario the capability gate skipped
23+
did run: it was asked, and the report accounts for it with a reason. A scenario
24+
that was never collected is missing, and no declaration makes it otherwise.
25+
26+
**An extension cannot close a gap.** An adopter's own scenarios are matched by
27+
neither uri nor path against the packaged set, and the executed side of the
28+
comparison is filtered to files that are genuinely inside this distribution --
29+
not merely to files reported under the canonical prefix, so that the check does
30+
not rest on the same derivation it is meant to corroborate.
31+
"""
32+
33+
from __future__ import annotations
34+
35+
import functools
36+
import os
37+
import typing
38+
39+
from .extensions import canonical_root, is_canonical, uri_for
40+
from .messages import FeatureCatalog, ScenarioKey, ScenarioRun
41+
42+
__all__ = [
43+
"PARTIAL_ENV",
44+
"canonical_scenarios",
45+
"describe",
46+
"missing_canonical",
47+
"partial_run_allowed",
48+
]
49+
50+
PARTIAL_ENV = "PROVIDER_TCK_PARTIAL"
51+
"""Set to acknowledge that a run is deliberately not a conformance run.
52+
53+
For working on one scenario with ``-k`` without the guard failing the run. It
54+
never makes a partial run publishable: no report is written for a suite that did
55+
not execute the canonical set, with or without it. The Java TCK spells the same
56+
escape hatch the same way, so the two are one thing to know rather than two.
57+
"""
58+
59+
_TRUTHY = {"1", "true", "yes", "on"}
60+
61+
62+
def partial_run_allowed(environment: typing.Mapping[str, str] | None = None) -> bool:
63+
"""Whether the run has declared itself partial."""
64+
source = os.environ if environment is None else environment
65+
return source.get(PARTIAL_ENV, "").strip().lower() in _TRUTHY
66+
67+
68+
@functools.cache
69+
def canonical_scenarios() -> frozenset[ScenarioKey]:
70+
"""Every scenario the packaged feature files define, row by row.
71+
72+
Empty when the assets are not reachable as files -- an installation from a
73+
zipimport, say. Everything built on this then degrades to "cannot tell",
74+
which is the honest answer and never a false accusation.
75+
76+
Cached because it is the same answer for the whole process and parsing it is
77+
the same work the results payload already does.
78+
"""
79+
root = canonical_root()
80+
if root is None or not root.is_dir():
81+
return frozenset()
82+
83+
catalog = FeatureCatalog()
84+
for path in sorted(root.rglob("*.feature")):
85+
uri = uri_for(path)
86+
if uri is not None:
87+
catalog.load_file(uri, path)
88+
return catalog.scenario_keys
89+
90+
91+
def missing_canonical(runs: typing.Iterable[ScenarioRun]) -> tuple[ScenarioKey, ...]:
92+
"""The canonical scenarios this suite did not execute, in reporting order.
93+
94+
``runs`` is everything the suite accounted for, extensions included; only
95+
the ones whose feature file is genuinely one of the packaged assets are
96+
counted, so an adopter's scenario can neither fill a gap nor be blamed for
97+
one.
98+
"""
99+
expected = canonical_scenarios()
100+
if not expected:
101+
return ()
102+
executed = {
103+
(run.identity.uri, run.identity.name, run.identity.example)
104+
for run in runs
105+
if is_canonical(run.identity.path)
106+
}
107+
return tuple(sorted(expected - executed))
108+
109+
110+
def describe(key: ScenarioKey) -> str:
111+
"""One missing scenario, named the way a failure message should name it."""
112+
uri, name, row = key
113+
if not row:
114+
return f"{uri}: {name}"
115+
cells = " ".join(f"{header}={cell}" for header, cell in row)
116+
return f"{uri}: {name} [{cells}]"

‎tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/emitter.py‎

Lines changed: 102 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333

3434
import pytest
3535

36+
from .canonical import (
37+
PARTIAL_ENV,
38+
canonical_scenarios,
39+
describe,
40+
missing_canonical,
41+
partial_run_allowed,
42+
)
3643
from .config import TckConfig
3744
from .extensions import (
3845
collision_problem,
@@ -97,6 +104,14 @@
97104
_SKIPPED = pytest.skip.Exception
98105
"""What ``pytest.skip`` raises, named so a step hook can recognise it."""
99106

107+
_MAX_MISSING = 10
108+
"""How many missing canonical scenarios a failure names before summarising.
109+
110+
Enough to act on and not so many that the reason is lost above them. A run with
111+
one scenario selected is missing twenty-eight, and listing all of them says
112+
nothing the count did not.
113+
"""
114+
100115

101116
def scenario_identity(node: pytest.Item) -> ScenarioIdentity | None:
102117
"""Describe a pytest node as a Gherkin scenario, or return ``None``.
@@ -222,11 +237,19 @@ def __init__(self, config: pytest.Config) -> None:
222237
self._step_started: dict[str, int] = {}
223238
config.stash[COLLECTOR_KEY] = self.collector
224239

240+
@pytest.hookimpl(trylast=True)
225241
def pytest_collection_modifyitems(self, items: list[pytest.Item]) -> None:
226-
"""Enumerate every TCK scenario the session collected.
242+
"""Enumerate every TCK scenario the session will run.
227243
228244
At collection rather than as each runs, so that the stream accounts for
229245
scenarios that never got as far as running a fixture.
246+
247+
``trylast`` so that pytest's own deselection -- ``-k``, ``-m``,
248+
``--deselect`` -- has already removed what it is going to remove.
249+
Enumerating before it does would make a filtered run report every
250+
deselected scenario as collected but never run, which is a true
251+
statement about a list nobody asked for and drowns the one message that
252+
matters: which canonical scenarios are missing.
230253
"""
231254
for item in items:
232255
identity = scenario_identity(item)
@@ -294,24 +317,48 @@ def _finish_step(
294317
)
295318

296319
def pytest_sessionfinish(self, session: pytest.Session) -> None:
320+
if session.config.getoption("collectonly", False):
321+
# Nothing ran, and nothing was meant to. Every check below asks what
322+
# a run executed, and the answer "nothing" is not a finding here.
323+
return
324+
325+
problems = self.collector.resolve(scenario_run)
326+
327+
# Whether a suite may be published is a property of the run rather than
328+
# of the report, so it is established whether or not one was asked for.
329+
# Both checks are made on every suite rather than short-circuited, so a
330+
# suite with two faults hears about both.
331+
unpublishable: set[int] = set()
332+
for suite in self.collector.suites:
333+
sound = self._identities_are_sound(session, suite)
334+
complete = self._canonical_set_ran(session, suite)
335+
if not sound or not complete:
336+
unpublishable.add(id(suite))
337+
297338
directory = os.environ.get(REPORT_DIR_ENV, "").strip()
298339
if not directory:
299340
return
300-
self.write(session, Path(directory))
341+
for problem in problems:
342+
self._fail(session, f"provider-tck: {problem}")
343+
self.write(session, Path(directory), unpublishable)
301344

302-
def write(self, session: pytest.Session, directory: Path) -> None:
345+
def write(
346+
self,
347+
session: pytest.Session,
348+
directory: Path,
349+
unpublishable: typing.AbstractSet[int] = frozenset(),
350+
) -> None:
303351
"""Write every suite's pair of files, failing the session if one cannot be.
304352
305353
A run that asked for a report and silently did not get one is how a
306354
publishing pipeline ends up serving a stale result forever, so both a
307355
write failure and an incomplete document are loud and change the exit
308356
status rather than being logged and forgotten.
309357
"""
310-
for problem in self.collector.resolve(scenario_run):
311-
self._fail(session, f"provider-tck: {problem}")
312-
313358
written: dict[str, str] = {}
314359
for suite in self.collector.suites:
360+
if id(suite) in unpublishable:
361+
continue
315362
name = suite.config.name
316363
file_name = envelope_file_name(name)
317364
if written.get(file_name, name) != name:
@@ -324,19 +371,56 @@ def write(self, session: pytest.Session, directory: Path) -> None:
324371
written[file_name] = name
325372
self._write_suite(session, directory, suite)
326373

374+
def _canonical_set_ran(self, session: pytest.Session, suite: SuiteReport) -> bool:
375+
"""Whether this suite executed every scenario the TCK ships.
376+
377+
The check a conformance claim rests on that no amount of reading the
378+
report can supply: the results payload says what happened to the
379+
scenarios that ran, and says nothing at all about the ones that did not.
380+
381+
A capability-gated skip counts -- it was asked, and the report accounts
382+
for it with a reason. An extension scenario does not count and cannot
383+
close a gap. :data:`~.canonical.PARTIAL_ENV` downgrades the failure to a
384+
note for someone working on a single scenario; it does not make the run
385+
publishable, because the report is withheld either way.
386+
"""
387+
missing = missing_canonical(suite.runs.values())
388+
if not missing:
389+
return True
390+
391+
name = suite.config.name
392+
total = len(canonical_scenarios())
393+
headline = (
394+
f"provider-tck [{name}]: {len(missing)} of {total} canonical scenarios "
395+
f"did not run, so this run cannot support a conformance claim and no "
396+
f"report is written for it. The canonical set is fixed by the "
397+
f"specification; running less of it is not a configuration. Decline "
398+
f"capabilities your provider does not have through TckConfig instead, "
399+
f"which reports the scenarios as skipped with their reason"
400+
)
401+
if partial_run_allowed():
402+
self._say(
403+
session,
404+
f"{headline}. {PARTIAL_ENV} is set, so the run is not failed for it",
405+
)
406+
else:
407+
self._fail(
408+
session,
409+
f"{headline}. To filter anyway while working on one scenario, set "
410+
f"{PARTIAL_ENV}=1 and accept that the run is not a conformance run",
411+
)
412+
for key in missing[:_MAX_MISSING]:
413+
self._say(session, f" - {describe(key)}")
414+
if len(missing) > _MAX_MISSING:
415+
self._say(session, f" ... and {len(missing) - _MAX_MISSING} more")
416+
return False
417+
327418
def _write_suite(
328419
self, session: pytest.Session, directory: Path, suite: SuiteReport
329420
) -> None:
330421
name = suite.config.name
331422
runs = suite.sorted_runs
332423

333-
if not self._identities_are_sound(session, suite):
334-
# Refusing to write is the point: a document that presents an
335-
# adopter's feature file as the specification's -- or reports one
336-
# file's scenarios against another's source -- is worse than no
337-
# document, because it is the one thing a consumer cannot check.
338-
return
339-
340424
catalog = FeatureCatalog()
341425
try:
342426
for run in runs:
@@ -406,6 +490,11 @@ def _identities_are_sound(
406490
has no way to tell that the specification did not write it. And two
407491
files must not share a uri, or the stream carries one source for both
408492
and the second file's scenarios are reported against the first's.
493+
494+
A suite that fails either writes no report. Refusing is the point: a
495+
document that presents an adopter's feature file as the specification's
496+
is worse than no document, because it is the one thing a consumer cannot
497+
check.
409498
"""
410499
name = suite.config.name
411500
runs = suite.sorted_runs

0 commit comments

Comments
 (0)