test(flagd): run the provider conformance suite against both resolvers - #411
Draft
aepfli wants to merge 24 commits into
Draft
test(flagd): run the provider conformance suite against both resolvers#411aepfli wants to merge 24 commits into
aepfli wants to merge 24 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
September 11, 2026 07:53
2924abd to
64845cd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/provider-tck #411 +/- ##
==================================================
Coverage 89.02% 89.02%
==================================================
Files 44 44
Lines 2160 2160
==================================================
Hits 1923 1923
Misses 237 237 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
4 times, most recently
from
September 11, 2026 14:37
a9da51f to
7bba47e
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
13 times, most recently
from
September 13, 2026 14:06
d625676 to
5163c26
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
3 times, most recently
from
September 13, 2026 17:36
4bb5b58 to
4e61eea
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
2 times, most recently
from
September 13, 2026 21:33
0936e22 to
751664e
Compare
Adopts the OpenFeature provider conformance suite in the flagd provider, for both resolvers, as two separate suites. They are separate because they are separately conformant. flagd resolves flags two quite different ways -- RPC evaluates remotely over gRPC, in-process syncs the ruleset and evaluates locally -- and any difference between the two results is a difference an application would see when it switches resolver, which is exactly the class of thing the suite exists to surface. flagd-testbed is not modified and the existing e2e suites are untouched. The TCK drives the testbed's launchpad through the standardised control API, which the launchpad already implements, and reuses the container lifecycle already in tests/e2e. The stack is started once per session and never restarted; scenario isolation comes from the control API instead, because container orchestrators assign host ports dynamically and cannot reliably preserve them across a restart, and a restarted backend on a new port looks like a flaky provider rather than a broken test. One stack and one HttpControl serve both suites. One flagd process serves both resolver ports, so there is nothing a second stack would isolate -- and the control has to be shared, because it tracks whether a disconnect has left the backend down so the next scenario starts it rather than merely resetting flag state. Two instances would each hold half of that knowledge. Every capability is declared on the strength of a line of provider code rather than of a green run, and each declaration carries its file and line. Both resolvers get the same set: EVENTS, STALE, CONFIGURATION_CHANGE, OBJECT, UNAVAILABLE_INIT and STRICT_NUMERIC_TYPING. TARGETING and CACHING are withheld from both, because no scenario carries either tag and a capability nothing exercises would be a claim with no evidence behind it. Worth recording that STALE is declared for RPC. The Go provider's RPC resolver never emits PROVIDER_STALE -- it sends ProviderError directly on connection loss (go-sdk-contrib#939) -- and the Go adoption withholds the capability for that reason. Python has no such asymmetry: both resolvers emit PROVIDER_STALE from the same channel-connectivity callback shape on TRANSIENT_FAILURE, and only escalate to PROVIDER_ERROR once the retry grace period expires. That is the behaviour the specification describes, and it is the reason the grace period is set well above the length of the scenario's outage: too short a value turns a scenario about staleness into one about failure. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The flagd dev group now depends on openfeature-provider-tck as a workspace source, so the lock has to carry it or uv sync --frozen fails for every package in the workspace. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The capability was renamed on the base branch, so these declarations named a symbol that no longer exists. Both resolvers keep declaring it: this provider satisfies the rule in both paths, which is worth noting because the other two languages' flagd providers do not. The rationale comments already recorded why, and they are now the evidence that the defect is narrower than it was described as being. RPC asks flagd for an Int and flagd answers INVALID_ARGUMENT for a float-valued flag, which the provider maps to TypeMismatchError. In-process is local and strict: an integer request admits only int, and the float mapping is deliberately the wider (int, float) because widening towards float loses nothing -- which is the lossless-coercion rule arrived at independently, before it had a name here. So flagd's float narrowing is a property of the Go and Java provider layers rather than of flagd itself. The Go adoption's comment currently says the opposite, calling it "a flagd-wide issue", and needs correcting. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Follow the conformance assets at spec@15fe8611, which the base now pins: @numeric-coercion requires lossless coercion in both directions, and the new @large-integers tag asks for 2^53 - 1. Neither resolver satisfies all three @numeric-coercion scenarios, so neither declares it: - RPC passes back whatever flagd answers, and flagd v0.16.0 -- the image flagd-testbed v3.8.0 builds on -- resolves an integer request as a float64 and casts it with a bare int64(val), so float-flag's 0.5 is narrowed to 0 with no error code. The earlier claim in this file that flagd answers INVALID_ARGUMENT was not borne out by the server source. open-feature/flagd#1996 implements the lossless rule. - In-process admits only int for an integer request (flagd_core.py:25), so integral-float-flag's 10.0 is a TYPE_MISMATCH where 10 is required. The other direction is widened (flagd_core.py:113-114), which is why the lossy scenario alone was passing before. Both declare @large-integers: RPC carries the value as an int64 that the provider hands over as a Python int, and in-process parses the ruleset with json.loads, so nothing narrows 9007199254740991. Record in the conftest that flagd-testbed v3.8.0 does not seed the six flags the assets added, so the untagged falsy and precision scenarios, and the @large-integers one, fail FLAG_NOT_FOUND until the testbed is updated. Left as failures rather than xfails: the backend is behind, not the provider. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The suite's note on what flagd-testbed does not serve was written against spec@15fe8611 and listed six missing flags. Three of them were never missing. spec@ba002ce8 renames them to boolean-zero-flag, integer-zero-flag and string-zero-flag, which is what flagd-testbed's own flags/zero-flags.json has called them all along, with the same zero/non-zero variants the scenarios assert. Verified against flagd-testbed v3.8.0: all three resolve with variant zero and reason STATIC on both resolvers. What remains genuinely unseeded is large-integer-flag, huge-integer-flag and integral-float-flag, so the note now names those three and lists only the two scenarios that still fail. The reasoning underneath is unchanged: the failures stay failures rather than xfails, because they say something true about the backend rather than about the provider. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
spec@fc99d5ac gates "A provider that was shut down can be initialized again" behind a new @reinitialization tag. Neither resolver declares it, and the reasoning is recorded next to each declaration because the arithmetic that suggested declaring it was wrong in an instructive way. The premise was that the scenario had been untagged at the previous pin, so it ran, and since it was not among the two failures it must have passed. It did not run. lifecycle.feature carries @lifecycle at the feature level, so every scenario in it inherits that tag, and the gate skips a scenario when any capability gating it is undeclared. Neither resolver declares LIFECYCLE, so the scenario is one of the six @lifecycle skips each resolver already reported -- absence from the failure list was absence from the run. Measured rather than reasoned about, by declaring LIFECYCLE and REINITIALIZATION together locally and letting the scenario run: in-process passes -- reuse genuinely works rpc fails -- boolean-flag resolves to the code default because grpc.py:420 raises "Cannot invoke RPC on closed channel!"; shutdown() closes the channel and the second initialize() does not rebuild it So the two resolvers genuinely differ, and RPC exhibits precisely the shape the specification's note on this capability describes: the provider evaluates against a closed connection rather than failing outright. Withheld for both regardless. For RPC because reuse does not work; for in-process because declaring REINITIALIZATION without LIFECYCLE leaves the scenario skipped on @lifecycle and the claim unexamined, which is the vacuous declaration this suite keeps the reserved tags out for. Whether to declare LIFECYCLE is a separate question and is not settled here. No KnownDeviation entry either way: Requirement 2.5.2 says a provider SHOULD revert to its uninitialized state and that "some providers MAY allow reinitialization", so declining reuse is a choice the specification offers rather than a requirement to deviate from. Tallies are unchanged from the previous pin, which is the check that no scenario silently changed status: rpc 29 passed / 2 failed / 9 skipped, in-process 28 / 3 / 9, nine skips per resolver being six @lifecycle and three @numeric-coercion. The failures remain large-integer-flag and huge-integer-flag, absent from flagd-testbed, plus in-process's boolean-flag-as-Float, which is #417. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Both resolvers, both declared, suite run, scenarios seen to pass. The numbers: 79 passed, 18 skipped, 7 failed across the two suites, which is 104 = 2 x 52 canonical scenario instances. @targeting is the interesting one, and it costs the testbed nothing. targeting-key-flag is the flag flagd-testbed's own targeting.feature already uses -- same key, same hit/miss variants, same uuid -- seeded from flags/testing-flags.json by the launchpad's default configuration. All three scenarios pass on both resolvers: RPC because grpc.py:492 puts the targeting key into the request context, in-process because targeting.py:40-41 puts it into the JSON-logic context. @Variants passes seven of its eight rows on both resolvers. The eighth asks for large-integer-flag's max-int32 and fails with FLAG_NOT_FOUND, which is the testbed gap already recorded in conftest.py rather than a new one: flagd-testbed v3.8.0 seeds neither large-integer-flag nor huge-integer-flag, and the same absence already fails the two precision scenarios. Withholding the capability over it would say flagd does not name variants, which the other seven rows show is false, and would attribute a missing flag to a capability the provider has. It is not a KnownDeviation either: a deviation is for a behaviour the provider is required to have and does not. Skips are unchanged in kind: six @lifecycle and three @numeric-coercion per resolver, each with its reason. The header claiming these declarations rest "on the strength of a line of provider code, not on the strength of a green run" is gone. Appendix F states the opposite rule as of 26362f85, and this file already contradicted itself -- its @reinitialization note says what was measured, and measuring is what showed the two resolvers genuinely differ. The code references stay, as places a reader can check a claim rather than as the evidence for it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
New at spec@009afe06. Declared for RPC and for in-process, on a run: 7 failed, 87 passed, 18 skipped, twice in a row, which is the same seven failures as before the pin moved and eight more passes -- the four rows per resolver. The four disabled-* flags need no testbed change, and for a sturdier reason than a coincidence of names: they are flagd-testbed's own, from flags/disabled-flags.json, which the launchpad merges into flags/allFlags.json along with every other non-selector- file in rawflags and serves under the default configuration. RPC passing is worth stating plainly, because the appendix's own rationale for gating the tag predicts it would not: it reasons that a provider "whose backend decides, such as one speaking OFREP", cannot substitute a default the server never saw. RPC is a remote evaluator by exactly that description and substitutes anyway. flagd answers a disabled flag with reason DISABLED, no variant and the zero value of the response proto, and grpc.py:468-472 replaces that with the caller's default_value on precisely that signal. So the substitution is local even though the evaluation is not: what crosses the wire is the signal, and the provider already holds the default. In-process is the unsurprising half. flagd_core.py:143-145 returns the caller's default_value with reason Reason.DISABLED before any targeting or variant selection, and flagd_core.py:199-200 skips the type check for that reason, which is what stops a substituted default from being re-typed against the flag it did not come from. Probed directly, both resolvers resolve all four flags to the caller's default with no variant and no error code. They differ on the reason: in-process returns the SDK's Reason.DISABLED, RPC the server's bare 'DISABLED' string. The scenarios assert neither and 2.2.5 requires neither, so it is recorded rather than acted on -- it is the sort of divergence between the two resolvers this pair of suites exists to surface. The harness also seeds disabled-object-flag and cross-flagset-flag, which the canonical set deliberately does not ask for: an Object resolution needs @object as well, and a scenario needing two capability tags cannot be one row of a single outline. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The TCK now owns the container lifecycle, so this adoption declares a
Compose file and stops doing everything else:
- the session-scoped testbed fixture, the control fixture built from a
launchpad port read off the compose object, and the reach into
tests.e2e.flagd_container for the container class are gone. The TCK's
tck_backend fixture starts the stack, discovers the mapped host ports,
builds the HttpControl against the launchpad and waits for it to
accept commands;
- tests/tck/docker-compose.yaml is new, and is the same stack the Java
adoption uses: the unmodified testbed image, service named `backend`,
three unpinned container ports. It is deliberately not the testbed's
own compose file, which stands up envoy for the forbidden-endpoint
e2e scenarios, names its service `flagd` and mounts a flags directory
from the host. None of that serves the TCK, which drives the launchpad
over HTTP;
- the dev dependency becomes openfeature-tck[compose]; the extra is what
pulls testcontainers in for the harness.
The TCK adoption no longer imports anything from tests/e2e, which is the
part that mattered beyond the line count: the two harnesses were coupled
through a class whose job is the e2e suites' needs, and a change there
would have broken conformance results for reasons unrelated to
conformance.
The provider host now comes from the endpoint rather than being written as
"localhost". That was a latent bug: with a remote Docker daemon, Docker
Desktop on some platforms or a rootless setup the stack is reachable at
neither localhost nor anything predictable.
Measured before and after, twice each: 7 failed, 87 passed, 18 skipped,
identical to the run before the rewrite, scenario for scenario. Dropping
Java's 50ms post-command settle -- which this suite never had -- changed
nothing, so there is no flakiness to report against the testbed.
The seventh failure is now recorded, which it was not. Three of the seven
are the documented testbed gaps on each resolver; the fourth is on
in-process alone and belongs to openfeature-flagd-core rather than to the
testbed: bool is a subclass of int in Python, so the int-to-float widening
in flagd_core.py sees boolean-flag as an integer and answers a Float
request with 1.0 instead of the caller's default. RPC passes that row
because the server type-checks it. A reader comparing "7 failed" against
the three documented gaps had no way to account for the difference.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… say so The TCK suite is excluded from `poe test` and `poe cov` and gets a task of its own, `poe test-tck`, which a maintainer runs before merging a change to it. The README says the same, with the reason and the current tally. It was running in the default build until now, and that is worth being plain about: the premise this was handed to me on -- that no CI job in any language executes a containerised suite -- is false for Python. `poe cov` is `pytest tests`, which collected 112 TCK tests alongside the 576 long-standing `tests/e2e` ones, on a runner that has Docker. So the flagd branch's build was red, for the same reason Java's ofrep one was. Docker is therefore not the reason for the exclusion; `tests/e2e` needs it too and keeps running. The reason is that a conformance suite reports what is true of the stack under test, and a full run is 7 failed, 87 passed, 18 skipped: three canonical flags flagd-testbed v3.8.0 does not seed, failing on each resolver, plus the `openfeature-flagd-core` widening that answers a Float request for `boolean-flag` with 1.0. Those failures are the report. A gate that has to be green cannot hold a suite whose honest output is red, and an xfail to make it green would blame the provider for the backend. Also follows the base's re-pin: the paragraph naming the revision whose assets ask for the three unseeded flags now names 93eb1a58. The two spec@009afe06 mentions that remain are provenance -- the revision a flag was introduced at -- and stay true. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…mporting The README and the pyproject comment both carried the reasoning for excluding a conformance suite from the default build, in this package's own words. That reasoning is now Appendix F's "Running the suite in CI", so both point at it instead. Four READMEs paraphrasing one decision is how the known-deviation and control-path decisions came to have three different answers. What stays is what is local: where the exclusion lives, the command a maintainer runs, the current tally with what each of the seven failures is, and that Docker is not what decides the exclusion here -- tests/e2e needs Docker too and does run in the default build. Appendix F also asks that an excluded suite keep building even when it does not run, because one that has quietly stopped building against its harness is worse than one that runs and fails. `--ignore` does not import the suite at all, and mypy in this package is configured over `src`, so nothing checked that. `poe test` and `poe test-cov` now end in a collect-only pass over tests/tck, which imports every test module and resolves the feature files without starting a container -- about a second. The two default tasks become sequences, so their commands move to `test-default` and `test-cov-default`. `poe cov`, which is what build.yml runs, is unchanged in name and now covers the collect as well. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
spec@c342461a moved every resolution-reason assertion into a new gated gherkin/reason.feature, so the reasons are now a claim a provider declares rather than a house rule every adopter is held to. Declare @standard-reasons on both resolvers. Declared on a run rather than on the source, as Appendix F requires. All nine scenarios pass on each resolver: the four rule-less rows as STATIC, an unknown flag and a type mismatch as ERROR beside their error codes, TARGETING_MATCH for the matched rule and DEFAULT for the miss, DISABLED for a disabled flag. The last three compose with @targeting and @disabled-flags, both of which these suites already declare, so none of the file is skipped here. Worth recording that the DISABLED row passes on RPC despite the reason arriving as flagd's bare string rather than the SDK's Reason enum -- the difference the DISABLED_FLAGS note already called out. The step compares the reason as text, so in-process's Reason.DISABLED and RPC's 'DISABLED' are the same answer. The tally moves from 7 failed, 87 passed, 18 skipped to 7 failed, 105 passed, 18 skipped: eighteen new passing scenarios, nine per resolver, and no new failure. The same seven failures as before -- three canonical flags flagd-testbed v3.8.0 does not seed, on each resolver, plus the boolean-as-Float widening in openfeature-flagd-core. Collection moves from 112 to 130. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…sured The base re-pins the conformance assets to spec@89b1519a, which adds a fifth declaring rule to Appendix F and corrects two $comment blocks in canonical-flags.json. No feature file changes and no declaration here moves: the run is 7 failed, 105 passed, 18 skipped before and after, with the same seven failures. The conftest's note about the flags flagd-testbed does not seed names the new revision. Neither suite declares @numeric-coercion and neither starts to. What changes is the evidence: both notes stated what the source would do, and one of them stated it as though it had been observed. Declaring the tag in both suites and running the three scenarios says something different. In-process passes both halves it claimed to -- 0.5 refused as an integer, 10 widened to a float. The third it blamed on the same `(int,)` rule cannot be observed at all: flagd-testbed seeds no `integral-float-flag`, so the scenario fails FLAG_NOT_FOUND, which is the gap the conftest already records for `large-integer-flag` and `huge-integer-flag` two paragraphs earlier. The source reading may well be right about a seeded 10.0; nothing here has seen it. RPC said one of three fails. Two do. The narrowing of 0.5 to 0 is real and measured, and `integer-flag` as a Float passes -- but `integral-float-flag` fails on the missing flag there too, so the note's "the two lossless scenarios pass" was false about an observable. Neither correction changes a declaration, and that is the point worth keeping: both resolvers were withheld for a good reason and the reason recorded was partly someone's reading of a source file. Two resolvers of one provider answering the same three questions differently is also the clearest evidence in this repository that Python can express @numeric-coercion at all -- a language that could not ask the questions could not produce two different answers to them. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…it measured Three declarations were wrong here, and one run settled all three. They are one commit because they are one decision applied three times -- and because two of them could only be measured once the first was fixed. @lifecycle: declared on both resolvers. It had been withheld since the first pass with nothing anywhere saying why. The only note that came close was the @reinitialization paragraph next door, which argued that declaring @reinitialization alone would be vacuous "and neither resolver declares LIFECYCLE" -- treating the withholding as given rather than justifying it. The consequence is that the six lifecycle scenarios had never been put to this provider at all, in six passes. Settled by running them. All six execute, and five pass on each resolver. The sixth is the @reinitialization scenario, which in-process passes and RPC fails against a closed channel. Java, Go and JavaScript all declare it on both of their flagd resolvers. Go's adoption records having made and reverted this exact mistake and says what it cost: withholding it left that suite blind to six scenarios another language was running against the same provider. Python was the last of the four still doing so. @REINITIALIZATION: declared on in-process, still withheld on RPC. It was withheld on in-process on the vacuity argument alone, which was sound at the time and is now spent: with @lifecycle declared the scenario runs, and it passes. RPC keeps withholding it, now on the measurement alone rather than on two reasons one of which rested on an unjustified absence. 2.5.2 makes reuse permitted rather than required, so neither withholding is a defect and neither carries a known-deviation entry. @NUMERIC-COERCION: declared on both, with the deviation on RPC only. The two resolvers do not agree, and withholding the tag on both hid that. Measured on one run: float-flag (0.5) requested as an Integer is a TYPE_MISMATCH returning the caller's default on in-process, and comes back as 0 with no error code at all on RPC. One provider, two resolvers, opposite answers to the question the capability exists to ask. In-process evaluates locally and never asks flagd; RPC passes back whatever the server answers, and flagd casts a float64 variant with a bare int64(val) -- open-feature/flagd#1996, which flagd's own numeric-coercion ADR commits it to fixing. Both declare the tag, because both attempt the coercion and both get the widening direction right; only RPC gets the narrowing direction wrong. That is the case Appendix F prefers the declared-and-failing shape for: withdrawing the tag from RPC would turn a specific, tracked defect into a skip indistinguishable from a provider that declines to coerce at all. The deviation entry is therefore on RPC only, which is what required a per-resolver known_deviations field on ResolverSuite. Java and Go each attach their equivalent entry to both of their resolvers and are right to -- in those languages both resolvers go through the server and both narrow. Python is the language where mirroring it would publish a defect against a resolver that does not have it. Worth flagging for Go, whose suite currently asserts the opposite: "the Python one does not [narrow], in either resolver: its RPC path asks flagd for an Int and gets INVALID_ARGUMENT for a float-valued flag". That is wrong about this resolver, and it is the same claim an earlier revision of this file made and retracted. INVALID_ARGUMENT is what grpc.py:461-462 would map to TypeMismatchError if it ever arrived, and for a float-valued flag it does not. @LARGE-INTEGERS: withheld, having been declared and failing on every run. Exactly one scenario carries the tag, and it asks for huge-integer-flag, which flagd-testbed v3.8.0 does not seed. The declaration rested on no evidence either way, and its failure read as a provider defect while establishing nothing. Nothing in either resolver would narrow the value. Go and JavaScript withhold it for this same reason; Java cannot declare it at all, its integer accessor being 32 bits, which is a third thing again. This looks like the same missing-fixture problem as integral-float-flag under @numeric-coercion, which this commit declares and leaves failing. The difference is worth stating as a rule rather than as two judgement calls: Declare when at least one scenario gating the tag can actually be put to the provider, and record the backend's gap where the others fail. Withhold when none of them can. @numeric-coercion has three scenarios and this backend can ask two -- and the two resolvers answer them differently, which is the finding a withholding would have buried. @large-integers has one, and this backend can ask none of it. One rule, two answers, because the facts differ. Neither gap gets a known-deviation entry: the gap is in the fixture, and an entry would attribute it to a provider that was never given the flag to get wrong. One finding that is neither a pass nor a fail. The in-process scenario "Shutting down a provider that cannot reach its backend completes promptly" passes and leaves a thread exception behind it: gRPC's connectivity poller raises "Cannot invoke RPC: Channel closed!" after shutdown has closed the channel underneath it, so the provider closes the channel without first stopping the watcher still using it. Reproducible on every run. Not a deviation -- nothing required is unmet -- but a stray traceback in a host application's logs during its own shutdown, and the one result the six lifecycle scenarios produced that neither a pass nor a failure would have carried. A full run is now 8 failed, 119 passed, 3 skipped, from 7 failed, 105 passed, 18 skipped. The conftest, the README and the pyproject comment all say so; the last of those had been stale at "7 failed, 87 passed" against a conftest saying 105. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Both suites carried the rule that decides @numeric-coercion and @large-integers in full, introduced as "stated once" -- which was true when nowhere else stated it. That wording is now Appendix F's sixth declaring rule (spec@4cab0320), taken from these files, so keeping a copy here makes two places for one rule to drift and this effort has already spent several passes undoing exactly that. The rule is named and its two consequences are attributed, and what stays is the part only this suite can say: which scenarios this backend can actually put to each resolver, and what it answered. The second consequence gets acted on rather than quoted. A capability withheld for a backend gap is temporary in a way one withheld by choice is not, so the @large-integers note now says what makes it expire -- flagd-testbed#392 seeding huge-integer-flag -- and what the cost of forgetting is: a withholding that outlives its reason reads as a claim about the provider. The numeric-coercion note pins which revision of the appendix it is following. That paragraph's guidance was reversed at spec@045950ca, and this repository's own TCK README and capability enum taught the old shape until this pass, so a reader who finds one of those in a cached copy can tell which came first. Also drops a stale cross-reference. The Go suite's note about the Python resolvers was wrong when it was written down here and has since been corrected there, so the paragraph keeps the finding -- RPC narrows, in-process does not, and reading the source got it backwards twice in two languages -- without telling a reader to go and check a file that no longer says it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
`FlagdContainer` creates `./flags` relative to the package directory and points the launchpad's FLAGS_DIR at it, so every e2e run leaves a 27KB `providers/openfeature-provider-flagd/flags/allFlags.json` behind. The directory is tracked through a `.gitkeep` and no ignore rule matches the file, so the working tree is dirty after any run and a `git add -A` commits a generated fixture. The rule is the one this package already wrote for the same file in the other flag directory: `tests/flags/.gitignore` has contained exactly `allFlags.json` since #121. This is that line, next to the directory that now receives it. Unrelated to the conformance suite, which drives its stack through Compose and writes nothing here -- it is in this branch because this is the branch that touches this package, and it reverts on its own. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
spec@aa2ad24f narrows the rule these two files supplied last pass. Without an opening clause it read as "declare wherever the scenarios are reachable", which forces a declaration even where the specification permits a provider to decline outright -- the Go implementation found it doing exactly that to its own self-tests, whose provider does not coerce by design. The rule now applies once a provider is attempting the capability, and whether an answer is owed at all is the known-deviation rule's question, asked first. Both citations carry the clause and then say why it is satisfied here, which is the part a reader of this file actually needs: this provider attempts the coercion -- correctly in-process, wrongly on RPC -- and nothing in either path declines to resolve a large integer. So what is left for rule six to decide is the fixture gap, which is what it was written for. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…uite The directory says conformance now that `tests/tck` is a sibling of `tests/e2e` rather than nested inside it, and `poe test-tck` selects it by that path. So `test_rpc_conformance.py` and `test_in_process_conformance.py` were saying it twice, and the half that duplicated the directory is the half that stopped being load-bearing: nothing selects on the module name. The `test_` prefix stays, because pytest still needs it -- that is a framework requirement rather than the naming convention being dropped. Node ids move with the rename. The only hardcoded one in this package's conformance suites is none: the xfail that names a node id lives on the OFREP branch and matches `item.name`, which a module rename does not touch. Same 130 collected, same 8 failed / 119 passed / 3 skipped, same eight scenarios by name. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
`test` and `test-cov` end in `test-tck-collect`, which is the one thing checking that the conformance suite still imports against the harness while `--ignore` keeps it out of the run. poe aborts a sequence at its first failing subtask, and this package's default suite is red on this branch stack -- the TCK requires openfeature-sdk >= 0.10 and this provider is not updated for it yet -- so that check has in fact never run here. `ignore_fail = "return_non_zero"` runs every subtask and still exits non-zero if any failed, so the build's verdict is unchanged and the check is reachable. No count moves: the collect step reports the same 130. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
flags/.gitignore ignored allFlags.json, which the launchpad writes into that directory when the pre-existing e2e suite mounts it. The conformance suite mounts no flags directory at all -- it drives the launchpad over HTTP, and the compose file says so -- so nothing this branch adds can produce that file. It is a reasonable fix for the e2e suite and belongs in a PR about the e2e suite. Here it is an unrelated change to a directory this branch otherwise does not touch. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Stripped of comments, this file and the OFREP adoption's were the same configuration with a different port list: same image, same service name, same absence of host bindings. Two hand-maintained copies of one backend is the mechanism that lets four languages claim to ask the same questions of two different images, which has already happened once across the four. So both now publish 8013, 8015, 8016 and 8080, and the two files are byte-identical. Exposing all four to both suites costs nothing: there are no host bindings, the harness maps container ports to dynamically assigned host ports and looks them up by container port, and it waits only on the ports an adoption declares. This suite never asks for 8016. Two files rather than one shared path, deliberately, and the packaging is the reason. Each provider is published independently, and this package's sdist includes its tests -- measured: `uv build --sdist` here ships tests/tck/docker-compose.yaml, while the OFREP package's ships no tests at all. A file outside the package directory would not travel with the distribution, and pointing one package's conftest at the other's directory is the cross-package coupling the OFREP adoption removed when it stopped reaching into this package's test-harness submodule. Drift is caught by `diff` instead, which both READMEs and the file's own header name. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
These four files were roughly three-quarters prose, and the surplus was not the per-capability reasoning -- it was everything around it: Appendix F's sixth declaring rule quoted at length in both suite modules, the two-shapes rule for a known deviation restated beside the one entry that uses it, the no-container-restart invariant explained twice, and a spec@ revision cited for every tag as though a reader needed the changelog to read the declaration. What is kept is what only this adoption can say. Both resolvers still carry the measurement behind every tag they declare and every tag they withhold, and the numeric-coercion split is kept whole: in-process refuses 0.5 as an Integer with TYPE_MISMATCH where RPC returns 0 with no error code, which is the only such split in the four languages and the reason the deviation is recorded against one resolver and not mirrored onto the other. conftest.py loses most of its length and gains the shape a reader wants: what a full run reports, and what each of the eight failures is. Three causes account for all of them, and the two that are the provider's now cite the issues they were filed as -- python-sdk-contrib#417 for the bool-as-float gap in flagd-core, #419 for the connectivity watcher a clean shutdown leaves running, #420 for the resolver split. The paragraphs explaining which flags flagd-testbed does not seed collapse to one sentence and open-feature/flagd-testbed#392, which names all three and says what each catches; the ones explaining that the canonical set's targeting, disabled and falsy flags need no testbed change collapse to one sentence, because they are flagd-testbed's own flags and that is why the canonical set took their names. Comments and docstrings only. Both suites still report 8 failed, 119 passed, 3 skipped. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
September 14, 2026 06:52
c3f3ffe to
a6159c6
Compare
Four languages pinned this image by hand and had drifted: JavaScript was already on v3.10.1 while Go, Java and Python sat on v3.8.0, so the suites whose results are only comparable if they asked the same backend were asking two. v3.10.1 is the current release, so aligning up rather than down. What this does NOT fix, stated because the tag is easy to mistake for a fix: open-feature/flagd-testbed#392 and #394 are both still open, so v3.10.1 carries neither the three missing precision flags nor the /start readiness fix. The fixture failures and the readiness race are unchanged. The one behavioural change in range is open-feature/flagd-testbed#390, which increases the simulated downtime -- and that is exactly the timing the @Stale and @unavailable scenarios depend on. Tallies recorded against v3.8.0 have not been re-measured on this image, including the failure ranges the adoption READMEs cite by tag; those figures stay as they are because they are a record of what v3.8.0 did, and re-taking them is follow-up work rather than a rewrite. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Several comments said flagd-testbed "v3.8.0" seeds neither large-integer-flag nor integral-float-flag. The Compose file moved to v3.10.1 and those flags are absent from that release too, so naming one version invited the reader to conclude the other serves them and that the failures were a pin away from going green. They are not: open-feature/flagd-testbed#392 is what adds the flags, and it is open. Now phrased as no released testbed seeding them, which stays true until #392 lands and false the moment it does -- which is when these comments should be revisited anyway. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts the conformance suite in the flagd provider, for both resolvers, in
tests/tck/.They are two suites because they are separately conformant. flagd resolves flags two quite different
ways — RPC evaluates remotely over gRPC, in-process syncs the ruleset and evaluates locally with
openfeature-flagd-core— and any difference between the two results is a difference an applicationwould see when it switches resolver, which is a configuration change rather than a code change. This
adoption found one.
flagd-testbedis not modified and the existing flagd e2e suites are untouched.tests/tck/is asibling of
tests/e2e/, and the directory is the whole of the selection:--ignore=tests/tckin thedefault build,
pytest tests/tckin the dedicated one. No-k, no name-shaped glob — which is whythe two modules could drop
_conformancefrom their names without a single count moving.The stack is started once per session and never restarted. Scenario isolation comes from the
control API instead, because container orchestrators assign host ports dynamically and cannot reliably
preserve them across a restart — a restarted backend comes back on a different host port, silently
invalidating every provider already pointed at it, and the failure looks like a flaky provider rather
than a broken test. One stack serves both suites, since one flagd process serves both resolver ports.
What it declares
Both resolvers declare
@events,@lifecycle,@stale,@configuration-change,@object,@variants,@targeting,@disabled-flags,@standard-reasons,@unavailableand@numeric-coercion. In-process declares@reinitializationas well; RPC does not.@large-integersis withheld from both:flagd-testbedserves no flag for it, so its one scenariocannot be put to the provider at all.
The one known deviation, and why it is on one resolver only
@numeric-coercionis declared in both, and the RPC suite carries a tracked deviation againstflagd#1996:
float-flag(0.5) through theinteger accessor returns
0with no error code, where the lossy half of flagd's own accepted coercionADR requires
TYPE_MISMATCH. The lossless half works, which is why the capability is declared and thescenario left to fail rather than the tag withheld.
The in-process suite has no such entry, because it does not have the defect. It evaluates locally
and refuses
0.5as an integer correctly. That makes Python the only one of the four languages whosetwo resolvers disagree here — Java's and Go's narrow identically in both, which places their defect in
a shared provider layer, while this one is in the server alone. Filed as a question rather than a bug:
#420.
Finding: Python's RPC resolver does emit
PROVIDER_STALE, and Go's does notThe Go adoption withholds
@stalefrom its RPC suite, because Go's RPC resolver sendsProviderErrordirectly on connection loss and never emits
PROVIDER_STALE(go-sdk-contrib#939).
Python has no such asymmetry.
resolvers/grpc.py: the channel-connectivity callback emitsPROVIDER_STALEonTRANSIENT_FAILUREand only then starts a timer escalating toPROVIDER_ERRORonce
retry_grace_periodexpires — structurally the same staged handling as the in-process watcher.So both suites declare
@stale.Not a point scored. It is a cross-language behavioural difference in the same provider against the
same specification, and the suite is what made it visible — which is roughly the argument for having
one.
The second finding, and it came from a passing scenario
shutdown()leaves the gRPC connectivity watcher running, so a clean shutdown emitsCannot invoke RPC: Channel closed!after the call has returned(#419). Worth reading twice: the
scenario passes — shutdown completes within its bound — so the results record no failure and the
report has nowhere to put this. It was found only because
@lifecyclehad never been declared in thisadoption, so those six scenarios had never run. Declaring it was a one-line change.
The backend stack
Both packages keep their own
tests/tck/docker-compose.yamland the two are byte-identical: theunmodified
flagd-testbedimage, servicebackend, publishing 8013, 8015, 8016 and 8080 with nohost port bindings. Each suite asks the harness only for the ports it needs.
Both are on
flagd-testbed:v3.10.1, aligned with the other three languages, which were splitbetween
v3.10.1and thev3.8.0this repository had been pinned to. The bump fixes nothing on itsown — flagd-testbed#392 and
#394 are both still open, so neither the
three missing precision flags nor the
/startreadiness fix is in it. The one behavioural change inrange, #390, increases the simulated downtime
that
@staleand@unavailabledepend on. The tallies below have been re-measured on v3.10.1 and areunchanged — same counts, same failures by name.
The other three languages consolidated to a single file per repository; Python deliberately did not,
and the reason is packaging rather than preference.
uv build --sdistshows the flagd package(hatchling) ships
tests/, while the OFREP package (uv_build,module-root = "src") ships notests at all — so a shared file above either package directory would not travel with flagd's
distribution, and pointing one package's conftest at the other's files is the coupling these
adoptions were separated to avoid. The duplication and the
diffthat catches drift are named inboth files' headers.
Running it, and the tally
130 scenarios over the two resolvers: 119 pass, 8 fail, 3 skip, measured against
flagd-testbed:v3.10.1. The eight account for exactly:@numeric-coerciondeviation, flagd#1996boolean-flagrequested as a Float resolves to1.0boolis a subclass ofint, so flagd-core coerces it and returnsSTATICwith no error code where the scenario requires the code default andTYPE_MISMATCHintegral-float-flagabsent from the testbed (#392)max-int32)large-integer-flagabsent, same PRlarge-integer-flagabsent, same PRSix of the eight are the backend's missing flags and get no deviation entry, because an entry there
would attribute a fixture gap to the provider.
The in-process one is a second place the two resolvers disagree, alongside the numeric-coercion
split above: RPC asks flagd server-side and is told
TYPE_MISMATCH, while in-process evaluateslocally through flagd-core and coerces. An earlier revision of this description said the eight were
"the RPC coercion deviation and backend fixture gaps", which accounts for seven.
The default build still typechecks the adoption, which Appendix F requires:
--ignoreleaves thefiles in the package, and
poe testends with a second pytest invocation that collectstests/tckwithout running it. That step cannot pass having checked nothing — pytest exits 5 on an empty
collection and 4 on a path that does not exist. It was, however, not being reached here: poe aborts
a sequence at its first failing subtask, and this package's default suite is red on this stack for the
SDK reason below, so the check had never run in the package where it matters most.
poe testnow setsignore_fail = "return_non_zero", which runs every subtask and still propagates a non-zero exit.Do not measure this package's default suite next to anything else. Three runs on one machine today
gave 404, 278 and 20 failures over the identical 784 selected tests; only the last was run alone, and
it matches the recorded figure exactly.
tests/e2edrives flagd containers through WSL Docker andfalls over under contention. I nearly reported the first two as a regression.
Why the flagd job is red, and why it is not this suite
openfeature-tckrequiresopenfeature-sdk>=0.10.0, and auvworkspace resolves one version permember, so the lock moves from
main's 0.8.4 to 0.10.0 for everyone — and 20 of this provider'sexisting tests fail on 0.10.0. That is a provider migration to a newer SDK; it wants its own PR and
its own reviewer, and is deliberately not fixed here.
unleashis a second casualty of the same bump— its
track()override no longer matches the SDK's signature, failingpoe mypyon the 3.14 job,the only one that type-checks — and is likewise left to its own PR. The rest of the workspace is clean
on 0.10.0.
Worth filing
PROVIDER_CONFIGURATION_CHANGEDon every reconnect, unconditionally._FlagStoreAdapter.update()calls it for every sync payload the watcher applies, including onewhose
changed_keysis empty. A reconnect that re-delivers an unchanged ruleset therefore announcesa change that did not happen, and an application cannot tell the two apart.
STALEbeforeERRORagainst a backend that was never reachable. The stagedhandling is right for losing a connection, but it also fires when the initial connection never
succeeded — and
STALEmeans "cannot reach the backend but still serving values" when there are novalues to serve. The suites keep short deadlines and a 1s grace period for the unavailable provider
to bound this, but bounding is not fixing.
What is not here
InProcessControland adifferent capability set. Left out to keep the reviewable surface small.