diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8c09e514..d80adb19 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -66,6 +66,9 @@ jobs: tools/openfeature-flagd-api-testkit: - 'tools/openfeature-flagd-api-testkit/**' - 'uv.lock' + tools/openfeature-provider-tck: + - 'tools/openfeature-provider-tck/**' + - 'uv.lock' build: needs: changes diff --git a/.gitmodules b/.gitmodules index 7e8bf9ed..31678c42 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,3 +7,6 @@ [submodule "providers/openfeature-provider-flagd/openfeature/test-harness"] path = providers/openfeature-provider-flagd/openfeature/test-harness url = https://github.com/open-feature/flagd-testbed.git +[submodule "tools/openfeature-provider-tck/spec"] + path = tools/openfeature-provider-tck/spec + url = https://github.com/open-feature/spec diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 94d58394..6bb08620 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -8,5 +8,6 @@ "providers/openfeature-provider-unleash": "0.1.2", "tools/openfeature-flagd-api": "1.0.0", "tools/openfeature-flagd-core": "1.0.0", - "tools/openfeature-flagd-api-testkit": "0.1.0" + "tools/openfeature-flagd-api-testkit": "0.1.0", + "tools/openfeature-provider-tck": "0.1.0" } diff --git a/pyproject.toml b/pyproject.toml index 647571bc..e250a4b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ "openfeature-flagd-api", "openfeature-flagd-core", "openfeature-flagd-api-testkit", + "openfeature-provider-tck", ] [dependency-groups] @@ -43,6 +44,7 @@ openfeature-provider-unleash = { workspace = true } openfeature-flagd-api = { workspace = true } openfeature-flagd-core = { workspace = true } openfeature-flagd-api-testkit = { workspace = true } +openfeature-provider-tck = { workspace = true } [tool.uv.workspace] members = [ @@ -58,7 +60,10 @@ exclude = [ ".venv", "__pycache__", "venv", - "providers/openfeature-provider-flagd/src/openfeature/schemas/**" + "providers/openfeature-provider-flagd/src/openfeature/schemas/**", + # Submodules of other repositories: not ours to lint or format. + "providers/openfeature-provider-flagd/openfeature/spec/**", + "tools/openfeature-provider-tck/spec/**", ] [tool.ruff.lint] diff --git a/release-please-config.json b/release-please-config.json index a6335415..29cfce44 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -99,6 +99,15 @@ "extra-files": [ "README.md" ] + }, + "tools/openfeature-provider-tck": { + "package-name": "openfeature-provider-tck", + "bump-minor-pre-major": true, + "bump-patch-for-minor-pre-major": true, + "versioning": "default", + "extra-files": [ + "README.md" + ] } }, "changelog-sections": [ diff --git a/tools/openfeature-provider-tck/.gitignore b/tools/openfeature-provider-tck/.gitignore new file mode 100644 index 00000000..06664622 --- /dev/null +++ b/tools/openfeature-provider-tck/.gitignore @@ -0,0 +1,7 @@ +# Copied from the open-feature/spec submodule by hatch_build_sync.py. +# DO NOT EDIT the copies, and do not commit them: the canonical definitions live +# in spec/specification/assets/provider-tck/, and the revision this package is +# built against is recorded by the submodule pin. +src/openfeature/contrib/tools/provider_tck/features/ +src/openfeature/contrib/tools/provider_tck/flag_data/ +src/openfeature/contrib/tools/provider_tck/control-api.yaml diff --git a/tools/openfeature-provider-tck/LICENSE b/tools/openfeature-provider-tck/LICENSE new file mode 100644 index 00000000..261eeb9e --- /dev/null +++ b/tools/openfeature-provider-tck/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/tools/openfeature-provider-tck/README.md b/tools/openfeature-provider-tck/README.md new file mode 100644 index 00000000..e3d40aae --- /dev/null +++ b/tools/openfeature-provider-tck/README.md @@ -0,0 +1,411 @@ +# OpenFeature Provider TCK (Python) + +A conformance suite any OpenFeature Python provider can adopt to verify that it implements the +provider contract of the specification. + +OpenFeature's central promise is that swapping providers does not change application behaviour. +Nothing verifies that today, and every provider tests differently — so "implements the provider +contract" is an unverified claim, and a behavioural difference between two providers is discovered +by the application that trips over it. + +This package is the Python implementation of [Appendix F][appendix-f]. It runs the same Gherkin +scenarios, against the same canonical flag set, driven through the same backend control API, as +every other language's TCK. That shared basis is the point: "conformant" only means something if the +question is identical everywhere. + +Tracking issue: [open-feature/spec#417][tracking]. + +## Status + +**Proof of concept.** The scenario set is a representative subset covering each architectural +mechanism once, not exhaustive coverage. Breaking changes should be expected. + +## Adopting it + +One fixture and one call. It uses **pytest-bdd**, the same runner the flagd provider and the flagd +testkit already use, so an adopting package gains no new test framework. + +```python +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.tools.provider_tck import ( + Capability, + TckConfig, + feature_paths, +) + + +@pytest.fixture(scope="session") +def tck_config(): + control = MyBackendControl() + return TckConfig( + name="my-provider", + control=control, + new_provider=lambda: MyProvider(control.address), + capabilities={Capability.EVENTS, Capability.OBJECT}, + ) + + +scenarios(*feature_paths()) +``` + +There is **no `conftest.py` to write and nothing to import for the steps**. The step definitions +arrive through this package's pytest plugin, registered via a `pytest11` entry point, so installing +the package is all it takes. + +The TCK owns the whole lifecycle: registering the provider under a suite-scoped domain, awaiting +events, resetting the backend between scenarios, releasing it at the end. **If you find yourself +writing test infrastructure, that is a defect here rather than something for you to work around.** + +pytest-bdd generates one test per scenario — and one per row of a Scenario Outline — so failures +name a scenario and `-k` selects one as usual. The feature files and canonical flag set are packaged +inside the distribution, so **adopting this package needs no git submodule** — see +[Where the assets come from](#where-the-assets-come-from). + +### Timings + +`TckConfig.event_timeout` is the knob that matters. Providers observe backend changes on wildly +different timescales — a streaming provider sees a configuration change in milliseconds, one that +polls every 30 seconds may need most of a poll interval. Set it to comfortably exceed your +worst-case detection latency, or the suite reports timeouts that are really just impatience. + +## Adding your own scenarios + +A provider is rarely only a provider. flagd has `fractional` targeting, another vendor has a +proprietary rollout rule, and the behaviour of those is as worth pinning as the contract they sit on +top of. Verifying them used to mean a second harness: a second backend lifecycle, a second set of +fixtures, a second thing to keep working. + +Put them in the same run instead. Create a directory named `tck-extensions` beside the module that +calls `scenarios()`, and write step definitions for whatever is new in a `conftest.py` beside it: + +``` +tests/ +├── conftest.py # your step definitions +├── test_conformance.py # the fixture and the one call, unchanged +└── tck-extensions/ + └── fractional.feature +``` + +```python +# conftest.py +from pytest_bdd import then + +from openfeature.contrib.tools.provider_tck import TckState + + +@then("the fractional rule splits the population") +def fractional_splits(tck_state: TckState) -> None: ... +``` + +That is the whole of it — **no registration, no option and no new argument**. pytest collects +`conftest.py` on its own, pytest-bdd resolves steps through the fixture system, and the canonical +step vocabulary is in scope in your feature file beside your own steps. `tck_state` is the same +per-scenario state the canonical steps use, so your scenario runs against the provider the suite +registered, in the same backend lifecycle, with the same reset between scenarios. + +The one thing pytest cannot find by itself is the feature files, because the canonical ones are +inside the installed distribution rather than in your repository. `feature_paths()` returns both: + +```python +scenarios(*feature_paths()) +``` + +That line does not change when you add an extension, and it is the only difference from +`scenarios(features_path())` — which still works and still sees only the canonical set. An adopter +with no `tck-extensions` directory runs exactly what they ran before: same scenarios, same count. + +### Your scenarios cannot stand in for ours + +Every feature file carries a uri, and it is how a canonical scenario is told from an adopter's: +canonical files are the ones under the `features/` prefix and yours are under `extensions/` — the +prefix Go and JavaScript mount theirs under too, so a consumer holding conformance reports from +several languages applies one rule. The prefix is derived from where a file *is*, not from what the +runner called it, and `extensions.py` reports two cases that derivation cannot rule out: + +- **A feature file of yours under the reserved `features/` prefix.** Handing `scenarios()` a + directory of your own named `features` is the one route left to a canonical-looking uri. +- **Two feature files that would share one uri.** A record of what ran holds one copy of a feature + file per uri, so the second file's scenarios would be attributed to the first file's. + +This is not hypothetical. Java's suite found that a same-named feature file in a second classpath +root *replaced* the canonical one, and the run went green having asked the adopter's questions +instead of the specification's — the worst outcome available to a conformance suite. The Python +route to the same place is narrower and just as quiet: pytest-bdd names a feature file by its parent +directory joined to its own name, so `tck-extensions/features/errors.feature` arrives under the uri +the canonical `errors.feature` already occupies. + +## Capabilities + +Not every provider implements every optional part of the contract. Each scenario exercising an +optional part carries a Gherkin tag, pytest-bdd turns that tag into a pytest marker, and a provider +declares what it supports. + +**A scenario whose capability was not declared is reported as skipped, with the reason — never as +passed.** A conformance suite that quietly goes green on scenarios it did not run is worse than no +suite at all, so `pytest.skip` carries the reason into the report: + +``` +SKIPPED provider does not declare capability @stale. + Declared: @events @large-integers @object +``` + +| Capability | Tag | Meaning | +| --- | --- | --- | +| `Capability.LIFECYCLE` | `@lifecycle` | reaches its backend during initialisation, observably and promptly | +| `Capability.EVENTS` | `@events` | emits lifecycle events at all | +| `Capability.STALE` | `@stale` | enters `STALE` and emits `PROVIDER_STALE` on backend loss | +| `Capability.CONFIGURATION_CHANGE` | `@configuration-change` | detects configuration changes and emits `PROVIDER_CONFIGURATION_CHANGED` | +| `Capability.OBJECT` | `@object` | supports structured flag values | +| `Capability.UNAVAILABLE_INIT` | `@unavailable` | reports an error state instead of hanging against a dead backend | +| `Capability.NUMERIC_COERCION` | `@numeric-coercion` | coerces between integer and float only when lossless, else `TYPE_MISMATCH` | +| `Capability.LARGE_INTEGERS` | `@large-integers` | resolves integers up to 2^53 − 1 exactly; undeclarable where the SDK's integer accessor is 32-bit | +| `Capability.REINITIALIZATION` | `@reinitialization` | can be initialised again after `shutdown`, which [Requirement 2.5.2](https://github.com/open-feature/spec/blob/main/specification/sections/02-providers.md) permits rather than requires | +| `Capability.TARGETING` | `@targeting` | reserved; **not declarable** — no scenarios yet | +| `Capability.CACHING` | `@caching` | reserved; **not declarable** — no scenarios yet | + +`@lifecycle` and `@events` are deliberately separate, and the split matters in both directions. An +SDK dispatches `PROVIDER_READY` around `initialize` for *any* provider, so a provider declaring only +`@events` passes the readiness scenario without demonstrating anything — a `NoOpProvider` passes it +identically. Meanwhile a stateless provider has a real initialisation to verify but no event stream +of its own to declare `@events` for, and gating on `@events` shut it out of a scenario it should be +held to. + +`@reinitialization` is separate from `@lifecycle` for a subtler reason. +[Requirement 2.5.2](https://github.com/open-feature/spec/blob/main/specification/sections/02-providers.md) +says a provider **SHOULD** revert to its uninitialized state after `shutdown`, and its supporting +text adds that *"some providers **may** allow reinitialization from this state"*. Reuse is therefore +permitted, not required: a provider that releases its client on shutdown and declines to be started +again is exercising a choice the specification offers it, so withholding this capability needs no +`KnownDeviation` entry. + +The scenario was untagged until spec revision `fc99d5ac`, on the reading that reverting to the +uninitialized state is observable as exactly one thing — being initialisable again. That inference +does not hold, and asserting it unconditionally reported a permitted choice as a conformance failure. +A false failure is the mirror image of a vacuous pass, and this suite cares about both. Reverting the +state is not separately observable either — a provider that reverts but refuses reuse presents +identically to one that did neither — so the gated reuse scenario is the only assertion the +requirement admits. It is worth keeping for the providers that do offer reuse, because releasing the +client on shutdown while leaving an initialised flag set behind is easy to write and leaves the +provider evaluating against a closed connection rather than failing outright. + +One practical note, because it is easy to get wrong: `@reinitialization` **narrows** `@lifecycle` +rather than standing beside it. The scenario lives in `lifecycle.feature`, which carries `@lifecycle` +at the feature level, so the scenario inherits it and carries both tags — and the gate skips a +scenario when *any* capability gating it is undeclared. Reuse is therefore exercised only by an +adoption declaring `Capability.LIFECYCLE` **and** `Capability.REINITIALIZATION`; declaring the latter +alone leaves the scenario skipped on `@lifecycle` and the declaration unverified. So a provider that +withholds `LIFECYCLE` has never run this scenario, and has no evidence either way on which to declare +reuse. + +Untagged scenarios are mandatory and always run. `capabilities` defaults to every *declarable* +capability — `DECLARABLE_CAPABILITIES` — and you should narrow it rather than widen it: start from +the default, run the suite, and remove only what your provider genuinely cannot do. + +A reserved capability is documented so the vocabulary has a place for it once scenarios exist, and +until then it **must not be declared**. Nothing carries the tag, so declaring it cannot be verified, +cannot produce a skip, and tells anyone reading the declaration only that something was claimed and +nothing examined. `TckConfig` raises if you name one in `capabilities` or in `not_applicable`, and +`DECLARABLE_CAPABILITIES` excludes them — which is the case that matters, because "every capability +except X" is how a reserved tag gets declared by accident rather than by decision. One +implementation's published conformance report asserts `@targeting` and `@caching` for exactly that +reason. + +`@numeric-coercion` deserves a note, because it is the one capability here that **the specification +does not define**. OpenFeature has a single numeric type on purpose — `number` is "a numeric value of +unspecified type or size", and languages **may** differentiate between integers and floats "as idioms +dictate" — so no requirement says what a provider must do when a value does not fit the accessor it +was asked through. That gap is [open-feature/spec#430](https://github.com/open-feature/spec/issues/430). + +The rule this tag is tested against is therefore **borrowed, not normative**: lossless coercion is +permitted, lossy coercion must fail — `10.0` requested as an integer must succeed, `0.5` must not. +It comes from flagd's +[numeric coercion ADR](https://github.com/open-feature/flagd/blob/main/docs/architecture-decisions/numeric-coercion.md), +which is scoped to flagd's own implementations, and the tag carries that name — it was +`@strict-numeric-typing` — because two vocabularies for one observable property is worse than one +borrowed name. **A provider that behaves differently is not violating the specification**, so +withholding this capability may be a deliberate choice as readily as a defect. + +Both halves are tested, and a provider declaring the tag must satisfy all three scenarios: `float-flag` +(`0.5`) requested as an integer is a `TYPE_MISMATCH`; `integral-float-flag` (`10.0`) requested as an +integer is `10`; `integer-flag` (`10`) requested as a float is `10.0`. Rejecting every float is an easy +way to pass the first, and the other two are what stop it. + +The width of the integer accessor is the related property, and it is a capability of its own because +it belongs to the SDK rather than to the provider. Every language can ask for 2^31 − 1, so that +precision scenario is untagged; only the one asking for 2^53 − 1 carries `@large-integers`. A Python +`int` is unbounded, so a Python provider declares it unless something of its own — a 32-bit field in +its wire format, a float on the way through — narrows the value. + +### Steps that reach the provider directly + +Everything the suite asks of a provider goes through an OpenFeature client, as an application's +would — except three steps. `the provider is shut down` and `the provider is initialized again` call +the provider's own `shutdown()` and `initialize()` on the registered instance, and +`the provider metadata name should not be empty` asks it for `get_metadata()`. Going through the SDK +would test the registry's bookkeeping as much as the provider, and Appendix B already does that; it +would also make a double shutdown impossible to express, since the registry calls `shutdown` once +per registration. + +The registry is not told. The client keeps pointing at the same instance, so an evaluation after +re-initialising reaches the very object that was shut down and brought back. When the scenario ends, +the SDK shuts the provider down once more on its own — requirement 2.5.3 makes that second call +harmless, and the suite relies on it. A direct call that outlasts `TckConfig.ready_timeout` is given +up on and fails its scenario with a message rather than hanging the session. + +### Declaring more than a capability set + +Two further fields on `TckConfig` say things a capability set cannot, and both are declarations +rather than switches: neither changes which scenarios run or what they assert. + +`not_applicable={Capability.X: "why"}` is for a capability that *cannot* hold rather than one you +chose not to declare. The suite treats the two identically — the scenarios are skipped either way, +with the reason — but collapsing them misrepresents a provider, and whole languages with it: +`@numeric-coercion` is unsatisfiable in JavaScript because the language has no integer type, and +recording that as a choice would show every JavaScript provider as declining something none of them +can have. Declining an optional feature is a choice; an impossibility is not. + +`known_deviations=(KnownDeviation(issue=..., summary=...),)` acknowledges a gap against something the +specification does *not* treat as optional, with somewhere it is tracked. It is an acknowledgement +and not an excuse: the scenario still fails and the suite still fails with it. What the declaration +adds is that the gap was known rather than a surprise. + +## Controlling the backend + +`BackendControl` is the single seam between the scenarios and whatever manipulates the backend. Step +definitions never talk to a backend directly, which is why the same Gherkin runs unchanged against a +containerised backend and against a provider manipulated in-process. + +**If your provider talks to a backend, drive it over the HTTP control API** — the document is +available as `control_api_spec()`. That API is the normative contract for those providers, and it is +what makes a conformance claim portable: another language's TCK drives the same endpoints against +the same stack and must get the same answers. + +Two of its requirements are easy to get wrong: + +- **Containers are never stopped or restarted mid-suite.** Unavailability is simulated *inside* the + running stack. Container orchestrators assign host ports dynamically and cannot reliably preserve + them across a restart, so restarting silently invalidates every provider already pointed at the + old port, and the failure looks like a flaky provider. +- **`/start` resets flag state; `/restart` preserves it.** An outage must be observable as a change + in availability, never as a change in flag values. + +### Providers with no backend + +An in-memory, environment-variable or file-based provider has nothing to connect to. Those may +control the backend in-process, where flag operations are direct manipulations of the provider's own +state. `InProcessControl` is the reference. + +This is a narrow allowance and the obvious thing to abuse. **A provider with an external backend +must use the control API.** Reaching into an external backend from inside the test process — a +test-only admin client, a shared database handle, a hook inside the provider — produces a suite that +passes while proving nothing, because the path it exercised is not the path the contract describes. + +Connection-dependent scenarios have no meaning without a connection, so a backend-less control +simply does not implement `ConnectionControl`, leaves `STALE` and `UNAVAILABLE_INIT` undeclared, and +those scenarios are skipped with their reason. + +## Findings + +Three, all confirmed by running the suite rather than by reading code. + +### 1. A boolean satisfies an Integer request + +`boolean-flag` evaluated through `get_integer_details` returns `True` with reason `STATIC` and **no +error code**, where the specification requires the code default and `TYPE_MISMATCH`. The client +type-checks with `isinstance(value, int)`, and `bool` is a subclass of `int` in Python. + +This is **Python-specific** — the identical scenario passes in every other language's suite, which +is a fair advertisement for having more than one implementation. Tracked as +[open-feature/python-sdk#619](https://github.com/open-feature/python-sdk/issues/619). + +The self-test marks that one row `xfail(strict=True)` with a pointer to the issue, so it stays +visible in the report and un-hides itself automatically once the SDK is fixed. + +### 2. The in-memory provider cannot update its flag set + +[Appendix A][appendix-a] requires an SDK's in-memory provider to support updating the flag set and +emitting `PROVIDER_CONFIGURATION_CHANGED`. Python's copies its mapping in the constructor and +exposes nothing to change it. Tracked as +[open-feature/python-sdk#620](https://github.com/open-feature/python-sdk/issues/620). + +Only half the machinery is missing — `AbstractProvider` already supplies +`emit_provider_configuration_changed` — which is why `ControllableInMemoryProvider` here is a small +subclass rather than a reimplementation, and why it should port back to the SDK as a method. + +### 3. The in-memory provider does not coerce numbers + +`integral-float-flag` (`10.0`) requested as an integer returns the code default with `TYPE_MISMATCH`, +and `integer-flag` (`10`) requested as a float does the same. The provider hands each variant back +untouched and the client's type check is `isinstance`-based, so neither lossless direction happens. +The lossy scenario passes — every float is rejected — which is exactly the shortcut the two lossless +scenarios exist to catch. + +This is not a defect: `@numeric-coercion` is optional, and the specification does not define the +behaviour. So neither in-memory self-test declares the tag, and the three scenarios are skipped with +that reason rather than failing. + +## Where the assets come from + +The Gherkin feature files, the canonical flag set and the control-API document are **not owned by +this repository**. They are the language-agnostic conformance artifacts defined in +[open-feature/spec][spec] under `specification/assets/provider-tck/`, and every language's TCK ships +the same ones — which is the only reason a conformance claim means the same thing in Python as it +does in Java. + +**Adopting this package needs no submodule.** The assets are copied into the wheel and the sdist at +build time, so `pip install openfeature-provider-tck` gives you everything the suite runs on. + +**Contributing to this package does.** The spec is a git submodule at +`tools/openfeature-provider-tck/spec`, and the copies under +`src/openfeature/contrib/tools/provider_tck/` are gitignored and generated: + +```bash +git submodule update --init tools/openfeature-provider-tck/spec +poe test # runs `poe sync-spec-assets` first +``` + +The copies carry a `DO-NOT-EDIT.txt` because editing them forks the definition of conformance, which +is the one thing this suite exists to prevent. A change goes to [open-feature/spec][spec] first; +then bump the submodule pin here. Committing no copies means the spec revision this package targets +is recorded by the pin and nowhere else, so the two cannot drift apart unnoticed. + +This mirrors what `openfeature-flagd-api-testkit` already does for the flagd test harness. + +## The self-tests + +| Suite | Subject | Why | +| --- | --- | --- | +| `test_in_memory_conformance` | the SDK's `InMemoryProvider` | reference adoption for a backend-less provider | +| `test_controllable_conformance` | `ControllableInMemoryProvider` | the only suite that exercises the configuration-change path — see finding 2 | +| `test_in_process_control` | `InProcessControl` and the canonical flag set | pins what the Gherkin cannot assert about itself, including that the in-memory flag set mirrors `canonical-flags.json` type for type | +| `test_lifecycle_steps` | the steps that call the provider directly | the in-memory suites skip `@lifecycle`, so the shutdown, re-initialise and metadata steps are driven against a recording provider instead | +| `test_declaration` | what a `TckConfig` claims | none of it is observable in a pass or a fail, so nothing else would catch it | +| `test_extensions` | an adopter's own scenarios | an extension runs inside the canonical suite, changes nothing for an adopter who has none, and cannot take a canonical scenario's identity | + +``` +106 passed, 21 skipped, 2 xfailed +``` + +No Docker and no network. The conformance suites take under a second; `test_extensions` takes most +of the rest, because the properties it checks are properties of a whole pytest session and it runs a +generated adoption in a subprocess to check them. + +Neither in-memory suite declares `@lifecycle`, so the six lifecycle scenarios — three about +initialisation, three about shutdown — are skipped in both. That is the point: with no backend to +reach, the initialisation ones would pass without testing anything — which is what they did while the +feature was gated on `@events`. Neither declares `@numeric-coercion` either, for the reason in +finding 3, so its three scenarios are skipped too. + +## Known gaps + +- **Evaluation context passthrough is unverifiable.** The scenarios build evaluation contexts but + cannot assert one *reached* the backend. That needs an echo operation on the control API. +- **No HTTP control client yet.** It arrives with the first containerised adopter. +- **Caching, hooks and flag metadata** are not covered. + +[appendix-a]: https://github.com/open-feature/spec/blob/main/specification/appendix-a-included-utilities.md +[appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md +[spec]: https://github.com/open-feature/spec +[tracking]: https://github.com/open-feature/spec/issues/417 diff --git a/tools/openfeature-provider-tck/hatch_build.py b/tools/openfeature-provider-tck/hatch_build.py new file mode 100644 index 00000000..4b6f1d84 --- /dev/null +++ b/tools/openfeature-provider-tck/hatch_build.py @@ -0,0 +1,51 @@ +"""Hatch build hook to copy the canonical conformance assets into the package. + +The feature files, the canonical flag set and the control-API document are owned +by open-feature/spec and reach this package through a git submodule, so nothing +in this repository can fork the definition of conformance. They are copied into +the source tree at build time and force-included into the distribution, which is +what lets an *adopter* install the wheel and run the suite with no submodule of +their own. +""" + +import sys +from pathlib import Path + +from hatchling.builders.hooks.plugin.interface import BuildHookInterface + +# Hatchling loads this file by path rather than importing it as part of a +# package, so its directory is not on sys.path and the sibling sync module -- +# the single definition of what gets copied where -- would not be importable. +sys.path.insert(0, str(Path(__file__).parent)) + +from hatch_build_sync import FILES, PACKAGE_REL, SPEC_ASSETS, TREES, sync + + +class SpecAssetsCopyHook(BuildHookInterface): + PLUGIN_NAME = "spec-assets-copy" + + def initialize(self, version: str, build_data: dict) -> None: + root = Path(self.root) + copies = [root / PACKAGE_REL / dest for _, dest in TREES + FILES] + + # Building from a checkout: refresh from the submodule, so what ships is + # always the revision the pin names. Building from an sdist: there is no + # submodule, but the copies are already in the tree. + if SPEC_ASSETS.exists(): + sync() + elif not all(path.exists() for path in copies): + missing = ", ".join(str(p) for p in copies if not p.exists()) + msg = ( + f"Conformance assets missing ({missing}) and the open-feature/spec " + f"submodule is not checked out at {SPEC_ASSETS}. Run " + "`git submodule update --init tools/openfeature-provider-tck/spec`." + ) + raise FileNotFoundError(msg) + + # Force-include the gitignored copies into both sdist and wheel. + force = build_data.setdefault("force_include", {}) + for path in copies: + for member in [path] if path.is_file() else path.rglob("*"): + if member.is_file(): + rel = str(member.relative_to(root)) + force[rel] = rel diff --git a/tools/openfeature-provider-tck/hatch_build_sync.py b/tools/openfeature-provider-tck/hatch_build_sync.py new file mode 100644 index 00000000..f31bc55b --- /dev/null +++ b/tools/openfeature-provider-tck/hatch_build_sync.py @@ -0,0 +1,56 @@ +"""Copy the canonical conformance assets from the spec submodule into the package. + +Used by `poe sync-spec-assets` for local development and CI testing. The hatch +build hook (hatch_build.py) handles inclusion in the wheel and sdist. + +The assets are owned by open-feature/spec, not by this repository. Copying them +in at build time -- rather than committing copies -- means the spec revision this +package was built against is recorded by the submodule pin and nowhere else, so +the two cannot drift apart unnoticed. An *adopter* installing the wheel still +needs no submodule: the copies are inside the distribution. +""" + +import shutil +from pathlib import Path + +ROOT = Path(__file__).parent +SPEC_ASSETS = (ROOT / "spec/specification/assets/provider-tck").resolve() +PACKAGE_REL = Path("src/openfeature/contrib/tools/provider_tck") +DEST_BASE = ROOT / PACKAGE_REL + +DO_NOT_EDIT = ( + "Generated by hatch_build_sync.py from the open-feature/spec submodule.\n" + "DO NOT EDIT. Changes belong in open-feature/spec under\n" + "specification/assets/provider-tck/, then bump the submodule pin.\n" +) + +# (source directory or file, destination) relative to SPEC_ASSETS / DEST_BASE. +TREES = [("gherkin", "features"), ("flags", "flag_data")] +FILES = [("openapi/control-api.yaml", "control-api.yaml")] + + +def sync() -> None: + if not SPEC_ASSETS.exists(): + msg = ( + f"Conformance assets not found at {SPEC_ASSETS}. " + "Make sure submodules are initialized: " + "`git submodule update --init tools/openfeature-provider-tck/spec`." + ) + raise FileNotFoundError(msg) + + for src_name, dest_name in TREES: + dest = DEST_BASE / dest_name + if dest.exists(): + shutil.rmtree(dest) + shutil.copytree(SPEC_ASSETS / src_name, dest) + (dest / "DO-NOT-EDIT.txt").write_text(DO_NOT_EDIT, encoding="utf-8") + + for src_name, dest_name in FILES: + dest = DEST_BASE / dest_name + if dest.exists(): + dest.unlink() + shutil.copy2(SPEC_ASSETS / src_name, dest) + + +if __name__ == "__main__": + sync() diff --git a/tools/openfeature-provider-tck/pyproject.toml b/tools/openfeature-provider-tck/pyproject.toml new file mode 100644 index 00000000..84ff7278 --- /dev/null +++ b/tools/openfeature-provider-tck/pyproject.toml @@ -0,0 +1,87 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "openfeature-provider-tck" +version = "0.1.0" +description = "OpenFeature provider conformance suite (TCK)" +readme = "README.md" +authors = [{ name = "OpenFeature", email = "openfeature-core@groups.io" }] +license = { file = "LICENSE" } +classifiers = [ + "License :: OSI Approved :: Apache Software License", + "Programming Language :: Python", + "Programming Language :: Python :: 3", + "Framework :: Pytest", +] +keywords = ["openfeature", "conformance", "tck", "feature-flags"] +dependencies = [ + "openfeature-sdk>=0.10.0", + "pytest>=8.4.0", + # Same runner the flagd provider and the flagd testkit already use, so an + # adopting module gains no new test framework. + "pytest-bdd>=8.1.0,<9.0.0", +] +requires-python = ">=3.10" + +[project.urls] +Homepage = "https://github.com/open-feature/python-sdk-contrib" + +# Shipping the step definitions as a pytest plugin is what keeps adoption to a +# single fixture: pytest-bdd resolves steps through the fixture system, and +# fixtures from an installed plugin are visible to every test, so an adopter +# never has to `from ... import *` to pull the vocabulary in. +[project.entry-points.pytest11] +openfeature_provider_tck = "openfeature.contrib.tools.provider_tck.plugin" + +[dependency-groups] +dev = [ + "coverage[toml]>=7.10.0,<8.0.0", + "mypy>=1.18.0,<2.0.0", + "poethepoet>=0.37.0", +] + +[tool.hatch.build.targets.sdist] +# The conformance assets are gitignored copies of the spec submodule; the build +# hook force-includes them so an sdist builds into a wheel without a submodule. +force-include = {} +# Which is why the submodule itself has no business in the sdist: it is the whole +# spec repository, and only the copies of the four assets are needed downstream. +exclude = ["/spec"] + +[tool.hatch.build.targets.wheel] +packages = ["src/openfeature"] +# Ship the conformance assets even though they are gitignored: an adopter +# installing this package must need no submodule of their own. +artifacts = [ + "src/openfeature/contrib/tools/provider_tck/features/", + "src/openfeature/contrib/tools/provider_tck/flag_data/", + "src/openfeature/contrib/tools/provider_tck/control-api.yaml", +] + +[tool.hatch.build.hooks.custom] + +[tool.mypy] +mypy_path = "src" +files = ["src", "tests"] +python_version = "3.10" +namespace_packages = true +explicit_package_bases = true +local_partial_types = true +allow_redefinition_new = true +fixed_format_cache = true +pretty = true +strict = true +disallow_any_generics = false + +[tool.coverage.run] +omit = ["tests/**"] + +[tool.poe.tasks] +sync-spec-assets = "python hatch_build_sync.py" +test = ["sync-spec-assets", {cmd = "pytest tests"}] +test-cov = ["sync-spec-assets", {cmd = "coverage run -m pytest tests"}] +cov-report = "coverage xml" +cov = ["test-cov", "cov-report"] +mypy = "mypy" diff --git a/tools/openfeature-provider-tck/spec b/tools/openfeature-provider-tck/spec new file mode 160000 index 00000000..fc99d5ac --- /dev/null +++ b/tools/openfeature-provider-tck/spec @@ -0,0 +1 @@ +Subproject commit fc99d5ace4da472a5fea0595fa4db8034bbbc769 diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py new file mode 100644 index 00000000..ed361a34 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/__init__.py @@ -0,0 +1,137 @@ +"""The OpenFeature Provider Conformance Suite (TCK) for Python. + +The suite answers one question: does this provider map its backend onto the +OpenFeature provider contract correctly? It is the Python implementation of +`Appendix F`_ of the specification, and it runs the same Gherkin scenarios, +against the same canonical flag set, that every other language's TCK runs. That +shared basis is the whole point -- "conformant" only means something if the +question is identical everywhere. + +**What a provider author writes.** One fixture and one call:: + + import pytest + from pytest_bdd import scenarios + + from openfeature.contrib.tools.provider_tck import ( + Capability, + InProcessControl, + TckConfig, + feature_paths, + ) + + @pytest.fixture(scope="session") + def tck_config(): + control = InProcessControl() + return TckConfig( + name="my-provider", + control=control, + new_provider=control.new_provider, + capabilities={Capability.EVENTS, Capability.OBJECT}, + ) + + scenarios(*feature_paths()) + +``scenarios()`` is pytest-bdd's own, called directly rather than wrapped: it +injects the generated tests into the *calling module* by walking the stack, so a +convenience wrapper around it would deposit them inside this package instead. +:func:`~.extensions.feature_paths` is the canonical assets plus a +``tck-extensions`` directory beside the calling module, if there is one -- see +:mod:`~.extensions`. + +The step definitions arrive through this package's pytest plugin, so there is +nothing to import for them and no ``conftest.py`` to write. Everything else -- +registering the provider, awaiting events, resetting the backend between +scenarios, tearing down -- belongs to the TCK. If you find yourself writing test +infrastructure, that is a defect here rather than something for you to work +around. + +.. _Appendix F: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md +""" + +from __future__ import annotations + +import importlib.resources + +from .capability import DECLARABLE_CAPABILITIES, RESERVED_CAPABILITIES, Capability +from .config import KnownDeviation, TckConfig +from .control import ( + BackendControl, + ConnectionControl, + UnsupportedControlError, +) +from .extensions import ( + EXTENSIONS_DIRECTORY, + feature_paths, + features_path, +) +from .inprocess import InProcessControl +from .provider import ( + CHANGING_FLAG_KEY, + ControllableInMemoryProvider, + canonical_flag_set, +) +from .state import TckState + +__all__ = [ + "CHANGING_FLAG_KEY", + "DECLARABLE_CAPABILITIES", + "EXTENSIONS_DIRECTORY", + "RESERVED_CAPABILITIES", + "BackendControl", + "Capability", + "ConnectionControl", + "ControllableInMemoryProvider", + "InProcessControl", + "KnownDeviation", + "TckConfig", + "TckState", + "UnsupportedControlError", + "canonical_flag_set", + "canonical_flags_json", + "control_api_spec", + "feature_paths", + "features_path", +] + +# NOTE ON THE SOURCE OF TRUTH +# +# The files under features/ and flag_data/, and control-api.yaml, are NOT owned +# by this repository and are NOT committed to it. They are copies of the +# language-agnostic conformance artifacts defined in open-feature/spec under +# specification/assets/provider-tck/, which reaches this package as a git +# submodule at tools/openfeature-provider-tck/spec and is copied in at build +# time by hatch_build.py. The copies are gitignored, so the only record of which +# spec revision this package targets is the submodule pin, and the two cannot +# drift apart unnoticed. +# +# They are copied into the distribution, so an adopter installing this package +# needs no submodule of their own; only a contributor to this package does. +# +# Changes belong in open-feature/spec first, followed by a bump of the submodule +# pin -- editing the copies locally forks the definition of conformance, which is +# the one thing this suite exists to prevent. +# See https://github.com/open-feature/spec/issues/417. + +_PACKAGE = "openfeature.contrib.tools.provider_tck" + + +def canonical_flags_json() -> str: + """Return the canonical flag set as raw JSON, in the flagd flag-definition format. + + This is the flag set every scenario assumes, and a backend under test must + serve an equivalent one. The format is not what matters -- the keys, types, + variant names and resolved values are. Seed them however your backend seeds + flags. + + Exposed so an adopting provider can seed a backend from the canonical + definition rather than transcribing it, transcription being the usual way + the two drift apart. + """ + ref = importlib.resources.files(_PACKAGE) / "flag_data" / "canonical-flags.json" + return ref.read_text(encoding="utf-8") + + +def control_api_spec() -> str: + """Return the OpenAPI document a containerised backend under test must implement.""" + ref = importlib.resources.files(_PACKAGE) / "control-api.yaml" + return ref.read_text(encoding="utf-8") diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/capability.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/capability.py new file mode 100644 index 00000000..69beb195 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/capability.py @@ -0,0 +1,237 @@ +"""Optional parts of the provider contract, and the Gherkin tags that gate them.""" + +from __future__ import annotations + +from enum import Enum + +__all__ = ["Capability"] + + +class Capability(str, Enum): + """An optional part of the OpenFeature provider contract. + + Not every provider implements every part of the specification. A provider + backed by a static file has no meaningful notion of going stale; one with no + streaming transport cannot emit configuration-change events. Rather than + forcing such providers to fail scenarios they were never going to satisfy, + each declares what it supports through :attr:`TckConfig.capabilities`. + + Every capability corresponds to exactly one Gherkin tag. pytest-bdd turns + those tags into pytest markers, and a scenario carrying a marker whose + capability was not declared is skipped with the reason reported -- never + passed. A conformance suite that quietly goes green on scenarios it did not + run is worse than no suite at all. + + Scenarios with no capability tag are mandatory and always run. + """ + + LIFECYCLE = "lifecycle" + """Provider reaches its backend during initialisation, observably and promptly. + + Deliberately separate from :attr:`EVENTS`, because the two are independent in + both directions. + + An SDK dispatches ``PROVIDER_READY`` around ``initialize`` for *any* + provider, so a provider that declares ``EVENTS`` passes the readiness + scenario without demonstrating anything -- a ``NoOpProvider`` passes it + identically. Gating on ``EVENTS`` therefore made the scenario vacuous for + exactly the providers that declared it. + + Conversely a stateless provider -- one that resolves every flag with a fresh + request and holds nothing between them -- has a real initialisation to + verify while having no event stream of its own to declare ``EVENTS`` for. + Gating on ``EVENTS`` shut it out of a scenario it should be held to. + + Declare it if initialisation actually contacts the backend and its outcome, + success or failure, is observable to the application. + """ + + EVENTS = "events" + """Provider emits lifecycle events at all, at minimum ``PROVIDER_READY``.""" + + STALE = "stale" + """Provider enters ``STALE`` and emits ``PROVIDER_STALE`` when it loses its backend.""" + + CONFIGURATION_CHANGE = "configuration-change" + """Provider detects configuration changes and emits ``PROVIDER_CONFIGURATION_CHANGED``.""" + + OBJECT = "object" + """Provider supports structured (object) flag values.""" + + UNAVAILABLE_INIT = "unavailable" + """Provider reports an error state promptly against a backend it cannot reach.""" + + NUMERIC_COERCION = "numeric-coercion" + """Provider coerces between integer and float only when lossless, else ``TYPE_MISMATCH``. + + This is the one entry here that **the specification does not define**. + OpenFeature has a single numeric type on purpose -- ``number`` is "a numeric + value of unspecified type or size", and languages *may* differentiate between + integers and floats "as idioms dictate" -- so no requirement says what a + provider must do when a value does not fit the accessor it was asked through. + That gap is `open-feature/spec#430 + `_. + + The rule this capability is tested against is therefore **borrowed, not + normative**: lossless coercion is permitted, lossy coercion must fail. An + integral float such as ``10.0`` requested as an integer must succeed; ``0.5`` + must not. It comes from flagd's `numeric coercion ADR + `_, + which is scoped to flagd's own implementations, and the tag carries that name + -- it was ``@strict-numeric-typing`` -- because two vocabularies for one + observable property is worse than one borrowed name. + + **A provider that behaves differently is not violating the specification.** + So this is genuinely optional, rather than optional as a concession to a + defect: withholding it may be a deliberate choice as readily as a known bug. + Where it is a bug, say so -- a report's ``knownDeviations`` is for exactly + that, and flagd's instance is tracked as `open-feature/flagd#1996 + `_. + + Both halves have scenarios, and a provider declaring the tag must satisfy + all three. The lossy half asks for ``float-flag`` (``0.5``) as an integer + and expects ``TYPE_MISMATCH``; the lossless half asks for + ``integral-float-flag`` (``10.0``) as an integer and for ``integer-flag`` + (``10``) as a float, and expects both to succeed. Rejecting every float is + an easy way to pass the first, and the other two are what stop it. + + The SDK's own ``InMemoryProvider`` cannot declare this: it hands values + back untouched and the client's type check is ``isinstance``-based, so + ``10.0`` requested as an integer is a ``TYPE_MISMATCH`` rather than ``10``. + The width of the integer accessor is a separate property, and a separate + capability: :attr:`LARGE_INTEGERS`. + """ + + LARGE_INTEGERS = "large-integers" + """Provider resolves integers up to 2^53 - 1 exactly. + + A property of the language's SDK as much as of the provider, which is why + it is a capability rather than mandatory: Java's integer accessor is a + 32-bit ``Integer``, and a provider cannot resolve a value the accessor has + no room for. Every language can ask for 2^31 - 1, so that precision + scenario is untagged; only the one asking for 2^53 - 1 carries this tag. + + Python's ``int`` is unbounded, so a Python provider declares it unless + something of its own -- a 32-bit field in its wire format, a float on the + way through -- narrows the value. Nothing above 2^53 - 1 is asked for: + JavaScript cannot represent it, and what a provider owes a value that does + not fit the requested accessor is the open question in + `open-feature/spec#430 `_. + """ + + REINITIALIZATION = "reinitialization" + """Provider can be initialised again after ``shutdown``, and serves flags afterwards. + + Gated rather than mandatory because the specification permits reuse without + requiring it. `Requirement 2.5.2 + `_ + says a provider **SHOULD** revert to its uninitialized state after + ``shutdown``, and its supporting text adds that "some providers **may** + allow reinitialization from this state". A provider that releases its client + on shutdown and declines to be started again is exercising a choice the + specification offers it, not exhibiting a defect -- so withholding this + capability needs no :class:`~.config.KnownDeviation` entry. + + The scenario was untagged until spec revision ``fc99d5ac``, on the reading + that reverting to the uninitialized state is observable as exactly one thing + -- being initialisable again. That inference does not hold, and asserting it + unconditionally reported a permitted choice as a conformance failure. A false + failure is the mirror image of a vacuous pass. + + Reverting the state is not separately observable either: a provider that + reverts but refuses reuse presents identically to one that did neither. So + the gated reuse scenario is the only assertion the requirement admits, and it + is worth keeping for the providers that do offer reuse -- releasing the client + on shutdown while leaving an initialised flag set behind is easy to write, + and leaves the provider evaluating against a closed connection rather than + failing outright. + + **This tag narrows :attr:`LIFECYCLE` rather than standing beside it.** The + scenario lives in ``lifecycle.feature``, which carries ``@lifecycle`` at the + feature level, so the scenario inherits that tag and carries both. The gate + skips a scenario if *any* capability gating it is undeclared, so reuse is + exercised only by an adoption declaring :attr:`LIFECYCLE` **and** this -- + declaring this one alone leaves the scenario skipped on ``@lifecycle``, and + the declaration unverified. Which is the trap worth naming: a provider that + withholds ``LIFECYCLE`` never ran this scenario, at this pin or the one + before it, so nothing about its behaviour on reuse has been observed either + way and there is no evidence on which to declare this. + """ + + TARGETING = "targeting" + """Reserved, and **not declarable**. No scenario carries this tag: targeting + is backend evaluation logic.""" + + CACHING = "caching" + """Reserved, and **not declarable**. No scenario carries this tag yet.""" + + @property + def tag(self) -> str: + """Return the Gherkin tag, with its leading at-sign, that gates this capability.""" + return f"@{self.value}" + + @property + def reserved(self) -> bool: + """Whether this capability exists in the vocabulary but gates no scenario.""" + return self in RESERVED_CAPABILITIES + + def __str__(self) -> str: + return self.tag + + +RESERVED_CAPABILITIES: frozenset[Capability] = frozenset( + {Capability.TARGETING, Capability.CACHING} +) +"""Capabilities that exist in the vocabulary and gate no scenario. + +They are documented so the vocabulary has a place for them when scenarios exist, +and until then they **must not be declared** and must not appear in a conformance +report's declaration. Nothing carries the tag, so declaring it cannot be +verified, cannot produce a skip, and tells a reader of the report only that +something was claimed and nothing examined. + +Listed once, here, and read everywhere else -- by +:data:`DECLARABLE_CAPABILITIES`, by :attr:`Capability.reserved` and by the +validation in :class:`~.config.TckConfig` -- so that the set and the rule cannot +drift apart. +""" + +DECLARABLE_CAPABILITIES: frozenset[Capability] = ( + frozenset(Capability) - RESERVED_CAPABILITIES +) +"""Every capability an adoption may declare: the vocabulary minus the reserved tags. + +A reasonable starting point for a new adoption: declare everything, run the +suite, and remove only what the provider genuinely cannot do. Narrowing from this +set surfaces gaps; widening towards it hides them. + +It excludes the reserved capabilities rather than spanning the whole enum, and it +is named for what it is rather than for "all", because the declare-everything +convenience is exactly how a reserved tag reaches a report by accident: an +adopter writing "every capability except X" picks up every reserved tag on the +way past, which is how one implementation came to report ``@targeting`` and +``@caching`` as declared without anyone deciding to claim them. +""" + +_BY_MARKER: dict[str, Capability] = {c.value: c for c in Capability} +_BY_TAG: dict[str, Capability] = {c.tag: c for c in Capability} + + +def capability_for_marker(name: str) -> Capability | None: + """Map a pytest marker name onto the capability it gates, if any. + + A marker that does not name a capability gates nothing, which is what lets + the canonical feature files carry organisational tags freely. + """ + return _BY_MARKER.get(name) + + +def capability_for_tag(tag: str) -> Capability | None: + """Map a Gherkin tag, leading at-sign included, onto the capability it gates. + + The tag form rather than the marker form because that is what the + conformance report carries: the report records a scenario's tags as the + feature files spell them, and deciding whether a failure counts against a + capability means reading them back. + """ + return _BY_TAG.get(tag) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/config.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/config.py new file mode 100644 index 00000000..71127051 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/config.py @@ -0,0 +1,333 @@ +"""The contract a provider author implements to run the suite.""" + +from __future__ import annotations + +import typing +from collections.abc import Callable, Collection, Iterable, Mapping, Sequence +from dataclasses import dataclass, field + +from openfeature.provider import FeatureProvider + +from .capability import DECLARABLE_CAPABILITIES, Capability +from .control import BackendControl + +__all__ = ["KnownDeviation", "ProviderFactory", "TckConfig"] + +ProviderFactory = Callable[[], FeatureProvider] +"""Creates the provider under test. + +A factory rather than a single instance because each scenario gets its own +provider, and because a provider often cannot be configured before the suite +starts -- a container stack's host ports do not exist until it is up. +""" + +DEFAULT_EVENT_TIMEOUT = 12.0 +DEFAULT_READY_TIMEOUT = 30.0 + + +@dataclass(frozen=True) +class KnownDeviation: + """A gap the provider is known to have, acknowledged rather than hidden. + + Distinct from an undeclared capability, which is a choice, and from a + not-applicable one, which is impossible: this is a defect against something + the specification does not treat as optional, with the gap tracked + somewhere. + + It changes nothing about how the suite runs. The scenario still fails, and + the results payload still reports it as failed -- a report that softened a + failure into a footnote would hide exactly what the acknowledgement exists to + keep visible. What this adds is the acknowledgement itself, in the envelope, + so that a consumer can tell a known and tracked gap from a surprise. + """ + + issue: str + """Where the gap is tracked. A URI, because the schema requires one.""" + + summary: str + """What is wrong, for a person reading a comparison page.""" + + capability: Capability | None = None + """The capability the deviation concerns, when it maps to one. + + Left out for a deviation against a mandatory scenario, which belongs to no + capability -- which is the common case, since a capability a provider fails + is usually one it should not have declared. + """ + + def as_json(self) -> dict[str, typing.Any]: + document: dict[str, typing.Any] = { + "issue": self.issue, + "summary": self.summary, + } + if self.capability is not None: + document["capability"] = self.capability.tag + return document + + +@dataclass(frozen=True) +class TckConfig: + """Everything the TCK needs to test one provider. + + An adopting module supplies this through a session-scoped ``tck_config`` + fixture; the TCK owns everything else -- registering the provider, awaiting + events, resetting the backend between scenarios, tearing down. If you find + yourself writing test infrastructure, that is a defect in this package + rather than something for you to work around. + """ + + name: str + """Identifies the suite in test output, and scopes the OpenFeature domain + the TCK registers providers under so two suites in the same session do not + observe each other's providers. + + Use something that reads well in a failure message: ``"flagd-rpc"``, + ``"in-memory"``. + """ + + control: BackendControl + """The seam through which the TCK manipulates the backend. + + See :class:`~.control.BackendControl` for which implementation is right for + your provider. The short version: a provider with a real backend drives it + over the HTTP control API; a provider with no backend at all may control it + in-process. + """ + + new_provider: ProviderFactory + """Creates the provider under test, against a backend that is already + running and seeded with the canonical flag set. Called once per scenario. + + Return a configured but uninitialised provider; the TCK initialises it. + """ + + new_unavailable_provider: ProviderFactory | None = None + """Creates a provider pointed at a backend that does not exist. + + Used by the initialisation-failure scenarios, which assert that a provider + unable to reach its backend settles into ``ERROR`` rather than hanging or + raising out of registration. + + Point it at a closed port on localhost. Do not point it at the backend under + test -- that must stay up, and simulated outages belong to :attr:`control`. + Configure a short connection deadline: the scenario allows a bounded time + for the error, and a provider with a 30-second connect timeout will not make + it. + + Required only if :attr:`capabilities` includes + :attr:`Capability.UNAVAILABLE_INIT`. Leaving both out is the honest + configuration for a provider with no backend, and those scenarios are then + skipped with the reason reported. + """ + + capabilities: Collection[Capability] = field(default=DECLARABLE_CAPABILITIES) + """Which optional parts of the provider contract this provider supports. + + Typed as a ``Collection`` rather than a ``frozenset`` so that the obvious + thing to write -- a set literal, which is what the README shows -- is also + the correctly typed thing to write. It is normalised to a frozenset on + construction, so a list, a set or a generator all behave identically. + + Scenarios tagged with an undeclared capability are reported as skipped with + the reason, never as passed. Defaults to every *declarable* capability -- + :data:`~.capability.DECLARABLE_CAPABILITIES`, which excludes the reserved + tags no scenario carries -- and narrowing it surfaces gaps where widening + towards it hides them. + + Naming a reserved capability here is rejected at construction rather than + passed into a report. See :data:`~.capability.RESERVED_CAPABILITIES`. + """ + + not_applicable: Mapping[Capability, str] = field(default_factory=dict) + """Capabilities that cannot hold for this provider, each with a reason. + + Kept apart from simply leaving a capability out of :attr:`capabilities`, + because the two are different claims and collapsing them misrepresents whole + languages: ``@numeric-coercion`` is unsatisfiable in JavaScript because + the language has no integer type, and reporting that as a choice would show + every JavaScript provider as missing something none of them can have. + + Scenarios behind a not-applicable capability are skipped exactly as an + undeclared one's are -- the gate makes no distinction, and neither does the + results payload. The difference is recorded once, here, and reaches the + report's declaration. + + Where the impossibility is a property of the language rather than of the + provider it belongs in the capability documentation rather than in every + report, so this is for provider-specific cases. + """ + + known_deviations: Sequence[KnownDeviation] = () + """Gaps this provider is known to have, with each one tracked somewhere. + + An acknowledgement, not an excuse: the scenarios still fail and the results + payload still says so. See :class:`KnownDeviation`. + """ + + event_timeout: float = DEFAULT_EVENT_TIMEOUT + """Seconds to wait for a provider event. + + The single most important knob for a provider author, because providers + observe backend changes on wildly different timescales. A streaming provider + sees a configuration change in milliseconds; one polling every 30 seconds + may need most of a poll interval. Set it to comfortably exceed your + worst-case detection latency, or the suite reports timeouts that are really + just impatience. + + Scenarios can tighten this with the explicit ``within {int}ms`` step, which + always wins over this value. + """ + + ready_timeout: float = DEFAULT_READY_TIMEOUT + """Seconds to wait for a provider to reach ``READY`` during initialisation. + + Also the longest the suite waits on a direct ``shutdown`` or ``initialize`` + call before giving up on it and recording the wait as a failure, so that a + provider whose shutdown hangs on a backend that is gone fails its scenario + with a message rather than hanging the session. + """ + + def __post_init__(self) -> None: + problems: list[str] = [] + + if not self.name: + problems.append( + "name is required: it scopes the OpenFeature domain and identifies " + "the suite in test output" + ) + if self.control is None: + problems.append( + "control is required: see BackendControl for which implementation " + "fits your provider" + ) + if self.new_provider is None: + problems.append( + "new_provider is required: the TCK has nothing to test without it" + ) + + # Normalise whatever iterable the caller passed into a frozenset, so a + # set literal, a list or a generator all behave the same. + object.__setattr__(self, "capabilities", frozenset(self.capabilities)) + + unknown = [c for c in self.capabilities if not isinstance(c, Capability)] + if unknown: + problems.append( + f"unknown capabilities {unknown!r}: capabilities are the members of " + f"the Capability enum" + ) + + # Normalised the same way, so a dict literal keyed by Capability is what + # an adopter writes and a plain mapping is what everything else reads. + object.__setattr__(self, "not_applicable", dict(self.not_applicable)) + object.__setattr__(self, "known_deviations", tuple(self.known_deviations)) + + stray = [c for c in self.not_applicable if not isinstance(c, Capability)] + if stray: + problems.append( + f"unknown capabilities {stray!r} in not_applicable: capabilities are " + f"the members of the Capability enum" + ) + + both = sorted( + capability.tag + for capability in self.not_applicable + if isinstance(capability, Capability) and capability in self.capabilities + ) + if both: + problems.append( + f"capabilities and not_applicable both claim {' '.join(both)}: a " + f"capability is either declared or impossible, and a report saying " + f"both leaves a consumer to guess which" + ) + + problems.extend( + reserved_problems(self.capabilities, self.not_applicable.keys()) + ) + + unreasoned = sorted( + capability.tag + for capability, reason in self.not_applicable.items() + if isinstance(capability, Capability) + and (not isinstance(reason, str) or not reason.strip()) + ) + if unreasoned: + problems.append( + f"not_applicable gives no reason for {' '.join(unreasoned)}: " + f"'impossible for this provider' is only useful to a reader who is " + f"told why, and the report schema requires the reason" + ) + + if ( + Capability.UNAVAILABLE_INIT in self.capabilities + and self.new_unavailable_provider is None + ): + problems.append( + "capabilities declares Capability.UNAVAILABLE_INIT but " + "new_unavailable_provider is None: the @unavailable scenarios need a " + "provider pointed at a backend that does not exist. Supply one, or " + "remove the capability so those scenarios are skipped with a reason" + ) + + if problems: + joined = "\n - ".join(problems) + msg = f"invalid TckConfig:\n - {joined}" + raise ValueError(msg) + + @property + def domain(self) -> str: + """The OpenFeature domain this suite registers its providers under. + + Suite-scoped rather than scenario-scoped on purpose. Registering a new + provider in the same domain replaces the previous one; a fresh domain + per scenario would leave every provider of the suite registered, which + for a provider holding a network connection means leaking one connection + per scenario. + """ + return f"provider-tck/{self.name}" + + def declares(self, capability: Capability) -> bool: + return capability in self.capabilities + + @property + def sorted_capabilities(self) -> list[str]: + return sorted(c.tag for c in self.capabilities) + + +def reserved_problems(*named: Iterable[Capability]) -> list[str]: + """Refuse a reserved capability named anywhere in a configuration. + + A reserved capability gates no scenario, so naming it cannot be verified + either way: declaring it claims something nothing examined, and calling it + not-applicable records an impossibility about a question that was never + asked. Either would reach the report's declaration, which the schema + forbids. + + Refused rather than dropped quietly. The adopter wrote it down and meant + something by it, so a configuration silently different from the one they + wrote is worse than one that will not build -- and construction is where + their own code is still on the stack to say which line to fix. The + alternative, a warning, is a line of CI output nobody reads while an + untested capability goes on being asserted in a published report, which is + how this got into one in the first place. + """ + reserved = sorted( + capability.tag + for group in named + for capability in group + if isinstance(capability, Capability) and capability.reserved + ) + if not reserved: + return [] + declarable = " ".join(sorted(c.tag for c in DECLARABLE_CAPABILITIES)) + return [ + f"reserved capabilities {' '.join(sorted(set(reserved)))} cannot be declared " + f"or called not-applicable: no scenario carries them, so the claim cannot be " + f"verified, cannot produce a skip, and would tell a reader of the report only " + f"that something was claimed and nothing examined. The declarable " + f"capabilities, which is what DECLARABLE_CAPABILITIES holds, are {declarable}" + ] + + +def capabilities_of(values: Iterable[Capability]) -> frozenset[Capability]: + """Convenience for building a capability set from any iterable.""" + return frozenset(values) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/control.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/control.py new file mode 100644 index 00000000..e92dd18d --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/control.py @@ -0,0 +1,128 @@ +"""The seam between the scenarios and whatever manipulates the backend.""" + +from __future__ import annotations + +import typing + +__all__ = [ + "BackendControl", + "ConnectionControl", + "UnsupportedControlError", + "unsupported_control", +] + + +class UnsupportedControlError(RuntimeError): + """Raised when a backend cannot perform a control operation. + + It is always a test-configuration bug rather than a provider defect. The + scenarios needing connection control are gated behind + :attr:`Capability.STALE` and :attr:`Capability.UNAVAILABLE_INIT`, so + reaching an unsupported operation means a capability was declared that the + backend cannot back up. The TCK fails loudly on it rather than skipping, + because a silent no-op would report the scenario as passed. + """ + + +@typing.runtime_checkable +class BackendControl(typing.Protocol): + """How the TCK puts the backend under test into the states a scenario needs. + + Step definitions never talk to a backend directly. They talk to this + protocol, which is why the same Gherkin runs unchanged against a + containerised backend driven over HTTP and against a provider manipulated + in-process. Nothing below this line knows about ports, containers or + transports. + + **Which implementation is right for your provider.** If your provider talks + to a backend -- a server, a service, anything out of process -- drive it + over the HTTP control API described in ``control-api.yaml``. That API is the + normative contract for those providers, and it is what makes a conformance + claim portable: another language's TCK drives the same endpoints against the + same stack and must get the same answers. + + Do not write an in-process control that reaches into an external backend + through a side channel -- a test-only admin client, a shared database + handle, a hook inside the provider. It will pass, and it will prove nothing, + because the path it exercised is not the path the contract describes. + + In-process control exists for providers with *no* backend to contract with: + in-memory, environment-variable and file-based providers, where "the + backend" is a data structure in the same process. See + :class:`InProcessControl`. + """ + + def prepare_scenario(self) -> None: + """Bring the backend to the state every scenario starts from. + + Reachable, with flag state at the baseline of the canonical flag set. + Called once before each scenario. This is the TCK's only isolation + mechanism -- scenarios share one backend for the whole suite, and + containers are never restarted between them. + """ + + def change_flag(self) -> None: + """Mutate flag configuration so a conforming provider observes a change. + + Afterwards the provider must resolve a different value for + ``changing-flag``. Which value it changes to is deliberately + unspecified; the suite asserts only that the resolved value differs. + """ + + @property + def description(self) -> str: + """A short description of what is being controlled, for messages a human reads.""" + + # OPTIONAL: ``control_api`` + # + # A control may also offer a ``control_api`` property returning ``"http"`` + # for the normative HTTP control API, or ``"in-process"`` for the narrow + # allowance made for providers with no backend. The conformance report + # records it, so that a claim of in-process control by a provider that does + # have a backend can be treated with the suspicion it deserves. + # + # It is deliberately not a member of this protocol. Adding one would make + # every existing control incomplete for the sake of one string, and there is + # nothing useful the TCK can do with a control that has not said: it cannot + # tell from the outside whether a control spoke HTTP or reached into the + # process, so the field is simply omitted. See ``report.control_api_of``. + + +@typing.runtime_checkable +class ConnectionControl(typing.Protocol): + """Implemented by a backend that can be cut off from the provider and restored. + + Separate from :class:`BackendControl` so a backend-less provider cannot + accidentally supply a no-op implementation: not implementing it at all is + the honest answer, and the TCK turns the resulting gap into an explicit, + reported skip. + """ + + def disconnect(self) -> None: + """Make the backend unreachable for the rest of the scenario, without stopping a container.""" + + def reconnect(self) -> None: + """Make the backend reachable again, preserving flag state. + + Preserving flag state is a requirement, not an implementation detail. An + outage must be observable as a change in availability and never as a + change in flag values, or the stale scenario cannot distinguish the two. + """ + + +def unsupported_control( + control: BackendControl, operation: str +) -> UnsupportedControlError: + """Build the error raised when a backend has no connection to control. + + The message names the fix, because the mistake it reports is always the same + one. + """ + return UnsupportedControlError( + f"{control.description} does not support {operation!r}. This is a " + f"test-configuration bug rather than a provider defect: a scenario needing " + f"connection control ran, so the suite declared Capability.STALE or " + f"Capability.UNAVAILABLE_INIT for a backend that cannot simulate an outage. " + f"Remove those capabilities from TckConfig.capabilities, or supply a " + f"BackendControl that also implements ConnectionControl." + ) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/extensions.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/extensions.py new file mode 100644 index 00000000..e5796d64 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/extensions.py @@ -0,0 +1,296 @@ +"""Where the scenarios come from: the canonical set, plus whatever an adopter adds. + +A provider is rarely only a provider. flagd has ``fractional`` targeting, another +vendor has a proprietary rollout rule, and the behaviour of those is as worth +pinning as the contract they sit on top of. Verifying them used to mean standing +up a second harness: a second backend lifecycle, a second set of fixtures, a +second thing to keep working. The canonical suite ran, then something else ran, +and nothing tied the two together. + +So an adopter's own scenarios run **inside** the canonical suite instead -- +against the same provider instance, in the same backend lifecycle, with the same +step vocabulary available. Almost nothing is needed to make that happen, because +pytest already scans. It collects ``conftest.py`` on its own and pytest-bdd +resolves step definitions through the fixture system, so a step defined in the +adopter's ``conftest.py`` -- or in the test module itself -- is in scope for the +scenarios ``scenarios()`` generates there. The only thing pytest cannot find by +itself is the feature files, which is what this module finds: a directory named +``tck-extensions`` beside the adopter's test module. + +That leaves one line, and it is the same line whether or not there are +extensions:: + + scenarios(*feature_paths()) + +**An extension can never stand in for a canonical scenario.** The two are told +apart by the uri each feature file is identified by, and this module derives that +uri from where the file *is* rather than taking what the runner offers: + +* ``features/…`` is the packaged canonical assets, and nothing else; +* ``extensions/…`` is a discovered extension, whatever the adopter's own + directory layout under ``tck-extensions`` looks like. + +The derivation is not decoration. pytest-bdd names a feature file by its parent +directory joined to its own name, so ``tck-extensions/features/errors.feature`` +arrives as ``features/errors.feature`` -- the same uri as a canonical file. A +record of what ran holds one copy of a feature file per uri, so the second file +is never read and its scenarios are attributed to the first one's or to nothing +at all. Java hit the same thing by a different route: a same-named feature file +in a second classpath root replaced the canonical one outright and the suite went +green having run the adopter's version. + +The derivation is public, and the two problems it cannot rule out are reported +rather than raised, because the consumer of all of this is a conformance report +and that is not written here. Appendix F requires a report to say which scenarios +ran; nothing else can tell an adopter's question from the specification's. +""" + +from __future__ import annotations + +import importlib.resources +import inspect +import typing +from pathlib import Path + +__all__ = [ + "CANONICAL_DIRECTORY", + "EXTENSIONS_DIRECTORY", + "EXTENSIONS_URI_PREFIX", + "canonical_root", + "collision_problem", + "extension_root", + "feature_paths", + "features_path", + "is_canonical", + "is_canonical_uri", + "reserved_prefix_problem", + "uri_collisions", + "uri_for", +] + +_PACKAGE = "openfeature.contrib.tools.provider_tck" + +CANONICAL_DIRECTORY = "features" +"""The packaged directory the canonical feature files live in. + +Also the uri prefix they are identified by, which is why it is reserved: anyone +reading ``features/errors.feature`` is entitled to assume it is the +specification's file rather than a local one that happened to land in a directory +of that name. +""" + +EXTENSIONS_DIRECTORY = "tck-extensions" +"""Where an adopter puts feature files of their own, beside their test module. + +Deliberately not ``features``: a directory sharing the canonical name is how an +extension comes to occupy a canonical file's identity, and a convention that +cannot collide is worth more than one that reads slightly better. The name is +the one Java's TCK scans for on the classpath, so an adopter who ships a provider +in both languages puts the same directory in both repositories. +""" + +EXTENSIONS_URI_PREFIX = "extensions" +"""The uri prefix an extension's scenarios are identified by. + +The Go and JavaScript suites mount extensions under the same prefix, so a +consumer holding reports from several languages applies one rule to tell an +adopter's scenario from the specification's. +""" + + +def features_path() -> str: + """Return the directory holding the canonical feature files. + + Packaged with this distribution, so a consumer needs no submodule and no + particular directory layout. This is the canonical set on its own; prefer + :func:`feature_paths`, which also picks up an adopter's own scenarios. + """ + return str(importlib.resources.files(_PACKAGE) / CANONICAL_DIRECTORY) + + +def feature_paths() -> tuple[str, ...]: + """Return every feature directory this adoption should run. + + The canonical set, always, and a ``tck-extensions`` directory beside the + calling module if there is one. Hand the result to pytest-bdd's + ``scenarios()``:: + + scenarios(*feature_paths()) + + That line does not change when an adopter adds an extension, which is what + makes adding one a matter of creating a directory rather than of configuring + anything. + + The calling module is located from the caller's frame, which is how + pytest-bdd locates it for ``scenarios()`` itself, so the two agree about + which module is adopting the suite. Call it from the test module rather than + from a helper: a helper's directory is what a helper would find. A caller + with no ``__file__`` -- an interactive session, an exec'd string -- gets the + canonical set alone. + """ + paths = [features_path()] + directory = _caller_directory() + if directory is not None: + extensions = extension_root(directory) + if extensions is not None: + paths.append(str(extensions)) + return tuple(paths) + + +def extension_root(module_directory: Path) -> Path | None: + """The extension directory beside a test module, or ``None`` if there is none. + + ``None`` rather than a path that contributes nothing, so that an adopter + without extensions hands ``scenarios()`` exactly what they handed it before: + same scenarios, same count, same report. + """ + candidate = module_directory / EXTENSIONS_DIRECTORY + return candidate if candidate.is_dir() else None + + +def canonical_root() -> Path | None: + """The packaged canonical features directory, as a real path. + + ``None`` if the assets are not on the filesystem -- an installation from a + zipimport, say. Everything built on this degrades to "cannot tell", which is + the honest answer and never a false accusation. + """ + try: + return _resolve(Path(features_path())) + except (OSError, TypeError): # pragma: no cover - assets outside a filesystem + return None + + +def is_canonical(path: Path) -> bool: + """Whether a feature file is one of the packaged canonical ones.""" + canonical = canonical_root() + return canonical is not None and _resolve(path).is_relative_to(canonical) + + +def is_canonical_uri(uri: str) -> bool: + """Whether a uri names a canonical feature file. + + The discriminator between a canonical scenario and an adopter's own wherever + it matters. Derived from the uri rather than carried beside it, so there is + no second fact to disagree with the first. + """ + return uri.startswith(f"{CANONICAL_DIRECTORY}/") + + +def uri_for(path: Path) -> str | None: + """The uri a feature file should be identified by. + + ``None`` when the file is neither canonical nor under an extension + directory, in which case the caller falls back to what pytest-bdd named it. + + Derived from the file's location rather than from pytest-bdd's + ``rel_filename``, which is the parent directory's name joined to the file's + own. That is what let ``tck-extensions/features/errors.feature`` present + itself as ``features/errors.feature``: the same uri as a canonical file, and + a record of what ran holds one copy of a feature file per uri. + """ + resolved = _resolve(path) + + canonical = canonical_root() + if canonical is not None and resolved.is_relative_to(canonical): + return _uri(Path(CANONICAL_DIRECTORY) / resolved.relative_to(canonical)) + + for parent in resolved.parents: + if parent.name == EXTENSIONS_DIRECTORY: + return _uri(Path(EXTENSIONS_URI_PREFIX) / resolved.relative_to(parent)) + return None + + +def reserved_prefix_problem(uri: str, path: Path) -> str | None: + """Report a feature file claiming the canonical uri prefix without being canonical. + + The one thing the naming convention cannot rule out on its own: an adopter + who hands ``scenarios()`` a directory of their own named ``features``. The + file is then named exactly as a canonical one would be, and a reader has no + way to tell that the specification did not write it. + + Returned rather than raised. The suite itself has no use for the answer -- + the scenarios run either way, and they are the adopter's to run -- so this is + for whatever writes a record of the run to refuse to publish one. + """ + if not is_canonical_uri(uri) or is_canonical(path): + return None + return ( + f"{uri} is not a canonical feature file -- it is {path} -- but it would be " + f"reported under the {CANONICAL_DIRECTORY}/ prefix, which is reserved for " + f"the packaged conformance assets. Move it into a directory named " + f"{EXTENSIONS_DIRECTORY} beside the test module, which feature_paths() " + f"finds on its own" + ) + + +def uri_collisions( + identified: typing.Iterable[tuple[str, Path]], +) -> dict[str, tuple[Path, ...]]: + """Feature files that would share one uri, keyed by that uri. + + Deriving the uri from the file's location removes the collision an adopter + is actually likely to hit, but it does not make one impossible. Two + extension roots contributing the same relative path to a single suite -- two + test modules sharing one ``tck_config`` from a conftest, each with a + ``tck-extensions/vendor.feature`` -- still land on ``extensions/vendor.feature`` + twice, and so does a ``tck-extensions`` directory nested inside another one. + + That has to be refused rather than resolved. A record of what ran holds one + copy of a feature file per uri, so the second file is never read: its + scenarios are attributed to the first file's where the names happen to + match, and go missing where they do not. The first is the silent form of + exactly the failure Java measured, and it is the one a consumer cannot + detect from the outside. + + Compared by resolved path, so the same file reached by two routes is one + file rather than a collision. + """ + files: dict[str, dict[Path, None]] = {} + for uri, path in identified: + files.setdefault(uri, {})[_resolve(path)] = None + return {uri: tuple(paths) for uri, paths in files.items() if len(paths) > 1} + + +def collision_problem(uri: str, paths: typing.Sequence[Path]) -> str: + """Say which files collided and what to do about it.""" + listed = ", ".join(str(path) for path in sorted(paths)) + return ( + f"{uri} is the uri of {len(paths)} different feature files -- {listed} -- " + f"and a record of what ran holds one copy of a feature file per uri, so " + f"one of them would be reported against the other's. Give them paths that " + f"differ below their {EXTENSIONS_DIRECTORY} directory" + ) + + +def _caller_directory() -> Path | None: + """The directory of the module two frames up, if it has a file.""" + frame = inspect.currentframe() + for _ in range(2): + if frame is None: # pragma: no cover - no Python frames to walk + return None + frame = frame.f_back + if frame is None: # pragma: no cover - called with no caller above + return None + file_name: typing.Any = frame.f_globals.get("__file__") + if not isinstance(file_name, str) or not file_name: + return None + return _resolve(Path(file_name)).parent + + +def _resolve(path: Path) -> Path: + try: + return path.resolve() + except OSError: # pragma: no cover - a path that cannot be resolved at all + return path + + +def _uri(path: Path) -> str: + """A relative path as a uri: slash-separated on every platform. + + These paths are assembled with ``pathlib``, so on Windows they arrive + backslash-separated. A uri is not, and the same string has to identify a + feature file wherever the suite ran or a run on Windows is not comparable + with one on Linux. + """ + return path.as_posix() diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/inprocess.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/inprocess.py new file mode 100644 index 00000000..11586e0f --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/inprocess.py @@ -0,0 +1,122 @@ +"""In-process backend control, for providers with no backend at all.""" + +from __future__ import annotations + +from openfeature.provider import FeatureProvider + +from .provider import ( + CHANGING_FLAG_KEY, + ControllableInMemoryProvider, + canonical_flag_set, + changing_flag, +) + +__all__ = ["InProcessControl"] + +_BASELINE = "foo" +_CHANGED = "bar" + + +class InProcessControl: + """Manipulates an in-process provider directly, with no backend and no HTTP. + + This exists so providers with nothing to connect to -- in-memory, + environment-variable and file-based providers -- can run the TCK. For those, + "the backend" is a data structure in the same process: seeding flags is + building a mapping, and changing one is an update on the live provider, so + the event the suite awaits is the provider's own + ``PROVIDER_CONFIGURATION_CHANGED`` rather than one the TCK synthesised. + + **This is not a shortcut for providers that do have a backend.** Reaching + into an external backend from inside the test process -- a test-only admin + client, a shared database handle, a hook in the provider -- produces a suite + that passes while proving nothing, because the path it exercised is not the + path the contract describes. Those providers drive the HTTP control API + instead. + + **Connection control.** :class:`InProcessControl` deliberately does not + implement :class:`~.control.ConnectionControl`. An in-memory provider has no + connection to lose, and pretending otherwise with a no-op would report the + ``@stale`` scenarios as passed. A suite using it leaves + :attr:`Capability.STALE` and :attr:`Capability.UNAVAILABLE_INIT` undeclared, + and those scenarios are skipped with the reason reported. + + **Ownership of the provider.** This type both seeds the flags and creates + the provider serving them, because in-process they are the same object: + :meth:`change_flag` has to reach the live instance to emit an event from it. + A suite therefore wires both through one control:: + + control = InProcessControl() + TckConfig( + name="in-memory", + control=control, + new_provider=control.new_provider, + capabilities={Capability.EVENTS, Capability.CONFIGURATION_CHANGE}, + ) + """ + + def __init__(self) -> None: + self._current: ControllableInMemoryProvider | None = None + self._changing_variant = _BASELINE + + @property + def description(self) -> str: + return "in-process control of an in-memory provider" + + @property + def control_api(self) -> str: + """Report how this backend was driven, for the conformance report. + + ``in-process`` is the narrow allowance for providers with no backend, + which is exactly what this control exists for. A provider that does have + a backend and reports this is claiming something it should not. + """ + return "in-process" + + def new_provider(self) -> FeatureProvider: + """Create the provider for the scenario about to run, at the baseline. + + Each call returns a fresh instance over a fresh copy of the canonical + flag set, which is what makes :meth:`prepare_scenario` nothing more than + dropping the previous reference. + """ + self._changing_variant = _BASELINE + self._current = ControllableInMemoryProvider(canonical_flag_set()) + return self._current + + def prepare_scenario(self) -> None: + """Drop the previous scenario's provider. + + That is the whole reset: the flag set is rebuilt per provider, so the + :meth:`new_provider` call that follows starts from an untouched + baseline. Clearing the reference rather than leaving it dangling means a + scenario that changes flags without creating a provider fails with a + clear message instead of mutating one that has already been shut down. + """ + self._current = None + + def change_flag(self) -> None: + """Flip ``changing-flag`` between its two variants on the live provider. + + The event the suite awaits is therefore the provider's own + ``PROVIDER_CONFIGURATION_CHANGED``, carrying ``changing-flag`` in + ``flags_changed``, and not a signal the TCK synthesised. + + Alternating rather than assigning a fixed variant keeps repeated calls + within one scenario meaningful; the suite asserts that the resolved + value differs, not what it became. + """ + if self._current is None: + msg = ( + "No in-memory provider exists for this scenario. In-process control " + "manipulates the provider itself, so the scenario must create one -- " + 'with "Given a stable provider" -- before any step that changes flag state.' + ) + raise RuntimeError(msg) + + self._changing_variant = ( + _BASELINE if self._changing_variant == _CHANGED else _CHANGED + ) + self._current.update_flag( + CHANGING_FLAG_KEY, changing_flag(self._changing_variant) + ) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/plugin.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/plugin.py new file mode 100644 index 00000000..d4b66bb4 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/plugin.py @@ -0,0 +1,110 @@ +"""The pytest plugin: capability gating, scenario state, and the shared step vocabulary. + +Registered through the ``pytest11`` entry point, so installing this package is +all it takes for the step definitions to be available. pytest-bdd resolves steps +through the fixture system and fixtures from an installed plugin are visible to +every test, which is what keeps an adoption down to one fixture and one call to +``scenarios(*feature_paths())``. + +The same mechanism is what makes the suite extensible: a step an adopter defines +in their own ``conftest.py`` is resolved by the same fixture lookup as one this +plugin ships, so their scenarios need no glue and no second harness. See +:mod:`~.extensions`. +""" + +from __future__ import annotations + +import typing + +import pytest + +from openfeature import api + +from .capability import Capability, capability_for_marker +from .config import TckConfig +from .state import TckState + +# The step modules are registered as plugins in their own right, not merely +# imported. pytest-bdd's decorators inject a generated fixture name into the +# *defining* module's namespace, so a step is only visible to pytest once the +# module defining it is a registered plugin -- importing it here would run the +# decorators but leave those fixtures where pytest never looks. +pytest_plugins = [ + "openfeature.contrib.tools.provider_tck.steps.provider_steps", + "openfeature.contrib.tools.provider_tck.steps.flag_steps", + "openfeature.contrib.tools.provider_tck.steps.event_steps", +] + + +def pytest_configure(config: pytest.Config) -> None: + """Register the capability tags as markers. + + pytest-bdd turns every Gherkin tag into a marker with + ``getattr(pytest.mark, tag)`` without registering it, which raises + ``PytestUnknownMarkWarning`` for each one -- noise at best, and a hard + failure in a project configured with ``-W error``. + """ + for capability in Capability: + config.addinivalue_line( + "markers", + f"{capability.value}: OpenFeature provider TCK capability {capability.tag}", + ) + + +@pytest.fixture +def tck_state(tck_config: TckConfig) -> typing.Iterator[TckState]: + """Per-scenario state, carried between step definitions.""" + # Resetting here rather than in an autouse fixture ties the reset to the + # scenarios that actually use the TCK, and guarantees it happens after the + # capability gate has had its say -- a skipped scenario never touches the + # backend. + tck_config.control.prepare_scenario() + state = TckState(config=tck_config) + yield state + state.teardown() + + +@pytest.fixture(autouse=True) +def _tck_capability_gate(request: pytest.FixtureRequest) -> None: + """Skip a scenario whose capability the provider did not declare. + + ``pytest.skip`` here reports the scenario as skipped **with the reason**, + which is exactly what the specification asks a TCK implementation to do. + Nothing about it can be mistaken for a pass. + + The gate keys off the node's markers rather than its requested fixtures. + pytest-bdd resolves a step's fixtures lazily, as each step runs, so + ``tck_config`` is not in ``request.fixturenames`` when this autouse fixture + is set up -- guarding on that silently disabled the gate and let + ``@unavailable`` scenarios run against a config that never declared it. + + Checking markers first also means the gate costs nothing, and instantiates + nothing, for tests that are not TCK scenarios. + """ + gated = [ + capability + for marker in request.node.iter_markers() + if (capability := capability_for_marker(marker.name)) is not None + ] + if not gated: + return + + try: + config: TckConfig = request.getfixturevalue("tck_config") + except pytest.FixtureLookupError: + return + + for capability in gated: + if not config.declares(capability): + pytest.skip( + f"provider does not declare capability {capability.tag}. " + f"Declared: {' '.join(config.sorted_capabilities) or '(none)'}" + ) + + +@pytest.fixture(scope="session", autouse=True) +def _tck_release_providers() -> typing.Iterator[None]: + """Shut down whatever the suite registered once it is over.""" + yield + api.shutdown() + api.clear_providers() diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/provider.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/provider.py new file mode 100644 index 00000000..fae7b564 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/provider.py @@ -0,0 +1,167 @@ +"""An in-memory provider that can be reconfigured at runtime, and the canonical flag set.""" + +from __future__ import annotations + +import typing + +from openfeature.event import ProviderEventDetails +from openfeature.provider.in_memory_provider import ( + FlagStorage, + InMemoryFlag, + InMemoryProvider, +) + +__all__ = [ + "CHANGING_FLAG_KEY", + "ControllableInMemoryProvider", + "canonical_flag_set", + "changing_flag", +] + +CHANGING_FLAG_KEY = "changing-flag" +"""The flag :meth:`BackendControl.change_flag` mutates.""" + +_CHANGING_BASELINE = "foo" +_CHANGING_CHANGED = "bar" + + +class ControllableInMemoryProvider(InMemoryProvider): + """An in-memory provider whose flag set can be replaced at runtime. + + **Why this exists.** `Appendix A`_ of the specification requires an SDK's + in-memory provider to "support a means of updating the ``flag set``, + resulting in the emission of ``PROVIDER_CONFIGURATION_CHANGED`` events". The + Python SDK's :class:`~openfeature.provider.in_memory_provider.InMemoryProvider` + has no such method: it copies the flag mapping in its constructor and never + exposes a way to change it. + + Only half the machinery is missing, which is what makes this a small class + rather than a reimplementation. :class:`~openfeature.provider.AbstractProvider` + already supplies ``emit_provider_configuration_changed``, and the registry + already attaches the emitter, so all that is needed is a method that swaps + the mapping and emits. Everything about *resolution* -- variants, reasons, + ``FLAG_NOT_FOUND`` -- is still the SDK's. + + That makes this an honest reference for what the SDK's provider should grow, + rather than a competing implementation that could drift from it. + + .. _Appendix A: https://github.com/open-feature/spec/blob/main/specification/appendix-a-included-utilities.md + """ + + def update_flags(self, flags: FlagStorage) -> None: + """Replace the whole flag set and emit a configuration-change event. + + The event names the union of the previous and new keys, which is what + Appendix A asks for: a consumer caching evaluations needs to know + everything that might have changed, and a key that disappeared has + changed as much as one that was added. + """ + changed = sorted(set(self._flags) | set(flags)) + self._flags = dict(flags) + self.emit_provider_configuration_changed( + ProviderEventDetails( + flags_changed=changed, message="flag configuration changed" + ) + ) + + def update_flag(self, key: str, flag: InMemoryFlag[typing.Any]) -> None: + """Replace a single flag and emit a configuration-change event naming it.""" + updated = dict(self._flags) + updated[key] = flag + self._flags = updated + self.emit_provider_configuration_changed( + ProviderEventDetails( + flags_changed=[key], message="flag configuration changed" + ) + ) + + def flag(self, key: str) -> InMemoryFlag[typing.Any] | None: + """Return the flag currently registered under ``key``.""" + return self._flags.get(key) + + +def changing_flag(default_variant: str) -> InMemoryFlag[str]: + return InMemoryFlag( + default_variant=default_variant, + variants={ + _CHANGING_BASELINE: _CHANGING_BASELINE, + _CHANGING_CHANGED: _CHANGING_CHANGED, + }, + ) + + +def canonical_flag_set() -> FlagStorage: + """Return the canonical flag set as SDK in-memory flags. + + Mirrors ``flag_data/canonical-flags.json`` entry for entry -- and the + self-tests check that it does, value for value and Python type for Python + type. Four properties of that file are load-bearing and hold here too: + + * ``missing-flag`` is absent, which is what the ``FLAG_NOT_FOUND`` scenario + tests. Adding it turns that scenario green for the wrong reason. + * no flag carries a ``context_evaluator``, so every evaluation reports reason + ``STATIC`` -- the TCK tests a provider's mapping of a response, not a + backend's evaluation logic. + * ``boolean-zero-flag``, ``integer-zero-flag`` and ``string-zero-flag`` + resolve to ``False``, ``0`` and ``""``. They are values, not absences, and + the falsy scenarios exist to catch a provider that cannot tell the + difference. Their ``zero``/``non-zero`` variant names are load-bearing + too: the scenarios assert the variant, not only the value. + * ``integral-float-flag`` is the ``float`` ``10.0`` and ``huge-integer-flag`` + is the ``int`` ``9007199254740991``. Writing the first as ``10`` makes the + lossless-coercion scenario pass without coercing; nothing here goes + through a float, so the second cannot be rounded. + """ + return { + "boolean-flag": InMemoryFlag( + default_variant="on", variants={"on": True, "off": False} + ), + "string-flag": InMemoryFlag( + default_variant="greeting", variants={"greeting": "hi", "parting": "bye"} + ), + "integer-flag": InMemoryFlag( + default_variant="ten", variants={"one": 1, "ten": 10} + ), + "float-flag": InMemoryFlag( + default_variant="half", variants={"tenth": 0.1, "half": 0.5} + ), + # 2^31 - 1: the largest value every language's integer accessor can ask for. + "large-integer-flag": InMemoryFlag( + default_variant="max-int32", variants={"one": 1, "max-int32": 2147483647} + ), + # 2^53 - 1: asked for only under @large-integers. A Python int is exact. + "huge-integer-flag": InMemoryFlag( + default_variant="max-safe", + variants={"one": 1, "max-safe": 9007199254740991}, + ), + # A float with no fractional part, for the lossless half of + # @numeric-coercion. The trailing ``.0`` is the whole point. + "integral-float-flag": InMemoryFlag( + default_variant="ten", variants={"tenth": 0.1, "ten": 10.0} + ), + "boolean-zero-flag": InMemoryFlag( + default_variant="zero", variants={"zero": False, "non-zero": True} + ), + "integer-zero-flag": InMemoryFlag( + default_variant="zero", variants={"zero": 0, "non-zero": 1} + ), + "string-zero-flag": InMemoryFlag( + default_variant="zero", variants={"zero": "", "non-zero": "str"} + ), + "object-flag": InMemoryFlag( + default_variant="template", + variants={ + "empty": {}, + "template": { + "showImages": True, + "title": "Check out these pics!", + "imagesPerPage": 100, + }, + }, + ), + # A string flag, evaluated as a boolean by the TYPE_MISMATCH scenario. + "wrong-flag": InMemoryFlag( + default_variant="one", variants={"one": "uno", "two": "dos"} + ), + CHANGING_FLAG_KEY: changing_flag(_CHANGING_BASELINE), + } diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/state.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/state.py new file mode 100644 index 00000000..074005f5 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/state.py @@ -0,0 +1,222 @@ +"""Per-scenario state: what a scenario accumulates, and how it observes events. + +Separate from :mod:`plugin` so the step modules can import these types at the top +level. The step modules are loaded by the plugin as plugins in their own right, +and a step importing from the plugin module that loads it reads like a cycle even +where it is not one. +""" + +from __future__ import annotations + +import queue +import typing +from dataclasses import dataclass, field + +from openfeature.client import OpenFeatureClient +from openfeature.event import EventDetails, ProviderEvent +from openfeature.flag_evaluation import FlagType +from openfeature.provider import FeatureProvider + +from .config import TckConfig + +__all__ = ["EvaluationRecord", "EventRecorder", "LifecycleRecord", "TckState"] + + +@dataclass +class EvaluationRecord: + """The outcome of one flag evaluation, flattened across the five typed calls.""" + + value: typing.Any = None + variant: str | None = None + reason: str | None = None + error_code: str | None = None + error_message: str | None = None + raised: BaseException | None = None + """The exception the call raised, if any. + + In Python an errored evaluation returns the code default in the details + rather than raising, so this stays ``None`` on the error paths the suite + exercises. It is what "no exception should have been thrown" asserts. + """ + + +@dataclass +class LifecycleRecord: + """The outcome of one direct call into the provider's lifecycle. + + The shutdown scenarios call the provider's own ``shutdown`` and + ``initialize`` rather than going through the SDK, because the SDK's + bookkeeping around them is Appendix B's business rather than this suite's. + Each call is recorded the same way an evaluation is -- what it raised, if + anything -- so that "no exception should have been thrown" reads one kind + of record for both, plus how long it took, which is what the prompt-shutdown + scenario bounds. + """ + + operation: str + """``shutdown`` or ``initialize``, for failure messages.""" + + duration: float + """Wall-clock seconds the call took to return, or to be given up on.""" + + raised: BaseException | None = None + """The exception the call raised, if any.""" + + +class EventRecorder: + """Captures the events of one type, in order, so a scenario consumes them one at a time. + + Consuming rather than merely observing is what makes the stale scenario + work: it awaits a ``PROVIDER_READY`` at the start and a second, different + ``PROVIDER_READY`` once the backend is back, and a recorder that only + remembered "ready has fired at some point" would report the second assertion + as satisfied by the first event. + + A queue rather than a list because a provider with a background thread -- + anything with a real backend -- delivers events from that thread while the + scenario waits on the main one. + """ + + def __init__(self, client: OpenFeatureClient, event: ProviderEvent) -> None: + self.event = event + self._client = client + self._events: queue.Queue[EventDetails] = queue.Queue() + self.last: EventDetails | None = None + + # The SDK replays a matching event on registration when the provider is + # already in the corresponding state, so a handler added after the + # provider became ready still observes its PROVIDER_READY. That is what + # lets the feature files register handlers after "Given a stable + # provider" without racing it. + client.add_handler(event, self._on_event) + + def _on_event(self, details: EventDetails) -> None: + self._events.put(details) + + def await_event(self, timeout: float) -> EventDetails: + """Consume the next event of this recorder's type.""" + try: + details = self._events.get(timeout=timeout) + except queue.Empty: + msg = ( + f"timed out after {timeout}s waiting for a {self.event.value} event. " + f"If the provider is simply slower than this to notice, raise " + f"TckConfig.event_timeout rather than treating it as a failure" + ) + raise AssertionError(msg) from None + self.last = details + return details + + def detach(self) -> None: + self._client.remove_handler(self.event, self._on_event) + + +@dataclass +class TckState: + """Everything one scenario accumulates.""" + + config: TckConfig + client: OpenFeatureClient | None = None + provider: FeatureProvider | None = None + """The provider under test, for the steps that call it directly. + + Everything else reaches the provider through :attr:`client`, which is how + an application would. The lifecycle and metadata steps are the exception: + they ask the provider itself, because what they verify is the provider's + own ``shutdown``, ``initialize`` and ``get_metadata`` rather than the SDK's + handling of them. + """ + flag_key: str | None = None + flag_type: FlagType | None = None + default_value: typing.Any = None + last: EvaluationRecord | None = None + lifecycle: list[LifecycleRecord] = field(default_factory=list) + """Every direct lifecycle call this scenario made, in order.""" + remembered: typing.Any = None + has_memory: bool = False + recorders: dict[ProviderEvent, EventRecorder] = field(default_factory=dict) + + def require_client(self) -> OpenFeatureClient: + if self.client is None: + msg = ( + "no provider has been registered in this scenario: a " + '"Given a stable provider" or "Given a unavailable provider" step ' + "must come first" + ) + raise AssertionError(msg) + return self.client + + def require_provider(self) -> FeatureProvider: + if self.provider is None: + msg = ( + "no provider has been registered in this scenario: a " + '"Given a stable provider" or "Given a unavailable provider" step ' + "must come first" + ) + raise AssertionError(msg) + return self.provider + + def require_shutdown(self) -> LifecycleRecord: + """The most recent direct ``shutdown`` call, for the steps that bound it.""" + for record in reversed(self.lifecycle): + if record.operation == "shutdown": + return record + msg = ( + "the provider has not been shut down in this scenario: a " + '"When the provider is shut down" step must come first' + ) + raise AssertionError(msg) + + def raised(self) -> list[tuple[str, BaseException]]: + """Every call into the provider that raised, as (what was called, exception). + + The evaluation and the lifecycle calls are recorded separately, since + they carry different things, but "did anything the scenario asked of + the provider raise" is one question and this is where it is answered. + """ + raised: list[tuple[str, BaseException]] = [ + (record.operation, record.raised) + for record in self.lifecycle + if record.raised is not None + ] + if self.last is not None and self.last.raised is not None: + raised.append(("the evaluation", self.last.raised)) + return raised + + def has_called_provider(self) -> bool: + """Whether the scenario has asked anything of the provider yet.""" + return self.last is not None or bool(self.lifecycle) + + def require_flag(self) -> tuple[str, FlagType, typing.Any]: + if self.flag_key is None or self.flag_type is None: + msg = ( + "no flag has been declared in this scenario: a " + '"Given a -flag with key ... and a default value ..." step ' + "must come first" + ) + raise AssertionError(msg) + return self.flag_key, self.flag_type, self.default_value + + def require_evaluation(self) -> EvaluationRecord: + if self.last is None: + msg = ( + "no flag has been evaluated in this scenario: a " + '"When the flag was evaluated with details" step must come first' + ) + raise AssertionError(msg) + return self.last + + def require_recorder(self, event: ProviderEvent) -> EventRecorder: + recorder = self.recorders.get(event) + if recorder is None: + msg = ( + f"no handler was registered for {event.value} in this scenario: a " + '"Given a event handler" step must come first' + ) + raise AssertionError(msg) + return recorder + + def teardown(self) -> None: + for recorder in self.recorders.values(): + recorder.detach() + self.recorders.clear() diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/__init__.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/__init__.py new file mode 100644 index 00000000..6581ee86 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/__init__.py @@ -0,0 +1,11 @@ +"""The shared step vocabulary. + +Each module here is registered as a pytest plugin by the TCK's own plugin, which +is what makes the steps visible: pytest-bdd's decorators inject a generated +fixture name into the *defining* module's namespace, so a step only reaches +pytest once its module is a registered plugin. + +Deliberately empty of imports. Pulling the submodules in here would import them +before pytest loads them as plugins, and pytest cannot rewrite assertions in a +module that is already imported. +""" diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/event_steps.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/event_steps.py new file mode 100644 index 00000000..6b635699 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/event_steps.py @@ -0,0 +1,165 @@ +"""Steps covering provider events, connection loss and client status.""" + +from __future__ import annotations + +from pytest_bdd import given, parsers, then, when + +from openfeature.event import ProviderEvent +from openfeature.provider import ProviderStatus + +from ..control import ConnectionControl, unsupported_control +from ..state import EventRecorder, TckState + +__all__ = [ + "an_event_handler", + "an_event_was_fired", + "the_client_should_be_in_state", + "the_connection_is_lost", + "the_connection_is_restored", + "the_event_handler_should_have_been_executed", + "the_event_handler_should_have_been_executed_within", + "the_flag_should_be_part_of_the_event_payload", +] + +_EVENT_BY_NAME: dict[str, ProviderEvent] = { + "ready": ProviderEvent.PROVIDER_READY, + "stale": ProviderEvent.PROVIDER_STALE, + "error": ProviderEvent.PROVIDER_ERROR, + "change": ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, +} + +_STATUS_BY_NAME: dict[str, ProviderStatus] = { + "ready": ProviderStatus.READY, + "stale": ProviderStatus.STALE, + "error": ProviderStatus.ERROR, +} + + +def _event(name: str) -> ProviderEvent: + try: + return _EVENT_BY_NAME[name] + except KeyError: + msg = f"unknown event kind {name!r}" + raise AssertionError(msg) from None + + +@given(parsers.re(r"^an? (?Pready|stale|error|change) event handler$")) +def an_event_handler(tck_state: TckState, kind: str) -> None: + """Attach a recorder for one event type. + + Handlers are attached after the provider is registered, which the SDK + handles by replaying a matching event on registration when the provider is + already in the corresponding state. That is why "Given a stable provider" + followed by "And a ready event handler" is not a race. + """ + event = _event(kind) + if event in tck_state.recorders: + return + client = tck_state.require_client() + tck_state.recorders[event] = EventRecorder(client, event) + + +@when(parsers.re(r"^a (?Pready|stale|error|change) event was fired$")) +def an_event_was_fired(tck_state: TckState, kind: str) -> None: + """Consume an event, so a later assertion observes the next one rather than this. + + The stale scenario depends on it: it consumes the initial ``PROVIDER_READY`` + here and then asserts a second, distinct one once the backend is back. + """ + recorder = tck_state.require_recorder(_event(kind)) + recorder.await_event(tck_state.config.event_timeout) + + +@then( + parsers.re( + r"^the (?Pready|stale|error|change) event handler should have been executed$" + ) +) +def the_event_handler_should_have_been_executed(tck_state: TckState, kind: str) -> None: + recorder = tck_state.require_recorder(_event(kind)) + recorder.await_event(tck_state.config.event_timeout) + + +@then( + parsers.re( + r"^the (?Pready|stale|error|change) event handler should have been " + r"executed within (?P\d+)ms$" + ) +) +def the_event_handler_should_have_been_executed_within( + tck_state: TckState, kind: str, millis: str +) -> None: + """Bound the wait explicitly. + + The scenarios using this assert promptness, not merely eventual arrival: a + provider that cannot reach its backend has to report that fact quickly, + because an application blocked on provider registration is down. The bound + therefore overrides ``event_timeout`` rather than being clamped by it. + """ + recorder = tck_state.require_recorder(_event(kind)) + recorder.await_event(int(millis) / 1000.0) + + +@then("the flag should be part of the event payload") +def the_flag_should_be_part_of_the_event_payload(tck_state: TckState) -> None: + """Assert the configuration-change event named the flag that changed. + + Naming the changed flags is what makes the event actionable: a consumer + caching evaluations needs to know what to invalidate, and an event carrying + no keys forces it to invalidate everything. + """ + key, _flag_type, _default = tck_state.require_flag() + recorder = tck_state.require_recorder(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED) + + if recorder.last is None: + msg = ( + "no configuration-change event has been consumed in this scenario: a " + '"the change event handler should have been executed" step must come first' + ) + raise AssertionError(msg) + + changed = recorder.last.flags_changed or [] + if key in changed: + return + + if not changed: + msg = ( + f"the configuration-change event carried no changed flags, expected it to " + f"name {key!r}" + ) + else: + msg = f"the configuration-change event named {changed}, expected it to include {key!r}" + raise AssertionError(msg) + + +def _connection_control(tck_state: TckState, operation: str) -> ConnectionControl: + control = tck_state.config.control + if not isinstance(control, ConnectionControl): + raise unsupported_control(control, operation) + return control + + +@when("the connection is lost") +def the_connection_is_lost(tck_state: TckState) -> None: + _connection_control(tck_state, "disconnect").disconnect() + + +@when("the connection is restored") +def the_connection_is_restored(tck_state: TckState) -> None: + _connection_control(tck_state, "reconnect").reconnect() + + +@then(parsers.re(r"^the client should be in (?Pready|stale|error) state$")) +def the_client_should_be_in_state(tck_state: TckState, name: str) -> None: + """Assert the provider status the client reports. + + Checked after the corresponding event has been consumed, and the SDK writes + provider status before running handlers, so no polling is needed: if the + event arrived, the status is already current. + """ + client = tck_state.require_client() + expected = _STATUS_BY_NAME[name] + actual = client.get_provider_status() + if actual != expected: + msg = f"client reports status {actual}, expected {expected}" + raise AssertionError(msg) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/flag_steps.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/flag_steps.py new file mode 100644 index 00000000..812c5047 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/flag_steps.py @@ -0,0 +1,275 @@ +"""Steps that declare, evaluate and assert flags.""" + +from __future__ import annotations + +import typing +from collections.abc import Callable + +from pytest_bdd import given, parsers, then, when + +from openfeature.flag_evaluation import FlagType + +from ..state import EvaluationRecord, TckState +from ..values import describe, parse_flag_type, parse_value, values_equal + +__all__ = [ + "a_flag_with_key_and_default", + "no_exception_should_have_been_thrown", + "the_error_code_should_be", + "the_error_message_should_be_empty", + "the_flag_was_evaluated_with_details", + "the_flag_was_modified", + "the_reason_should_be", + "the_resolved_object_value_should_contain", + "the_resolved_value_is_remembered", + "the_resolved_value_should_be", + "the_resolved_value_should_have_changed", + "the_variant_should_be", +] + + +@given( + parsers.re( + r'^an? (?P[A-Za-z]+)-flag with key "(?P[^"]*)" ' + r'and a default value "(?P[^"]*)"$' + ) +) +def a_flag_with_key_and_default( + tck_state: TckState, flag_type: str, key: str, default: str +) -> None: + """Declare the flag the scenario is about, and the type it is requested as. + + The two are independent on purpose: most of ``errors.feature`` asks for a + flag as a type it is not. + """ + parsed_type = parse_flag_type(flag_type) + tck_state.flag_key = key + tck_state.flag_type = parsed_type + tck_state.default_value = parse_value(parsed_type, default) + + +@when("the flag was evaluated with details") +def the_flag_was_evaluated_with_details(tck_state: TckState) -> None: + """Resolve the declared flag through the typed client call matching its type.""" + client = tck_state.require_client() + key, flag_type, default = tck_state.require_flag() + + # Annotated explicitly: the five typed getters have different signatures, so + # an unannotated mapping infers a value type mypy will not let us call. + calls: dict[FlagType, Callable[[str, typing.Any], typing.Any]] = { + FlagType.BOOLEAN: client.get_boolean_details, + FlagType.STRING: client.get_string_details, + FlagType.INTEGER: client.get_integer_details, + FlagType.FLOAT: client.get_float_details, + FlagType.OBJECT: client.get_object_details, + } + + record = EvaluationRecord() + try: + details = calls[flag_type](key, default) + except BaseException as exc: # recorded here, asserted on by its own step + record.raised = exc + record.value = default + else: + record.value = details.value + record.variant = details.variant + record.reason = str(details.reason) if details.reason is not None else None + record.error_code = ( + details.error_code.value if details.error_code is not None else None + ) + record.error_message = details.error_message + + tck_state.last = record + + +@then(parsers.re(r'^the resolved details value should be "(?P[^"]*)"$')) +def the_resolved_value_should_be(tck_state: TckState, expected: str) -> None: + _key, flag_type, _default = tck_state.require_flag() + record = tck_state.require_evaluation() + wanted = parse_value(flag_type, expected) + + if not values_equal(wanted, record.value): + detail = ( + f" (the client also reported: {record.error_message})" + if record.error_message + else "" + ) + msg = ( + f"flag {tck_state.flag_key!r} resolved to {describe(record.value)}, " + f"expected {describe(wanted)}{detail}" + ) + raise AssertionError(msg) + + +@then(parsers.re(r'^the variant should be "(?P[^"]*)"$')) +def the_variant_should_be(tck_state: TckState, expected: str) -> None: + record = tck_state.require_evaluation() + if record.variant != expected: + msg = ( + f"variant was {record.variant!r}, expected {expected!r}. A variant that " + f"does not survive the trip from the backend is one of the easiest parts " + f"of the contract to drop" + ) + raise AssertionError(msg) + + +@then(parsers.re(r'^the reason should be "(?P[^"]*)"$')) +def the_reason_should_be(tck_state: TckState, expected: str) -> None: + record = tck_state.require_evaluation() + if record.reason != expected: + msg = f"reason was {record.reason!r}, expected {expected!r}" + raise AssertionError(msg) + + +@then(parsers.re(r'^the error-code should be "(?P[^"]*)"$')) +def the_error_code_should_be(tck_state: TckState, expected: str) -> None: + """Assert the reported error code, where the empty string means none at all. + + The empty case matters as much as the populated ones. A provider that + reports a plausible value with no error code is the failure mode the suite + is most concerned with, because the application has no way to notice. + """ + record = tck_state.require_evaluation() + actual = record.error_code or "" + + if actual == expected: + return + + if expected == "": + msg = f"error-code was {actual!r}, expected none" + elif actual == "": + msg = ( + f"no error-code was reported, expected {expected!r}. Returning a value " + f"without an error code leaves the application unable to tell that " + f"anything went wrong" + ) + else: + msg = f"error-code was {actual!r}, expected {expected!r}" + raise AssertionError(msg) + + +@then("the error message should be empty") +def the_error_message_should_be_empty(tck_state: TckState) -> None: + """Assert no error message was reported (requirement 2.3.2). + + Asserted on the success paths, where a message contradicts the value beside + it: an application reading the message will believe the wrong one of the + two signals. ``None`` and ``""`` are both "none": the SDK's resolution + details default the field to ``None`` and a provider that writes the empty + string has said the same thing. + """ + record = tck_state.require_evaluation() + if record.error_message: + msg = ( + f"an error message was reported alongside a successful evaluation: " + f"{record.error_message!r}. A value and an error message are two " + f"contradictory signals, and the application cannot tell which to believe" + ) + raise AssertionError(msg) + + +@then("no exception should have been thrown") +def no_exception_should_have_been_thrown(tck_state: TckState) -> None: + """Assert that nothing the scenario asked of the provider raised. + + That is the evaluation, if there was one, and every direct lifecycle call: + each records what it raised rather than propagating it, and this is the + one step that reads those records back. + + In Python an errored evaluation returns the code default in the details and + does not raise, so this holds on the error paths too. A provider that raises + instead takes the calling application down with it -- and one that raises + from ``shutdown`` does so from the application's own shutdown, where an + exception is least welcome. Both are what the feature files forbid. + """ + if not tck_state.has_called_provider(): + msg = ( + "nothing has been asked of the provider in this scenario: a " + '"When the flag was evaluated with details" or "When the provider is ' + 'shut down" step must come first' + ) + raise AssertionError(msg) + + raised = tck_state.raised() + if raised: + what, exc = raised[0] + msg = ( + f"{what} raised {exc!r}. A provider must return from an evaluation with a " + f"value and an error code, and from a lifecycle call quietly -- never raise" + ) + raise AssertionError(msg) from exc + + +@then("the resolved object value should contain") +def the_resolved_object_value_should_contain( + tck_state: TckState, datatable: list[list[str]] +) -> None: + """Assert members of a structured value, each with its own expected type.""" + record = tck_state.require_evaluation() + header, *rows = datatable + + if header != ["key", "type", "value"]: + msg = f"expected a data table with columns key, type, value; got {header}" + raise AssertionError(msg) + + if not isinstance(record.value, dict): + msg = ( + f"resolved object value is {describe(record.value)}, which has no members " + f"to check" + ) + raise AssertionError(msg) + + for key, raw_type, raw_value in rows: + wanted = parse_value(parse_flag_type(raw_type), raw_value) + if key not in record.value: + msg = f"resolved object value has no member {key!r}" + raise AssertionError(msg) + actual = record.value[key] + if not values_equal(wanted, actual): + msg = f"object member {key!r} was {describe(actual)}, expected {describe(wanted)}" + raise AssertionError(msg) + + +@when("the resolved value is remembered") +def the_resolved_value_is_remembered(tck_state: TckState) -> None: + """Store the current value so a later step can assert it changed.""" + record = tck_state.require_evaluation() + tck_state.remembered = record.value + tck_state.has_memory = True + + +@then("the resolved details value should have changed") +def the_resolved_value_should_have_changed(tck_state: TckState) -> None: + """Assert that re-evaluation produced a different value. + + This is the half of the configuration-change contract providers actually get + wrong. Emitting ``PROVIDER_CONFIGURATION_CHANGED`` and then continuing to + resolve the old value is worse than emitting nothing, because the + application acted on a signal that was not true. + """ + record = tck_state.require_evaluation() + if not tck_state.has_memory: + msg = ( + "no value was remembered in this scenario: a " + '"the resolved value is remembered" step must come first' + ) + raise AssertionError(msg) + + if values_equal(tck_state.remembered, record.value): + msg = ( + f"the resolved value is still {describe(record.value)} after the " + f"configuration changed. The change was signalled but not applied, so the " + f"event told the application something untrue" + ) + raise AssertionError(msg) + + +@when("the flag was modified") +def the_flag_was_modified(tck_state: TckState) -> None: + """Change flag configuration on the backend.""" + control = tck_state.config.control + try: + control.change_flag() + except Exception as exc: + msg = f"could not change flag configuration on {control.description}: {exc}" + raise AssertionError(msg) from exc diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/provider_steps.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/provider_steps.py new file mode 100644 index 00000000..cf84f8ea --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/steps/provider_steps.py @@ -0,0 +1,251 @@ +"""Steps that put a provider under test, and the ones that talk to it directly.""" + +from __future__ import annotations + +import concurrent.futures +import contextlib +import time +from collections.abc import Callable + +from pytest_bdd import given, parsers, then, when + +from openfeature import api +from openfeature.evaluation_context import EvaluationContext + +from ..state import LifecycleRecord, TckState + +__all__ = [ + "a_stable_provider", + "an_unavailable_provider", + "the_provider_is_initialized_again", + "the_provider_is_shut_down", + "the_provider_metadata_name_should_not_be_empty", + "the_shutdown_should_have_completed_within", +] + + +@given(parsers.re(r"^an? stable provider$")) +def a_stable_provider(tck_state: TckState) -> None: + """Register the provider under test against the running, seeded backend. + + ``api.set_provider_and_wait`` initialises the provider before it returns and + dispatches ``PROVIDER_READY``, so by the time this step returns the provider + is ready and every scenario that follows can assume it. A suite that started + evaluating before that would report races in the TCK as defects in the + provider. + + It has to be the waiting variant. Plain ``api.set_provider`` registers and + initialises on a worker thread, returning long before the provider is up, so + the very first evaluation of every scenario answers ``PROVIDER_NOT_READY`` -- + a TCK defect that reads exactly like a provider that cannot resolve + anything. + """ + config = tck_state.config + provider = config.new_provider() + if provider is None: + msg = "TckConfig.new_provider returned None" + raise AssertionError(msg) + + try: + _call_within( + lambda: api.set_provider_and_wait(provider, config.domain), + config.ready_timeout, + ) + except TimeoutError: + msg = ( + f"the provider did not become ready within {config.ready_timeout}s. The backend " + f"is up and seeded at this point, so either initialisation is genuinely hanging " + f"or TckConfig.ready_timeout is too short" + ) + raise AssertionError(msg) from None + except Exception as exc: + msg = ( + f"registering the provider raised {exc!r}. The backend is up and seeded " + f"at this point, so this is a genuine initialisation failure rather than " + f"the unavailable-backend case" + ) + raise AssertionError(msg) from exc + + tck_state.provider = provider + tck_state.client = api.get_client(config.domain) + + +@given(parsers.re(r"^an? unavailable provider$")) +def an_unavailable_provider(tck_state: TckState) -> None: + """Register a provider pointed at a backend that does not exist. + + Neither a failed initialisation nor a raised exception during registration + is a failure here: what the contract requires is that the provider settles + into an observable error state promptly, which the scenario asserts through + the event and the client status. The SDK's registry already converts a + raising ``initialize`` into ``PROVIDER_ERROR``, so registration itself is + expected to return normally -- but a provider that raises anyway must not + take the scenario down with it, which is why this is caught rather than + propagated. + """ + config = tck_state.config + + if config.new_unavailable_provider is None: + msg = ( + "TckConfig.new_unavailable_provider is None but an @unavailable scenario " + "ran. This is a test-configuration bug rather than a provider defect: the " + "suite declared Capability.UNAVAILABLE_INIT without supplying a provider " + "that cannot reach its backend. Remove that capability, or supply the factory" + ) + raise AssertionError(msg) + + provider = config.new_unavailable_provider() + if provider is None: + msg = "TckConfig.new_unavailable_provider returned None" + raise AssertionError(msg) + + # The waiting variant for the same reason as the stable provider: plain + # set_provider initialises on a worker thread, so registration would return + # before the provider had even tried to reach its backend, and the scenario + # would assert an error state that had not happened yet. + # + # A raising initialize is already converted to PROVIDER_ERROR by the SDK's + # registry, so the suppression is belt and braces: a provider that raises + # anyway must not take the scenario down with it, because the contract is + # about the observable error state rather than about how registration + # returned. + with contextlib.suppress(Exception): + api.set_provider_and_wait(provider, config.domain) + + tck_state.provider = provider + tck_state.client = api.get_client(config.domain) + + +@when("the provider is shut down") +def the_provider_is_shut_down(tck_state: TckState) -> None: + """Call the provider's own ``shutdown``, directly. + + Not through the SDK. The SDK shuts a provider down when it is replaced or + when the API is shut down, but going that way would test the registry's + bookkeeping as much as the provider, and Appendix B already does that. + Calling ``shutdown`` on the instance is also what lets a scenario call it + twice: the registry only ever calls it once per registration. + + The registry is not told. The client still points at the same instance, so + an evaluation after "the provider is initialized again" reaches the very + object that was shut down and brought back, which is what that scenario + asserts. And when the scenario ends the SDK shuts the provider down once + more on its own -- a second call, which requirement 2.5.3 makes harmless. + """ + _record_lifecycle_call(tck_state, "shutdown", tck_state.require_provider().shutdown) + + +@when("the provider is initialized again") +def the_provider_is_initialized_again(tck_state: TckState) -> None: + """Call the provider's own ``initialize`` after it was shut down. + + With an empty context, as the SDK would with none set. Direct for the same + reason as the shutdown step: re-registering through the SDK would create a + new registration around the same instance, and what is under test is that + the instance itself reverts to an initialisable state. + """ + provider = tck_state.require_provider() + _record_lifecycle_call( + tck_state, "initialize", lambda: provider.initialize(EvaluationContext()) + ) + + +@then(parsers.re(r"^the shutdown should have completed within (?P\d+)ms$")) +def the_shutdown_should_have_completed_within(tck_state: TckState, millis: str) -> None: + """Bound the most recent shutdown. + + The scenario using this runs against a backend that will never answer, so + what it asserts is that shutdown returns rather than waiting for a graceful + close that cannot happen. A shutdown that was given up on because it + outlasted ``TckConfig.ready_timeout`` fails here too: its recorded duration + is however long the suite waited before moving on. + """ + record = tck_state.require_shutdown() + bound = int(millis) / 1000.0 + if record.duration > bound: + msg = ( + f"shutdown took {record.duration * 1000:.0f}ms, expected it to complete within " + f"{millis}ms. A shutdown that waits on a backend that is gone hangs the host " + f"application's own shutdown" + ) + raise AssertionError(msg) + + +@then("the provider metadata name should not be empty") +def the_provider_metadata_name_should_not_be_empty(tck_state: TckState) -> None: + """Assert the provider identifies itself (requirement 2.1.1). + + Asked of the provider rather than of ``api.get_provider_metadata``, which + would answer for whatever the registry holds under the domain: the same + object here, but the question is about the provider. + """ + provider = tck_state.require_provider() + try: + metadata = provider.get_metadata() + except Exception as exc: + msg = f"get_metadata raised {exc!r}: the provider cannot say what it is" + raise AssertionError(msg) from exc + + name = getattr(metadata, "name", None) + if not isinstance(name, str) or not name.strip(): + msg = ( + f"the provider metadata name is {name!r}, expected a non-empty string. A " + f"conformance report keyed on the name cannot be attributed without one" + ) + raise AssertionError(msg) + + +def _record_lifecycle_call( + tck_state: TckState, operation: str, call: Callable[[], object] +) -> None: + """Make one direct lifecycle call and record how it went, raising nothing. + + An exception is recorded rather than propagated, for the same reason an + evaluation's is: "no exception should have been thrown" is a step of its + own, and a scenario that wants a raise to fail says so there. Only + ``Exception`` is caught, though. The shutdown scenario that matters most is + the one against a backend that is gone, which is exactly where somebody + might reach for Ctrl-C, and a ``KeyboardInterrupt`` recorded as "shutdown + raised" would carry the run on past the thing they interrupted. + + A call that outlasts ``TckConfig.ready_timeout`` is given up on and recorded + as a ``TimeoutError`` with the time waited, so a hanging shutdown fails its + scenario with a message rather than hanging the session. + """ + started = time.perf_counter() + raised: BaseException | None = None + try: + _call_within(call, tck_state.config.ready_timeout) + except TimeoutError: + raised = TimeoutError( + f"{operation} did not return within {tck_state.config.ready_timeout}s" + ) + except Exception as exc: # recorded here, asserted on by its own step + raised = exc + duration = time.perf_counter() - started + tck_state.lifecycle.append( + LifecycleRecord(operation=operation, duration=duration, raised=raised) + ) + + +def _call_within(call: Callable[[], object], timeout: float) -> None: + """Make a call into the provider, giving up if it has not returned in time. + + Neither ``api.set_provider_and_wait`` nor a provider's own ``shutdown`` has a + timeout of its own, so one that hangs while talking to its backend would hang + the whole session with no useful message. Running it on a worker thread bounds + it. + + The worker is deliberately not cancelled on timeout -- Python cannot interrupt a + thread blocked in a socket call -- so it is left to finish or die with the process. + That is acceptable here because a timeout already means the scenario is failing. + """ + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: + future = pool.submit(call) + try: + future.result(timeout=timeout) + except concurrent.futures.TimeoutError: + raise TimeoutError from None + finally: + # Do not block __exit__ on a worker that is still stuck. + pool.shutdown(wait=False) diff --git a/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/values.py b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/values.py new file mode 100644 index 00000000..4459d466 --- /dev/null +++ b/tools/openfeature-provider-tck/src/openfeature/contrib/tools/provider_tck/values.py @@ -0,0 +1,125 @@ +"""Turning Gherkin strings into typed values, and comparing them with what a provider resolved.""" + +from __future__ import annotations + +import json +import typing + +from openfeature.flag_evaluation import FlagType + +__all__ = ["describe", "parse_flag_type", "parse_value", "values_equal"] + +_BY_NAME: dict[str, FlagType] = { + "boolean": FlagType.BOOLEAN, + "string": FlagType.STRING, + "integer": FlagType.INTEGER, + "float": FlagType.FLOAT, + "object": FlagType.OBJECT, +} + + +def parse_flag_type(raw: str) -> FlagType: + """Resolve the type named in a scenario, case-insensitively.""" + try: + return _BY_NAME[raw.strip().lower()] + except KeyError: + names = ", ".join(sorted(n.capitalize() for n in _BY_NAME)) + msg = f"unknown flag type {raw!r}: expected one of {names}" + raise ValueError(msg) from None + + +def _parse_bool(raw: str) -> bool: + lowered = raw.strip().lower() + if lowered in {"true", "t", "yes", "1"}: + return True + if lowered in {"false", "f", "no", "0"}: + return False + msg = f"{raw!r} is not a boolean" + raise ValueError(msg) + + +def parse_value(flag_type: FlagType, raw: str) -> typing.Any: + """Convert a value written in a scenario into the type the API uses. + + Everything in Gherkin is a string, so this is where ``"0.5"`` becomes a + float and ``"{}"`` becomes an empty object. Parsing per declared type rather + than guessing is what keeps the integer and float scenarios + distinguishable: ``"1"`` is an ``int`` in an Integer scenario and a ``float`` + in a Float one. + """ + if flag_type is FlagType.BOOLEAN: + return _parse_bool(raw) + if flag_type is FlagType.STRING: + return raw + if flag_type is FlagType.INTEGER: + return int(raw) + if flag_type is FlagType.FLOAT: + return float(raw) + if flag_type is FlagType.OBJECT: + # Gherkin escapes quotes in table cells; pytest-bdd keeps the backslash, + # so strip it before handing the text to json. + return json.loads(raw.replace('\\"', '"')) + msg = f"unknown flag type {flag_type!r}" + raise ValueError(msg) + + +def _as_number(value: typing.Any) -> float | None: + """Return a numeric value as a float, or None if it is not numeric. + + Booleans are deliberately excluded. Python makes ``bool`` a subclass of + ``int``, so an unguarded numeric comparison would quietly report ``True`` and + ``1`` as equal -- which is the exact confusion several of these scenarios + exist to detect. + """ + if isinstance(value, bool): + return None + if isinstance(value, (int, float)): + return float(value) + return None + + +def values_equal(expected: typing.Any, actual: typing.Any) -> bool: + """Compare an expected value from a scenario with what a provider resolved. + + Numbers are compared numerically rather than by Python type. A provider that + deserialises its backend's JSON hands back ``float`` for every number, so the + ``100`` inside ``object-flag`` arrives as ``100.0`` from one provider and + ``100`` from another while both are correct. Type distinctness is asserted + where it belongs -- by requesting a flag as a specific type and checking the + error code -- not by accident of how a number was decoded. + """ + # A boolean only ever equals a boolean. Without this, Python's bool-is-an-int + # rule would make True == 1 and quietly satisfy the scenario that exists to + # catch exactly that confusion. + if isinstance(expected, bool) or isinstance(actual, bool): + return ( + isinstance(expected, bool) + and isinstance(actual, bool) + and expected == actual + ) + + expected_number = _as_number(expected) + if expected_number is not None: + actual_number = _as_number(actual) + return actual_number is not None and expected_number == actual_number + + if isinstance(expected, dict) and isinstance(actual, dict): + if set(expected) != set(actual): + return False + return all(values_equal(v, actual[k]) for k, v in expected.items()) + + if isinstance(expected, list) and isinstance(actual, list): + return len(expected) == len(actual) and all( + values_equal(e, a) for e, a in zip(expected, actual, strict=True) + ) + + return bool(expected == actual) + + +def describe(value: typing.Any) -> str: + """Render a value for a failure message, including its type. + + "expected 100 but got 100" is the single most confusing failure a + cross-language conformance suite can produce. + """ + return f"{value!r} ({type(value).__name__})" diff --git a/tools/openfeature-provider-tck/tests/conftest.py b/tools/openfeature-provider-tck/tests/conftest.py new file mode 100644 index 00000000..a5e6726f --- /dev/null +++ b/tools/openfeature-provider-tck/tests/conftest.py @@ -0,0 +1,39 @@ +"""Known deviations of the Python SDK, recorded rather than hidden. + +A conformance suite that quietly goes green on scenarios it did not run is worse +than no suite at all -- and the same is true of one that quietly goes green on a +scenario it *did* run and fail. So the one scenario the Python SDK cannot +currently satisfy is marked ``xfail(strict=True)`` here, which: + +* keeps it visible in the report, as XFAIL with the reason attached; +* fails the suite if it ever *passes*, so the marker is removed the moment the + SDK is fixed rather than lingering as a lie. + +This lives in the TCK's own self-test rather than in the shared package. It is a +fact about the SDK under test, not part of the conformance definition, and +Appendix F deliberately leaves a general "known deviations" concept as an open +question (spec#417, Q4). If that concept lands, this moves into it. +""" + +from __future__ import annotations + +import pytest + +# The Scenario Outline row that asks for boolean-flag as an Integer. +_BOOL_AS_INT = ( + "test_requesting_the_wrong_type_returns_the_code_default[boolean-flag-Integer-1]" +) + +_REASON = ( + "python-sdk: a boolean satisfies an Integer request. The client type-checks with " + "isinstance(value, int) and bool is a subclass of int in Python, so boolean-flag " + "requested as an Integer returns True with reason STATIC and no error code, where " + "the specification requires the code default and TYPE_MISMATCH. " + "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/tools/openfeature-provider-tck/tests/test_controllable_conformance.py b/tools/openfeature-provider-tck/tests/test_controllable_conformance.py new file mode 100644 index 00000000..1d36dec2 --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_controllable_conformance.py @@ -0,0 +1,62 @@ +"""Run the conformance suite against the TCK's own updatable in-memory provider. + +This is the suite that exercises the configuration-change path, and it exists +because the SDK's in-memory provider cannot: it has no way to update a flag set, +so ``test_in_memory_conformance`` necessarily skips those scenarios. Without +this suite the change-event step definitions would ship with no coverage at all, +and a break in them would first surface in a containerised provider suite where +it looks like a provider defect. + +It is also the reference for what an in-process control path looks like when the +provider does support updates, which is what a file-based or +environment-variable provider should be able to do. +""" + +from __future__ import annotations + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.tools.provider_tck import ( + Capability, + InProcessControl, + TckConfig, + features_path, +) + + +@pytest.fixture(scope="session") +def tck_config() -> TckConfig: + """Declare the provider under test and what it can do. + + ``STALE`` and ``UNAVAILABLE_INIT`` stay undeclared: there is still no + connection to lose, and ``InProcessControl`` does not implement + ``ConnectionControl``. ``CONFIGURATION_CHANGE`` is what this suite adds over + the plain in-memory one, and it is the whole point of it. + + ``LIFECYCLE`` stays undeclared for the same reason as in + ``test_in_memory_conformance``: there is no backend to reach during + initialisation, so the readiness scenario would pass here without testing + anything. It did exactly that while the feature was gated on ``@events``. + + ``NUMERIC_COERCION`` stays undeclared for the reason given there too. + ``ControllableInMemoryProvider`` changes nothing about resolution, so it + inherits the SDK provider's refusal to coerce: ``10.0`` requested as an + integer is a ``TYPE_MISMATCH`` rather than ``10``. ``LARGE_INTEGERS`` is + declared, since a Python ``int`` is exact at 2^53 - 1. + """ + control = InProcessControl() + return TckConfig( + name="controllable-in-memory", + control=control, + new_provider=control.new_provider, + capabilities={ + Capability.EVENTS, + Capability.CONFIGURATION_CHANGE, + Capability.OBJECT, + Capability.LARGE_INTEGERS, + }, + ) + + +scenarios(features_path()) diff --git a/tools/openfeature-provider-tck/tests/test_declaration.py b/tools/openfeature-provider-tck/tests/test_declaration.py new file mode 100644 index 00000000..d6cc3dda --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_declaration.py @@ -0,0 +1,312 @@ +"""What an adoption may declare about its provider, and what it may not. + +A ``TckConfig`` is two things at once. It is the configuration a run needs, and +it is a *declaration*: the set of claims an adopter makes about the provider, +which is what turns a skipped scenario from a hole in the run into a recorded +answer. Everything checked here belongs to the second role, so none of it is +observable in the pass/fail of a suite -- which is exactly why it is pinned by +tests of its own rather than by the conformance suites. + +The declaration vocabulary is public because something outside this package has +to read it back. A reporter deciding whether a skip was legitimate needs the tag +lookup and the reserved set; a comparison page needs to tell a declined +capability from an impossible one. None of those consumers is here, and the API +is complete for them anyway -- a follow-up that adds one should widen nothing. + +The one property that makes "reserved" mean anything is checked against the +packaged assets rather than asserted: a reserved tag is reserved because no +canonical scenario carries it, and that stops being true the moment the spec +adds one. +""" + +from __future__ import annotations + +import re +import typing +from pathlib import Path + +import pytest + +from openfeature.contrib.tools.provider_tck import ( + DECLARABLE_CAPABILITIES, + RESERVED_CAPABILITIES, + BackendControl, + Capability, + InProcessControl, + KnownDeviation, + TckConfig, + features_path, +) +from openfeature.contrib.tools.provider_tck.capability import ( + capability_for_marker, + capability_for_tag, +) + + +class _StubControl: + """A control that says nothing it is not obliged to say. + + Which includes ``control_api``: the property is documented as optional, and + a control leaving it out has to remain a ``BackendControl``. + """ + + def prepare_scenario(self) -> None: ... + + def change_flag(self) -> None: ... + + @property + def description(self) -> str: + return "a stub" + + +def _config(**overrides: typing.Any) -> TckConfig: + """A configuration that is valid but declares nothing in particular.""" + settings: dict[str, typing.Any] = { + "name": "stub", + "control": _StubControl(), + "new_provider": lambda: None, + "capabilities": frozenset(), + } + settings.update(overrides) + return TckConfig(**settings) + + +def _canonical_tags() -> set[str]: + """Every tag the packaged feature files carry, at any level. + + Read off tag lines only. A tag is the whole of the line it appears on in + Gherkin, which is what tells one apart from the same word written in a + comment -- ``events.feature`` mentions ``@caching`` in prose, saying where + those scenarios will go once they exist. + """ + tags: set[str] = set() + for feature in sorted(Path(features_path()).glob("*.feature")): + for line in feature.read_text(encoding="utf-8").splitlines(): + stripped = line.strip() + if stripped.startswith("@"): + tags.update(re.findall(r"@[\w-]+", stripped)) + return tags + + +# -- the vocabulary ---------------------------------------------------------- + + +def test_every_capability_is_either_declarable_or_reserved() -> None: + """Two sets, one enum, and no member in both or neither. + + ``DECLARABLE_CAPABILITIES`` is derived from ``RESERVED_CAPABILITIES`` rather + than listed beside it, so this is really a check that the derivation is the + one the documentation promises. + """ + assert frozenset(Capability) == DECLARABLE_CAPABILITIES | RESERVED_CAPABILITIES + assert not DECLARABLE_CAPABILITIES & RESERVED_CAPABILITIES + assert RESERVED_CAPABILITIES, "the whole rule is vacuous if nothing is reserved" + + for capability in Capability: + assert capability.reserved is (capability in RESERVED_CAPABILITIES) + + +def test_a_reserved_capability_is_one_no_canonical_scenario_carries() -> None: + """The fact the rule rests on, read off the assets rather than asserted. + + "Reserved" claims that nothing carries the tag, so declaring it cannot be + verified. If the specification adds a scenario for ``@targeting``, that + stops being true and this fails -- which is the moment the capability should + become declarable, and the moment somebody has to notice. + """ + carried = _canonical_tags() + for capability in RESERVED_CAPABILITIES: + assert capability.tag not in carried, ( + f"{capability.tag} is no longer reserved: the canonical assets now " + f"carry it, so it can be verified and should be declarable" + ) + for capability in DECLARABLE_CAPABILITIES: + assert capability.tag in carried, ( + f"{capability.tag} is declarable but no canonical scenario carries " + f"it, so declaring it would be a claim nothing examines" + ) + + +def test_a_tag_maps_onto_the_capability_it_gates() -> None: + """The lookup a reporter outside this package needs, in the tag form. + + The tag form rather than the marker form, because that is the form a + scenario's tags are recorded in: deciding whether a skip was legitimate + means reading them back as the feature files spell them. Nothing in this + package calls it -- it is exported for the consumer that does. + """ + for capability in Capability: + assert capability_for_tag(capability.tag) is capability + assert capability_for_marker(capability.value) is capability + + # An organisational tag gates nothing, and must not be mistaken for a + # capability: the feature files carry them freely. + assert capability_for_tag("@smoke") is None + assert capability_for_tag("events") is None, "the at-sign is part of the tag" + + +# -- declaring a capability set ---------------------------------------------- + + +def test_the_default_is_every_declarable_capability() -> None: + """And so cannot pick up a reserved tag on the way past. + + "Declare everything, then narrow it" is the advice, which makes the default + the one place a reserved tag would otherwise get declared by accident. One + implementation's published report asserts ``@targeting`` and ``@caching`` + for precisely that reason. + """ + # Not routed through ``_config``, which narrows the set: the field default is + # the whole point of this one. It needs an unavailable-provider factory + # because ``@unavailable`` is declarable, so the default declares it. + settings: dict[str, typing.Any] = { + "name": "stub", + "control": _StubControl(), + "new_provider": lambda: None, + "new_unavailable_provider": lambda: None, + } + declared = TckConfig(**settings).capabilities + assert declared == DECLARABLE_CAPABILITIES + for capability in RESERVED_CAPABILITIES: + assert capability not in declared + + +def test_a_capability_set_is_normalised_however_it_was_written() -> None: + """A list, a set or a generator all arrive as the same frozenset.""" + expected = frozenset({Capability.EVENTS, Capability.OBJECT}) + written = [ + [Capability.EVENTS, Capability.OBJECT, Capability.EVENTS], + {Capability.EVENTS, Capability.OBJECT}, + (c for c in (Capability.EVENTS, Capability.OBJECT)), + ] + for capabilities in written: + assert _config(capabilities=capabilities).capabilities == expected + + +def test_something_that_is_not_a_capability_is_refused() -> None: + with pytest.raises(ValueError, match="unknown capabilities"): + _config(capabilities={"events"}) + + +def test_a_reserved_capability_cannot_be_declared() -> None: + """A tag no scenario carries is a claim nothing can check, so it is refused. + + At construction rather than at the point something reads the declaration: + the adopter wrote it down and meant something by it, so a configuration + silently different from the one they wrote is worse than one that will not + build -- and construction is where their own code is still on the stack to + say which line to fix. + """ + for reserved in RESERVED_CAPABILITIES: + with pytest.raises(ValueError, match=f"reserved capabilities {reserved.tag}"): + _config(capabilities={Capability.EVENTS, reserved}) + with pytest.raises(ValueError, match=f"reserved capabilities {reserved.tag}"): + _config(not_applicable={reserved: "no scenario asks"}) + + +def test_the_refusal_says_what_may_be_declared_instead() -> None: + """A message that names the rule and not just the violation.""" + with pytest.raises(ValueError) as raised: + _config(capabilities={Capability.TARGETING}) + message = str(raised.value) + assert "DECLARABLE_CAPABILITIES" in message + for capability in DECLARABLE_CAPABILITIES: + assert capability.tag in message + + +# -- declaring an impossibility ---------------------------------------------- + + +def test_not_applicable_is_normalised_and_keeps_its_reasons() -> None: + """Written as a dict literal keyed by ``Capability``; read as a mapping.""" + config = _config(not_applicable={Capability.NUMERIC_COERCION: "no integer type"}) + assert dict(config.not_applicable) == { + Capability.NUMERIC_COERCION: "no integer type" + } + + +def test_a_capability_cannot_be_both_declared_and_impossible() -> None: + """The two are different claims, and a declaration asserting both says neither.""" + with pytest.raises(ValueError, match="both claim @events"): + _config( + capabilities={Capability.EVENTS}, + not_applicable={Capability.EVENTS: "a reason"}, + ) + + +def test_a_not_applicable_capability_must_say_why() -> None: + """A reason is required: "impossible for this provider" is useless without one.""" + for empty in ("", " ", "\n"): + with pytest.raises(ValueError, match="no reason for @stale"): + _config(not_applicable={Capability.STALE: empty}) + + +def test_something_that_is_not_a_capability_cannot_be_not_applicable() -> None: + with pytest.raises(ValueError, match="in not_applicable"): + _config(not_applicable={"stale": "a reason"}) + + +# -- acknowledging a gap ----------------------------------------------------- + + +def test_a_known_deviation_carries_its_capability_only_when_it_has_one() -> None: + """The common case has none: a mandatory scenario belongs to no capability. + + Omitted rather than null, because the field is the answer to "which + capability does this concern" and there is not always one. + """ + issue = "https://github.com/open-feature/python-sdk/issues/619" + mandatory = KnownDeviation(issue=issue, summary="a boolean satisfies an Integer") + assert mandatory.as_json() == {"issue": issue, "summary": mandatory.summary} + + attributed = KnownDeviation( + issue=issue, + summary="a lossy float satisfies an Integer", + capability=Capability.NUMERIC_COERCION, + ) + assert attributed.as_json() == { + "issue": issue, + "summary": attributed.summary, + "capability": Capability.NUMERIC_COERCION.tag, + } + + +def test_known_deviations_are_normalised_and_change_nothing_about_the_run() -> None: + """Declared as any sequence; read as a tuple. + + And that is all they do. A deviation is an acknowledgement, not a licence: + nothing here makes a scenario pass, skip, or be collected differently, which + is why a suite declaring one still fails on it. + """ + deviation = KnownDeviation(issue="https://example.invalid/1", summary="a gap") + config = _config(known_deviations=[deviation]) + assert config.known_deviations == (deviation,) + assert _config().known_deviations == () + assert _config(known_deviations=[deviation]).capabilities == _config().capabilities + + +# -- saying how the backend is driven ---------------------------------------- + + +def test_a_control_need_not_say_how_it_drives_the_backend() -> None: + """``control_api`` is documented as optional, and means it. + + Making it a member of ``BackendControl`` would make every existing control + incomplete for the sake of one string, and there is nothing the suite can do + with the answer: it cannot tell from the outside whether a control spoke + HTTP or reached into the process. + """ + quiet = _StubControl() + assert isinstance(quiet, BackendControl) + assert not hasattr(quiet, "control_api") + + +def test_in_process_control_says_it_is_in_process() -> None: + """The narrow allowance, and the control that exists to take it. + + A provider that does have a backend and reports this is claiming something + it should not, which is only detectable if the honest case says so plainly. + """ + control = InProcessControl() + assert isinstance(control, BackendControl) + assert control.control_api == "in-process" diff --git a/tools/openfeature-provider-tck/tests/test_extensions.py b/tools/openfeature-provider-tck/tests/test_extensions.py new file mode 100644 index 00000000..8d0b407e --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_extensions.py @@ -0,0 +1,488 @@ +"""What an adopter's own scenarios may and may not do. + +An adopter with provider-specific behaviour -- flagd's ``fractional`` targeting, a +proprietary rollout rule -- has to be able to pin it in the same run as the +contract it sits on top of, or they end up maintaining a second harness beside +the one the TCK gives them. So the properties checked here are the ones that make +that safe rather than merely possible: + +* an extension scenario runs **inside** the canonical suite -- same session, same + provider registration, same backend control -- with a step definition the + adopter wrote in their own ``conftest.py`` and nothing else registered; +* an adoption without extensions runs exactly what it ran before, scenario for + scenario and outcome for outcome; +* a feature file's identity comes from where the file is, so an extension cannot + take a canonical scenario's. + +The last is not hypothetical. Java's suite discovered a same-named feature file +in a second classpath root silently *replacing* the canonical one, and the run +went green having asked the adopter's questions instead of the specification's. +The Python route to the same place is narrower and just as quiet: pytest-bdd +names a feature file by its parent directory joined to its own name, so a file at +``tck-extensions/features/errors.feature`` arrives under the uri the canonical +``errors.feature`` already occupies. + +The first three are properties of how a whole session runs rather than of what a +function returns, so they are checked against real pytest sessions in +subprocesses, read back through pytest's own JUnit XML. Reading them back from a +conformance report would be circular here and impossible anyway: this package +writes none. +""" + +from __future__ import annotations + +import dataclasses +import subprocess +import sys +import xml.etree.ElementTree as ElementTree +from pathlib import Path + +import pytest + +from openfeature.contrib.tools.provider_tck import ( + EXTENSIONS_DIRECTORY, + feature_paths, + features_path, +) +from openfeature.contrib.tools.provider_tck.extensions import ( + CANONICAL_DIRECTORY, + EXTENSIONS_URI_PREFIX, + collision_problem, + extension_root, + is_canonical, + is_canonical_uri, + reserved_prefix_problem, + uri_collisions, + uri_for, +) + +CANONICAL_FEATURE = "errors.feature" +"""The canonical file the collision cases are written against, chosen because it +is the one whose scenarios an extension could most plausibly want to restate.""" + +VENDOR_SCENARIO = "A vendor rule resolves through the suite's own provider" + + +# -- the generated adoption -------------------------------------------------- + +_SUITE_MODULE = '''\ +"""A one-fixture adoption, generated so extensions can be checked end to end.""" + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.tools.provider_tck import ( + Capability, + InProcessControl, + TckConfig, + feature_paths, + features_path, +) + + +@pytest.fixture(scope="session") +def tck_config(): + control = InProcessControl() + return TckConfig( + name="{name}", + control=control, + new_provider=control.new_provider, + capabilities={{Capability.EVENTS, Capability.OBJECT}}, + ) + + +scenarios({call}) +''' + +_EXTENSION_CALL = "*feature_paths()" +_CANONICAL_CALL = "features_path()" + +# The step the adopter writes, in the adopter's own conftest.py and nowhere else. +# It asks the TCK's own per-scenario state what happened, which is what makes +# "the same session and the same provider" checkable rather than asserted: a +# second harness would have a second provider, or none, and none of the canonical +# steps would have run. +_CONFTEST_MODULE = """\ +import pytest +from pytest_bdd import then + +from openfeature.contrib.tools.provider_tck import TckState + +DEVIATION = "[boolean-flag-Integer-1]" + + +@then("the vendor rule ran against the provider the suite registered") +def vendor_rule_ran(tck_state: TckState) -> None: + assert tck_state.client is not None, "no provider was registered" + assert tck_state.last is not None, "the canonical steps did not run here" + assert tck_state.last.value == "hi", tck_state.last + + +def pytest_collection_modifyitems(items): + for item in items: + if item.name.endswith(DEVIATION): + item.add_marker(pytest.mark.xfail(reason="python-sdk#619")) +""" + +# Deliberately reuses the canonical step vocabulary and adds exactly one step of +# its own, which is the shape an adopter's feature file actually takes. +_VENDOR_FEATURE = """\ +Feature: Vendor rules + + Background: + Given a stable provider + + Scenario: A vendor rule resolves through the suite's own provider + Given a String-flag with key "string-flag" and a default value "bye" + When the flag was evaluated with details + Then the resolved details value should be "hi" + And the vendor rule ran against the provider the suite registered +""" + + +# -- reading a run back ------------------------------------------------------ + + +@dataclasses.dataclass(frozen=True) +class Run: + """One subprocess run of a generated adoption.""" + + directory: Path + result: subprocess.CompletedProcess[str] + outcomes: dict[str, str] + + +def _outcomes(report: Path) -> dict[str, str]: + """Read a JUnit XML report into ``node id -> passed | failed | skipped``. + + pytest's own results format, because the question is what the session did + and pytest is the thing that knows. It records one ``testcase`` per test with + the file it came from, which is what lets two suites in one directory be + told apart. + """ + outcomes: dict[str, str] = {} + # Not untrusted input: the file is one pytest wrote seconds ago, in a + # temporary directory this test made, from a subprocess this test started. + root = ElementTree.parse(report).getroot() # noqa: S314 + for case in root.iter("testcase"): + statuses = { + "failure": "failed", + "error": "failed", + "skipped": "skipped", + } + status = "passed" + for tag, named in statuses.items(): + if case.find(tag) is not None: + status = named + break + node = f"{case.get('classname', '')}::{case.get('name', '')}" + outcomes[node] = status + return outcomes + + +def _run( + tmp_path_factory: pytest.TempPathFactory, + modules: dict[str, str], + features: dict[str, str] | None = None, +) -> Run: + """Write an adoption, run it in a subprocess, and read the results back. + + Both mappings are keyed by a path relative to the adoption directory, so a + case can put a module or a feature file wherever the property under test + needs it. + """ + directory = tmp_path_factory.mktemp("adoption") + for relative, body in {**modules, **(features or {})}.items(): + path = directory / Path(relative) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + + report = directory / "results.xml" + result = subprocess.run( # noqa: S603 + [ + sys.executable, + "-m", + "pytest", + "-q", + "-p", + "no:cacheprovider", + f"--junitxml={report}", + str(directory), + ], + capture_output=True, + text=True, + check=False, + ) + outcomes = _outcomes(report) if report.exists() else {} + return Run(directory=directory, result=result, outcomes=outcomes) + + +def _suite(name: str, call: str = _EXTENSION_CALL) -> str: + return _SUITE_MODULE.format(name=name, call=call) + + +@pytest.fixture(scope="module") +def adoption(tmp_path_factory: pytest.TempPathFactory) -> Run: + """One session running two adoptions of the same provider. + + ``before`` is the call an adopter wrote before any of this, + ``scenarios(features_path())``, which sees no extension however many are + lying beside it. ``after`` is ``scenarios(*feature_paths())`` with an + ordinary extension beside it. Having both in one run is what lets "an + extension adds and does not alter" be a comparison rather than a number + written down here. + + One session rather than two, because a subprocess pytest run is by far the + most expensive thing in this file and the two suites are independent: each + resolves its own ``TckConfig`` under its own OpenFeature domain. + """ + return _run( + tmp_path_factory, + { + "test_before.py": _suite("before", _CANONICAL_CALL), + "test_after.py": _suite("after", _EXTENSION_CALL), + "conftest.py": _CONFTEST_MODULE, + }, + {f"{EXTENSIONS_DIRECTORY}/vendor.feature": _VENDOR_FEATURE}, + ) + + +def _of(run: Run, module: str) -> dict[str, str]: + """The outcomes belonging to one of the generated suites, keyed by test name. + + JUnit XML names the module in dotted form, so the two generated suites in one + directory are told apart by the last segment of it. + """ + return { + node.split("::", 1)[1]: status + for node, status in run.outcomes.items() + if node.split("::", 1)[0].rsplit(".", 1)[-1] == module + } + + +def _contributed(run: Run) -> dict[str, str]: + """What the extension added: the tests ``after`` ran and ``before`` did not. + + Identified by difference rather than by name. pytest-bdd derives a test + function's name from the scenario name by a munging of its own -- an + apostrophe disappears where a space becomes an underscore -- and reproducing + that here would pin pytest-bdd's spelling rather than this package's + behaviour. + """ + before = _of(run, "test_before") + return { + name: status + for name, status in _of(run, "test_after").items() + if name not in before + } + + +# -- an extension runs inside the canonical suite ---------------------------- + + +def test_an_extension_scenario_runs_in_the_canonical_suite(adoption: Run) -> None: + """One suite, one session, both sets of scenarios in it. + + The extension scenario passes only if it reached the provider the suite + registered and the canonical steps ran in it, because that is what its own + step asserts -- so this is not a presentational fact about collection. It is + the same suite, which is the same provider registration and the same backend + control. + """ + assert adoption.result.returncode == 0, adoption.result.stdout + + contributed = _contributed(adoption) + assert len(contributed) == 1, adoption.outcomes + [(name, status)] = contributed.items() + assert "vendor_rule" in name, name + assert status == "passed" + + canonical = _of(adoption, "test_after") + assert len(canonical) > 1, "the canonical scenarios must have run too" + assert "passed" in canonical.values() + + +def test_the_extension_step_came_from_the_adopters_conftest(adoption: Run) -> None: + """Nothing was registered, imported or configured to make that step resolve. + + pytest collects ``conftest.py`` on its own and pytest-bdd resolves steps + through the fixture system, so a step defined beside the test module is in + scope for the scenarios generated into it. An unresolved step is a *failure* + rather than an omission, which is why asserting that the scenario passed is + enough to pin this. + """ + conftest = (adoption.directory / "conftest.py").read_text(encoding="utf-8") + assert "the vendor rule ran against the provider the suite registered" in conftest + assert set(_contributed(adoption).values()) == {"passed"} + + +# -- and changes nothing for an adopter who has none ------------------------- + + +def test_an_extension_adds_scenarios_and_alters_none(adoption: Run) -> None: + """``before`` is what an adopter ran before extensions existed. + + Every test it generated, ``after`` generated too, with the same outcome, and + the only difference between the two is the one scenario the extension added. + An adopter who has no extensions is the same comparison with the right-hand + side empty, which is what ``feature_paths()`` returning the canonical path + alone makes true by construction rather than by luck. + """ + before = _of(adoption, "test_before") + after = _of(adoption, "test_after") + assert before, "the baseline suite generated nothing" + + assert len(_contributed(adoption)) == 1 + assert {name: after[name] for name in before} == before + + +def test_features_path_sees_no_extension_however_many_are_beside_it( + adoption: Run, +) -> None: + """The older call still means exactly what it meant: the canonical set. + + Both generated suites sit in the same directory as the ``tck-extensions`` + directory, so the one that asks for ``features_path()`` is asking with an + extension in arm's reach and must still not see it. + """ + assert (adoption.directory / EXTENSIONS_DIRECTORY).is_dir() + assert set(_of(adoption, "test_before")) < set(_of(adoption, "test_after")) + + +# -- finding an adopter's feature files -------------------------------------- + + +def test_feature_paths_is_the_canonical_set_when_there_is_no_extension_directory() -> ( + None +): + """This test module has no ``tck-extensions`` beside it, and gets one path.""" + assert not (Path(__file__).parent / EXTENSIONS_DIRECTORY).exists() + assert feature_paths() == (features_path(),) + + +def test_an_extension_directory_counts_only_when_it_is_a_directory( + tmp_path: Path, +) -> None: + """``None`` rather than a path that contributes nothing. + + So that an adopter without extensions hands ``scenarios()`` exactly what + they handed it before -- and so that a *file* of that name, which pytest-bdd + would choke on, is not offered as a feature directory. + """ + assert extension_root(tmp_path) is None + + (tmp_path / EXTENSIONS_DIRECTORY).write_text("not a directory", encoding="utf-8") + assert extension_root(tmp_path) is None + + (tmp_path / EXTENSIONS_DIRECTORY).unlink() + (tmp_path / EXTENSIONS_DIRECTORY).mkdir() + assert extension_root(tmp_path) == tmp_path / EXTENSIONS_DIRECTORY + + +def test_the_canonical_features_are_found_inside_the_distribution() -> None: + """No submodule and no directory layout of the adopter's own.""" + packaged = Path(features_path()) + assert (packaged / CANONICAL_FEATURE).is_file() + assert is_canonical(packaged / CANONICAL_FEATURE) + assert not is_canonical(Path(__file__)) + + +# -- deriving the uri -------------------------------------------------------- + + +def test_the_canonical_assets_keep_the_reserved_prefix() -> None: + canonical = Path(features_path()) / CANONICAL_FEATURE + assert uri_for(canonical) == f"{CANONICAL_DIRECTORY}/{CANONICAL_FEATURE}" + assert is_canonical_uri(f"{CANONICAL_DIRECTORY}/{CANONICAL_FEATURE}") + assert reserved_prefix_problem(f"features/{CANONICAL_FEATURE}", canonical) is None + + +def test_an_extension_keeps_its_layout_below_the_extensions_prefix( + tmp_path: Path, +) -> None: + """Whatever the adopter's own directory layout under the root looks like. + + Including one that reproduces the canonical name, which is the collision the + derivation exists for: pytest-bdd would have called the second of these + ``features/errors.feature``. + """ + root = tmp_path / EXTENSIONS_DIRECTORY + assert uri_for(root / "vendor.feature") == "extensions/vendor.feature" + assert ( + uri_for(root / CANONICAL_DIRECTORY / CANONICAL_FEATURE) + == f"{EXTENSIONS_URI_PREFIX}/{CANONICAL_DIRECTORY}/{CANONICAL_FEATURE}" + ) + assert ( + uri_for(root / "a" / "b" / "vendor.feature") == "extensions/a/b/vendor.feature" + ) + assert not is_canonical_uri(f"extensions/features/{CANONICAL_FEATURE}") + + +def test_a_derived_uri_is_slash_separated_on_every_platform(tmp_path: Path) -> None: + """A uri identifies a feature file, so it cannot depend on where it ran. + + The paths these are built from are ``pathlib`` paths, which are + backslash-separated on Windows. A run there has to be comparable with one on + Linux, and it is not if the same file is identified two ways. + """ + nested = tmp_path / EXTENSIONS_DIRECTORY / "a" / "b" / "vendor.feature" + derived = uri_for(nested) + assert derived == "extensions/a/b/vendor.feature" + assert derived is not None and "\\" not in derived + + +def test_a_file_that_is_neither_is_left_to_pytest_bdd(tmp_path: Path) -> None: + """``None`` rather than a guess: the caller falls back to what the runner said.""" + assert uri_for(tmp_path / "loose.feature") is None + + +def test_a_local_file_under_the_reserved_prefix_is_a_problem(tmp_path: Path) -> None: + """The one route to a canonical-looking uri the convention cannot close. + + An adopter may still hand ``scenarios()`` a directory of their own named + ``features``, and its files are then named exactly as canonical ones would + be. Reported rather than raised: the scenarios are the adopter's to run, and + it is publishing them as the specification's that has to be refused. + """ + local = tmp_path / CANONICAL_DIRECTORY / "local.feature" + problem = reserved_prefix_problem(f"{CANONICAL_DIRECTORY}/local.feature", local) + assert problem is not None + assert EXTENSIONS_DIRECTORY in problem, "the message must say the fix" + assert str(local) in problem + + +def test_two_files_that_would_share_one_uri_are_reported(tmp_path: Path) -> None: + """Deriving the uri from the location narrows the collision; it does not end it. + + A ``tck-extensions`` directory nested inside another one reaches the same uri + as its namesake at the root, and so would two test modules sharing one + ``tck_config``. + """ + root = tmp_path / EXTENSIONS_DIRECTORY + nested = root / "nested" / EXTENSIONS_DIRECTORY / "vendor.feature" + collisions = uri_collisions( + [ + ("extensions/vendor.feature", root / "vendor.feature"), + ("extensions/vendor.feature", nested), + ] + ) + assert set(collisions) == {"extensions/vendor.feature"} + + problem = collision_problem( + "extensions/vendor.feature", collisions["extensions/vendor.feature"] + ) + assert "2 different feature files" in problem + assert EXTENSIONS_DIRECTORY in problem + + +def test_distinct_extension_paths_do_not_collide(tmp_path: Path) -> None: + """The layouts that are fine, including the one that only looks like a clash.""" + root = tmp_path / EXTENSIONS_DIRECTORY + assert not uri_collisions( + [ + ("extensions/vendor.feature", root / "vendor.feature"), + ("extensions/a/vendor.feature", root / "a" / "vendor.feature"), + # One file reached by two routes is one file, not a collision. + ("extensions/vendor.feature", root / "a" / ".." / "vendor.feature"), + ] + ) diff --git a/tools/openfeature-provider-tck/tests/test_in_memory_conformance.py b/tools/openfeature-provider-tck/tests/test_in_memory_conformance.py new file mode 100644 index 00000000..18fef5c0 --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_in_memory_conformance.py @@ -0,0 +1,122 @@ +"""Run the conformance suite against the SDK's own in-memory provider. + +This is the TCK's self-test, and it earns its keep twice over. + +It is the **reference adoption** for a provider with no backend. Everything a +file-based or environment-variable provider has to write is here: one fixture +and one call. + +It is also the **Docker-free canary**. Needing no container and no network, it +runs in a fraction of a second, which makes it the fast check that catches a +broken step definition, a mis-wired capability gate or a regression in the +shared harness long before a containerised suite would. + +What it does not do is license providers that have a backend to test themselves +this way -- see ``BackendControl`` for why. +""" + +from __future__ import annotations + +import pytest +from pytest_bdd import scenarios + +from openfeature.contrib.tools.provider_tck import ( + Capability, + TckConfig, + canonical_flag_set, + features_path, +) +from openfeature.provider import FeatureProvider +from openfeature.provider.in_memory_provider import InMemoryProvider + + +class PlainMemoryControl: + """Backend control for the SDK's stock in-memory provider. + + ``prepare_scenario`` is a no-op because the provider is rebuilt from the + canonical flag set for every scenario, so each one already starts from an + untouched baseline. + + ``change_flag`` cannot be implemented at all, and the error says why. + Appendix A of the specification requires an SDK's in-memory provider to + "support a means of updating the flag set, resulting in the emission of + PROVIDER_CONFIGURATION_CHANGED events"; the Python SDK's copies its mapping + in the constructor and exposes no way to change it. The suite below + therefore leaves ``CONFIGURATION_CHANGE`` undeclared and the scenario is + reported as skipped with its reason, which is the honest outcome. Reaching + this error would mean the capability had been declared anyway. + """ + + @property + def description(self) -> str: + return "the Python SDK's InMemoryProvider, rebuilt per scenario" + + def prepare_scenario(self) -> None: + return None + + def change_flag(self) -> None: + msg = ( + "openfeature.provider.in_memory_provider.InMemoryProvider cannot change its " + "flag set: it copies the mapping in its constructor and exposes no update " + "method, so a configuration change can be neither applied nor signalled. " + "Appendix A of the specification requires it. See " + "ControllableInMemoryProvider for what the SDK's provider is missing" + ) + raise NotImplementedError(msg) + + +def _new_provider() -> FeatureProvider: + return InMemoryProvider(canonical_flag_set()) + + +@pytest.fixture(scope="session") +def tck_config() -> TckConfig: + """Declare the provider under test and what it can do. + + Each omission is a fact about the provider rather than a convenience: + + * ``CONFIGURATION_CHANGE`` -- omitted because the SDK's in-memory provider + cannot update its flag set. That is a finding, not a configuration choice; + see ``PlainMemoryControl``. + * ``STALE`` and ``UNAVAILABLE_INIT`` -- omitted because there is no + connection to lose. ``PlainMemoryControl`` does not implement + ``ConnectionControl`` for the same reason, and the two omissions keep each + other honest: the scenarios are skipped before any step can reach an + operation the control cannot perform. + * ``TARGETING`` and ``CACHING`` -- omitted because no scenario carries their + tags yet, so leaving them out skips nothing. + * ``LIFECYCLE`` -- omitted because there is no backend to reach. The + capability asserts that initialisation actually contacts a backend and + that the outcome is observable; this provider's ``initialize`` is a no-op + and the SDK dispatches ``PROVIDER_READY`` around it regardless, so the + readiness scenario would pass here without testing anything. It passed + vacuously while the feature was gated on ``EVENTS``, which is precisely + the failure mode the split of ``@lifecycle`` from ``@events`` exists to + end. A skip with a reason is the honest outcome. + * ``NUMERIC_COERCION`` -- omitted because the SDK's in-memory provider does + not coerce. It hands each variant back untouched, and the client's type + check is ``isinstance``-based, so ``integral-float-flag`` (``10.0``) + requested as an integer is a ``TYPE_MISMATCH`` rather than ``10``, and + ``integer-flag`` (``10``) requested as a float is one rather than + ``10.0``. The lossy scenario passes for the wrong reason -- every float + is rejected -- which is exactly what the two lossless scenarios exist to + catch, and declaring the tag would have them catch it here. The + capability is optional, so this is a choice the provider is entitled to + rather than a deviation. + + ``LARGE_INTEGERS`` is declared: a Python ``int`` is unbounded and nothing + in this provider routes a value through a float. + """ + return TckConfig( + name="in-memory", + control=PlainMemoryControl(), + new_provider=_new_provider, + capabilities={ + Capability.EVENTS, + Capability.OBJECT, + Capability.LARGE_INTEGERS, + }, + ) + + +scenarios(features_path()) diff --git a/tools/openfeature-provider-tck/tests/test_in_process_control.py b/tools/openfeature-provider-tck/tests/test_in_process_control.py new file mode 100644 index 00000000..ea19864e --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_in_process_control.py @@ -0,0 +1,192 @@ +"""Things the Gherkin cannot assert about itself. + +Each of these is a way the in-process control path could look correct while +quietly making the conformance suites meaningless. +""" + +from __future__ import annotations + +import json +import typing + +import pytest + +from openfeature.contrib.tools.provider_tck import ( + CHANGING_FLAG_KEY, + ConnectionControl, + ControllableInMemoryProvider, + InProcessControl, + canonical_flag_set, + canonical_flags_json, +) +from openfeature.event import ProviderEvent + + +def _resolve_changing(provider: ControllableInMemoryProvider) -> str: + return provider.resolve_string_details(CHANGING_FLAG_KEY, "unset").value + + +def test_change_flag_actually_changes_the_resolved_value() -> None: + """The assumption every configuration-change scenario rests on. + + If ``change_flag`` emitted an event without altering what the provider + resolves, the scenario would still pass its event assertion and the suite + would be certifying a signal with nothing behind it. + """ + control = InProcessControl() + provider = control.new_provider() + assert isinstance(provider, ControllableInMemoryProvider) + + before = _resolve_changing(provider) + control.change_flag() + after = _resolve_changing(provider) + + assert before != after, "change_flag did not change the resolved value" + + +def test_change_flag_emits_a_configuration_change_event_naming_the_flag() -> None: + """The event the scenarios await is the provider's own, and it names the flag.""" + control = InProcessControl() + provider = control.new_provider() + + seen: list[tuple[ProviderEvent, list[str] | None]] = [] + + def record(_provider: object, event: ProviderEvent, details: object) -> None: + seen.append((event, getattr(details, "flags_changed", None))) + + # attach() is how the SDK registry wires a provider's emitter; doing it by + # hand keeps this a unit test of the provider rather than of the registry. + provider.attach(record) + control.change_flag() + + assert seen, "no event was emitted" + event, flags_changed = seen[-1] + assert event is ProviderEvent.PROVIDER_CONFIGURATION_CHANGED + assert flags_changed == [CHANGING_FLAG_KEY] + + +def test_change_does_not_leak_into_the_next_scenario() -> None: + """Scenario isolation. + + A leak here would make the suite order-dependent: a scenario running after + the configuration-change one would start with ``changing-flag`` already + flipped, and the failure would look like a provider defect. + """ + control = InProcessControl() + + first = control.new_provider() + assert isinstance(first, ControllableInMemoryProvider) + baseline = _resolve_changing(first) + + control.change_flag() + assert _resolve_changing(first) != baseline, ( + "precondition: change_flag had no effect" + ) + + control.prepare_scenario() + + second = control.new_provider() + assert isinstance(second, ControllableInMemoryProvider) + assert _resolve_changing(second) == baseline, ( + "the next scenario did not start from the baseline" + ) + + +def test_change_flag_without_a_provider_fails_clearly() -> None: + """In-process the flag store and the provider are the same object, so there is + nothing to change before one exists. Saying so beats an AttributeError.""" + control = InProcessControl() + with pytest.raises(RuntimeError, match="must create one"): + control.change_flag() + + +def test_in_process_control_does_not_pretend_to_have_a_connection() -> None: + """The load-bearing one. + + A no-op ``disconnect`` would report the ``@stale`` scenarios as passed + against a provider that cannot go stale -- precisely the silent-green + failure a conformance suite must never have. ``InProcessControl`` therefore + does not implement ``ConnectionControl`` at all, and the TCK turns that into + a skip with a reason. + """ + assert not isinstance(InProcessControl(), ConnectionControl), ( + "InProcessControl implements ConnectionControl: an in-memory provider has no " + "connection to lose, and a no-op implementation would make the @stale " + "scenarios pass without testing anything" + ) + + +def test_canonical_flag_set_omits_missing_flag() -> None: + """The property the FLAG_NOT_FOUND scenario depends on. + + Seeding ``missing-flag`` would turn that scenario green for the wrong + reason, and nothing else in the suite would notice. + """ + assert "missing-flag" not in canonical_flag_set() + + +def _same_value_and_type(expected: typing.Any, actual: typing.Any) -> bool: + """Equal, and of the same Python type, member by member. + + ``==`` alone is what a seeding step that "cleans up" gets past: ``10 == 10.0`` + and ``0 == False`` in Python, so the integral float and the falsy values + would compare equal to exactly the mistranslations they exist to catch. + """ + if type(expected) is not type(actual): + return False + if isinstance(expected, dict): + return set(expected) == set(actual) and all( + _same_value_and_type(v, actual[k]) for k, v in expected.items() + ) + if isinstance(expected, list): + return len(expected) == len(actual) and all( + _same_value_and_type(e, a) for e, a in zip(expected, actual, strict=True) + ) + return bool(expected == actual) + + +def test_canonical_flag_set_mirrors_the_canonical_json_type_for_type() -> None: + """The in-memory flag set is transcribed, so this is what stops it drifting. + + Key for key, default variant for default variant, and every variant's value + with its Python type: ``json.loads`` keeps ``10.0`` a ``float`` and ``0`` + an ``int``, and the transcription has to as well. The four load-bearing + properties the flag file documents -- no ``missing-flag``, no targeting, + falsy values kept, ``10.0`` a float and 2^53 - 1 an integer -- all follow + from being an exact mirror of it. + """ + canonical = json.loads(canonical_flags_json())["flags"] + transcribed = canonical_flag_set() + + assert set(transcribed) == set(canonical) + for key, definition in canonical.items(): + flag = transcribed[key] + assert flag.default_variant == definition["defaultVariant"], key + assert flag.context_evaluator is None, f"{key} has targeting" + assert set(flag.variants) == set(definition["variants"]), key + for variant, value in definition["variants"].items(): + assert _same_value_and_type(value, flag.variants[variant]), ( + f"{key}/{variant}: canonical {value!r} ({type(value).__name__}), " + f"transcribed {flag.variants[variant]!r} " + f"({type(flag.variants[variant]).__name__})" + ) + + +def test_update_flags_names_the_union_of_old_and_new_keys() -> None: + """Appendix A asks for the union, not just the new keys. + + A consumer caching evaluations needs to know everything that might have + changed, and a key that disappeared has changed as much as one that arrived. + """ + provider = ControllableInMemoryProvider(canonical_flag_set()) + + seen: list[list[str] | None] = [] + provider.attach(lambda _p, _e, details: seen.append(details.flags_changed)) + + provider.update_flags({}) + + assert seen, "no event was emitted" + assert seen[-1] is not None + assert set(seen[-1]) == set(canonical_flag_set()), ( + "the event did not name every flag that disappeared" + ) diff --git a/tools/openfeature-provider-tck/tests/test_lifecycle_steps.py b/tools/openfeature-provider-tck/tests/test_lifecycle_steps.py new file mode 100644 index 00000000..9f202439 --- /dev/null +++ b/tools/openfeature-provider-tck/tests/test_lifecycle_steps.py @@ -0,0 +1,278 @@ +"""The steps that talk to the provider directly, pinned outside the Gherkin. + +The shutdown scenarios live in ``lifecycle.feature``, which is gated on +``@lifecycle``, and neither in-memory self-test declares that -- there is no +backend to reach, so the readiness scenario would pass without testing anything. +That leaves the shutdown, re-initialise and shutdown-bound steps with no +canonical scenario running them here, and a step that first runs in a +containerised adopter's suite fails there looking like a provider defect. + +So they are driven directly, with a provider that records what was called of it +and can be told to misbehave. What is pinned is the contract the feature file +relies on: that the calls reach the provider's *own* methods rather than the +SDK's, that a raise is recorded and surfaces through the one "no exception" +step rather than through a second mechanism, that the client still reaches the +instance after it was brought back, and that the scenario's teardown copes with +a provider that was shut down underneath it. +""" + +from __future__ import annotations + +import typing +from collections.abc import Iterator + +import pytest + +from openfeature import api +from openfeature.contrib.tools.provider_tck import ( + Capability, + TckConfig, + TckState, + canonical_flag_set, +) +from openfeature.contrib.tools.provider_tck.steps.flag_steps import ( + a_flag_with_key_and_default, + no_exception_should_have_been_thrown, + the_flag_was_evaluated_with_details, + the_resolved_value_should_be, +) +from openfeature.contrib.tools.provider_tck.steps.provider_steps import ( + a_stable_provider, + the_provider_is_initialized_again, + the_provider_is_shut_down, + the_provider_metadata_name_should_not_be_empty, + the_shutdown_should_have_completed_within, +) +from openfeature.evaluation_context import EvaluationContext +from openfeature.provider import Metadata +from openfeature.provider.in_memory_provider import InMemoryProvider + + +class RecordingProvider(InMemoryProvider): + """The SDK's in-memory provider, remembering its lifecycle calls. + + ``fail_shutdown`` and ``fail_initialize`` make the corresponding call raise, + which is how the recording half of the steps is checked; ``metadata_name`` + is what the metadata step is checked against. + """ + + def __init__(self) -> None: + super().__init__(canonical_flag_set()) + self.calls: list[str] = [] + self.fail_shutdown = False + self.fail_initialize = False + self.metadata_name: typing.Any = "recording" + + def initialize(self, evaluation_context: EvaluationContext) -> None: + self.calls.append("initialize") + if self.fail_initialize: + msg = "initialize refused" + raise RuntimeError(msg) + + def shutdown(self) -> None: + self.calls.append("shutdown") + if self.fail_shutdown: + msg = "already closed" + raise RuntimeError(msg) + + def get_metadata(self) -> Metadata: + return Metadata(name=self.metadata_name) + + +class _NoControl: + @property + def description(self) -> str: + return "nothing" + + def prepare_scenario(self) -> None: ... + + def change_flag(self) -> None: ... + + +@pytest.fixture +def provider() -> RecordingProvider: + return RecordingProvider() + + +@pytest.fixture +def state(provider: RecordingProvider) -> Iterator[TckState]: + """A scenario's state, with the recording provider registered by the real step. + + Through ``a_stable_provider`` rather than by hand, so what is tested is the + hand-off the feature files rely on: the step that registers the provider is + the one that makes it available to the steps that call it directly. + """ + config = TckConfig( + name="lifecycle-steps", + control=_NoControl(), + new_provider=lambda: provider, + capabilities={Capability.EVENTS}, + ) + state = TckState(config=config) + a_stable_provider(state) + yield state + state.teardown() + api.shutdown() + api.clear_providers() + + +# -- the calls reach the provider itself ------------------------------------- + + +def test_shutdown_calls_the_providers_own_shutdown_each_time( + state: TckState, provider: RecordingProvider +) -> None: + """Twice asked, twice called -- which the SDK would never do on its own. + + The registry shuts a provider down once per registration. The double-close + scenario needs two calls on one instance, and gets them only because the + step bypasses the registry. + """ + before = list(provider.calls) + the_provider_is_shut_down(state) + the_provider_is_shut_down(state) + assert provider.calls[len(before) :] == ["shutdown", "shutdown"] + assert [record.operation for record in state.lifecycle] == ["shutdown", "shutdown"] + no_exception_should_have_been_thrown(state) + + +def test_initialize_again_reaches_the_same_instance_the_client_uses( + state: TckState, provider: RecordingProvider +) -> None: + """The scenario's whole point: after the round trip, the client serves flags + from the very object that was shut down and brought back. + """ + the_provider_is_shut_down(state) + the_provider_is_initialized_again(state) + assert provider.calls[-2:] == ["shutdown", "initialize"] + + a_flag_with_key_and_default(state, "Boolean", "boolean-flag", "false") + the_flag_was_evaluated_with_details(state) + the_resolved_value_should_be(state, "true") + no_exception_should_have_been_thrown(state) + assert state.client is not None + assert state.client.get_provider_status().value == "READY" + + +def test_initialize_again_passes_an_empty_context( + state: TckState, provider: RecordingProvider +) -> None: + seen: list[EvaluationContext] = [] + original = provider.initialize + + def spy(evaluation_context: EvaluationContext) -> None: + seen.append(evaluation_context) + original(evaluation_context) + + provider.initialize = spy # type: ignore[method-assign] + the_provider_is_initialized_again(state) + assert len(seen) == 1 + assert seen[0].attributes == {} + assert seen[0].targeting_key is None + + +# -- a raise is recorded, and surfaces through the one step ------------------ + + +def test_a_raising_shutdown_fails_the_no_exception_step( + state: TckState, provider: RecordingProvider +) -> None: + """Recorded, not propagated: the step returns and the assertion is elsewhere.""" + provider.fail_shutdown = True + the_provider_is_shut_down(state) + + assert state.lifecycle[-1].raised is not None + with pytest.raises(AssertionError, match="shutdown raised RuntimeError"): + no_exception_should_have_been_thrown(state) + + +def test_a_raising_initialize_fails_the_no_exception_step( + state: TckState, provider: RecordingProvider +) -> None: + provider.fail_initialize = True + the_provider_is_shut_down(state) + the_provider_is_initialized_again(state) + + with pytest.raises(AssertionError, match="initialize raised RuntimeError"): + no_exception_should_have_been_thrown(state) + + +def test_a_lifecycle_raise_is_reported_even_after_a_clean_evaluation( + state: TckState, provider: RecordingProvider +) -> None: + """One mechanism for both kinds of call. + + The re-initialise scenario ends with an evaluation and then the no-exception + step. A raise from the shutdown before it must not be hidden behind the + evaluation that went fine. + """ + provider.fail_shutdown = True + the_provider_is_shut_down(state) + provider.fail_initialize = False + the_provider_is_initialized_again(state) + a_flag_with_key_and_default(state, "Boolean", "boolean-flag", "false") + the_flag_was_evaluated_with_details(state) + + assert state.last is not None and state.last.raised is None + with pytest.raises(AssertionError, match="shutdown raised"): + no_exception_should_have_been_thrown(state) + + +def test_the_no_exception_step_needs_something_to_have_been_called( + state: TckState, +) -> None: + """Before anything was asked of the provider the step has nothing to assert, + and says so rather than passing on an empty record.""" + with pytest.raises(AssertionError, match="nothing has been asked of the provider"): + no_exception_should_have_been_thrown(state) + + +# -- the shutdown bound ------------------------------------------------------ + + +def test_the_shutdown_bound_reads_the_most_recent_shutdown(state: TckState) -> None: + the_provider_is_shut_down(state) + the_shutdown_should_have_completed_within(state, "10000") + + state.lifecycle[-1].duration = 11.0 + with pytest.raises(AssertionError, match="shutdown took 11000ms"): + the_shutdown_should_have_completed_within(state, "10000") + + +def test_the_shutdown_bound_needs_a_shutdown(state: TckState) -> None: + with pytest.raises(AssertionError, match="has not been shut down"): + the_shutdown_should_have_completed_within(state, "10000") + + +# -- metadata ---------------------------------------------------------------- + + +def test_the_metadata_step_accepts_a_name_and_refuses_an_empty_one( + state: TckState, provider: RecordingProvider +) -> None: + the_provider_metadata_name_should_not_be_empty(state) + + for empty in ("", " ", None): + provider.metadata_name = empty + with pytest.raises(AssertionError, match="expected a non-empty string"): + the_provider_metadata_name_should_not_be_empty(state) + + +# -- the scenario after this one --------------------------------------------- + + +def test_a_shut_down_provider_does_not_break_the_next_registration( + state: TckState, provider: RecordingProvider +) -> None: + """What the fixture teardown and the next "Given a stable provider" do. + + Both shut the provider down again through the SDK. Requirement 2.5.3 makes + the second call harmless, and the suite relies on that: a provider that was + shut down directly is still the registered one when the scenario ends. + """ + the_provider_is_shut_down(state) + + replacement = RecordingProvider() + api.set_provider(replacement, state.config.domain) + assert state.client is not None + assert state.client.get_boolean_details("boolean-flag", False).value is True diff --git a/uv.lock b/uv.lock index 3168e251..804640a9 100644 --- a/uv.lock +++ b/uv.lock @@ -17,6 +17,7 @@ members = [ "openfeature-provider-flagd", "openfeature-provider-flipt", "openfeature-provider-ofrep", + "openfeature-provider-tck", "openfeature-provider-unleash", "openfeature-python-contrib", ] @@ -1868,7 +1869,7 @@ dev = [ [[package]] name = "openfeature-provider-flagd" -version = "0.5.1" +version = "0.5.2" source = { editable = "providers/openfeature-provider-flagd" } dependencies = [ { name = "cachebox" }, @@ -1989,6 +1990,37 @@ dev = [ { name = "types-requests", specifier = ">=2.32.0,<3.0.0" }, ] +[[package]] +name = "openfeature-provider-tck" +version = "0.1.0" +source = { editable = "tools/openfeature-provider-tck" } +dependencies = [ + { name = "openfeature-sdk" }, + { name = "pytest" }, + { name = "pytest-bdd" }, +] + +[package.dev-dependencies] +dev = [ + { name = "coverage", extra = ["toml"] }, + { name = "mypy" }, + { name = "poethepoet" }, +] + +[package.metadata] +requires-dist = [ + { name = "openfeature-sdk", specifier = ">=0.10.0" }, + { name = "pytest", specifier = ">=8.4.0" }, + { name = "pytest-bdd", specifier = ">=8.1.0,<9.0.0" }, +] + +[package.metadata.requires-dev] +dev = [ + { name = "coverage", extras = ["toml"], specifier = ">=7.10.0,<8.0.0" }, + { name = "mypy", specifier = ">=1.18.0,<2.0.0" }, + { name = "poethepoet", specifier = ">=0.37.0" }, +] + [[package]] name = "openfeature-provider-unleash" version = "0.1.2" @@ -2042,6 +2074,7 @@ dependencies = [ { name = "openfeature-provider-flagd" }, { name = "openfeature-provider-flipt" }, { name = "openfeature-provider-ofrep" }, + { name = "openfeature-provider-tck" }, { name = "openfeature-provider-unleash" }, ] @@ -2063,6 +2096,7 @@ requires-dist = [ { name = "openfeature-provider-flagd", editable = "providers/openfeature-provider-flagd" }, { name = "openfeature-provider-flipt", editable = "providers/openfeature-provider-flipt" }, { name = "openfeature-provider-ofrep", editable = "providers/openfeature-provider-ofrep" }, + { name = "openfeature-provider-tck", editable = "tools/openfeature-provider-tck" }, { name = "openfeature-provider-unleash", editable = "providers/openfeature-provider-unleash" }, ] @@ -2075,11 +2109,11 @@ dev = [ [[package]] name = "openfeature-sdk" -version = "0.8.4" +version = "0.10.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/3b/08/f6698d0614b8703170117b786bd77b7b0a04f3ee00f19fbe9b360d2dee69/openfeature_sdk-0.8.4.tar.gz", hash = "sha256:66abf71f928ec8c0db1111072bb0ef2635dfbd09510f77f4b548e5d0ea0e6c1a", size = 29676, upload-time = "2025-12-09T07:31:13.137Z" } +sdist = { url = "https://files.pythonhosted.org/packages/24/30/cfc684b7d8314398d476ae8ed515c10db99c4d7f950989db464b4ded12ce/openfeature_sdk-0.10.0.tar.gz", hash = "sha256:938c2540bdea4da3b01ef507517ee636f223a35abaaca845c5587e594151b052", size = 33516, upload-time = "2026-06-01T19:45:35.136Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/9c/80/f6532778188c573cc83790b11abccde717d4c1442514e722d6bb6140e55c/openfeature_sdk-0.8.4-py3-none-any.whl", hash = "sha256:805ba090669798fc343ca9fdcbc56ff0f4b57bf6757533f0854d2021192e620a", size = 35986, upload-time = "2025-12-09T07:31:12.092Z" }, + { url = "https://files.pythonhosted.org/packages/da/44/8a4f5225e930ff0d999fd43f5d743a4babeb6c7e76dddc00f0e118878ef3/openfeature_sdk-0.10.0-py3-none-any.whl", hash = "sha256:75497ea75d73f684eef509a25f79ad6386368862e050af80ab70a44ae49b33e4", size = 38941, upload-time = "2026-06-01T19:45:33.011Z" }, ] [[package]]