From 8662b36fb16e0095d9ab2fbfe4d2914abe5501d4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 24 Aug 2026 15:15:56 +0200 Subject: [PATCH 01/22] test(ofrep): run the provider conformance suite against flagd's OFREP API Adopts the OpenFeature provider conformance suite in the OFREP provider. No new infrastructure. flagd serves the OFREP API on port 8016 alongside its own protocols, and flagd-testbed's compose file already publishes it, so the OFREP provider runs against the existing testbed, seeded with the same canonical flag set, driven through the same launchpad control API as the flagd suites. Running two providers against one backend is the point of a cross-provider conformance suite: a difference in the results is a difference an application would see when it switches provider. tests/e2e/flagd_container.FlagdContainer would have been the natural thing to reuse and is not importable here -- a package's tests are not part of its distribution -- so tests/tck/testbed.py drives compose directly. What it duplicates is deliberately minimal: compose up, read two mapped ports, poll /readyz. This is a concrete instance of the "no shared containerised-backend helper" gap the TCK's README records. Two capabilities, both on the strength of a line of provider code rather than of a green run: OBJECT and STRICT_NUMERIC_TYPING. The same two the Go and Java OFREP adoptions reached independently, from the same architecture. Every omission is a fact about the provider. OFREPProvider is stateless -- it holds a requests.Session and a rate-limit timestamp, and nothing else survives between evaluations. It does not override initialize, so it inherits AbstractProvider's, which is `pass`, and it never emits: `_on_emit` is not called anywhere in the provider. So EVENTS, STALE and CONFIGURATION_CHANGE have nothing behind them, and UNAVAILABLE_INIT is false in the strong sense -- a provider pointed at a closed port reaches READY, because the SDK's registry dispatches PROVIDER_READY around an initialize that does nothing. events.feature and lifecycle.feature are gated at feature level and skip with their reasons; 24 of the 29 scenarios run. @lifecycle, which lands on the TCK branch this is stacked under, would also be withheld once it is available here: nothing contacts the backend before the first evaluation, so initialisation has no outcome to observe. That capability was split out of EVENTS precisely so a stateless provider can decline it accurately, and this is the case it was split out for. One scenario is marked xfail(strict=True): boolean-flag requested as an Integer. OFREP is untyped on the wire -- the request carries no type and the backend returns the JSON value regardless -- so the whole type check is the provider's, and it is isinstance(value, int), which bool is a subclass of in Python. The value True comes back with reason STATIC and no error code where the specification requires the code default and TYPE_MISMATCH. The SDK client type-checks the same way, so this is the provider-side half of open-feature/python-sdk#619 and fixing one half is not enough. Strict, so the marker fails the suite once it starts passing rather than lingering as a lie. Recorded as a finding: POST /start returns before the backend serves the flag set. The control API specifies that /start reseeds flag state; it does not specify that it returns only once that state is being served, and flagd-testbed's launchpad returns as soon as flagd answers /readyz, which is roughly 40ms before its file sources are in the flag store. The flagd suites never see this because both resolvers block inside initialize until the stream is up or the ruleset has synced, absorbing the window. A stateless provider is the first adopter with no initialisation to hide a backend's warm-up behind, and its first evaluation lands squarely in the gap -- reported, before the fix, as FLAG_NOT_FOUND on every flag. SettledControl closes it by delegating to HttpControl and then polling the public OFREP endpoint until the flag set is actually served. It manipulates nothing and weakens no scenario, but "reseeded" and "serving" should be the same instant in the control API contract, and until they are this belongs in the adoption. 23 passed, 5 skipped, 1 xfailed. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/pyproject.toml | 11 ++ .../tests/tck/__init__.py | 0 .../tests/tck/conftest.py | 79 ++++++++ .../tests/tck/settled_control.py | 136 ++++++++++++++ .../tests/tck/test_ofrep_conformance.py | 153 ++++++++++++++++ .../tests/tck/testbed.py | 172 ++++++++++++++++++ uv.lock | 6 + 7 files changed, 557 insertions(+) create mode 100644 providers/openfeature-provider-ofrep/tests/tck/__init__.py create mode 100644 providers/openfeature-provider-ofrep/tests/tck/conftest.py create mode 100644 providers/openfeature-provider-ofrep/tests/tck/settled_control.py create mode 100644 providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py create mode 100644 providers/openfeature-provider-ofrep/tests/tck/testbed.py diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index bb054ad55..fbe7a04be 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -29,12 +29,23 @@ Homepage = "https://github.com/open-feature/python-sdk-contrib" dev = [ "coverage[toml]>=7.10.0,<8.0.0", "mypy>=1.18.0,<2.0.0", + # The OpenFeature provider conformance suite. Ships the feature files, the flag + # set and the control-API client, and registers its step definitions through a + # pytest11 entry point, so tests/tck needs no conftest of its own for them. + "openfeature-provider-tck", "poethepoet>=0.37.0", "pytest>=9.0.0,<10.0.0", + "pytest-bdd>=8.1.0,<9.0.0", "requests-mock>=1.12.0,<2.0.0", + # Starts the flagd testbed, which serves the OFREP API on port 8016 alongside + # flagd's own protocols. See tests/tck/testbed.py. + "testcontainers>=4.12.0,<5.0.0", "types-requests>=2.32.0,<3.0.0", ] +[tool.uv.sources] +openfeature-provider-tck = { workspace = true } + [tool.uv.build-backend] module-name = "openfeature" module-root = "src" diff --git a/providers/openfeature-provider-ofrep/tests/tck/__init__.py b/providers/openfeature-provider-ofrep/tests/tck/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py new file mode 100644 index 000000000..c692ab0da --- /dev/null +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -0,0 +1,79 @@ +"""Session fixtures for the OFREP conformance suite, and one recorded deviation. + +The stack is started once and never restarted, because compose assigns host +ports dynamically and cannot preserve them across a restart: a restarted backend +comes back on a different port, silently invalidating a provider already pointed +at the old one, and the failure reads as a flaky provider rather than a broken +test. Scenario isolation comes from the control API instead -- see the +no-container-restart invariant in the TCK's ``control-api.yaml``. +""" + +from __future__ import annotations + +import typing + +import pytest + +from openfeature.contrib.tools.provider_tck import HttpControl +from tests.tck.settled_control import SettledControl +from tests.tck.testbed import FlagdTestbed, running_testbed + + +@pytest.fixture(scope="session") +def flagd_testbed() -> typing.Iterator[FlagdTestbed]: + """The testbed stack, up for the whole session.""" + yield from running_testbed() + + +@pytest.fixture(scope="session") +def ofrep_control(flagd_testbed: FlagdTestbed) -> SettledControl: + """The control API client, pointed at the testbed's launchpad. + + The launchpad registers only ``/start``, ``/restart``, ``/stop`` and + ``/change`` (flagd-testbed ``launchpad/main.go:29-32``), so ``/reset`` + answers 404 and every ``prepare_scenario`` takes the documented ``/start`` + fallback. The probe costs one 404 for the whole session. + + Wrapped in :class:`SettledControl` because ``/start`` returns before the + backend serves the flag set, and a stateless provider has no initialisation + to hide that window behind. See that module -- it is a finding about the + control API's guarantee, not a convenience. + """ + return SettledControl( + HttpControl(flagd_testbed.get_launchpad_url()), + flagd_testbed.get_ofrep_url(), + ) + + +# --------------------------------------------------------------------------- +# One known deviation, recorded rather than hidden. +# +# A conformance suite that quietly goes green on a scenario it ran and failed is +# as bad as one that goes green on a scenario it skipped. So the single scenario +# this provider cannot satisfy is marked xfail(strict=True), which keeps it in +# the report with its reason attached and fails the suite the moment it starts +# passing -- so the marker is removed when the bug is fixed rather than +# lingering as a lie. Same mechanism, and same bug, as the TCK's own self-test +# (tools/openfeature-provider-tck/tests/conftest.py). + +_BOOL_AS_INT = ( + "test_requesting_the_wrong_type_returns_the_code_default[boolean-flag-Integer-1]" +) + +_REASON = ( + "bool satisfies an Integer request. OFREP is an untyped protocol -- the " + "backend returns the JSON value with no knowledge of the requested type -- so " + "the whole type check is the provider's, at ofrep/__init__.py:244-256: " + "FlagType.INTEGER maps to `int` and the check is isinstance(value, int), which " + "bool is a subclass of in Python. boolean-flag requested as an Integer " + "therefore returns True with reason STATIC and no error code, where the " + "specification requires the code default and TYPE_MISMATCH. The Python SDK " + "client type-checks the same way, so fixing only one of the two is not enough. " + "See https://github.com/open-feature/python-sdk/issues/619" +) + + +def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: + for item in items: + if item.name == _BOOL_AS_INT: + item.add_marker(pytest.mark.xfail(reason=_REASON, strict=True)) diff --git a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py new file mode 100644 index 000000000..e834b47b7 --- /dev/null +++ b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py @@ -0,0 +1,136 @@ +"""``HttpControl``, plus a wait for the backend to actually serve the flag set. + +**The problem this exists for is worth stating carefully, because it is a +finding rather than a workaround.** + +``POST /start`` is specified to reseed flag state to the named configuration's +baseline (normative requirement 2 in the TCK's ``control-api.yaml``). It is not +specified to *return only once that state is being served*, and flagd-testbed's +launchpad does not: it returns as soon as flagd answers ``/readyz`` +(``launchpad/pkg/flagd.go``), which flagd does before its file sources have been +loaded into the flag store. Measured against this testbed, the window is short +-- around 40ms -- but it is real and reliably hit: + + start:200 {"errorCode":"FLAG_NOT_FOUND","errorDetails":"flag `float-flag` does not exist"} + start:200 {"value":0.5,"key":"float-flag","reason":"STATIC","variant":"half"} + start:200 {"errorCode":"FLAG_NOT_FOUND","errorDetails":"flag `float-flag` does not exist"} + +The flagd suites never see it, and that is the interesting part. Both flagd +resolvers block inside ``initialize`` until the evaluation stream is up or the +ruleset has synced, so their initialisation absorbs the window before any +scenario evaluates. OFREP is stateless -- no ``initialize``, no connection, no +warm-up -- so its first evaluation lands directly in the gap and the suite +reports FLAG_NOT_FOUND for every flag, which reads as a catastrophically broken +provider. + +**A stateless provider is the first adopter with no initialisation to hide a +backend's warm-up behind**, which makes it the one that discovers whether the +control API's guarantee is strong enough. It is not: "reseeded" and "serving" +need to be the same instant, or every stateless provider reimplements this. That +belongs in the control API contract, and until it is there it belongs here. + +**Why this is not cheating.** It manipulates nothing. It is a readiness probe +over the same public OFREP endpoint the provider uses, on a canonical flag, +asserting only that the backend has finished doing what ``/start`` already +promised. No scenario is weakened, no step is bypassed, and no side channel into +the backend is opened -- the normative control path is still ``HttpControl``, +which this delegates to unchanged. + +Deliberately not a :class:`ConnectionControl`: it has no ``disconnect`` or +``reconnect``, matching a suite that declares neither ``STALE`` nor +``UNAVAILABLE_INIT``. The two omissions keep each other honest. +""" + +from __future__ import annotations + +import json +import time +import urllib.error +import urllib.request + +from openfeature.contrib.tools.provider_tck import HttpControl + +__all__ = ["SettledControl"] + +PROBE_FLAG_KEY = "boolean-flag" +"""A canonical flag, used only to ask whether the flag set is being served yet.""" + +SETTLE_TIMEOUT_SECONDS = 15.0 +"""How long to wait for the backend to serve the flag set after ``/start``. + +Two orders of magnitude above the ~40ms observed, because the cost of being +generous is nothing -- the loop exits on the first success -- while the cost of +being tight is a suite that fails intermittently on a loaded CI runner and gets +diagnosed as a provider bug. +""" + +SETTLE_POLL_SECONDS = 0.02 + + +class SettledControl: + """Delegates to :class:`HttpControl`, then waits for the flags to appear.""" + + def __init__( + self, + control: HttpControl, + ofrep_url: str, + *, + timeout: float = SETTLE_TIMEOUT_SECONDS, + ) -> None: + self._control = control + self._probe_url = ( + f"{ofrep_url.rstrip('/')}/ofrep/v1/evaluate/flags/{PROBE_FLAG_KEY}" + ) + self._timeout = timeout + + @property + def description(self) -> str: + return f"{self._control.description}, awaited through the OFREP endpoint" + + def prepare_scenario(self) -> None: + self._control.prepare_scenario() + self._await_flags() + + def change_flag(self) -> None: + self._control.change_flag() + + def _await_flags(self) -> None: + """Block until the probe flag resolves, or fail saying what was seen. + + Raising rather than proceeding is deliberate. A scenario allowed to run + against a backend that is not serving its flag set does not report a + harness problem; it reports FLAG_NOT_FOUND as a conformance result, + which is the one outcome a conformance suite must never produce. + """ + deadline = time.monotonic() + self._timeout + last = "no response" + + while time.monotonic() < deadline: + status, body = self._probe() + if status == 200: + return + last = f"HTTP {status}: {body}" + time.sleep(SETTLE_POLL_SECONDS) + + msg = ( + f"the backend did not serve {PROBE_FLAG_KEY!r} within {self._timeout}s of " + f"a successful control-API reseed. Last response from {self._probe_url}: " + f"{last}. This is a problem with the stack under test or its control API, " + f"not with the provider" + ) + raise RuntimeError(msg) + + def _probe(self) -> tuple[int, str]: + request = urllib.request.Request( # noqa: S310 + self._probe_url, + data=json.dumps({}).encode("utf-8"), + headers={"Content-Type": "application/json"}, + method="POST", + ) + try: + with urllib.request.urlopen(request, timeout=5.0) as response: # noqa: S310 + return int(response.status), "" + except urllib.error.HTTPError as err: + return int(err.code), err.read().decode("utf-8", "replace")[:200] + except (urllib.error.URLError, OSError) as err: + return 0, str(err) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py new file mode 100644 index 000000000..9f9b47c10 --- /dev/null +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -0,0 +1,153 @@ +"""The OpenFeature provider conformance suite, run against the OFREP provider. + +OFREP is the vendor-neutral remote evaluation protocol, so what is under test +here is a pure mapping: one HTTP request per evaluation, and the translation of +its JSON response -- or its error status -- into typed resolution details. There +is no cache, no stream and no local ruleset, so unlike the flagd suites there is +nothing here that a lifecycle could be wrong about. + +The backend is flagd, which serves OFREP on port 8016 alongside its own +protocols, driven through the same launchpad control API and seeded with the +same canonical flag set as the flagd conformance suites. Running two providers +against one backend is the point of a cross-provider conformance suite: a +difference in the results is a difference an application would see when it +switches provider. +""" + +from __future__ import annotations + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.provider.ofrep import OFREPProvider +from openfeature.contrib.tools.provider_tck import ( + Capability, + TckConfig, + features_path, +) +from openfeature.provider import FeatureProvider +from tests.tck.settled_control import SettledControl +from tests.tck.testbed import FlagdTestbed + +TIMEOUT_SECONDS = 10.0 +"""Bounds a single OFREP request. + +Generous, because every scenario is preceded by a control-API ``/start`` that +restarts the flagd process, so the first evaluation of a scenario routinely hits +a backend that came up milliseconds ago. It is the only timing knob this +provider has (``ofrep/__init__.py:52``); everything else the TCK offers -- event +timeouts, ready timeouts -- has nothing to bound, for the reasons below. +""" + +# Every capability below is declared on the strength of a line of provider code, +# not on the strength of a green run. +# +# OBJECT +# ofrep/__init__.py:105-113 resolves structured values, and the type check at +# ofrep/__init__.py:248 admits `(dict, list)` for FlagType.OBJECT -- so a JSON +# object comes back as one rather than being rejected or flattened. +# +# STRICT_NUMERIC_TYPING +# ofrep/__init__.py:250 maps FlagType.INTEGER to `int`, and the isinstance +# check at ofrep/__init__.py:255 fails for the float 0.5, raising +# TypeMismatchError. So float-flag's 0.5 is reported as a mismatch rather +# than narrowed to 0. Worth stating plainly that this is the provider's own +# doing: OFREP is untyped on the wire, the request carries no type at all, +# and flagd returns 0.5 whatever was asked for -- so unlike flagd-RPC, where +# the server answers INVALID_ARGUMENT, there is no backend here to catch a +# numeric mismatch. Every type decision in this suite is made at those two +# lines, which is also why the one deviation recorded in conftest.py lives +# there. +# +# Not declared, and why. Each is a fact about the provider, established by +# reading it -- OFREPProvider is stateless: it holds a requests.Session and a +# rate-limit timestamp, and nothing else survives between evaluations. +# +# LIFECYCLE +# Not in the Capability enum on this branch yet (it lands with +# feat/provider-tck), and it would not be declared once it does. +# OFREPProvider does not override `initialize`, so it inherits +# AbstractProvider's, which is `pass` (python-sdk +# openfeature/provider/__init__.py:138-139). Nothing contacts the backend +# before the first evaluation, so initialisation has no outcome to observe. +# lifecycle.feature is gated on @events at feature level on this branch and +# skips for that reason; when it is retagged to @lifecycle it must keep +# skipping, for this one. +# +# EVENTS +# The provider never emits. It extends AbstractProvider, so it inherits +# `attach`, but `_on_emit` is never called anywhere in +# ofrep/__init__.py -- there is no stream, no poll and no background thread +# to notice anything worth emitting about. +# +# The SDK's registry does dispatch PROVIDER_READY around `initialize` for any +# provider (python-sdk openfeature/provider/_registry.py:73-77), so declaring +# EVENTS would make lifecycle.feature's readiness scenario pass without +# demonstrating anything -- a NoOpProvider passes it identically. That is +# exactly the vacuity the @lifecycle capability was split out to end, and +# claiming the capability to collect the pass would be the dishonest use of +# it. +# +# STALE, CONFIGURATION_CHANGE +# Both are event capabilities and follow from EVENTS. There is no connection +# to lose -- every evaluation is an independent HTTP request -- so there is no +# state between them that could go stale, and nothing watches the backend for +# a configuration change. events.feature is gated @events at feature level +# and skips as a whole. +# +# Note that a *change* is nonetheless visible to an application: the next +# evaluation issues a fresh request and returns the new value. What is +# missing is the signal, and the @configuration-change scenario asserts the +# event as well as the behaviour, deliberately -- a provider that changes +# silently is not conformant, it is just not broken. +# +# UNAVAILABLE_INIT +# A provider pointed at a closed port reaches READY, because `initialize` +# does nothing and the registry dispatches PROVIDER_READY unconditionally +# (python-sdk openfeature/provider/_registry.py:73-77). The failure surfaces +# on the first evaluation as GeneralError from ofrep/__init__.py:167, not as +# PROVIDER_ERROR, so the scenario's premise does not hold. `TckConfig` also +# rejects the capability without a `new_unavailable_provider`, and none is +# supplied here for the same reason. +# +# TARGETING, CACHING +# Reserved in the Capability enum; no scenario carries either tag. Declaring +# a capability nothing exercises would be a claim with no evidence behind it. +# +# The result matches the Go and Java OFREP adoptions, which reached the same two +# capabilities from the same architecture, independently. +CAPABILITIES = frozenset( + { + Capability.OBJECT, + Capability.STRICT_NUMERIC_TYPING, + } +) + + +@pytest.fixture(scope="session") +def tck_config( + flagd_testbed: FlagdTestbed, + ofrep_control: SettledControl, +) -> TckConfig: + """Wire the provider up to the running testbed. + + The port is read here, after the stack is up: compose maps host ports + dynamically, so it does not exist earlier -- and it stays valid for the whole + session because nothing ever restarts a container. Outages, had this suite + any use for them, would be simulated inside the running stack through + ``ofrep_control``. + """ + base_url = flagd_testbed.get_ofrep_url() + + def new_provider() -> FeatureProvider: + return OFREPProvider(base_url, timeout=TIMEOUT_SECONDS) + + return TckConfig( + name="ofrep", + control=ofrep_control, + new_provider=new_provider, + capabilities=CAPABILITIES, + ) + + +scenarios(features_path()) diff --git a/providers/openfeature-provider-ofrep/tests/tck/testbed.py b/providers/openfeature-provider-ofrep/tests/tck/testbed.py new file mode 100644 index 000000000..feb6930f6 --- /dev/null +++ b/providers/openfeature-provider-ofrep/tests/tck/testbed.py @@ -0,0 +1,172 @@ +"""The flagd testbed, started for the OFREP conformance suite. + +flagd serves the **OFREP** HTTP API alongside its own protocols -- port 8016, +next to 8013 for RPC and 8015 for sync -- and ``flagd-testbed``'s compose file +already publishes it. So an OFREP provider needs no backend of its own: it runs +against the same stack, seeded with the same canonical flag set, driven through +the same launchpad control API as the flagd suites. A second stack would be a +second definition of "the canonical flags", which is the one thing a conformance +suite exists to prevent. + +**Why this is not ``tests/e2e/flagd_container.FlagdContainer``.** That helper +would be the natural thing to reuse, and it is not importable here: it lives +under ``providers/openfeature-provider-flagd/tests/``, and a package's tests are +not part of its distribution, so nothing in this package can import it. It also +depends on ``grpcio`` and ``grpcio-health-checking`` for its readiness probe, +neither of which the OFREP provider has any other reason to pull in. What is +duplicated is therefore deliberately minimal -- compose up, read two mapped +ports, poll one HTTP endpoint -- and this is a concrete instance of the "no +shared containerised-backend helper" gap the TCK's README already records. + +The compose file is reached through the flagd package's ``test-harness`` +submodule rather than a second checkout of the same repository. Run +``git submodule update --init`` if it is missing; :func:`testbed_path` says so +rather than failing inside testcontainers. +""" + +from __future__ import annotations + +import os +import tempfile +import time +import typing +import urllib.error +import urllib.request +from pathlib import Path + +from testcontainers.compose import DockerCompose + +__all__ = ["LAUNCHPAD_PORT", "OFREP_PORT", "FlagdTestbed", "testbed_path"] + +OFREP_PORT = 8016 +"""flagd's OFREP HTTP port. + +flagd's own default (``flags.Int32P("ofrep-port", "r", 8016, ...)`` in flagd's +``cmd/start.go``), published unchanged by the testbed's compose file. The +testbed's launchpad starts flagd with no ``--ofrep-port`` override, so this is +what it listens on. +""" + +HEALTH_PORT = 8014 +"""flagd's HTTP management port, serving ``/readyz``. + +The same endpoint the launchpad itself polls after starting flagd +(``launchpad/pkg/flagd.go``), so waiting on it here means waiting on exactly the +condition the backend considers "up". +""" + +LAUNCHPAD_PORT = 8080 +"""The testbed's control-API port. ``HttpControl`` is pointed at its mapped host port.""" + +READY_TIMEOUT_SECONDS = 60.0 +READY_POLL_SECONDS = 0.5 + + +def testbed_path() -> Path: + """Return the directory holding the testbed's compose file. + + Reaching across into the flagd package is a real coupling and is called out + where it will be read: this package has no submodule of its own, and adding + a second checkout of ``flagd-testbed`` would let the two drift to different + testbed versions -- which for a shared canonical flag set is precisely the + failure the suite is meant to detect rather than to cause. + """ + providers = Path(__file__).resolve().parents[3] + path = providers / "openfeature-provider-flagd" / "openfeature" / "test-harness" + + if not (path / "docker-compose.yaml").is_file(): + msg = ( + f"the flagd testbed is not checked out at {path}. It is a git " + f"submodule of this repository; run 'git submodule update --init' " + f"from the repository root" + ) + raise RuntimeError(msg) + return path + + +class FlagdTestbed: + """The testbed stack, and the two host ports the OFREP suite needs from it. + + Started once per session and **never restarted**: compose assigns host ports + dynamically and cannot preserve them across a restart, so a restart would + silently invalidate every provider already pointed at the old port. Scenario + isolation comes from the control API instead -- see the no-container-restart + invariant in the TCK's ``control-api.yaml``. + """ + + def __init__(self) -> None: + self._path = testbed_path() + self._version = (self._path / "version.txt").read_text().rstrip() + + # The compose file substitutes these. FLAGS_DIR is bind-mounted at + # /flags, where the launchpad writes the flag set it assembles for the + # configuration it was asked to start, so the directory has to exist + # before compose runs. A temporary one, because nothing outside the + # container reads it and a directory inside the checkout would be a + # test artifact left in the tree. + self._flags_dir = tempfile.mkdtemp(prefix="ofrep-tck-flags-") + os.environ["IMAGE"] = "ghcr.io/open-feature/flagd-testbed" + os.environ["VERSION"] = f"v{self._version}" + os.environ["FLAGS_DIR"] = self._flags_dir + + self._compose = DockerCompose( + context=str(self._path), + compose_file_name="docker-compose.yaml", + wait=True, + ) + + def start(self) -> FlagdTestbed: + self._compose.start() + self._await_ready() + return self + + def stop(self) -> None: + self._compose.stop() + + def get_ofrep_url(self) -> str: + """Return the base URL to hand to ``OFREPProvider``. + + The provider appends ``ofrep/v1/evaluate/flags/{key}`` itself + (``ofrep/__init__.py:115-119``), so this is the bare origin. + """ + return f"http://localhost:{self._mapped(OFREP_PORT)}" + + def get_launchpad_url(self) -> str: + return f"http://localhost:{self._mapped(LAUNCHPAD_PORT)}" + + def _mapped(self, port: int) -> int: + return int(self._compose.get_service_port("flagd", port)) + + def _await_ready(self) -> None: + """Block until flagd answers ``/readyz``. + + ``wait=True`` above waits for compose's own healthcheck, which polls + ``/healthz`` -- liveness, not readiness. The OFREP endpoint is only + useful once the flag sources are loaded, so this waits for the stricter + of the two rather than letting the first scenario race the load. + """ + url = f"http://localhost:{self._mapped(HEALTH_PORT)}/readyz" + deadline = time.monotonic() + READY_TIMEOUT_SECONDS + last: Exception | None = None + + while time.monotonic() < deadline: + try: + with urllib.request.urlopen(url, timeout=2.0) as response: # noqa: S310 + if response.status == 200: + return + except (urllib.error.URLError, OSError) as err: # pragma: no cover + last = err + time.sleep(READY_POLL_SECONDS) + + msg = f"flagd testbed was not ready within {READY_TIMEOUT_SECONDS}s ({url})" + raise ConnectionError(msg) from last + + +def running_testbed() -> typing.Iterator[FlagdTestbed]: + """Yield a started testbed and stop it afterwards. Used by the session fixture.""" + testbed = FlagdTestbed() + testbed.start() + try: + yield testbed + finally: + testbed.stop() diff --git a/uv.lock b/uv.lock index 8017df40a..0af4c1e6b 100644 --- a/uv.lock +++ b/uv.lock @@ -1970,9 +1970,12 @@ dependencies = [ dev = [ { name = "coverage", extra = ["toml"] }, { name = "mypy" }, + { name = "openfeature-provider-tck" }, { name = "poethepoet" }, { name = "pytest" }, + { name = "pytest-bdd" }, { name = "requests-mock" }, + { name = "testcontainers" }, { name = "types-requests" }, ] @@ -1986,9 +1989,12 @@ requires-dist = [ dev = [ { name = "coverage", extras = ["toml"], specifier = ">=7.10.0,<8.0.0" }, { name = "mypy", specifier = ">=1.18.0,<2.0.0" }, + { name = "openfeature-provider-tck", editable = "tools/openfeature-provider-tck" }, { name = "poethepoet", specifier = ">=0.37.0" }, { name = "pytest", specifier = ">=9.0.0,<10.0.0" }, + { name = "pytest-bdd", specifier = ">=8.1.0,<9.0.0" }, { name = "requests-mock", specifier = ">=1.12.0,<2.0.0" }, + { name = "testcontainers", specifier = ">=4.12.0,<5.0.0" }, { name = "types-requests", specifier = ">=2.32.0,<3.0.0" }, ] From c22e73b5293519932dad2f6ec13b7d35645e3c9e Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 10 Sep 2026 19:50:14 +0200 Subject: [PATCH 02/22] fix(ofrep): stop leaking the testbed when it never becomes ready Two leaks on the same path, both reported by coderabbit on #414. running_testbed called start() outside the try, and start() brings the compose stack up before waiting for readiness. So a readiness timeout left containers running that nothing would stop -- and a suite that cannot reach its backend is exactly when someone runs it again, against the stack the last run abandoned. stop() also never removed the temporary directory that __init__ creates for the bind mount, so every constructed testbed left one behind whether or not anything failed. It now comes off in a finally, because a compose failure is precisely when it would otherwise be missed. Verified on the failing path rather than by inspection: with readiness forced to raise, the temporary directory is removed and no containers remain. The suite is unchanged at 23 passed, 5 skipped, 1 xfailed. Signed-off-by: Simon Schrottner --- .../tests/tck/testbed.py | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/testbed.py b/providers/openfeature-provider-ofrep/tests/tck/testbed.py index feb6930f6..9bcd62463 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/testbed.py +++ b/providers/openfeature-provider-ofrep/tests/tck/testbed.py @@ -27,6 +27,7 @@ from __future__ import annotations import os +import shutil import tempfile import time import typing @@ -121,7 +122,16 @@ def start(self) -> FlagdTestbed: return self def stop(self) -> None: - self._compose.stop() + """Stop the stack and remove the temporary flag directory. + + The directory comes off in a ``finally`` because a compose failure is + exactly when it would otherwise be left behind, and it is created in + ``__init__`` -- so every constructed testbed leaks one until this runs. + """ + try: + self._compose.stop() + finally: + shutil.rmtree(self._flags_dir, ignore_errors=True) def get_ofrep_url(self) -> str: """Return the base URL to hand to ``OFREPProvider``. @@ -163,10 +173,16 @@ def _await_ready(self) -> None: def running_testbed() -> typing.Iterator[FlagdTestbed]: - """Yield a started testbed and stop it afterwards. Used by the session fixture.""" + """Yield a started testbed and stop it afterwards. Used by the session fixture. + + ``start`` is inside the ``try`` on purpose. It brings the stack up and then + waits for readiness, so a readiness timeout leaves containers running that + nothing would otherwise stop -- and a suite that cannot reach its backend is + precisely when a developer is going to run it again. + """ testbed = FlagdTestbed() - testbed.start() try: + testbed.start() yield testbed finally: testbed.stop() From 64478835a0658abfc5fc984a63370fbb9cd6f660 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 11 Sep 2026 09:53:29 +0200 Subject: [PATCH 03/22] test(ofrep): follow the @numeric-coercion rename The capability was renamed on the base branch, so this declaration named a symbol that no longer exists. OFREP keeps declaring it -- the provider satisfies the rule, for the same reason the Go OFREP provider does: OFREP is JSON, JSON has one number type, and the provider checks whether the round trip through an integer is lossy rather than assuming it is not. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 9f9b47c10..61dd3b7a9 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -47,7 +47,7 @@ # ofrep/__init__.py:248 admits `(dict, list)` for FlagType.OBJECT -- so a JSON # object comes back as one rather than being rejected or flattened. # -# STRICT_NUMERIC_TYPING +# NUMERIC_COERCION # ofrep/__init__.py:250 maps FlagType.INTEGER to `int`, and the isinstance # check at ofrep/__init__.py:255 fails for the float 0.5, raising # TypeMismatchError. So float-flag's 0.5 is reported as a mismatch rather @@ -119,7 +119,7 @@ CAPABILITIES = frozenset( { Capability.OBJECT, - Capability.STRICT_NUMERIC_TYPING, + Capability.NUMERIC_COERCION, } ) From 02441047f8358db9ac11e7a2f9e58b44f8cae3f4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 11 Sep 2026 16:36:29 +0200 Subject: [PATCH 04/22] test(ofrep): account for @reinitialization, and drop two stale claims spec@fc99d5ac gates "A provider that was shut down can be initialized again" behind a new @reinitialization tag. This adoption withholds it, and the "Not declared, and why" block now says so rather than leaving the newest capability the only one without an entry. Reuse would in fact work here -- a stateless provider holding only a Session has nothing to release and nothing to rebuild, and `shutdown` is inherited and does nothing -- but the scenario cannot be reached to demonstrate it. It lives in lifecycle.feature, so it inherits @lifecycle at feature level, and the gate skips a scenario when any capability gating it is undeclared. With LIFECYCLE withheld, declaring this would leave the scenario skipped on @lifecycle and the claim unexamined. Requirement 2.5.2 makes reuse permitted rather than required, so withholding needs no KnownDeviation. The LIFECYCLE entry also carried two claims that the restacking has since falsified: that the capability was "not in the Capability enum on this branch yet", and that lifecycle.feature was "gated on @events at feature level on this branch". Both were true when written, above a base that did not yet have the split; this branch now sits above the commit that added it, so the enum has LIFECYCLE and the feature carries @lifecycle. The reason for withholding is unchanged -- `initialize` is inherited and does nothing, so initialisation has no outcome to observe -- and only the description of the surroundings is corrected. Conformance is unchanged at the new pin: 27 passed, 3 failed, 9 skipped, 1 xfailed over the 40 canonical scenarios, the nine skips being three @unavailable, two @events, two @lifecycle, one @large-integers and one @reinitialization. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 61dd3b7a9..21a8b11fa 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -64,15 +64,13 @@ # rate-limit timestamp, and nothing else survives between evaluations. # # LIFECYCLE -# Not in the Capability enum on this branch yet (it lands with -# feat/provider-tck), and it would not be declared once it does. # OFREPProvider does not override `initialize`, so it inherits # AbstractProvider's, which is `pass` (python-sdk # openfeature/provider/__init__.py:138-139). Nothing contacts the backend # before the first evaluation, so initialisation has no outcome to observe. -# lifecycle.feature is gated on @events at feature level on this branch and -# skips for that reason; when it is retagged to @lifecycle it must keep -# skipping, for this one. +# lifecycle.feature carries @lifecycle at feature level and skips as a +# whole, which is the intended outcome: it was gated on @events before the +# capability was split out, and the retag had to keep it skipping here. # # EVENTS # The provider never emits. It extends AbstractProvider, so it inherits @@ -110,6 +108,23 @@ # rejects the capability without a `new_unavailable_provider`, and none is # supplied here for the same reason. # +# REINITIALIZATION +# New at spec@fc99d5ac, which gated the scenario "A provider that was shut +# down can be initialized again" that had been untagged before it. +# Requirement 2.5.2 says a provider SHOULD revert to its uninitialized +# state and that "some providers MAY allow reinitialization", so reuse is +# permitted rather than required and withholding needs no KnownDeviation. +# +# Reuse would in fact work here -- a stateless provider holding only a +# Session has nothing to release and nothing to rebuild, and `shutdown` is +# inherited and does nothing either -- but the scenario cannot be reached to +# demonstrate it. It lives in lifecycle.feature, so it inherits @lifecycle +# at feature level, and the gate skips a scenario when any capability +# gating it is undeclared. With LIFECYCLE withheld above, declaring this +# would leave the scenario skipped on @lifecycle and the claim unexamined: +# the same declare-what-nothing-exercises error the reserved tags below are +# kept out for. +# # TARGETING, CACHING # Reserved in the Capability enum; no scenario carries either tag. Declaring # a capability nothing exercises would be a claim with no evidence behind it. From 4648afafad97d65e1eed4755a92183df98ac46ec Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 12 Sep 2026 08:28:32 +0200 Subject: [PATCH 05/22] test(ofrep): declare @variants and @targeting, on a measured run Declared, the suite run, and the scenarios seen to pass. The numbers: 52 canonical scenario instances, 38 passed, 9 skipped, 1 xfailed, 4 failed. @targeting costs the testbed nothing. This suite's backend is the same flagd testbed the flagd suites use, so targeting-key-flag is already seeded from flags/testing-flags.json by the launchpad's default configuration. All three scenarios pass -- the matching context, the non-matching one, and no context at all -- because ofrep/__init__.py:229-230 puts the evaluation context's targeting key into the request body's `context` object. That is what makes context passthrough observable on a protocol with no echo endpoint: a provider that dropped the context would resolve `miss` where `hit` is expected. Worth noting that the whole of what is verified is that the key reached flagd, since nothing else about the context is keyed on by any canonical flag. @variants passes seven of its eight rows. The eighth asks for large-integer-flag's max-int32 and fails with the flag missing from the backend -- flagd-testbed v3.8.0 seeds neither large-integer-flag nor huge-integer-flag, which already fails the untagged precision scenario here and does the same in both flagd suites. Withholding the capability over it would say this provider does not name variants, which the other seven rows show is false, and would attribute a missing flag to a capability the provider has. It is not a KnownDeviation either: a deviation is for a behaviour the provider is required to have and does not. The header claiming these declarations rest "on the strength of a line of provider code, not on the strength of a green run" is gone, as it now is in the flagd suites: Appendix F states the opposite rule as of 26362f85. The code references stay, as places a reader can check a claim rather than as the evidence for it. That rule turns up something about a declaration this commit does not otherwise touch, so it is written down rather than left to be discovered. @numeric-coercion is declared here and the run fails two of its three scenarios: the lossy half passes, where rejecting 0.5 is correct, and the two lossless ones do not -- integral-float-flag's 10.0 is a TYPE_MISMATCH where 10 is required, and integer-flag's 10 is one where 10.0 is. That is the shortcut errors.feature warns of, and its comment states that a provider declaring the tag must satisfy all three. The flagd suites reached the same finding at the previous pin and withdrew the tag for it. The same withdrawal is the consistent end of it here, but it changes what this adoption claims rather than how it is worded, so it is left for a deliberate decision and recorded in the note meanwhile. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 73 +++++++++++++++++-- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 21a8b11fa..b970ccf2b 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -39,14 +39,49 @@ timeouts, ready timeouts -- has nothing to bound, for the reasons below. """ -# Every capability below is declared on the strength of a line of provider code, -# not on the strength of a green run. +# VARIANTS and TARGETING below were declared, the suite run, and their scenarios +# seen to pass -- bar the one row named under VARIANTS, which fails on a flag the +# testbed does not seed. The code references say where the behaviour lives, so a +# reader can check a claim; they are not the evidence for it. That order is +# Appendix F's rule as of spec@26362f85, and this file used to state the reverse. +# +# NUMERIC_COERCION is the entry that rule has yet to be applied to, and it is +# stated here rather than left to be discovered: the run fails two of its three +# scenarios, so it is declared on something other than a green run. See its note +# below for the measurement and for what the flagd suites did with the same +# finding. # # OBJECT # ofrep/__init__.py:105-113 resolves structured values, and the type check at # ofrep/__init__.py:248 admits `(dict, list)` for FlagType.OBJECT -- so a JSON # object comes back as one rather than being rejected or flattened. # +# VARIANTS +# ofrep/__init__.py:160 carries the response's `variant` field into the +# resolution details, and flagd names a variant for every flag it serves. +# Seven of the outline's eight rows pass. The eighth asks for +# large-integer-flag's `max-int32` and fails with the flag missing from the +# backend -- flagd-testbed v3.8.0 seeds neither large-integer-flag nor +# huge-integer-flag, which already fails the untagged precision scenario +# here and does the same in both flagd suites. Withholding the capability +# over it would say this provider does not name variants, which the other +# seven rows show is false, and would blame a missing flag on a capability +# the provider has. It is not a KnownDeviation either: a deviation is for a +# behaviour the provider is required to have and does not. +# +# TARGETING +# ofrep/__init__.py:229-230 puts the evaluation context's targeting key into +# the request body's `context` object, so flagd evaluates +# targeting-key-flag's rule against it. All three scenarios pass -- the +# matching context, the non-matching one and no context at all -- which is +# what makes context passthrough observable here without an echo endpoint: +# a provider that dropped the context would resolve `miss` where `hit` is +# expected. +# +# Worth noting for a protocol with no types on the wire: the whole of what +# is verified is that the key reached flagd, since nothing else about the +# context is keyed on by any canonical flag. +# # NUMERIC_COERCION # ofrep/__init__.py:250 maps FlagType.INTEGER to `int`, and the isinstance # check at ofrep/__init__.py:255 fails for the float 0.5, raising @@ -59,6 +94,24 @@ # lines, which is also why the one deviation recorded in conftest.py lives # there. # +# Measured, and it does not support the declaration. One of the three +# scenarios passes -- the lossy half, where rejecting 0.5 is correct -- and +# the two lossless ones fail: integral-float-flag's 10.0 is a TYPE_MISMATCH +# where 10 is required, and integer-flag's 10 is one where 10.0 is. That is +# precisely the shortcut errors.feature warns of, "easy to get right by +# rejecting every float", and its comment states that a provider declaring +# the tag must satisfy all three. +# +# The flagd suites reached this finding first and withdrew the tag for it: +# "Neither resolver satisfies all three @numeric-coercion scenarios, so +# neither declares it". The same conclusion follows here, and the same +# withdrawal is the consistent end of it -- the two failures would become +# skips carrying their reason, as they already do in both flagd suites. It is +# left declared for now because withdrawing it is a change to what this +# adoption claims rather than to how it is worded, and the claim is one to +# settle deliberately rather than in passing. Until then this note is the +# honest form of it: the tag is declared, the evidence is a run that fails. +# # Not declared, and why. Each is a fact about the provider, established by # reading it -- OFREPProvider is stateless: it holds a requests.Session and a # rate-limit timestamp, and nothing else survives between evaluations. @@ -122,18 +175,22 @@ # at feature level, and the gate skips a scenario when any capability # gating it is undeclared. With LIFECYCLE withheld above, declaring this # would leave the scenario skipped on @lifecycle and the claim unexamined: -# the same declare-what-nothing-exercises error the reserved tags below are +# the same declare-what-nothing-exercises error the reserved tag below is # kept out for. # -# TARGETING, CACHING -# Reserved in the Capability enum; no scenario carries either tag. Declaring -# a capability nothing exercises would be a claim with no evidence behind it. +# CACHING +# Reserved in the Capability enum; no scenario carries the tag. Declaring a +# capability nothing exercises would be a claim with no evidence behind it. +# @targeting was reserved alongside it until spec@26362f85 gave it three +# scenarios, and is now declared above. # -# The result matches the Go and Java OFREP adoptions, which reached the same two -# capabilities from the same architecture, independently. +# The withheld set matches the Go and Java OFREP adoptions, which reached the +# same conclusions from the same architecture, independently. CAPABILITIES = frozenset( { Capability.OBJECT, + Capability.VARIANTS, + Capability.TARGETING, Capability.NUMERIC_COERCION, } ) From c799ff037c5fd41548634e8020009dcf476afa84 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 12 Sep 2026 09:12:08 +0200 Subject: [PATCH 06/22] test(ofrep): withhold @numeric-coercion, which this provider does not have The tag was declared and two of its three scenarios failed. errors.feature says a declarer must satisfy all three, and says why: the two lossless rows exist to catch the shortcut of rejecting every float. This provider takes that shortcut. json.loads yields `int` for 10 and `float` for 0.5, and the check at ofrep/__init__.py:249-256 admits a value only on an exact isinstance against one of them. Nothing in that path widens or narrows a number. So the lossy row passes -- float-flag asked for as an Integer is a TYPE_MISMATCH rather than a silent 0 -- and integer-flag asked for as a Float fails, because 10 is not an instance of float. That is the same architecture as the Java OFREP adoption, which withholds the tag for the same reason. The Go adoption declares it, and the difference is the JSON decoder rather than a decision either author made: encoding/json makes every JSON number a float64, so integer-ness never survives the wire and ResolveInt has to round-trip through int64 -- which yields lossless coercion, and TYPE_MISMATCH on loss, for free. Over OFREP this capability follows the language's JSON library. No knownDeviation accompanies the withdrawal. A deviation records a gap in behaviour a provider is required to have, and numeric coercion is a declared capability rather than a requirement. The honest record is the undeclared tag and the three skips it produces. Measured before and after, 52 scenarios both times: 4 failed / 38 passed / 9 skipped / 1 xfailed becomes 2 failed / 37 passed / 12 skipped / 1 xfailed. The two remaining failures are both large-integer-flag, which the pinned testbed does not serve (open-feature/flagd-testbed#392). The header also claimed every declared capability had been seen to pass, which was untrue while this tag was declared, and the closing note claimed the withheld set matched Go's as well as Java's. Both corrected. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index b970ccf2b..8ddc3cd1f 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -39,17 +39,11 @@ timeouts, ready timeouts -- has nothing to bound, for the reasons below. """ -# VARIANTS and TARGETING below were declared, the suite run, and their scenarios -# seen to pass -- bar the one row named under VARIANTS, which fails on a flag the -# testbed does not seed. The code references say where the behaviour lives, so a -# reader can check a claim; they are not the evidence for it. That order is -# Appendix F's rule as of spec@26362f85, and this file used to state the reverse. -# -# NUMERIC_COERCION is the entry that rule has yet to be applied to, and it is -# stated here rather than left to be discovered: the run fails two of its three -# scenarios, so it is declared on something other than a green run. See its note -# below for the measurement and for what the flagd suites did with the same -# finding. +# Every capability below was declared, the suite run, and its scenarios seen to +# pass -- bar the one row named under VARIANTS, which fails on a flag the testbed +# does not seed. The code references say where the behaviour lives, so a reader +# can check a claim; they are not the evidence for it. That order is Appendix F's +# rule as of spec@26362f85, and this file used to state the reverse. # # OBJECT # ofrep/__init__.py:105-113 resolves structured values, and the type check at @@ -82,17 +76,37 @@ # is verified is that the key reached flagd, since nothing else about the # context is keyed on by any canonical flag. # -# NUMERIC_COERCION -# ofrep/__init__.py:250 maps FlagType.INTEGER to `int`, and the isinstance -# check at ofrep/__init__.py:255 fails for the float 0.5, raising -# TypeMismatchError. So float-flag's 0.5 is reported as a mismatch rather -# than narrowed to 0. Worth stating plainly that this is the provider's own -# doing: OFREP is untyped on the wire, the request carries no type at all, -# and flagd returns 0.5 whatever was asked for -- so unlike flagd-RPC, where -# the server answers INVALID_ARGUMENT, there is no backend here to catch a -# numeric mismatch. Every type decision in this suite is made at those two -# lines, which is also why the one deviation recorded in conftest.py lives -# there. +# NUMERIC_COERCION is withheld, and the reason is worth recording because two +# other languages answered it differently over the same protocol. +# +# The capability has three scenarios and errors.feature says a declarer must +# satisfy all three -- the two lossless rows exist precisely to catch the +# shortcut of rejecting every float. This provider takes that shortcut. It keeps +# the two numeric types strictly apart: json.loads yields `int` for 10 and +# `float` for 0.5, and the check at ofrep/__init__.py:249-256 admits a value only +# on an exact isinstance against one of them. Nothing in that path widens or +# narrows a number. So the lossy row passes -- float-flag asked for as an Integer +# is a TYPE_MISMATCH rather than a silent 0 -- and integer-flag asked for as a +# Float fails, because 10 is not an instance of float. +# +# That is the same architecture as the Java OFREP adoption, which withholds the +# tag for the same reason: Jackson maps a JSON integer to Integer and a fraction +# to Double, and handleResolved admits the value only on an exact +# type.isInstance. The Go adoption declares it, and the difference is the JSON +# decoder rather than anything the provider author chose -- encoding/json makes +# every JSON number a float64, so integer-ness never survives the wire and +# ResolveInt has to round-trip through int64, which gives lossless coercion and a +# TYPE_MISMATCH on loss for free. +# +# Which is to say: over OFREP this capability follows the language's JSON +# library. Declaring it here would claim a behaviour two of these three lines of +# code rule out. +# +# No knownDeviation entry accompanies this. A deviation records a gap in +# behaviour the provider is required to have, and numeric coercion is a declared +# capability rather than a requirement -- Appendix F stopped presenting the rule +# as normative OpenFeature. The honest record is the undeclared tag and the three +# skips it produces. # # Measured, and it does not support the declaration. One of the three # scenarios passes -- the lossy half, where rejecting 0.5 is correct -- and @@ -184,14 +198,14 @@ # @targeting was reserved alongside it until spec@26362f85 gave it three # scenarios, and is now declared above. # -# The withheld set matches the Go and Java OFREP adoptions, which reached the -# same conclusions from the same architecture, independently. +# The withheld set matches the Java OFREP adoption, which reached the same +# conclusions from the same architecture, independently. It differs from Go's, +# which declares NUMERIC_COERCION for the decoder reason recorded above. CAPABILITIES = frozenset( { Capability.OBJECT, Capability.VARIANTS, Capability.TARGETING, - Capability.NUMERIC_COERCION, } ) From d64fab4c551e6e27857d94c37e8143de5a18b5e8 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 12 Sep 2026 12:52:58 +0200 Subject: [PATCH 07/22] test(ofrep): withhold @disabled-flags, which is a bug rather than a wall New at spec@009afe06 and withheld here, on a measured run: 2 failed, 37 passed, 16 skipped, 1 xfailed -- the same two failures as before the pin moved, with the four new rows skipped carrying their reason. Declaring it instead gives 6 failed, and each of the four fails on the error code rather than on the value. The appendix gates this tag on the reasoning that a provider "whose backend decides, such as one speaking OFREP, cannot: the server never sees the caller's default, so it has no way to return it". Neither half of that is the obstacle, which is why the note records a defect rather than an architecture. Probed at the wire: flagd's OFREP endpoint answers a disabled flag 200 {"key": ..., "reason": "DISABLED", "metadata": {}} -- no value and no variant. The server does never return a value, exactly as the appendix says, and it does not need to: ofrep/__init__.py:153 already reads data.get("value", default_value) and substitutes the caller's default for an absent one, and the type check on the next line passes on it. What fails is line 160, which indexes data["variant"] unconditionally, where flagd omits the member and types.md types the field "variant (string, optional)". The KeyError surfaces as GENERAL. So the whole of the difference between passing and failing these four rows is one .get, and flagd's own RPC resolver satisfies the tag from the same signal in a different envelope. Withheld because a declaration has to rest on a run and the run fails, not because the protocol forbids it. The gap is an unfiled defect in openfeature-provider-ofrep and is not confined to disabled flags: the same index breaks on any OFREP response omitting the variant. No knownDeviation, for the reason the @numeric-coercion note above it gives -- a deviation records a gap in behaviour the provider is required to have, and this is a declared capability rather than a requirement. Also drops the stale half of that note. eebf35a withdrew @numeric-coercion and added the paragraphs saying so, but left the pre-withdrawal sub-block in place underneath them, ending "the tag is declared, the evidence is a run that fails" about a tag no longer in CAPABILITIES. A comment that contradicts the code beside it is worse than no comment; the paragraphs above it already carry everything it said. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 57 +++++++++++++------ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 8ddc3cd1f..ecf2820e0 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -108,23 +108,46 @@ # as normative OpenFeature. The honest record is the undeclared tag and the three # skips it produces. # -# Measured, and it does not support the declaration. One of the three -# scenarios passes -- the lossy half, where rejecting 0.5 is correct -- and -# the two lossless ones fail: integral-float-flag's 10.0 is a TYPE_MISMATCH -# where 10 is required, and integer-flag's 10 is one where 10.0 is. That is -# precisely the shortcut errors.feature warns of, "easy to get right by -# rejecting every float", and its comment states that a provider declaring -# the tag must satisfy all three. -# -# The flagd suites reached this finding first and withdrew the tag for it: -# "Neither resolver satisfies all three @numeric-coercion scenarios, so -# neither declares it". The same conclusion follows here, and the same -# withdrawal is the consistent end of it -- the two failures would become -# skips carrying their reason, as they already do in both flagd suites. It is -# left declared for now because withdrawing it is a change to what this -# adoption claims rather than to how it is worded, and the claim is one to -# settle deliberately rather than in passing. Until then this note is the -# honest form of it: the tag is declared, the evidence is a run that fails. +# DISABLED_FLAGS is withheld as well, new at spec@009afe06, and this one is a +# provider defect rather than an architecture. Which is the opposite of what the +# appendix predicts, so it is worth being exact about. +# +# The appendix gates the tag on the reasoning that a provider "whose backend +# decides, such as one speaking OFREP, cannot: the server never sees the +# caller's default, so it has no way to return it". Neither half of that is the +# obstacle here. +# +# Measured first. Declaring the tag fails all four rows -- 6 failed, 37 passed, +# 12 skipped, 1 xfailed, against the 2 failed of the run without it -- and each +# fails on the error code rather than on the value: "error-code was 'GENERAL', +# expected none". Then read back, and probed at the wire to be sure of the +# reading. flagd's OFREP endpoint answers a disabled flag +# `200 {"key": ..., "reason": "DISABLED", "metadata": {}}`: no `value`, and no +# `variant`. The server does indeed never return a value, exactly as the +# appendix says. It does not need to -- ofrep/__init__.py:153 already reads +# `data.get("value", default_value)` and substitutes the caller's default for an +# absent one, and the type check on the next line passes on it. +# +# What fails is ofrep/__init__.py:160, which indexes `data["variant"]` +# unconditionally. flagd omits the member for a disabled flag, types.md types +# the field `variant (string, optional)`, and the resolution raises +# KeyError: 'variant'; the SDK catches it and reports GENERAL. So the whole of +# the difference between passing and failing these four rows is one `.get`. +# +# Which puts the capability within reach of this provider rather than outside +# it, and flagd's own RPC resolver satisfies the tag from the same signal in a +# different envelope. It is withheld because a declaration has to rest on a run +# and the run fails -- not because the architecture forbids it. The gap is an +# unfiled defect in openfeature-provider-ofrep, and it is not confined to +# disabled flags: the same index breaks on any OFREP response that omits the +# variant, which the field being optional permits for any reason a server +# reports without one. +# +# No knownDeviation entry, for the reason the numeric-coercion note above gives: +# a deviation records a gap in behaviour the provider is required to have, and +# @disabled-flags is a declared capability rather than a requirement. The honest +# record is the undeclared tag, the four skips it produces, and this note saying +# the gap is a bug somebody can fix rather than a fact of the protocol. # # Not declared, and why. Each is a fact about the provider, established by # reading it -- OFREPProvider is stateless: it holds a requests.Session and a From 0e76639ad69acc70542cd1ca5cddc1614b4ead5a Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 12 Sep 2026 14:47:24 +0200 Subject: [PATCH 08/22] test(ofrep): declare the stack instead of wiring it This suite carried the largest hand-rolled container wrapper of the three adoptions -- a 188-line tests/tck/testbed.py, 74 lines of it code -- and it is deleted. The TCK's Compose harness starts the stack, discovers the mapped host ports, builds the HttpControl against the launchpad and waits for it to accept commands; what is left is a Compose file and the port the provider connects to. tests/tck/docker-compose.yaml wraps the same unmodified testbed image the flagd adoption uses, publishing flagd's OFREP port and the launchpad's. It replaces reaching across into the flagd package's `test-harness` submodule for that package's compose file, which was a real coupling and was called out as one: this package's conformance results depended on another package's test-harness checkout, on a compose file that also stands up envoy and bind-mounts a flags directory for reasons belonging to the flagd e2e suites. Two packages now pin the same image tag in two files, which is the cost of each owning its stack, and they are bumped together. Gone with it: the temporary flags directory the bind mount needed, the IMAGE/VERSION/FLAGS_DIR environment substitution, the /readyz poll on port 8014, and the leak fix that was needed because a readiness timeout left containers running -- the harness tears the stack down in a finally of its own. SettledControl stays, and stays a finding rather than a workaround. It is not the post-command settle that was dropped from the harness: that was a fixed 50ms sleep after every control call, and this is a bounded readiness probe over the provider's own public OFREP endpoint, on a canonical flag, asserting only that the backend has finished doing what POST /start already promised. It wraps the harness's control unchanged, so the normative control path is untouched. The window it covers is real at flagd-testbed v3.8.0 -- around 40ms, measured -- and a stateless provider is the first adopter with no initialisation to hide it behind. Measured before and after, twice each: 2 failed, 37 passed, 16 skipped, 1 xfailed, identical to the run before the rewrite, scenario for scenario. The two failures are the documented flags the testbed does not seed. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/pyproject.toml | 15 +- .../tests/tck/conftest.py | 73 +++++-- .../tests/tck/docker-compose.yaml | 26 +++ .../tests/tck/settled_control.py | 2 +- .../tests/tck/test_ofrep_conformance.py | 20 +- .../tests/tck/testbed.py | 188 ------------------ uv.lock | 6 +- 7 files changed, 100 insertions(+), 230 deletions(-) create mode 100644 providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml delete mode 100644 providers/openfeature-provider-ofrep/tests/tck/testbed.py diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index fbe7a04be..d3347c85a 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -29,22 +29,21 @@ Homepage = "https://github.com/open-feature/python-sdk-contrib" dev = [ "coverage[toml]>=7.10.0,<8.0.0", "mypy>=1.18.0,<2.0.0", - # The OpenFeature provider conformance suite. Ships the feature files, the flag - # set and the control-API client, and registers its step definitions through a - # pytest11 entry point, so tests/tck needs no conftest of its own for them. - "openfeature-provider-tck", + # The OpenFeature conformance suite. Ships the feature files, the flag set, + # the control-API client and the Compose harness that owns the container + # stack, and registers its step definitions through a pytest11 entry point -- + # so tests/tck declares a Compose file and nothing else. The `compose` extra + # is what pulls testcontainers in for the harness. + "openfeature-tck[compose]", "poethepoet>=0.37.0", "pytest>=9.0.0,<10.0.0", "pytest-bdd>=8.1.0,<9.0.0", "requests-mock>=1.12.0,<2.0.0", - # Starts the flagd testbed, which serves the OFREP API on port 8016 alongside - # flagd's own protocols. See tests/tck/testbed.py. - "testcontainers>=4.12.0,<5.0.0", "types-requests>=2.32.0,<3.0.0", ] [tool.uv.sources] -openfeature-provider-tck = { workspace = true } +openfeature-tck = { workspace = true } [tool.uv.build-backend] module-name = "openfeature" diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py index c692ab0da..f0b0aa568 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/conftest.py +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -1,6 +1,12 @@ """Session fixtures for the OFREP conformance suite, and one recorded deviation. -The stack is started once and never restarted, because compose assigns host +The container lifecycle belongs to the TCK: it starts the Compose file beside +this module once per session, discovers the dynamically mapped host ports, builds +the ``HttpControl`` against the launchpad and waits for it to accept commands. +What is left here is the declaration, the one wrapper this provider needs around +the control, and the xfail for the single scenario it cannot satisfy. + +The stack is started once and never restarted, because Compose assigns host ports dynamically and cannot preserve them across a restart: a restarted backend comes back on a different port, silently invalidating a provider already pointed at the old one, and the failure reads as a flaky provider rather than a broken @@ -10,39 +16,68 @@ from __future__ import annotations -import typing +from pathlib import Path import pytest -from openfeature.contrib.tools.provider_tck import HttpControl +from openfeature.contrib.tools.tck import ComposeBackend, RunningBackend from tests.tck.settled_control import SettledControl -from tests.tck.testbed import FlagdTestbed, running_testbed + +OFREP_PORT = 8016 +"""flagd's OFREP HTTP port. + +flagd's own default (``flags.Int32P("ofrep-port", "r", 8016, ...)`` in flagd's +``cmd/start.go``). The testbed's launchpad starts flagd with no +``--ofrep-port`` override, so this is what it listens on, and it is the one port +the provider connects to. The launchpad's own 8080 is exposed automatically. +""" @pytest.fixture(scope="session") -def flagd_testbed() -> typing.Iterator[FlagdTestbed]: - """The testbed stack, up for the whole session.""" - yield from running_testbed() +def compose_backend() -> ComposeBackend: + """The stack under test, as the TCK's ``tck_backend`` fixture wants it. + + The path is absolute rather than relative to the package directory -- which + is what the harness resolves a relative one against, and where ``poe test`` + runs from -- so that running pytest from the repository root works too. + """ + return ComposeBackend( + compose_file=Path(__file__).parent / "docker-compose.yaml", + backend_ports=[OFREP_PORT], + ) @pytest.fixture(scope="session") -def ofrep_control(flagd_testbed: FlagdTestbed) -> SettledControl: - """The control API client, pointed at the testbed's launchpad. +def ofrep_base_url(tck_backend: RunningBackend) -> str: + """The origin flagd serves OFREP on, resolved once the stack is up. - The launchpad registers only ``/start``, ``/restart``, ``/stop`` and - ``/change`` (flagd-testbed ``launchpad/main.go:29-32``), so ``/reset`` - answers 404 and every ``prepare_scenario`` takes the documented ``/start`` - fallback. The probe costs one 404 for the whole session. + A fixture rather than a constant the suite module reads, because the mapped + host port does not exist until the stack has started. The provider appends + ``ofrep/v1/evaluate/flags/{key}`` itself (``ofrep/__init__.py:115-119``), so + this is the bare origin. + """ + endpoint = tck_backend.endpoint + return f"http://{endpoint.host}:{endpoint.port(OFREP_PORT)}" + + +@pytest.fixture(scope="session") +def ofrep_control(tck_backend: RunningBackend, ofrep_base_url: str) -> SettledControl: + """The control API client, wrapped in a wait for the flag set to be served. + + ``tck_backend.control`` is the TCK's own ``HttpControl``, already pointed at + the launchpad's mapped port and awaited ready. The launchpad registers only + ``/start``, ``/restart``, ``/stop`` and ``/change`` (flagd-testbed + ``launchpad/main.go:29-32``), so ``/reset`` answers 404 and every + ``prepare_scenario`` takes the documented ``/start`` fallback. The probe + costs one 404 for the whole session. Wrapped in :class:`SettledControl` because ``/start`` returns before the backend serves the flag set, and a stateless provider has no initialisation to hide that window behind. See that module -- it is a finding about the - control API's guarantee, not a convenience. + control API's guarantee, not a convenience, and it is a readiness probe over + the provider's own public endpoint rather than a sleep. """ - return SettledControl( - HttpControl(flagd_testbed.get_launchpad_url()), - flagd_testbed.get_ofrep_url(), - ) + return SettledControl(tck_backend.control, ofrep_base_url) # --------------------------------------------------------------------------- @@ -54,7 +89,7 @@ def ofrep_control(flagd_testbed: FlagdTestbed) -> SettledControl: # the report with its reason attached and fails the suite the moment it starts # passing -- so the marker is removed when the bug is fixed rather than # lingering as a lie. Same mechanism, and same bug, as the TCK's own self-test -# (tools/openfeature-provider-tck/tests/conftest.py). +# (tools/openfeature-tck/tests/conftest.py). _BOOL_AS_INT = ( "test_requesting_the_wrong_type_returns_the_code_default[boolean-flag-Integer-1]" diff --git a/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml new file mode 100644 index 000000000..1eecdb483 --- /dev/null +++ b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml @@ -0,0 +1,26 @@ +# Backend stack for the OpenFeature TCK, wrapping the unmodified flagd testbed image. +# +# flagd serves the OFREP HTTP API on 8016 alongside its own protocols, and the "launchpad" +# control API on 8080. So an OFREP provider needs no backend of its own: it runs against the +# same image, seeded with the same canonical flag set, driven through the same control API as +# the flagd conformance suites. A second stack would be a second definition of "the canonical +# flags", which is the one thing a conformance suite exists to prevent. +# +# Deliberately not flagd-testbed's own compose file, which this suite used to reach across into +# the flagd package's submodule for. That file stands up envoy for the forbidden-endpoint e2e +# scenarios, names its service `flagd`, and bind-mounts a flags directory the launchpad writes +# into -- none of which the TCK needs. Reaching for it also coupled this package's conformance +# results to another package's test-harness submodule. +# +# The tag is pinned here and in the flagd adoption's copy of this file. Two files naming one +# version is the cost of each package owning its own stack; they serve different ports of the +# same image and are bumped together. +# +# Note there are no host port bindings. The TCK requires dynamically mapped ports and discovers +# them after startup. +services: + backend: + image: ghcr.io/open-feature/flagd-testbed:v3.8.0 + ports: + - 8016 # flagd's OFREP HTTP API + - 8080 # launchpad control API diff --git a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py index e834b47b7..41ce674dc 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py +++ b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py @@ -48,7 +48,7 @@ import urllib.error import urllib.request -from openfeature.contrib.tools.provider_tck import HttpControl +from openfeature.contrib.tools.tck import HttpControl __all__ = ["SettledControl"] diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index ecf2820e0..65daa0d61 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -20,14 +20,13 @@ from pytest_bdd import scenarios from openfeature.contrib.provider.ofrep import OFREPProvider -from openfeature.contrib.tools.provider_tck import ( +from openfeature.contrib.tools.tck import ( Capability, TckConfig, - features_path, + feature_paths, ) from openfeature.provider import FeatureProvider from tests.tck.settled_control import SettledControl -from tests.tck.testbed import FlagdTestbed TIMEOUT_SECONDS = 10.0 """Bounds a single OFREP request. @@ -235,18 +234,19 @@ @pytest.fixture(scope="session") def tck_config( - flagd_testbed: FlagdTestbed, + ofrep_base_url: str, ofrep_control: SettledControl, ) -> TckConfig: """Wire the provider up to the running testbed. - The port is read here, after the stack is up: compose maps host ports - dynamically, so it does not exist earlier -- and it stays valid for the whole - session because nothing ever restarts a container. Outages, had this suite - any use for them, would be simulated inside the running stack through + Both fixtures come from ``tests/tck/conftest.py`` and both resolve after the + TCK has started the stack: Compose maps host ports dynamically, so the + address does not exist earlier -- and it stays valid for the whole session + because nothing ever restarts a container. Outages, had this suite any use + for them, would be simulated inside the running stack through ``ofrep_control``. """ - base_url = flagd_testbed.get_ofrep_url() + base_url = ofrep_base_url def new_provider() -> FeatureProvider: return OFREPProvider(base_url, timeout=TIMEOUT_SECONDS) @@ -259,4 +259,4 @@ def new_provider() -> FeatureProvider: ) -scenarios(features_path()) +scenarios(*feature_paths()) diff --git a/providers/openfeature-provider-ofrep/tests/tck/testbed.py b/providers/openfeature-provider-ofrep/tests/tck/testbed.py deleted file mode 100644 index 9bcd62463..000000000 --- a/providers/openfeature-provider-ofrep/tests/tck/testbed.py +++ /dev/null @@ -1,188 +0,0 @@ -"""The flagd testbed, started for the OFREP conformance suite. - -flagd serves the **OFREP** HTTP API alongside its own protocols -- port 8016, -next to 8013 for RPC and 8015 for sync -- and ``flagd-testbed``'s compose file -already publishes it. So an OFREP provider needs no backend of its own: it runs -against the same stack, seeded with the same canonical flag set, driven through -the same launchpad control API as the flagd suites. A second stack would be a -second definition of "the canonical flags", which is the one thing a conformance -suite exists to prevent. - -**Why this is not ``tests/e2e/flagd_container.FlagdContainer``.** That helper -would be the natural thing to reuse, and it is not importable here: it lives -under ``providers/openfeature-provider-flagd/tests/``, and a package's tests are -not part of its distribution, so nothing in this package can import it. It also -depends on ``grpcio`` and ``grpcio-health-checking`` for its readiness probe, -neither of which the OFREP provider has any other reason to pull in. What is -duplicated is therefore deliberately minimal -- compose up, read two mapped -ports, poll one HTTP endpoint -- and this is a concrete instance of the "no -shared containerised-backend helper" gap the TCK's README already records. - -The compose file is reached through the flagd package's ``test-harness`` -submodule rather than a second checkout of the same repository. Run -``git submodule update --init`` if it is missing; :func:`testbed_path` says so -rather than failing inside testcontainers. -""" - -from __future__ import annotations - -import os -import shutil -import tempfile -import time -import typing -import urllib.error -import urllib.request -from pathlib import Path - -from testcontainers.compose import DockerCompose - -__all__ = ["LAUNCHPAD_PORT", "OFREP_PORT", "FlagdTestbed", "testbed_path"] - -OFREP_PORT = 8016 -"""flagd's OFREP HTTP port. - -flagd's own default (``flags.Int32P("ofrep-port", "r", 8016, ...)`` in flagd's -``cmd/start.go``), published unchanged by the testbed's compose file. The -testbed's launchpad starts flagd with no ``--ofrep-port`` override, so this is -what it listens on. -""" - -HEALTH_PORT = 8014 -"""flagd's HTTP management port, serving ``/readyz``. - -The same endpoint the launchpad itself polls after starting flagd -(``launchpad/pkg/flagd.go``), so waiting on it here means waiting on exactly the -condition the backend considers "up". -""" - -LAUNCHPAD_PORT = 8080 -"""The testbed's control-API port. ``HttpControl`` is pointed at its mapped host port.""" - -READY_TIMEOUT_SECONDS = 60.0 -READY_POLL_SECONDS = 0.5 - - -def testbed_path() -> Path: - """Return the directory holding the testbed's compose file. - - Reaching across into the flagd package is a real coupling and is called out - where it will be read: this package has no submodule of its own, and adding - a second checkout of ``flagd-testbed`` would let the two drift to different - testbed versions -- which for a shared canonical flag set is precisely the - failure the suite is meant to detect rather than to cause. - """ - providers = Path(__file__).resolve().parents[3] - path = providers / "openfeature-provider-flagd" / "openfeature" / "test-harness" - - if not (path / "docker-compose.yaml").is_file(): - msg = ( - f"the flagd testbed is not checked out at {path}. It is a git " - f"submodule of this repository; run 'git submodule update --init' " - f"from the repository root" - ) - raise RuntimeError(msg) - return path - - -class FlagdTestbed: - """The testbed stack, and the two host ports the OFREP suite needs from it. - - Started once per session and **never restarted**: compose assigns host ports - dynamically and cannot preserve them across a restart, so a restart would - silently invalidate every provider already pointed at the old port. Scenario - isolation comes from the control API instead -- see the no-container-restart - invariant in the TCK's ``control-api.yaml``. - """ - - def __init__(self) -> None: - self._path = testbed_path() - self._version = (self._path / "version.txt").read_text().rstrip() - - # The compose file substitutes these. FLAGS_DIR is bind-mounted at - # /flags, where the launchpad writes the flag set it assembles for the - # configuration it was asked to start, so the directory has to exist - # before compose runs. A temporary one, because nothing outside the - # container reads it and a directory inside the checkout would be a - # test artifact left in the tree. - self._flags_dir = tempfile.mkdtemp(prefix="ofrep-tck-flags-") - os.environ["IMAGE"] = "ghcr.io/open-feature/flagd-testbed" - os.environ["VERSION"] = f"v{self._version}" - os.environ["FLAGS_DIR"] = self._flags_dir - - self._compose = DockerCompose( - context=str(self._path), - compose_file_name="docker-compose.yaml", - wait=True, - ) - - def start(self) -> FlagdTestbed: - self._compose.start() - self._await_ready() - return self - - def stop(self) -> None: - """Stop the stack and remove the temporary flag directory. - - The directory comes off in a ``finally`` because a compose failure is - exactly when it would otherwise be left behind, and it is created in - ``__init__`` -- so every constructed testbed leaks one until this runs. - """ - try: - self._compose.stop() - finally: - shutil.rmtree(self._flags_dir, ignore_errors=True) - - def get_ofrep_url(self) -> str: - """Return the base URL to hand to ``OFREPProvider``. - - The provider appends ``ofrep/v1/evaluate/flags/{key}`` itself - (``ofrep/__init__.py:115-119``), so this is the bare origin. - """ - return f"http://localhost:{self._mapped(OFREP_PORT)}" - - def get_launchpad_url(self) -> str: - return f"http://localhost:{self._mapped(LAUNCHPAD_PORT)}" - - def _mapped(self, port: int) -> int: - return int(self._compose.get_service_port("flagd", port)) - - def _await_ready(self) -> None: - """Block until flagd answers ``/readyz``. - - ``wait=True`` above waits for compose's own healthcheck, which polls - ``/healthz`` -- liveness, not readiness. The OFREP endpoint is only - useful once the flag sources are loaded, so this waits for the stricter - of the two rather than letting the first scenario race the load. - """ - url = f"http://localhost:{self._mapped(HEALTH_PORT)}/readyz" - deadline = time.monotonic() + READY_TIMEOUT_SECONDS - last: Exception | None = None - - while time.monotonic() < deadline: - try: - with urllib.request.urlopen(url, timeout=2.0) as response: # noqa: S310 - if response.status == 200: - return - except (urllib.error.URLError, OSError) as err: # pragma: no cover - last = err - time.sleep(READY_POLL_SECONDS) - - msg = f"flagd testbed was not ready within {READY_TIMEOUT_SECONDS}s ({url})" - raise ConnectionError(msg) from last - - -def running_testbed() -> typing.Iterator[FlagdTestbed]: - """Yield a started testbed and stop it afterwards. Used by the session fixture. - - ``start`` is inside the ``try`` on purpose. It brings the stack up and then - waits for readiness, so a readiness timeout leaves containers running that - nothing would otherwise stop -- and a suite that cannot reach its backend is - precisely when a developer is going to run it again. - """ - testbed = FlagdTestbed() - try: - testbed.start() - yield testbed - finally: - testbed.stop() diff --git a/uv.lock b/uv.lock index 0af4c1e6b..de3b3fea6 100644 --- a/uv.lock +++ b/uv.lock @@ -1970,12 +1970,11 @@ dependencies = [ dev = [ { name = "coverage", extra = ["toml"] }, { name = "mypy" }, - { name = "openfeature-provider-tck" }, + { name = "openfeature-tck", extra = ["compose"] }, { name = "poethepoet" }, { name = "pytest" }, { name = "pytest-bdd" }, { name = "requests-mock" }, - { name = "testcontainers" }, { name = "types-requests" }, ] @@ -1989,12 +1988,11 @@ requires-dist = [ dev = [ { name = "coverage", extras = ["toml"], specifier = ">=7.10.0,<8.0.0" }, { name = "mypy", specifier = ">=1.18.0,<2.0.0" }, - { name = "openfeature-provider-tck", editable = "tools/openfeature-provider-tck" }, + { name = "openfeature-tck", extras = ["compose"], editable = "tools/openfeature-tck" }, { name = "poethepoet", specifier = ">=0.37.0" }, { name = "pytest", specifier = ">=9.0.0,<10.0.0" }, { name = "pytest-bdd", specifier = ">=8.1.0,<9.0.0" }, { name = "requests-mock", specifier = ">=1.12.0,<2.0.0" }, - { name = "testcontainers", specifier = ">=4.12.0,<5.0.0" }, { name = "types-requests", specifier = ">=2.32.0,<3.0.0" }, ] From 9db8d5d154746b5c803dad32ea517a41d9d09786 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 12 Sep 2026 16:48:16 +0200 Subject: [PATCH 09/22] test(ofrep): keep the conformance suite out of the default build, and say so Same change as the flagd adoption's, for the same reason: the TCK suite is excluded from `poe test` and `poe cov`, gets a `poe test-tck` of its own, and the README records the policy and the tally -- 2 failed, 37 passed, 16 skipped, 1 xfailed, where both failures are canonical flags flagd-testbed v3.8.0 does not seed and the xfail is the one genuine provider gap. It had been running in the default build, so this branch's build was red too. SettledControl also needed correcting, and the correction is a reversal of its own premise. It claimed `POST /start` "is not specified to return only once that state is being served". It is, and it already was at the pin this branch carried: control-api.yaml says "MUST NOT return until the seeded flag state is actually being served", and names this very case -- "the reference implementation exhibits this: its /start returns roughly 40ms before flagd's file sources reach the flag store". So this is not a gap in the contract that the adoption fills in; it is a backend that breaks the contract, and flagd-testbed#394 is open and unmerged. That changes what the file is, for the better. Appendix F now states that a suite must not paper over such a backend from the shared harness -- a fixed delay there "buys silence, not correctness" and every future adopter inherits it -- and that the wait belongs in the adoption, "set explicitly and citing the defect, so that it reads as a named workaround for a specific backend and disappears when the backend is fixed". SettledControl is exactly that, so it now says so instead of arguing for itself. It also forwards `control_api` from the control it delegates to, which the base's now-required protocol member makes necessary and which is the honest answer: the normative control path is still the HTTP control API, and this adds a readiness probe over the provider's own public endpoint rather than replacing anything. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/README.md | 32 ++++++++++ .../openfeature-provider-ofrep/pyproject.toml | 12 +++- .../tests/tck/conftest.py | 12 ++-- .../tests/tck/settled_control.py | 58 +++++++++++++------ 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index cad6a584c..8e3da475d 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -23,6 +23,38 @@ api.set_provider(OFREPProvider()) +## Provider conformance suite + +This provider runs the [OpenFeature Provider Conformance Suite][tck] against a flagd-testbed stack +serving OFREP, in `tests/tck`. The suite owns the container stack: `tests/tck/conftest.py` declares +a Compose file and the port the provider connects to, and nothing else. + +**It is excluded from the default build, and a maintainer runs it by hand before merging a change to +it.** + +``` +poe test-tck # needs Docker +poe test # everything else, which is what CI runs +``` + +The exclusion is a decision rather than an oversight. It needs Docker, and — the part that actually +decides it — a conformance suite reports what is true of the *stack* under test. A full run today is +**2 failed, 37 passed, 16 skipped, 1 xfailed**: both failures are canonical flags that flagd-testbed +v3.8.0 does not seed yet, and the `xfail` is the one genuine provider gap, recorded as a +`KnownDeviation` rather than hidden. A gate that has to be green cannot hold a suite whose honest +output is red, and turning the two backend gaps into `xfail`s would say the provider is at fault +where the backend is. + +`tests/tck/conftest.py` and `tests/tck/test_ofrep_conformance.py` account for each one, so a +reviewer running the suite can tell a new failure from a known one. + +`tests/tck/settled_control.py` is worth reading before you touch the suite: it is a named workaround +for one backend defect — flagd-testbed's `/start` returns about 40 ms before it serves the flag set, +which the control API forbids — and the specification prescribes that such a wait live in the +adoption, citing the defect, rather than in the shared harness. + +[tck]: ../../tools/openfeature-tck/README.md + ## License Apache 2.0 - See [LICENSE](./LICENSE) for more information. diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index d3347c85a..22d10fe29 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -79,8 +79,16 @@ disallow_any_generics = false strict = true [tool.poe.tasks] -test = "pytest tests" -test-cov = "coverage run -m pytest tests" +# `tests/tck` is excluded from the default build on purpose, and a maintainer +# runs `poe test-tck` by hand before merging a change to it. It needs Docker, +# and more to the point a conformance suite reports what is true of the *stack* +# under test: two of its failures today are canonical flags flagd-testbed does +# not seed yet, which are the report rather than something to hide. A gate that +# has to be green cannot hold a suite whose honest output is red. See +# tests/tck/conftest.py for the tally, and the TCK's README for the policy. +test = "pytest tests --ignore=tests/tck" +test-cov = "coverage run -m pytest tests --ignore=tests/tck" +test-tck = "pytest tests/tck" cov-report = "coverage xml" cov = [ "test-cov", diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py index f0b0aa568..70beaf017 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/conftest.py +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -71,11 +71,13 @@ def ofrep_control(tck_backend: RunningBackend, ofrep_base_url: str) -> SettledCo ``prepare_scenario`` takes the documented ``/start`` fallback. The probe costs one 404 for the whole session. - Wrapped in :class:`SettledControl` because ``/start`` returns before the - backend serves the flag set, and a stateless provider has no initialisation - to hide that window behind. See that module -- it is a finding about the - control API's guarantee, not a convenience, and it is a readiness probe over - the provider's own public endpoint rather than a sleep. + Wrapped in :class:`SettledControl` because this backend's ``/start`` + returns before it serves the flag set -- which ``control-api.yaml`` forbids + in those words -- and a stateless provider has no initialisation to hide + that window behind. See that module: it is a named workaround for one + backend's defect, which is where Appendix F says such a wait belongs, and it + is a readiness probe over the provider's own public endpoint rather than a + sleep. """ return SettledControl(tck_backend.control, ofrep_base_url) diff --git a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py index 41ce674dc..739a37f3f 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py +++ b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py @@ -1,15 +1,28 @@ -"""``HttpControl``, plus a wait for the backend to actually serve the flag set. - -**The problem this exists for is worth stating carefully, because it is a -finding rather than a workaround.** - -``POST /start`` is specified to reseed flag state to the named configuration's -baseline (normative requirement 2 in the TCK's ``control-api.yaml``). It is not -specified to *return only once that state is being served*, and flagd-testbed's -launchpad does not: it returns as soon as flagd answers ``/readyz`` -(``launchpad/pkg/flagd.go``), which flagd does before its file sources have been -loaded into the flag store. Measured against this testbed, the window is short --- around 40ms -- but it is real and reliably hit: +"""``HttpControl``, plus a wait for a backend that returns before it serves. + +**This is a named workaround for one backend's defect, and it belongs here +rather than in the shared harness.** That is not a preference; it is what the +specification prescribes. ``control-api.yaml`` requires every state-changing +endpoint to serve the new state before it returns, and Appendix F adds that a +suite must not paper over a backend that breaks it -- a fixed delay in the +harness "buys silence, not correctness", is un-tunable because the window +belongs to the backend, and would be inherited by every future adopter without +knowing why. Where an adopter is stuck with such a backend, "the wait belongs in +**that adoption**, set explicitly and citing the defect, so that it reads as a +named workaround for a specific backend and disappears when the backend is +fixed". This file is that. + +**The defect.** ``POST /start`` reseeds flag state to the named configuration's +baseline and **MUST NOT return until that state is actually being served** -- +the control API says so in those words, and names this very case: "the reference +implementation exhibits this: its ``/start`` returns roughly 40ms before flagd's +file sources reach the flag store". flagd-testbed's launchpad returns as soon as +flagd answers ``/readyz`` (``launchpad/pkg/flagd.go``), which flagd does before +its file sources have been loaded into the flag store. +`flagd-testbed#394 `_ +would close it and is open and unmerged, so the window is still there. Measured +against the pinned image it is short -- around 40ms -- but real and reliably +hit: start:200 {"errorCode":"FLAG_NOT_FOUND","errorDetails":"flag `float-flag` does not exist"} start:200 {"value":0.5,"key":"float-flag","reason":"STATIC","variant":"half"} @@ -24,10 +37,10 @@ provider. **A stateless provider is the first adopter with no initialisation to hide a -backend's warm-up behind**, which makes it the one that discovers whether the -control API's guarantee is strong enough. It is not: "reseeded" and "serving" -need to be the same instant, or every stateless provider reimplements this. That -belongs in the control API contract, and until it is there it belongs here. +backend's warm-up behind**, which is what made it the one that found this. The +guarantee itself is not in doubt -- "reseeded" and "serving" are already +required to be the same instant -- so what is left is a backend that does not +keep it, and this wait goes away the day the testbed does. **Why this is not cheating.** It manipulates nothing. It is a readiness probe over the same public OFREP endpoint the provider uses, on a canonical flag, @@ -48,7 +61,7 @@ import urllib.error import urllib.request -from openfeature.contrib.tools.tck import HttpControl +from openfeature.contrib.tools.tck import ControlApi, HttpControl __all__ = ["SettledControl"] @@ -87,6 +100,17 @@ def __init__( def description(self) -> str: return f"{self._control.description}, awaited through the OFREP endpoint" + @property + def control_api(self) -> ControlApi: + """Whatever the control being delegated to says, which is ``"http"``. + + Forwarded rather than answered, because this class adds a readiness + probe and manipulates nothing: the normative control path is still the + HTTP control API, and a report that said otherwise would understate what + was exercised. + """ + return self._control.control_api + def prepare_scenario(self) -> None: self._control.prepare_scenario() self._await_flags() From 995d597850d2cf3479e662245a50d9edae113c8b Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 00:04:30 +0200 Subject: [PATCH 10/22] test(ofrep): point at Appendix F for the policy, and keep the suite importing The README and the pyproject comment both carried the reasoning for excluding a conformance suite from the default build, in this package's own words, and the two did not quite agree -- the README made Docker a reason, which it is not. That reasoning is now Appendix F's "Running the suite in CI", so both point at it instead. What stays is what is local: where the exclusion lives, the command a maintainer runs, the current tally with what each failure and the one xfail is, and that Docker does not decide it -- the flagd package's tests/e2e needs Docker too and does run in the default build. Appendix F also asks that an excluded suite keep building even when it does not run. `--ignore` does not import the suite at all, and mypy here is configured over `src`, so nothing checked that. `poe test` and `poe test-cov` now end in a collect-only pass over tests/tck, which imports every test module and resolves the feature files without starting a container -- under a second, 56 tests collected. The two default tasks become sequences, so their commands move to `test-default` and `test-cov-default`. `poe cov`, which is what build.yml runs, is unchanged in name and now covers the collect as well. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/README.md | 27 +++++++++++------- .../openfeature-provider-ofrep/pyproject.toml | 28 +++++++++++++------ 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index 8e3da475d..dd0a57f97 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -37,16 +37,22 @@ poe test-tck # needs Docker poe test # everything else, which is what CI runs ``` -The exclusion is a decision rather than an oversight. It needs Docker, and — the part that actually -decides it — a conformance suite reports what is true of the *stack* under test. A full run today is -**2 failed, 37 passed, 16 skipped, 1 xfailed**: both failures are canonical flags that flagd-testbed -v3.8.0 does not seed yet, and the `xfail` is the one genuine provider gap, recorded as a -`KnownDeviation` rather than hidden. A gate that has to be green cannot hold a suite whose honest -output is red, and turning the two backend gaps into `xfail`s would say the provider is at fault -where the backend is. - -`tests/tck/conftest.py` and `tests/tck/test_ofrep_conformance.py` account for each one, so a -reviewer running the suite can tell a new failure from a known one. +The exclusion lives in `pyproject.toml`: `--ignore=tests/tck` on the two tasks `build.yml` reaches, +with the reason in a comment above them. Why a conformance suite is not a required gate is +[Appendix F, "Running the suite in CI"][appendix-f], and is not restated here. + +Two things that are this provider's rather than the policy's: + +- **Docker is not what decides it.** The flagd package's `tests/e2e` needs Docker too and does run in + the default build. What decides it is the run: **2 failed, 37 passed, 16 skipped, 1 xfailed** — + both failures are canonical flags that flagd-testbed v3.8.0 does not seed yet, and the `xfail` is + the one genuine provider gap, recorded as a `KnownDeviation` rather than hidden. + `tests/tck/conftest.py` and `tests/tck/test_ofrep_conformance.py` account for each one, so a + reviewer running the suite can tell a new failure from a known one. +- **The default build still collects the suite** — `poe test` and `poe test-cov` end in + `pytest tests/tck --collect-only`, which imports every module and starts no container. An excluded + suite that has quietly stopped importing against the harness is worse than one that runs and + fails, and `mypy` here is configured over `src` alone, so nothing else would notice. `tests/tck/settled_control.py` is worth reading before you touch the suite: it is a named workaround for one backend defect — flagd-testbed's `/start` returns about 40 ms before it serves the flag set, @@ -54,6 +60,7 @@ which the control API forbids — and the specification prescribes that such a w adoption, citing the defect, rather than in the shared harness. [tck]: ../../tools/openfeature-tck/README.md +[appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md ## License diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index 22d10fe29..c3c8ac350 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -80,15 +80,27 @@ strict = true [tool.poe.tasks] # `tests/tck` is excluded from the default build on purpose, and a maintainer -# runs `poe test-tck` by hand before merging a change to it. It needs Docker, -# and more to the point a conformance suite reports what is true of the *stack* -# under test: two of its failures today are canonical flags flagd-testbed does -# not seed yet, which are the report rather than something to hide. A gate that -# has to be green cannot hold a suite whose honest output is red. See -# tests/tck/conftest.py for the tally, and the TCK's README for the policy. -test = "pytest tests --ignore=tests/tck" -test-cov = "coverage run -m pytest tests --ignore=tests/tck" +# runs `poe test-tck` by hand before merging a change to it. Why a conformance +# suite is not a required gate is Appendix F, "Running the suite in CI" -- +# linked from tools/openfeature-tck/README.md -- and is deliberately not +# restated here. +# +# What is local to this package: a full run is 2 failed, 37 passed, 16 skipped, +# 1 xfailed. Both failures are canonical flags flagd-testbed does not seed yet, +# and the xfail is the one genuine provider gap; tests/tck/conftest.py accounts +# for them. Docker is not what decides the exclusion -- the flagd package's +# `tests/e2e` needs Docker too and does run. +# +# The suite is still *collected* on every default build, so it cannot quietly +# stop importing against the harness while nobody runs it. `--ignore` would +# otherwise leave nothing checking that, since mypy here is configured over +# `src` alone. +test = ["test-default", "test-tck-collect"] +test-cov = ["test-cov-default", "test-tck-collect"] +test-default = "pytest tests --ignore=tests/tck" +test-cov-default = "coverage run -m pytest tests --ignore=tests/tck" test-tck = "pytest tests/tck" +test-tck-collect = "pytest tests/tck --collect-only -q" cov-report = "coverage xml" cov = [ "test-cov", From 0f195046388df7a525b4c8960da67d2a661f36e0 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 10:05:06 +0200 Subject: [PATCH 11/22] test(ofrep): claim the standard resolution reasons spec@c342461a moved every resolution-reason assertion into a new gated gherkin/reason.feature, so the reasons are now a claim a provider declares rather than a house rule every adopter is held to. Declare @standard-reasons. Eight of the file's nine scenarios run here and all eight pass: the four rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR beside their error codes, and -- because @targeting is declared -- TARGETING_MATCH for the matched rule and DEFAULT for the miss. The ninth composes with @disabled-flags, which stays withheld, so it is skipped with that reason. The obstacle expected here is not the one that exists, and it was measured rather than reasoned about. ofrep/__init__.py:159 indexes `Reason[data["reason"]]` by name, so a reason outside the SDK's enum raises KeyError -- which made this capability look like the risky one. It is not: every reason reason.feature asserts is an enum member, DISABLED included. Declaring @disabled-flags beside this one and running the ninth scenario shows that index surviving the DISABLED reason and the next keyword argument failing instead -- `variant=data["variant"]` on line 160, KeyError: 'variant', reported to the application as GENERAL. That is the same one-line defect the @disabled-flags note already records, and it is unrelated to the reason vocabulary. The tally moves from 2 failed, 37 passed, 16 skipped, 1 xfailed to 2 failed, 45 passed, 17 skipped, 1 xfailed. The two failures are unchanged -- large-integer-flag is still not seeded by flagd-testbed v3.8.0. Collection moves from 56 to 65. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/README.md | 2 +- .../tests/tck/test_ofrep_conformance.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index dd0a57f97..7b7a5cc61 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -44,7 +44,7 @@ with the reason in a comment above them. Why a conformance suite is not a requir Two things that are this provider's rather than the policy's: - **Docker is not what decides it.** The flagd package's `tests/e2e` needs Docker too and does run in - the default build. What decides it is the run: **2 failed, 37 passed, 16 skipped, 1 xfailed** — + the default build. What decides it is the run: **2 failed, 45 passed, 17 skipped, 1 xfailed** — both failures are canonical flags that flagd-testbed v3.8.0 does not seed yet, and the `xfail` is the one genuine provider gap, recorded as a `KnownDeviation` rather than hidden. `tests/tck/conftest.py` and `tests/tck/test_ofrep_conformance.py` account for each one, so a diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 65daa0d61..6111b0291 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -75,6 +75,32 @@ # is verified is that the key reached flagd, since nothing else about the # context is keyed on by any canonical flag. # +# STANDARD_REASONS +# New at spec@c342461a, which moved every resolution-reason assertion out of +# the other feature files and into reason.feature, gated as a whole. A claim +# rather than an exemption: 2.2.5 is a SHOULD that permits "some other +# string", so declaring the tag says this provider uses the standard +# vocabulary with the standard meanings. +# +# Eight of the file's nine scenarios run here and all eight pass: the four +# rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR +# beside their error codes, and -- because TARGETING is declared above -- +# TARGETING_MATCH for the matched rule and DEFAULT for the miss. The ninth +# composes with @disabled-flags, withheld below, so it is skipped with that +# reason. +# +# The obstacle that was expected here is not the one that exists, and it was +# measured rather than reasoned about. ofrep/__init__.py:159 indexes +# `Reason[data["reason"]]` by *name*, so a server reporting a reason outside +# the SDK's enum raises KeyError -- which is why this capability looked like +# the risky one. It is not: every reason reason.feature asserts is an enum +# member, DISABLED included. Declaring @disabled-flags alongside this one and +# running the ninth scenario shows the index surviving the DISABLED reason +# and the *next* keyword argument failing, `variant=data["variant"]` on line +# 160, with KeyError: 'variant' reported to the application as GENERAL. That +# is the same one-line defect the @disabled-flags note below records, and it +# is unrelated to the reason vocabulary. +# # NUMERIC_COERCION is withheld, and the reason is worth recording because two # other languages answered it differently over the same protocol. # @@ -228,6 +254,7 @@ Capability.OBJECT, Capability.VARIANTS, Capability.TARGETING, + Capability.STANDARD_REASONS, } ) From 1de45cddd4cbaef592fadbfdc57272290882a423 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 18:51:37 +0200 Subject: [PATCH 12/22] docs(ofrep): say which side of the corrected declaring rules each withholding is on Appendix F gained a sixth declaring rule and a corrected numeric-coercion note (spec@4cab0320, spec@045950ca). Three of this suite's withholdings are affected and none of them is affected the same way, so each now says which. @numeric-coercion is unchanged and correct. The corrected note says a provider that attempts the coercion and gets a direction wrong declares the tag and lets the scenario fail; this provider never coerces at all, keeps the two JSON types apart end to end, and is the kind the note still says should withhold. Worth stating because the flagd adoption in this repository now reads the opposite way from the same rule, and a reader comparing the two files deserves to know that is the rule working rather than two suites disagreeing. @large-integers had no note at all, which the sixth rule's second consequence makes a defect in its own right: a capability withheld for a backend gap is temporary, and without a note saying why it outlives its reason. Its single scenario asks for huge-integer-flag, flagd-testbed v3.8.0 seeds none, so not one of the tag's scenarios reaches this provider -- the mirror image of @variants two entries above, where seven of eight rows do and the tag is declared. @disabled-flags is the one decision here the correction says should change, and this commit does not change it, because a documentation pass that moves a count is no longer a documentation pass. The note records the argument in full instead: this provider does attempt the behaviour and fails on one unconditional index, so the appendix's preferred shape is to declare the tag, accept four failing rows and record the defect as an untracked deviation. The self-test carve-out added in the same revision licenses a withholding for an identified defect, but is explicit that an adoption has none. The next change to this file is that one. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 6111b0291..10579393d 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -133,6 +133,16 @@ # as normative OpenFeature. The honest record is the undeclared tag and the three # skips it produces. # +# This withholding survives the correction that appendix made at spec@045950ca, +# and it is worth saying which side of it this is on, because the flagd adoption +# in this repository reads the opposite way. That note now says a provider which +# *attempts* the coercion and gets a direction wrong declares the tag and lets +# the scenario fail -- flagd's RPC resolver narrows 0.5 to 0 and does exactly +# that -- and that withholding is for a provider which cannot attempt it at all. +# This provider is the second kind: it never widens or narrows anything, the two +# JSON types stay apart end to end, and there is no coercion here to get wrong. +# Same rule, different provider, opposite answer. +# # DISABLED_FLAGS is withheld as well, new at spec@009afe06, and this one is a # provider defect rather than an architecture. Which is the opposite of what the # appendix predicts, so it is worth being exact about. @@ -174,6 +184,23 @@ # record is the undeclared tag, the four skips it produces, and this note saying # the gap is a bug somebody can fix rather than a fact of the protocol. # +# **That reasoning does not survive spec@045950ca, and this is the one decision +# in these suites that the corrected appendix says should change.** It is left +# standing here only because the pass that found it was a documentation pass and +# changing it moves a count. The difference from the numeric-coercion note above +# is that this provider *does* attempt the behaviour: it resolves a disabled flag +# and gets it wrong on one unconditional index, which is the case the appendix +# tells an adopter to declare and let fail, with a KnownDeviation.untracked +# beside it -- "withdrawing the capability replaces a failing scenario with a +# skip and hides a defect behind something that looks deliberate". The self-test +# carve-out in that same revision, which does license a withholding for an +# identified defect, is explicit that an adoption has none: this suite exists to +# report on a provider, and a skip here is a claim about that provider. +# +# So the next change to this file declares @disabled-flags, accepts four failing +# rows, and records the `data["variant"]` defect as an untracked deviation -- and +# files it, which is what makes the deviation tracked and the failures temporary. +# # Not declared, and why. Each is a fact about the provider, established by # reading it -- OFREPProvider is stateless: it holds a requests.Session and a # rate-limit timestamp, and nothing else survives between evaluations. @@ -240,6 +267,30 @@ # the same declare-what-nothing-exercises error the reserved tag below is # kept out for. # +# LARGE_INTEGERS +# The one withholding in this list that is about the backend rather than the +# provider, and until this pass the one with no reason written down at all. +# Appendix F's sixth declaring rule (spec@4cab0320) is what decides it: +# exactly one scenario carries the tag, it asks for `huge-integer-flag`, and +# flagd-testbed v3.8.0 seeds no such flag -- so not one of the tag's +# scenarios can be put to this provider, and nothing about the capability +# can be established either way. Contrast VARIANTS above, where seven of +# eight rows do reach the provider and the tag is declared on their strength. +# +# Nothing in this path would narrow the value: a JSON number decodes through +# `json.loads` into an unbounded Python `int` and the type check at +# ofrep/__init__.py:249-256 admits it unchanged. The suite cannot show that, +# which is the point -- a declaration would be a claim with no evidence +# behind it in either direction. +# +# No KnownDeviation, in either shape: the gap is the fixture's, and an entry +# would attribute it to the provider. And per the same rule's second +# consequence this withholding is temporary in a way the ones above are not. +# open-feature/flagd-testbed#392 adds the flag; declare the tag when the +# image carries it, or this outlives its reason and starts reading as a +# claim about the provider. Both flagd suites here withhold it on the same +# ground and say so in the same terms. +# # CACHING # Reserved in the Capability enum; no scenario carries the tag. Declaring a # capability nothing exercises would be a claim with no evidence behind it. From 985779065b8ec9ba7ae6b06404f3210ecddb73cf Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 19:15:00 +0200 Subject: [PATCH 13/22] docs(ofrep): say which rule each withholding reaches, after rule six was narrowed spec@aa2ad24f puts a condition on the rule cited here a commit ago: it applies once a provider is attempting the capability, and decides whether a question is askable rather than whether an answer is owed. Applied to this suite that is not a detail, it is the difference between three right answers and one manufactured failure. @numeric-coercion would have been that failure. All three of its scenarios are askable of this provider and two of them fail, so the rule as first written forced a declaration -- of a capability no requirement defines, from a provider that does not coerce by design. It never reaches rule six now. The Go implementation found the over-reach against its own self-tests; this suite would have been the second one. @large-integers does reach it, and the note says why: nothing here declines to resolve a large integer, so attempting is not in doubt and askability is the only question left. The backend seeds no huge-integer-flag, so the answer is withhold. @disabled-flags reaches neither, and the note now says which rule its argument actually turns on -- the two shapes, not rule six. The evidence is that this provider attempts and fails rather than declining: it has no design position on disabled flags, it parses the response flagd sends for one, and it raises KeyError on a member the protocol types as optional. A provider that declined would have nothing in the path that could be right or wrong, which is exactly what the numeric-coercion entry above looks like. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep_conformance.py | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index 10579393d..e088ecbae 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -143,6 +143,16 @@ # JSON types stay apart end to end, and there is no coercion here to get wrong. # Same rule, different provider, opposite answer. # +# It also survives rule six, and spec@aa2ad24f is why. As that rule was first +# written -- declare when at least one scenario gating the tag can be put to the +# provider -- it would have forced a declaration here, since all three scenarios +# are perfectly askable of this provider and two of them fail. The rule now +# opens by saying it applies *once a provider is attempting the capability*, and +# decides whether a question is askable rather than whether an answer is owed. +# Nothing requires numeric coercion, this provider does not attempt it, so the +# question never reaches rule six. The Go implementation found that over-reach +# against its own self-tests; this suite would have been the second casualty. +# # DISABLED_FLAGS is withheld as well, new at spec@009afe06, and this one is a # provider defect rather than an architecture. Which is the opposite of what the # appendix predicts, so it is worth being exact about. @@ -197,6 +207,18 @@ # identified defect, is explicit that an adoption has none: this suite exists to # report on a provider, and a skip here is a claim about that provider. # +# Note which rule that turns on, because spec@aa2ad24f makes the difference +# sharp. It is *not* rule six: that rule decides askability and applies only +# once a provider is attempting the capability, and if attempting were in doubt +# it would not reach the question. It is the two-shapes rule, and the evidence +# says this provider attempts and fails rather than declining -- it has no +# design position on disabled flags at all, it parses the response flagd sends +# for one and raises KeyError on a member the protocol types as optional, which +# breaks on any variant-less response and not only this one. A provider that +# declined by design would look like the numeric-coercion entry above: nothing +# in the code path that could be right or wrong, rather than one index that is +# wrong. +# # So the next change to this file declares @disabled-flags, accepts four failing # rows, and records the `data["variant"]` defect as an untracked deviation -- and # files it, which is what makes the deviation tracked and the failures temporary. @@ -270,8 +292,11 @@ # LARGE_INTEGERS # The one withholding in this list that is about the backend rather than the # provider, and until this pass the one with no reason written down at all. -# Appendix F's sixth declaring rule (spec@4cab0320) is what decides it: -# exactly one scenario carries the tag, it asks for `huge-integer-flag`, and +# Appendix F's sixth declaring rule (spec@4cab0320, narrowed at +# spec@aa2ad24f) is what decides it, and this is a tag that reaches that +# rule: nothing about this provider declines to resolve a large integer, so +# it is attempting the capability and only askability is left in question. +# Exactly one scenario carries the tag, it asks for `huge-integer-flag`, and # flagd-testbed v3.8.0 seeds no such flag -- so not one of the tag's # scenarios can be put to this provider, and nothing about the capability # can be established either way. Contrast VARIANTS above, where seven of From 02de2d54437f7f77e58fd14439b05db786da5b58 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 19:27:50 +0200 Subject: [PATCH 14/22] docs(ofrep): correct two tallies that reason.feature left behind Running the suite to confirm this pass moved nothing found a count that had already moved and not been written down. `poe test-tck` is 2 failed, 45 passed, 17 skipped, 1 xfailed; `pyproject.toml` still said 2 / 37 / 16 / 1, which was true when this suite collected 56 scenarios and stopped being true when reason.feature took it to 65. The README already carried the right figure, so the two disagreed with each other as well as with the run. The @disabled-flags note quoted the same superseded run as its measurement of what declaring the tag would cost. The measurement stands -- all four rows fail, on the error code rather than the value -- so it is kept and dated rather than rewritten, with today's baseline beside it and the arithmetic said plainly: the same four rows move from skipped to failed. Same lesson as the three stale tallies the flagd suite carried a pass ago. A count in prose is a claim, it ages the moment the assets move, and the only thing that finds it is running the suite and comparing. Signed-off-by: Simon Schrottner --- providers/openfeature-provider-ofrep/pyproject.toml | 2 +- .../tests/tck/test_ofrep_conformance.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index c3c8ac350..d38940e5a 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -85,7 +85,7 @@ strict = true # linked from tools/openfeature-tck/README.md -- and is deliberately not # restated here. # -# What is local to this package: a full run is 2 failed, 37 passed, 16 skipped, +# What is local to this package: a full run is 2 failed, 45 passed, 17 skipped, # 1 xfailed. Both failures are canonical flags flagd-testbed does not seed yet, # and the xfail is the one genuine provider gap; tests/tck/conftest.py accounts # for them. Docker is not what decides the exclusion -- the flagd package's diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py index e088ecbae..846828f1b 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py @@ -163,9 +163,12 @@ # obstacle here. # # Measured first. Declaring the tag fails all four rows -- 6 failed, 37 passed, -# 12 skipped, 1 xfailed, against the 2 failed of the run without it -- and each -# fails on the error code rather than on the value: "error-code was 'GENERAL', -# expected none". Then read back, and probed at the wire to be sure of the +# 12 skipped, 1 xfailed at the time, against the 2 failed of the run without it +# -- and each fails on the error code rather than on the value: "error-code was +# 'GENERAL', expected none". That probe predates reason.feature, which took this +# suite from 56 collected to 65; today's run without the tag is 2 failed, 45 +# passed, 17 skipped, 1 xfailed, and declaring it would move the same four rows +# from skipped to failed. Then read back, and probed at the wire to be sure of the # reading. flagd's OFREP endpoint answers a disabled flag # `200 {"key": ..., "reason": "DISABLED", "metadata": {}}`: no `value`, and no # `variant`. The server does indeed never return a value, exactly as the From 9a45c68bc2dafe68608c5585d215105e9e2398c7 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 22:21:49 +0200 Subject: [PATCH 15/22] test(ofrep): name the conformance module for the provider, not the suite `tests/tck` is already a sibling of `tests/e2e` and `poe test-tck` selects it by that path, so `test_ofrep_conformance.py` was saying conformance twice. Only the half that duplicated the directory is dropped; the `test_` prefix stays because pytest needs it. The renamed module's node ids all move, and this branch is the one with a hardcoded node id: the `xfail(strict=True)` for python-sdk#619 in `tests/tck/conftest.py`. It matches on `item.name`, which is the test function plus its parameters and carries no module, so the rename does not reach it -- and the run after confirms it: the scenario still reports XFAIL with its reason, which a strict xfail that stopped matching could not do. Same tally: 2 failed, 45 passed, 17 skipped, 1 xfailed. Signed-off-by: Simon Schrottner --- providers/openfeature-provider-ofrep/README.md | 2 +- .../tests/tck/{test_ofrep_conformance.py => test_ofrep.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename providers/openfeature-provider-ofrep/tests/tck/{test_ofrep_conformance.py => test_ofrep.py} (100%) diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index 7b7a5cc61..93a3c03db 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -47,7 +47,7 @@ Two things that are this provider's rather than the policy's: the default build. What decides it is the run: **2 failed, 45 passed, 17 skipped, 1 xfailed** — both failures are canonical flags that flagd-testbed v3.8.0 does not seed yet, and the `xfail` is the one genuine provider gap, recorded as a `KnownDeviation` rather than hidden. - `tests/tck/conftest.py` and `tests/tck/test_ofrep_conformance.py` account for each one, so a + `tests/tck/conftest.py` and `tests/tck/test_ofrep.py` account for each one, so a reviewer running the suite can tell a new failure from a known one. - **The default build still collects the suite** — `poe test` and `poe test-cov` end in `pytest tests/tck --collect-only`, which imports every module and starts no container. An excluded diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py similarity index 100% rename from providers/openfeature-provider-ofrep/tests/tck/test_ofrep_conformance.py rename to providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py From b22f2ab1c7e5fe5b2b9a19e4fbafb1b37f1b0da9 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sun, 13 Sep 2026 23:06:06 +0200 Subject: [PATCH 16/22] test(ofrep): let the collect step run even when the default suite fails Same pair as the flagd package: poe aborts a sequence at its first failing subtask, so `test-tck-collect` -- the one check that the conformance suite still imports against the harness while `--ignore` keeps it out of the run -- sat behind the default suite's result. It is green here today, which is exactly why this is worth fixing before it is not: a check that only runs while everything else passes is not a check. `ignore_fail = "return_non_zero"`: every subtask runs, a non-zero exit still propagates. The collect step reports the same 65. Signed-off-by: Simon Schrottner --- providers/openfeature-provider-ofrep/pyproject.toml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/providers/openfeature-provider-ofrep/pyproject.toml b/providers/openfeature-provider-ofrep/pyproject.toml index d38940e5a..0e1ff9ff2 100644 --- a/providers/openfeature-provider-ofrep/pyproject.toml +++ b/providers/openfeature-provider-ofrep/pyproject.toml @@ -95,8 +95,15 @@ strict = true # stop importing against the harness while nobody runs it. `--ignore` would # otherwise leave nothing checking that, since mypy here is configured over # `src` alone. -test = ["test-default", "test-tck-collect"] -test-cov = ["test-cov-default", "test-tck-collect"] +# +# `ignore_fail = "return_non_zero"` because poe aborts a sequence at its first +# failing subtask, which would put the collect step behind the default suite's +# result. It is green in this package today, and a check that only runs while +# everything else passes is not a check. Every subtask runs and a non-zero exit +# still propagates. The flagd package carries the same pair, where the default +# suite is red and the collect step was in fact never reached. +test = { sequence = ["test-default", "test-tck-collect"], ignore_fail = "return_non_zero" } +test-cov = { sequence = ["test-cov-default", "test-tck-collect"], ignore_fail = "return_non_zero" } test-default = "pytest tests --ignore=tests/tck" test-cov-default = "coverage run -m pytest tests --ignore=tests/tck" test-tck = "pytest tests/tck" From 020c23db9fb87838daa6837d39686339ddaaf799 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 14 Sep 2026 08:25:17 +0200 Subject: [PATCH 17/22] test(ofrep): take the same compose definition as the flagd adoption Byte-identical to providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml, so the repository has one definition of the backend under test rather than two that differ only in a port list. It publishes 8013, 8015, 8016 and 8080; this suite declares 8016 alone, and the harness waits only on what an adoption declares, so the two extra ports cost nothing. Copied rather than shared, for the reason the flagd commit records: each provider package is published independently, so a file outside the package directory would not travel with the distribution, and pointing this conftest at the flagd package's directory is the coupling this adoption removed when it stopped reaching into that package's test-harness submodule for a compose file. `diff` the two paths after changing either. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/README.md | 5 ++++ .../tests/tck/docker-compose.yaml | 30 ++++++++----------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index 93a3c03db..86933509f 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -29,6 +29,11 @@ This provider runs the [OpenFeature Provider Conformance Suite][tck] against a f serving OFREP, in `tests/tck`. The suite owns the container stack: `tests/tck/conftest.py` declares a Compose file and the port the provider connects to, and nothing else. +`tests/tck/docker-compose.yaml` is one definition of the backend for the whole repository, and the +flagd adoption carries a byte-identical copy — each provider package publishes its own distribution +and must not read the other's files, so the two are kept in step by `diff` rather than by sharing a +path. Change one, copy it to the other. + **It is excluded from the default build, and a maintainer runs it by hand before merging a change to it.** diff --git a/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml index 1eecdb483..5c27bf279 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml +++ b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml @@ -1,26 +1,20 @@ -# Backend stack for the OpenFeature TCK, wrapping the unmodified flagd testbed image. +# The backend every conformance suite in this repository runs against: the unmodified +# flagd testbed image, serving flagd and its launchpad control API. # -# flagd serves the OFREP HTTP API on 8016 alongside its own protocols, and the "launchpad" -# control API on 8080. So an OFREP provider needs no backend of its own: it runs against the -# same image, seeded with the same canonical flag set, driven through the same control API as -# the flagd conformance suites. A second stack would be a second definition of "the canonical -# flags", which is the one thing a conformance suite exists to prevent. +# Not flagd-testbed's own compose file, which adds an envoy sidecar for its +# forbidden-endpoint scenarios, names its service `flagd`, and bind-mounts a flags +# directory the launchpad writes into. None of that is needed to drive the launchpad. # -# Deliberately not flagd-testbed's own compose file, which this suite used to reach across into -# the flagd package's submodule for. That file stands up envoy for the forbidden-endpoint e2e -# scenarios, names its service `flagd`, and bind-mounts a flags directory the launchpad writes -# into -- none of which the TCK needs. Reaching for it also coupled this package's conformance -# results to another package's test-harness submodule. -# -# The tag is pinned here and in the flagd adoption's copy of this file. Two files naming one -# version is the cost of each package owning its own stack; they serve different ports of the -# same image and are bumped together. -# -# Note there are no host port bindings. The TCK requires dynamically mapped ports and discovers -# them after startup. +# This file exists twice, once per provider package, and the two are byte-identical: +# providers/openfeature-provider-flagd/tests/tck/docker-compose.yaml +# providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml +# Each package publishes its own distribution and must not read the other's files, so +# `diff` the two paths after changing either -- that diff is what catches drift. services: backend: image: ghcr.io/open-feature/flagd-testbed:v3.8.0 ports: + - 8013 # flagd RPC evaluation (gRPC) + - 8015 # flagd in-process sync (gRPC) - 8016 # flagd's OFREP HTTP API - 8080 # launchpad control API From 8b12e66dc99825b95a0071fc73af7d1b28bc26e3 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 14 Sep 2026 08:25:18 +0200 Subject: [PATCH 18/22] docs(ofrep): keep the evidence, link the rules, delete the retellings test_ofrep.py was 368 lines to 35 of code, and the surplus was around the reasoning rather than in it: Appendix F's declaring rules quoted at length, the same spec revisions cited tag by tag, and two paragraphs comparing what the Java and Go OFREP adoptions did -- which is in PR #414's body, where a reviewer comparing four languages is actually looking. Both withholdings keep their measurements whole, because they are the two things here nothing else records: - @numeric-coercion, withheld because this provider never coerces -- json.loads keeps int and float apart and the check admits a value only on an exact isinstance, so the lossy row passes and a lossless one fails. Over OFREP the capability follows the language's JSON library. - @disabled-flags, withheld for a defect rather than an architecture, and the whole gap is one unconditional `data["variant"]` index on a member the protocol types optional. The wire response is kept, the measurement is kept, and the acknowledgement that this is the one declaration the corrected appendix says should change shape is kept in short form, pointing at PR #414 where the decision and the same measurement are recorded in full. The note also stops calling the defect unfiled: it is open-feature/python-sdk-contrib#418. Two claims went because they had gone stale rather than because they were duplication. The file argued at length with the appendix's rationale for gating @disabled-flags -- "a provider whose backend decides, such as one speaking OFREP, cannot" -- and the appendix no longer says it, so the rebuttal had nothing to rebut. settled_control.py keeps what it is and why it lives in this adoption rather than in the shared harness, and hands the mechanism of the window to open-feature/flagd-testbed#394, which explains it down to the buffered channel in flagd's file sync and measures it. Comments and docstrings only. The suite still reports 2 failed, 45 passed, 17 skipped, 1 xfailed. Signed-off-by: Simon Schrottner --- .../tests/tck/conftest.py | 61 ++- .../tests/tck/settled_control.py | 71 ++-- .../tests/tck/test_ofrep.py | 373 ++++++------------ 3 files changed, 173 insertions(+), 332 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py index 70beaf017..60150ccd8 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/conftest.py +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -1,17 +1,16 @@ """Session fixtures for the OFREP conformance suite, and one recorded deviation. -The container lifecycle belongs to the TCK: it starts the Compose file beside -this module once per session, discovers the dynamically mapped host ports, builds -the ``HttpControl`` against the launchpad and waits for it to accept commands. -What is left here is the declaration, the one wrapper this provider needs around -the control, and the xfail for the single scenario it cannot satisfy. - -The stack is started once and never restarted, because Compose assigns host -ports dynamically and cannot preserve them across a restart: a restarted backend -comes back on a different port, silently invalidating a provider already pointed -at the old one, and the failure reads as a flaky provider rather than a broken -test. Scenario isolation comes from the control API instead -- see the -no-container-restart invariant in the TCK's ``control-api.yaml``. +The container lifecycle belongs to the TCK -- see its README for what +``tck_backend`` does with the declaration below, and Appendix F, "The control +API", for why the stack is started once and never restarted. What is left here is +the declaration, the one wrapper this provider needs around the control, and the +xfail for the single scenario it cannot satisfy. + +A full run is ``2 failed, 45 passed, 17 skipped, 1 xfailed``. Both failures ask +for ``large-integer-flag``, which flagd-testbed v3.8.0 does not seed -- +open-feature/flagd-testbed#392 adds it, along with the two other canonical flags +the image is missing, and says what each catches. Neither carries a +``KnownDeviation``: the gap is the backend's flag set, not the provider's. """ from __future__ import annotations @@ -24,12 +23,10 @@ from tests.tck.settled_control import SettledControl OFREP_PORT = 8016 -"""flagd's OFREP HTTP port. +"""flagd's OFREP HTTP port, and the one port this provider connects to. -flagd's own default (``flags.Int32P("ofrep-port", "r", 8016, ...)`` in flagd's -``cmd/start.go``). The testbed's launchpad starts flagd with no -``--ofrep-port`` override, so this is what it listens on, and it is the one port -the provider connects to. The launchpad's own 8080 is exposed automatically. +flagd's own default, which the testbed's launchpad does not override. The +launchpad's control port is exposed by the harness and must not be listed here. """ @@ -37,9 +34,13 @@ def compose_backend() -> ComposeBackend: """The stack under test, as the TCK's ``tck_backend`` fixture wants it. - The path is absolute rather than relative to the package directory -- which - is what the harness resolves a relative one against, and where ``poe test`` - runs from -- so that running pytest from the repository root works too. + The Compose file is the same one the flagd adoption uses -- one definition of + the backend, copied per package -- so it publishes flagd's two resolver ports + as well. Nothing here declares them, and a port nobody declares is neither + waited on nor looked up. + + The path is absolute so that pytest run from the repository root works too; + a relative one resolves against the working directory. """ return ComposeBackend( compose_file=Path(__file__).parent / "docker-compose.yaml", @@ -65,19 +66,13 @@ def ofrep_control(tck_backend: RunningBackend, ofrep_base_url: str) -> SettledCo """The control API client, wrapped in a wait for the flag set to be served. ``tck_backend.control`` is the TCK's own ``HttpControl``, already pointed at - the launchpad's mapped port and awaited ready. The launchpad registers only - ``/start``, ``/restart``, ``/stop`` and ``/change`` (flagd-testbed - ``launchpad/main.go:29-32``), so ``/reset`` answers 404 and every - ``prepare_scenario`` takes the documented ``/start`` fallback. The probe - costs one 404 for the whole session. - - Wrapped in :class:`SettledControl` because this backend's ``/start`` - returns before it serves the flag set -- which ``control-api.yaml`` forbids - in those words -- and a stateless provider has no initialisation to hide - that window behind. See that module: it is a named workaround for one - backend's defect, which is where Appendix F says such a wait belongs, and it - is a readiness probe over the provider's own public endpoint rather than a - sleep. + the launchpad's mapped port and awaited ready. The launchpad registers no + ``/reset``, so every ``prepare_scenario`` takes the harness's documented + ``/start`` fallback and one 404 is logged per session. + + Wrapped in :class:`SettledControl` because this backend's ``/start`` returns + before it serves the flag set, and a stateless provider has no initialisation + to hide that window behind. See that module. """ return SettledControl(tck_backend.control, ofrep_base_url) diff --git a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py index 739a37f3f..f75421844 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py +++ b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py @@ -1,53 +1,30 @@ """``HttpControl``, plus a wait for a backend that returns before it serves. -**This is a named workaround for one backend's defect, and it belongs here -rather than in the shared harness.** That is not a preference; it is what the -specification prescribes. ``control-api.yaml`` requires every state-changing -endpoint to serve the new state before it returns, and Appendix F adds that a -suite must not paper over a backend that breaks it -- a fixed delay in the -harness "buys silence, not correctness", is un-tunable because the window -belongs to the backend, and would be inherited by every future adopter without -knowing why. Where an adopter is stuck with such a backend, "the wait belongs in -**that adoption**, set explicitly and citing the defect, so that it reads as a -named workaround for a specific backend and disappears when the backend is -fixed". This file is that. - -**The defect.** ``POST /start`` reseeds flag state to the named configuration's -baseline and **MUST NOT return until that state is actually being served** -- -the control API says so in those words, and names this very case: "the reference -implementation exhibits this: its ``/start`` returns roughly 40ms before flagd's -file sources reach the flag store". flagd-testbed's launchpad returns as soon as -flagd answers ``/readyz`` (``launchpad/pkg/flagd.go``), which flagd does before -its file sources have been loaded into the flag store. +**A named workaround for one backend's defect, in the adoption rather than in +the shared harness**, which is the shape Appendix F prescribes for exactly this +situation -- see "The control API" there for why a delay in the harness would be +the wrong instrument. + +**The defect.** ``control-api.yaml`` requires ``POST /start`` not to return until +the state it reseeded is actually being served. flagd-testbed's launchpad returns +as soon as flagd answers ``/readyz``, which flagd does before its file sources +have reached the flag store. `flagd-testbed#394 `_ -would close it and is open and unmerged, so the window is still there. Measured -against the pinned image it is short -- around 40ms -- but real and reliably -hit: - - start:200 {"errorCode":"FLAG_NOT_FOUND","errorDetails":"flag `float-flag` does not exist"} - start:200 {"value":0.5,"key":"float-flag","reason":"STATIC","variant":"half"} - start:200 {"errorCode":"FLAG_NOT_FOUND","errorDetails":"flag `float-flag` does not exist"} - -The flagd suites never see it, and that is the interesting part. Both flagd -resolvers block inside ``initialize`` until the evaluation stream is up or the -ruleset has synced, so their initialisation absorbs the window before any -scenario evaluates. OFREP is stateless -- no ``initialize``, no connection, no -warm-up -- so its first evaluation lands directly in the gap and the suite -reports FLAG_NOT_FOUND for every flag, which reads as a catastrophically broken -provider. - -**A stateless provider is the first adopter with no initialisation to hide a -backend's warm-up behind**, which is what made it the one that found this. The -guarantee itself is not in doubt -- "reseeded" and "serving" are already -required to be the same instant -- so what is left is a backend that does not -keep it, and this wait goes away the day the testbed does. - -**Why this is not cheating.** It manipulates nothing. It is a readiness probe -over the same public OFREP endpoint the provider uses, on a canonical flag, -asserting only that the backend has finished doing what ``/start`` already -promised. No scenario is weakened, no step is bypassed, and no side channel into -the backend is opened -- the normative control path is still ``HttpControl``, -which this delegates to unchanged. +explains the mechanism, measures the window and closes it; it is open and +unmerged, so the window is still there. + +**Why this suite is the one that found it.** Both flagd resolvers block inside +``initialize`` until their stream or ruleset is up, so their initialisation +absorbs the window before any scenario evaluates. A stateless provider has no +initialisation to hide behind: its first evaluation lands directly in the gap and +the suite reports FLAG_NOT_FOUND for every flag, which reads as a +catastrophically broken provider. This wait goes away the day the testbed does. + +**Why it is not cheating.** It manipulates nothing. It is a readiness probe over +the same public OFREP endpoint the provider uses, on a canonical flag, asserting +only that the backend finished what ``/start`` already promised. No scenario is +weakened and no side channel into the backend is opened -- the normative control +path is still ``HttpControl``, which this delegates to unchanged. Deliberately not a :class:`ConnectionControl`: it has no ``disconnect`` or ``reconnect``, matching a suite that declares neither ``STALE`` nor diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py index 846828f1b..cb4c3f3db 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py @@ -32,239 +32,135 @@ """Bounds a single OFREP request. Generous, because every scenario is preceded by a control-API ``/start`` that -restarts the flagd process, so the first evaluation of a scenario routinely hits -a backend that came up milliseconds ago. It is the only timing knob this -provider has (``ofrep/__init__.py:52``); everything else the TCK offers -- event -timeouts, ready timeouts -- has nothing to bound, for the reasons below. +restarts the flagd process. It is the only timing knob this provider has +(``ofrep/__init__.py:52``); everything the TCK offers -- event timeouts, ready +timeouts -- has nothing to bound here, for the reasons below. """ # Every capability below was declared, the suite run, and its scenarios seen to # pass -- bar the one row named under VARIANTS, which fails on a flag the testbed -# does not seed. The code references say where the behaviour lives, so a reader -# can check a claim; they are not the evidence for it. That order is Appendix F's -# rule as of spec@26362f85, and this file used to state the reverse. -# -# OBJECT -# ofrep/__init__.py:105-113 resolves structured values, and the type check at -# ofrep/__init__.py:248 admits `(dict, list)` for FlagType.OBJECT -- so a JSON -# object comes back as one rather than being rejected or flattened. -# -# VARIANTS -# ofrep/__init__.py:160 carries the response's `variant` field into the -# resolution details, and flagd names a variant for every flag it serves. -# Seven of the outline's eight rows pass. The eighth asks for -# large-integer-flag's `max-int32` and fails with the flag missing from the -# backend -- flagd-testbed v3.8.0 seeds neither large-integer-flag nor -# huge-integer-flag, which already fails the untagged precision scenario -# here and does the same in both flagd suites. Withholding the capability -# over it would say this provider does not name variants, which the other -# seven rows show is false, and would blame a missing flag on a capability -# the provider has. It is not a KnownDeviation either: a deviation is for a -# behaviour the provider is required to have and does not. -# -# TARGETING -# ofrep/__init__.py:229-230 puts the evaluation context's targeting key into -# the request body's `context` object, so flagd evaluates -# targeting-key-flag's rule against it. All three scenarios pass -- the -# matching context, the non-matching one and no context at all -- which is -# what makes context passthrough observable here without an echo endpoint: -# a provider that dropped the context would resolve `miss` where `hit` is -# expected. -# -# Worth noting for a protocol with no types on the wire: the whole of what -# is verified is that the key reached flagd, since nothing else about the -# context is keyed on by any canonical flag. +# does not seed. The code references say where the behaviour lives, so that a +# reader can check the claim; the run is what it rests on. +# +# OBJECT ofrep/__init__.py:105-113 resolves structured values, and the +# type check at ofrep/__init__.py:248 admits `(dict, list)` for +# FlagType.OBJECT -- so a JSON object comes back as one rather than +# being rejected or flattened. +# VARIANTS ofrep/__init__.py:160 carries the response's `variant` into the +# resolution details. Seven of the outline's eight rows pass; the +# eighth asks for large-integer-flag's `max-int32`, which +# flagd-testbed v3.8.0 does not seed (see conftest.py). Withholding +# over it would say this provider does not name variants, which the +# other seven rows show is false. +# TARGETING ofrep/__init__.py:229-230 puts the evaluation context's targeting +# key into the request body's `context`, so flagd evaluates +# targeting-key-flag's rule against it. All three scenarios pass -- +# the matching context, the non-matching one and no context at all +# -- and for a protocol with no types on the wire that is the whole +# of what is verified about passthrough: the key reached flagd. # # STANDARD_REASONS -# New at spec@c342461a, which moved every resolution-reason assertion out of -# the other feature files and into reason.feature, gated as a whole. A claim -# rather than an exemption: 2.2.5 is a SHOULD that permits "some other -# string", so declaring the tag says this provider uses the standard -# vocabulary with the standard meanings. -# # Eight of the file's nine scenarios run here and all eight pass: the four # rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR # beside their error codes, and -- because TARGETING is declared above -- # TARGETING_MATCH for the matched rule and DEFAULT for the miss. The ninth -# composes with @disabled-flags, withheld below, so it is skipped with that -# reason. +# composes with @disabled-flags, withheld below, so it skips with that reason. # -# The obstacle that was expected here is not the one that exists, and it was -# measured rather than reasoned about. ofrep/__init__.py:159 indexes +# **The obstacle that was expected here is not the one that exists**, and it +# was measured rather than reasoned about. ofrep/__init__.py:159 indexes # `Reason[data["reason"]]` by *name*, so a server reporting a reason outside # the SDK's enum raises KeyError -- which is why this capability looked like # the risky one. It is not: every reason reason.feature asserts is an enum # member, DISABLED included. Declaring @disabled-flags alongside this one and -# running the ninth scenario shows the index surviving the DISABLED reason -# and the *next* keyword argument failing, `variant=data["variant"]` on line -# 160, with KeyError: 'variant' reported to the application as GENERAL. That -# is the same one-line defect the @disabled-flags note below records, and it -# is unrelated to the reason vocabulary. -# -# NUMERIC_COERCION is withheld, and the reason is worth recording because two -# other languages answered it differently over the same protocol. -# -# The capability has three scenarios and errors.feature says a declarer must -# satisfy all three -- the two lossless rows exist precisely to catch the -# shortcut of rejecting every float. This provider takes that shortcut. It keeps -# the two numeric types strictly apart: json.loads yields `int` for 10 and -# `float` for 0.5, and the check at ofrep/__init__.py:249-256 admits a value only -# on an exact isinstance against one of them. Nothing in that path widens or -# narrows a number. So the lossy row passes -- float-flag asked for as an Integer -# is a TYPE_MISMATCH rather than a silent 0 -- and integer-flag asked for as a -# Float fails, because 10 is not an instance of float. -# -# That is the same architecture as the Java OFREP adoption, which withholds the -# tag for the same reason: Jackson maps a JSON integer to Integer and a fraction -# to Double, and handleResolved admits the value only on an exact -# type.isInstance. The Go adoption declares it, and the difference is the JSON -# decoder rather than anything the provider author chose -- encoding/json makes -# every JSON number a float64, so integer-ness never survives the wire and -# ResolveInt has to round-trip through int64, which gives lossless coercion and a -# TYPE_MISMATCH on loss for free. -# -# Which is to say: over OFREP this capability follows the language's JSON -# library. Declaring it here would claim a behaviour two of these three lines of -# code rule out. -# -# No knownDeviation entry accompanies this. A deviation records a gap in -# behaviour the provider is required to have, and numeric coercion is a declared -# capability rather than a requirement -- Appendix F stopped presenting the rule -# as normative OpenFeature. The honest record is the undeclared tag and the three -# skips it produces. -# -# This withholding survives the correction that appendix made at spec@045950ca, -# and it is worth saying which side of it this is on, because the flagd adoption -# in this repository reads the opposite way. That note now says a provider which -# *attempts* the coercion and gets a direction wrong declares the tag and lets -# the scenario fail -- flagd's RPC resolver narrows 0.5 to 0 and does exactly -# that -- and that withholding is for a provider which cannot attempt it at all. -# This provider is the second kind: it never widens or narrows anything, the two -# JSON types stay apart end to end, and there is no coercion here to get wrong. -# Same rule, different provider, opposite answer. -# -# It also survives rule six, and spec@aa2ad24f is why. As that rule was first -# written -- declare when at least one scenario gating the tag can be put to the -# provider -- it would have forced a declaration here, since all three scenarios -# are perfectly askable of this provider and two of them fail. The rule now -# opens by saying it applies *once a provider is attempting the capability*, and -# decides whether a question is askable rather than whether an answer is owed. -# Nothing requires numeric coercion, this provider does not attempt it, so the -# question never reaches rule six. The Go implementation found that over-reach -# against its own self-tests; this suite would have been the second casualty. -# -# DISABLED_FLAGS is withheld as well, new at spec@009afe06, and this one is a -# provider defect rather than an architecture. Which is the opposite of what the -# appendix predicts, so it is worth being exact about. -# -# The appendix gates the tag on the reasoning that a provider "whose backend -# decides, such as one speaking OFREP, cannot: the server never sees the -# caller's default, so it has no way to return it". Neither half of that is the -# obstacle here. -# -# Measured first. Declaring the tag fails all four rows -- 6 failed, 37 passed, -# 12 skipped, 1 xfailed at the time, against the 2 failed of the run without it -# -- and each fails on the error code rather than on the value: "error-code was -# 'GENERAL', expected none". That probe predates reason.feature, which took this -# suite from 56 collected to 65; today's run without the tag is 2 failed, 45 -# passed, 17 skipped, 1 xfailed, and declaring it would move the same four rows -# from skipped to failed. Then read back, and probed at the wire to be sure of the -# reading. flagd's OFREP endpoint answers a disabled flag -# `200 {"key": ..., "reason": "DISABLED", "metadata": {}}`: no `value`, and no -# `variant`. The server does indeed never return a value, exactly as the -# appendix says. It does not need to -- ofrep/__init__.py:153 already reads -# `data.get("value", default_value)` and substitutes the caller's default for an -# absent one, and the type check on the next line passes on it. -# -# What fails is ofrep/__init__.py:160, which indexes `data["variant"]` -# unconditionally. flagd omits the member for a disabled flag, types.md types -# the field `variant (string, optional)`, and the resolution raises -# KeyError: 'variant'; the SDK catches it and reports GENERAL. So the whole of -# the difference between passing and failing these four rows is one `.get`. -# -# Which puts the capability within reach of this provider rather than outside -# it, and flagd's own RPC resolver satisfies the tag from the same signal in a -# different envelope. It is withheld because a declaration has to rest on a run -# and the run fails -- not because the architecture forbids it. The gap is an -# unfiled defect in openfeature-provider-ofrep, and it is not confined to -# disabled flags: the same index breaks on any OFREP response that omits the -# variant, which the field being optional permits for any reason a server -# reports without one. -# -# No knownDeviation entry, for the reason the numeric-coercion note above gives: -# a deviation records a gap in behaviour the provider is required to have, and -# @disabled-flags is a declared capability rather than a requirement. The honest -# record is the undeclared tag, the four skips it produces, and this note saying -# the gap is a bug somebody can fix rather than a fact of the protocol. -# -# **That reasoning does not survive spec@045950ca, and this is the one decision -# in these suites that the corrected appendix says should change.** It is left -# standing here only because the pass that found it was a documentation pass and -# changing it moves a count. The difference from the numeric-coercion note above -# is that this provider *does* attempt the behaviour: it resolves a disabled flag -# and gets it wrong on one unconditional index, which is the case the appendix -# tells an adopter to declare and let fail, with a KnownDeviation.untracked -# beside it -- "withdrawing the capability replaces a failing scenario with a -# skip and hides a defect behind something that looks deliberate". The self-test -# carve-out in that same revision, which does license a withholding for an -# identified defect, is explicit that an adoption has none: this suite exists to -# report on a provider, and a skip here is a claim about that provider. -# -# Note which rule that turns on, because spec@aa2ad24f makes the difference -# sharp. It is *not* rule six: that rule decides askability and applies only -# once a provider is attempting the capability, and if attempting were in doubt -# it would not reach the question. It is the two-shapes rule, and the evidence -# says this provider attempts and fails rather than declining -- it has no -# design position on disabled flags at all, it parses the response flagd sends -# for one and raises KeyError on a member the protocol types as optional, which -# breaks on any variant-less response and not only this one. A provider that -# declined by design would look like the numeric-coercion entry above: nothing -# in the code path that could be right or wrong, rather than one index that is -# wrong. -# -# So the next change to this file declares @disabled-flags, accepts four failing -# rows, and records the `data["variant"]` defect as an untracked deviation -- and -# files it, which is what makes the deviation tracked and the failures temporary. -# -# Not declared, and why. Each is a fact about the provider, established by -# reading it -- OFREPProvider is stateless: it holds a requests.Session and a -# rate-limit timestamp, and nothing else survives between evaluations. +# running the ninth scenario shows that index surviving the DISABLED reason +# and the *next* keyword argument failing -- `variant=data["variant"]` on line +# 160, KeyError: 'variant', reported to the application as GENERAL. Same +# one-line defect the @disabled-flags note below records, and nothing to do +# with the reason vocabulary. +# +# Not declared, and why. +# +# NUMERIC_COERCION +# Withheld, because this provider does not coerce at all -- which is the case +# Appendix F reserves withholding for, and the reason the flagd adoption in +# this repository reads the opposite way on the same rule. +# +# A declarer must satisfy all three scenarios; the two lossless rows exist to +# catch the shortcut of rejecting every float, and this provider takes that +# shortcut. It keeps the two numeric types strictly apart: json.loads yields +# `int` for 10 and `float` for 0.5, and the check at +# ofrep/__init__.py:249-256 admits a value only on an exact isinstance +# against one of them. Nothing in that path widens or narrows. So the lossy +# row passes -- float-flag asked for as an Integer is a TYPE_MISMATCH rather +# than a silent 0 -- and integer-flag asked for as a Float fails, because 10 +# is not an instance of float. Over OFREP the capability follows the +# language's JSON library rather than anything the provider author chose. +# +# No KnownDeviation: nothing requires numeric coercion, so there is no +# requirement to deviate from. The honest record is the undeclared tag and +# the three skips it produces. +# +# DISABLED_FLAGS +# Withheld, and **this is the one declaration in these suites that should +# change shape**: the provider attempts the behaviour and fails on one +# unconditional index, which is the declare-and-deviate case rather than the +# withhold case. It is left standing only because the pass that found it was a +# documentation pass and changing it moves a count; PR #414 records the +# decision, and the next change here declares the tag, accepts four failing +# rows and records the defect as a deviation. +# +# Measured, then read back, then probed at the wire. Declaring the tag fails +# all four rows, each on the error code rather than the value: "error-code was +# 'GENERAL', expected none" (probed at 56 collected, before reason.feature +# took the suite to 65; the four rows move from skipped to failed and nothing +# else changes). flagd's OFREP endpoint answers a disabled flag +# `200 {"key": ..., "reason": "DISABLED", "metadata": {}}` -- no `value` and +# no `variant`. The absent value is not the obstacle: +# ofrep/__init__.py:153 already reads `data.get("value", default_value)` and +# the type check on the next line passes on it. What fails is +# ofrep/__init__.py:160, indexing `data["variant"]` unconditionally, where +# types.md types the field `variant (string, optional)`. **The whole of the +# difference between passing and failing these four rows is one `.get`**, and +# the same index breaks on any variant-less OFREP response, not only a +# disabled flag. Filed as open-feature/python-sdk-contrib#418. +# +# So the capability is within reach of this provider rather than outside it, +# and flagd's RPC resolver satisfies the tag from the same signal in a +# different envelope. +# +# Everything below follows from the provider being stateless: it holds a +# requests.Session and a rate-limit timestamp, and nothing else survives +# between evaluations. # # LIFECYCLE # OFREPProvider does not override `initialize`, so it inherits # AbstractProvider's, which is `pass` (python-sdk # openfeature/provider/__init__.py:138-139). Nothing contacts the backend -# before the first evaluation, so initialisation has no outcome to observe. -# lifecycle.feature carries @lifecycle at feature level and skips as a -# whole, which is the intended outcome: it was gated on @events before the -# capability was split out, and the retag had to keep it skipping here. +# before the first evaluation, so initialisation has no outcome to observe, +# and lifecycle.feature -- which carries @lifecycle at feature level -- skips +# as a whole. # # EVENTS -# The provider never emits. It extends AbstractProvider, so it inherits -# `attach`, but `_on_emit` is never called anywhere in -# ofrep/__init__.py -- there is no stream, no poll and no background thread -# to notice anything worth emitting about. +# The provider never emits: it inherits `attach` from AbstractProvider, but +# `_on_emit` is never called anywhere in ofrep/__init__.py, because there is +# no stream, no poll and no background thread to notice anything. # # The SDK's registry does dispatch PROVIDER_READY around `initialize` for any # provider (python-sdk openfeature/provider/_registry.py:73-77), so declaring -# EVENTS would make lifecycle.feature's readiness scenario pass without -# demonstrating anything -- a NoOpProvider passes it identically. That is -# exactly the vacuity the @lifecycle capability was split out to end, and -# claiming the capability to collect the pass would be the dishonest use of -# it. +# EVENTS would make the readiness scenario pass without demonstrating +# anything -- a NoOpProvider passes it identically. That is the vacuity +# @lifecycle was split out to end. # # STALE, CONFIGURATION_CHANGE -# Both are event capabilities and follow from EVENTS. There is no connection -# to lose -- every evaluation is an independent HTTP request -- so there is no -# state between them that could go stale, and nothing watches the backend for -# a configuration change. events.feature is gated @events at feature level -# and skips as a whole. +# Both follow from EVENTS. There is no connection to lose -- every evaluation +# is an independent HTTP request -- so no state between them can go stale, and +# nothing watches the backend for a change. events.feature is gated @events at +# feature level and skips as a whole. # # Note that a *change* is nonetheless visible to an application: the next -# evaluation issues a fresh request and returns the new value. What is -# missing is the signal, and the @configuration-change scenario asserts the -# event as well as the behaviour, deliberately -- a provider that changes -# silently is not conformant, it is just not broken. +# evaluation issues a fresh request and returns the new value. What is missing +# is the signal, and the @configuration-change scenario asserts the event as +# well as the behaviour, deliberately. # # UNAVAILABLE_INIT # A provider pointed at a closed port reaches READY, because `initialize` @@ -276,58 +172,35 @@ # supplied here for the same reason. # # REINITIALIZATION -# New at spec@fc99d5ac, which gated the scenario "A provider that was shut -# down can be initialized again" that had been untagged before it. -# Requirement 2.5.2 says a provider SHOULD revert to its uninitialized -# state and that "some providers MAY allow reinitialization", so reuse is -# permitted rather than required and withholding needs no KnownDeviation. -# -# Reuse would in fact work here -- a stateless provider holding only a -# Session has nothing to release and nothing to rebuild, and `shutdown` is -# inherited and does nothing either -- but the scenario cannot be reached to -# demonstrate it. It lives in lifecycle.feature, so it inherits @lifecycle -# at feature level, and the gate skips a scenario when any capability -# gating it is undeclared. With LIFECYCLE withheld above, declaring this -# would leave the scenario skipped on @lifecycle and the claim unexamined: -# the same declare-what-nothing-exercises error the reserved tag below is -# kept out for. +# Reuse would in fact work -- a stateless provider holding only a Session has +# nothing to release and nothing to rebuild, and `shutdown` is inherited and +# does nothing either -- but the scenario cannot be reached to demonstrate it: +# it inherits @lifecycle from its feature, withheld above, so declaring this +# would leave the scenario skipped and the claim unexamined. Requirement 2.5.2 +# permits reuse rather than requiring it, so withholding needs no +# KnownDeviation. # # LARGE_INTEGERS -# The one withholding in this list that is about the backend rather than the -# provider, and until this pass the one with no reason written down at all. -# Appendix F's sixth declaring rule (spec@4cab0320, narrowed at -# spec@aa2ad24f) is what decides it, and this is a tag that reaches that -# rule: nothing about this provider declines to resolve a large integer, so -# it is attempting the capability and only askability is left in question. +# The one withholding here that is about the backend rather than the provider. # Exactly one scenario carries the tag, it asks for `huge-integer-flag`, and -# flagd-testbed v3.8.0 seeds no such flag -- so not one of the tag's -# scenarios can be put to this provider, and nothing about the capability -# can be established either way. Contrast VARIANTS above, where seven of -# eight rows do reach the provider and the tag is declared on their strength. +# flagd-testbed v3.8.0 seeds no such flag -- so none of the tag's scenarios +# can be put to this provider, and Appendix F's sixth declaring rule says +# withhold. Contrast VARIANTS above, where seven of eight rows do reach the +# provider and the tag is declared on their strength. # # Nothing in this path would narrow the value: a JSON number decodes through # `json.loads` into an unbounded Python `int` and the type check at -# ofrep/__init__.py:249-256 admits it unchanged. The suite cannot show that, -# which is the point -- a declaration would be a claim with no evidence -# behind it in either direction. +# ofrep/__init__.py:249-256 admits it unchanged -- and the suite cannot show +# that, which is the point. # -# No KnownDeviation, in either shape: the gap is the fixture's, and an entry -# would attribute it to the provider. And per the same rule's second -# consequence this withholding is temporary in a way the ones above are not. -# open-feature/flagd-testbed#392 adds the flag; declare the tag when the -# image carries it, or this outlives its reason and starts reading as a -# claim about the provider. Both flagd suites here withhold it on the same -# ground and say so in the same terms. +# No KnownDeviation, in either shape: the gap is the fixture's and an entry +# would attribute it to the provider. Unlike the withholdings above, this one +# is temporary -- open-feature/flagd-testbed#392 adds the flag; declare the +# tag when the image carries it, or it outlives its reason and starts reading +# as a claim about the provider. # # CACHING -# Reserved in the Capability enum; no scenario carries the tag. Declaring a -# capability nothing exercises would be a claim with no evidence behind it. -# @targeting was reserved alongside it until spec@26362f85 gave it three -# scenarios, and is now declared above. -# -# The withheld set matches the Java OFREP adoption, which reached the same -# conclusions from the same architecture, independently. It differs from Go's, -# which declares NUMERIC_COERCION for the decoder reason recorded above. +# Reserved, and the harness refuses it: no scenario carries the tag. CAPABILITIES = frozenset( { Capability.OBJECT, @@ -346,11 +219,7 @@ def tck_config( """Wire the provider up to the running testbed. Both fixtures come from ``tests/tck/conftest.py`` and both resolve after the - TCK has started the stack: Compose maps host ports dynamically, so the - address does not exist earlier -- and it stays valid for the whole session - because nothing ever restarts a container. Outages, had this suite any use - for them, would be simulated inside the running stack through - ``ofrep_control``. + TCK has started the stack, which is when the mapped host port exists. """ base_url = ofrep_base_url From de6cc16b1a5837daf56f10f9eaabb1f71206bb83 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 14 Sep 2026 10:04:03 +0200 Subject: [PATCH 19/22] test(tck): run the conformance suite against flagd-testbed v3.10.1 Four languages pinned this image by hand and had drifted: JavaScript was already on v3.10.1 while Go, Java and Python sat on v3.8.0, so the suites whose results are only comparable if they asked the same backend were asking two. v3.10.1 is the current release, so aligning up rather than down. What this does NOT fix, stated because the tag is easy to mistake for a fix: open-feature/flagd-testbed#392 and #394 are both still open, so v3.10.1 carries neither the three missing precision flags nor the /start readiness fix. The fixture failures and the readiness race are unchanged. The one behavioural change in range is open-feature/flagd-testbed#390, which increases the simulated downtime -- and that is exactly the timing the @stale and @unavailable scenarios depend on. Tallies recorded against v3.8.0 have not been re-measured on this image, including the failure ranges the adoption READMEs cite by tag; those figures stay as they are because they are a record of what v3.8.0 did, and re-taking them is follow-up work rather than a rewrite. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/tests/tck/docker-compose.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml index 5c27bf279..9ed476d52 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml +++ b/providers/openfeature-provider-ofrep/tests/tck/docker-compose.yaml @@ -12,7 +12,7 @@ # `diff` the two paths after changing either -- that diff is what catches drift. services: backend: - image: ghcr.io/open-feature/flagd-testbed:v3.8.0 + image: ghcr.io/open-feature/flagd-testbed:v3.10.1 ports: - 8013 # flagd RPC evaluation (gRPC) - 8015 # flagd in-process sync (gRPC) From e8df02074e61ef84dbff25e4835769f1d9493179 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 14 Sep 2026 13:01:42 +0200 Subject: [PATCH 20/22] test(ofrep): stop compensating for a backend defect this suite does not have SettledControl wrapped the control API in a poll over the OFREP endpoint until a reseeded flag actually resolved, because flagd-testbed's POST /start returns before it serves the flag set -- open-feature/flagd-testbed#394 -- and a stateless provider has no initialisation to hide that window behind. Appendix F used to prescribe exactly this shape, and no longer does: a backend that returns before it serves has a defect to fix in the backend, and an adoption that compensates cannot be compared with one that does not against the same backend. Removing it changes nothing here, which is the point. Four consecutive runs give the same 2 failed / 45 passed / 17 skipped / 1 xfailed as before, and each takes 20s rather than 45s because it is no longer polling for a condition that was already true. The wrapper was 160 lines defending against a window this suite was not in fact losing to -- which is how a compensating wait usually ends up: hard to show is load-bearing, and easy to leave in long after its defect is fixed. The race is still real and still open upstream. A red run is read against the documented floor and repeated before the provider is blamed: the race moves between scenarios, a defect does not. Signed-off-by: Simon Schrottner --- .../openfeature-provider-ofrep/README.md | 15 +- .../tests/tck/conftest.py | 25 +++- .../tests/tck/settled_control.py | 137 ------------------ .../tests/tck/test_ofrep.py | 4 +- 4 files changed, 30 insertions(+), 151 deletions(-) delete mode 100644 providers/openfeature-provider-ofrep/tests/tck/settled_control.py diff --git a/providers/openfeature-provider-ofrep/README.md b/providers/openfeature-provider-ofrep/README.md index 86933509f..14871f9bd 100644 --- a/providers/openfeature-provider-ofrep/README.md +++ b/providers/openfeature-provider-ofrep/README.md @@ -59,10 +59,17 @@ Two things that are this provider's rather than the policy's: suite that has quietly stopped importing against the harness is worse than one that runs and fails, and `mypy` here is configured over `src` alone, so nothing else would notice. -`tests/tck/settled_control.py` is worth reading before you touch the suite: it is a named workaround -for one backend defect — flagd-testbed's `/start` returns about 40 ms before it serves the flag set, -which the control API forbids — and the specification prescribes that such a wait live in the -adoption, citing the defect, rather than in the shared harness. +**This suite races a known backend defect, and deliberately does not compensate for it.** +flagd-testbed's `POST /start` returns before it serves the reseeded flag set, which the control API +forbids, and a stateless provider has no initialisation to hide that window behind — so a run can +report `FLAG_NOT_FOUND` for flags the configuration plainly defines. The defect is +[flagd-testbed#394](https://github.com/open-feature/flagd-testbed/pull/394). + +This suite used to wrap the control in a `SettledControl` that polled until the flags were served. +That is removed. Compensating here made this suite's results incomparable with every other adoption +run against the same backend: it read a clean floor while the others bounced, and the difference was +the wait rather than the provider. **Read a red run against the documented floor and repeat it before +blaming the provider** — the race moves between scenarios, a real defect does not. [tck]: ../../tools/openfeature-tck/README.md [appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py index 60150ccd8..6167e4d9e 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/conftest.py +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -19,8 +19,7 @@ import pytest -from openfeature.contrib.tools.tck import ComposeBackend, RunningBackend -from tests.tck.settled_control import SettledControl +from openfeature.contrib.tools.tck import BackendControl, ComposeBackend, RunningBackend OFREP_PORT = 8016 """flagd's OFREP HTTP port, and the one port this provider connects to. @@ -62,19 +61,29 @@ def ofrep_base_url(tck_backend: RunningBackend) -> str: @pytest.fixture(scope="session") -def ofrep_control(tck_backend: RunningBackend, ofrep_base_url: str) -> SettledControl: - """The control API client, wrapped in a wait for the flag set to be served. +def ofrep_control(tck_backend: RunningBackend) -> BackendControl: + """The control API client, used exactly as the harness provides it. ``tck_backend.control`` is the TCK's own ``HttpControl``, already pointed at the launchpad's mapped port and awaited ready. The launchpad registers no ``/reset``, so every ``prepare_scenario`` takes the harness's documented ``/start`` fallback and one 404 is logged per session. - Wrapped in :class:`SettledControl` because this backend's ``/start`` returns - before it serves the flag set, and a stateless provider has no initialisation - to hide that window behind. See that module. + This suite used to wrap it in a ``SettledControl`` that polled the OFREP + endpoint until the reseeded flags were actually served, because this + backend's ``/start`` returns before that is true and a stateless provider + has no initialisation to hide the window behind. That wrapper is gone. + A backend returning before it serves breaks the control API contract, and + compensating for it here made this suite's results incomparable with every + other adoption run against the same backend -- this one read a clean floor + while the others bounced, and the difference was the wait, not the provider. + The defect is open-feature/flagd-testbed#394 and belongs there. + + So this suite now races the window like the others do. Read a red run + against the documented floor and repeat it before blaming the provider: the + race moves between scenarios, a real defect does not. """ - return SettledControl(tck_backend.control, ofrep_base_url) + return tck_backend.control # --------------------------------------------------------------------------- diff --git a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py b/providers/openfeature-provider-ofrep/tests/tck/settled_control.py deleted file mode 100644 index f75421844..000000000 --- a/providers/openfeature-provider-ofrep/tests/tck/settled_control.py +++ /dev/null @@ -1,137 +0,0 @@ -"""``HttpControl``, plus a wait for a backend that returns before it serves. - -**A named workaround for one backend's defect, in the adoption rather than in -the shared harness**, which is the shape Appendix F prescribes for exactly this -situation -- see "The control API" there for why a delay in the harness would be -the wrong instrument. - -**The defect.** ``control-api.yaml`` requires ``POST /start`` not to return until -the state it reseeded is actually being served. flagd-testbed's launchpad returns -as soon as flagd answers ``/readyz``, which flagd does before its file sources -have reached the flag store. -`flagd-testbed#394 `_ -explains the mechanism, measures the window and closes it; it is open and -unmerged, so the window is still there. - -**Why this suite is the one that found it.** Both flagd resolvers block inside -``initialize`` until their stream or ruleset is up, so their initialisation -absorbs the window before any scenario evaluates. A stateless provider has no -initialisation to hide behind: its first evaluation lands directly in the gap and -the suite reports FLAG_NOT_FOUND for every flag, which reads as a -catastrophically broken provider. This wait goes away the day the testbed does. - -**Why it is not cheating.** It manipulates nothing. It is a readiness probe over -the same public OFREP endpoint the provider uses, on a canonical flag, asserting -only that the backend finished what ``/start`` already promised. No scenario is -weakened and no side channel into the backend is opened -- the normative control -path is still ``HttpControl``, which this delegates to unchanged. - -Deliberately not a :class:`ConnectionControl`: it has no ``disconnect`` or -``reconnect``, matching a suite that declares neither ``STALE`` nor -``UNAVAILABLE_INIT``. The two omissions keep each other honest. -""" - -from __future__ import annotations - -import json -import time -import urllib.error -import urllib.request - -from openfeature.contrib.tools.tck import ControlApi, HttpControl - -__all__ = ["SettledControl"] - -PROBE_FLAG_KEY = "boolean-flag" -"""A canonical flag, used only to ask whether the flag set is being served yet.""" - -SETTLE_TIMEOUT_SECONDS = 15.0 -"""How long to wait for the backend to serve the flag set after ``/start``. - -Two orders of magnitude above the ~40ms observed, because the cost of being -generous is nothing -- the loop exits on the first success -- while the cost of -being tight is a suite that fails intermittently on a loaded CI runner and gets -diagnosed as a provider bug. -""" - -SETTLE_POLL_SECONDS = 0.02 - - -class SettledControl: - """Delegates to :class:`HttpControl`, then waits for the flags to appear.""" - - def __init__( - self, - control: HttpControl, - ofrep_url: str, - *, - timeout: float = SETTLE_TIMEOUT_SECONDS, - ) -> None: - self._control = control - self._probe_url = ( - f"{ofrep_url.rstrip('/')}/ofrep/v1/evaluate/flags/{PROBE_FLAG_KEY}" - ) - self._timeout = timeout - - @property - def description(self) -> str: - return f"{self._control.description}, awaited through the OFREP endpoint" - - @property - def control_api(self) -> ControlApi: - """Whatever the control being delegated to says, which is ``"http"``. - - Forwarded rather than answered, because this class adds a readiness - probe and manipulates nothing: the normative control path is still the - HTTP control API, and a report that said otherwise would understate what - was exercised. - """ - return self._control.control_api - - def prepare_scenario(self) -> None: - self._control.prepare_scenario() - self._await_flags() - - def change_flag(self) -> None: - self._control.change_flag() - - def _await_flags(self) -> None: - """Block until the probe flag resolves, or fail saying what was seen. - - Raising rather than proceeding is deliberate. A scenario allowed to run - against a backend that is not serving its flag set does not report a - harness problem; it reports FLAG_NOT_FOUND as a conformance result, - which is the one outcome a conformance suite must never produce. - """ - deadline = time.monotonic() + self._timeout - last = "no response" - - while time.monotonic() < deadline: - status, body = self._probe() - if status == 200: - return - last = f"HTTP {status}: {body}" - time.sleep(SETTLE_POLL_SECONDS) - - msg = ( - f"the backend did not serve {PROBE_FLAG_KEY!r} within {self._timeout}s of " - f"a successful control-API reseed. Last response from {self._probe_url}: " - f"{last}. This is a problem with the stack under test or its control API, " - f"not with the provider" - ) - raise RuntimeError(msg) - - def _probe(self) -> tuple[int, str]: - request = urllib.request.Request( # noqa: S310 - self._probe_url, - data=json.dumps({}).encode("utf-8"), - headers={"Content-Type": "application/json"}, - method="POST", - ) - try: - with urllib.request.urlopen(request, timeout=5.0) as response: # noqa: S310 - return int(response.status), "" - except urllib.error.HTTPError as err: - return int(err.code), err.read().decode("utf-8", "replace")[:200] - except (urllib.error.URLError, OSError) as err: - return 0, str(err) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py index cb4c3f3db..1e8a1b440 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py @@ -21,12 +21,12 @@ from openfeature.contrib.provider.ofrep import OFREPProvider from openfeature.contrib.tools.tck import ( + BackendControl, Capability, TckConfig, feature_paths, ) from openfeature.provider import FeatureProvider -from tests.tck.settled_control import SettledControl TIMEOUT_SECONDS = 10.0 """Bounds a single OFREP request. @@ -214,7 +214,7 @@ @pytest.fixture(scope="session") def tck_config( ofrep_base_url: str, - ofrep_control: SettledControl, + ofrep_control: BackendControl, ) -> TckConfig: """Wire the provider up to the running testbed. From cb9bc5b32b88c44bd1d87cb381355ecbbeccdcdd Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 14 Sep 2026 17:30:03 +0200 Subject: [PATCH 21/22] test(tck): stop naming one testbed release as the flag set's boundary Several comments said flagd-testbed "v3.8.0" seeds neither large-integer-flag nor integral-float-flag. The Compose file moved to v3.10.1 and those flags are absent from that release too, so naming one version invited the reader to conclude the other serves them and that the failures were a pin away from going green. They are not: open-feature/flagd-testbed#392 is what adds the flags, and it is open. Now phrased as no released testbed seeding them, which stays true until #392 lands and false the moment it does -- which is when these comments should be revisited anyway. Signed-off-by: Simon Schrottner --- providers/openfeature-provider-ofrep/tests/tck/conftest.py | 2 +- providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/providers/openfeature-provider-ofrep/tests/tck/conftest.py b/providers/openfeature-provider-ofrep/tests/tck/conftest.py index 6167e4d9e..f1a9f4a00 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/conftest.py +++ b/providers/openfeature-provider-ofrep/tests/tck/conftest.py @@ -7,7 +7,7 @@ xfail for the single scenario it cannot satisfy. A full run is ``2 failed, 45 passed, 17 skipped, 1 xfailed``. Both failures ask -for ``large-integer-flag``, which flagd-testbed v3.8.0 does not seed -- +for ``large-integer-flag``, which no released flagd-testbed seeds -- open-feature/flagd-testbed#392 adds it, along with the two other canonical flags the image is missing, and says what each catches. Neither carries a ``KnownDeviation``: the gap is the backend's flag set, not the provider's. diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py index 1e8a1b440..eb489f846 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py @@ -49,7 +49,7 @@ # VARIANTS ofrep/__init__.py:160 carries the response's `variant` into the # resolution details. Seven of the outline's eight rows pass; the # eighth asks for large-integer-flag's `max-int32`, which -# flagd-testbed v3.8.0 does not seed (see conftest.py). Withholding +# no released flagd-testbed seeds (see conftest.py). Withholding # over it would say this provider does not name variants, which the # other seven rows show is false. # TARGETING ofrep/__init__.py:229-230 puts the evaluation context's targeting @@ -183,7 +183,7 @@ # LARGE_INTEGERS # The one withholding here that is about the backend rather than the provider. # Exactly one scenario carries the tag, it asks for `huge-integer-flag`, and -# flagd-testbed v3.8.0 seeds no such flag -- so none of the tag's scenarios +# no released flagd-testbed seeds such a flag -- so none of the tag's scenarios # can be put to this provider, and Appendix F's sixth declaring rule says # withhold. Contrast VARIANTS above, where seven of eight rows do reach the # provider and the tag is declared on their strength. From 0af2f073aecf65b2b28f68efa8758e6937fb2c79 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 15 Sep 2026 22:44:15 +0200 Subject: [PATCH 22/22] test(ofrep): declare @string-typing, and separate two kinds of untyped The base branch's pin moves four scenarios out of the mandatory mismatch matrix and behind the new `@string-typing` tag: `boolean-flag`, `integer-flag` and `float-flag` asked for through the string accessor, plus `object-flag`, which reaches this suite because @object is declared. All four pass here, so the tag is declared. Measured rather than argued from the protocol, and the measurement is the point: those four were mandatory at the previous pin and passing, the full run is 2 failed, 45 passed, 17 skipped, 1 xfailed before and after, and the four were re-run on their own to see them selected rather than gated away. The note in the suite spends its words on one distinction, because this is the adoption where it bites. "OFREP is an untyped protocol" is already written in this package, as the reason boolean-flag satisfies an Integer request -- and it is not the property this capability asks about. OFREP carries no requested type on the wire, but its response body is JSON, and JSON tells `true` from `"true"`. The values this provider receives are therefore typed, and `_typecheck_flag_value` maps FlagType.STRING to the bare `str`, so a bool, a number or a structure fails the isinstance and becomes TYPE_MISMATCH. What the capability is actually about is the backend's storage: a provider over a backend that stores flag values as strings answers `"true"` to the string accessor and has nothing to reject. flagd holds typed variants and serialises them as typed JSON, so this provider is never handed one -- a property of the stack rather than of OFREP, and the reason the same protocol over a string-storing backend would be a withholding. The subclass hazard does not carry over either: `str` is not in bool's ancestry, so the defect that makes the Integer accessor admit `True` has no counterpart on the string accessor. Signed-off-by: Simon Schrottner --- .../tests/tck/test_ofrep.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py index eb489f846..6685137fd 100644 --- a/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py +++ b/providers/openfeature-provider-ofrep/tests/tck/test_ofrep.py @@ -78,6 +78,40 @@ # one-line defect the @disabled-flags note below records, and nothing to do # with the reason vocabulary. # +# STRING_TYPING +# Declared, on a run: all four scenarios pass -- three Examples rows asking +# for `boolean-flag`, `integer-flag` and `float-flag` through the string +# accessor, and the `@object` one asking for `object-flag`, which reaches +# this suite because OBJECT is declared above. +# +# **"Untyped protocol" is not the same claim as "untyped backend"**, and +# this is the tag where the two come apart, so it is worth being explicit +# rather than inheriting the sentence used elsewhere in this file. OFREP +# carries no *requested* type on the wire -- which is exactly why +# boolean-flag satisfies an Integer request here and is xfailed in +# conftest.py -- but the response body is JSON, and JSON distinguishes +# `true` from `"true"` and `10` from `"10"`. So the values this provider +# receives are typed, and ofrep/__init__.py:249-256 checks them: FlagType +# .STRING maps to the bare `str`, and a bool, a number or a structure fails +# that isinstance and becomes TYPE_MISMATCH. +# +# The capability is about the backend's storage rather than the protocol's +# request envelope: the provider a string-storing backend produces answers +# `"true"` to the string accessor and has nothing to reject. flagd holds +# typed variants and serialises them as typed JSON, so this provider is +# never handed one. That is a property of the stack rather than of OFREP, +# and it is the reason the tag is declared on a run rather than argued from +# the protocol -- the same protocol over a Flipt-like backend would be a +# withholding. +# +# Note how little of the bool-as-int defect carries over: `str` is not in +# bool's ancestry, so the subclass hazard that makes the Integer accessor +# admit `True` has no counterpart on the string accessor. +# +# Four scenarios that were mandatory at the previous pin, and passing, so +# the declaration keeps them running and changes no number: the full run is +# 2 failed, 45 passed, 17 skipped, 1 xfailed before and after. +# # Not declared, and why. # # NUMERIC_COERCION @@ -205,6 +239,7 @@ { Capability.OBJECT, Capability.VARIANTS, + Capability.STRING_TYPING, Capability.TARGETING, Capability.STANDARD_REASONS, }