test(flagd): run the provider conformance suite against both resolvers - #941
Draft
aepfli wants to merge 22 commits into
Draft
test(flagd): run the provider conformance suite against both resolvers#941aepfli wants to merge 22 commits into
aepfli wants to merge 22 commits into
Conversation
github-actions
Bot
requested review from
Kavindu-Dodan,
bacherfl and
toddbaert
August 24, 2026 10:35
|
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
branch
from
August 24, 2026 10:37
c4bc447 to
521f042
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
August 24, 2026 10:37
c614a94 to
6b74e5c
Compare
aepfli
force-pushed
the
feat/provider-tck
branch
from
August 24, 2026 10:41
521f042 to
3755b42
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
August 24, 2026 10:41
6b74e5c to
d40c7b2
Compare
aepfli
force-pushed
the
feat/provider-tck
branch
from
August 24, 2026 11:02
3755b42 to
55e90a3
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
August 24, 2026 11:02
d40c7b2 to
791922a
Compare
This was referenced Aug 24, 2026
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
5 times, most recently
from
September 11, 2026 11:09
7033576 to
1af9cac
Compare
aepfli
force-pushed
the
feat/provider-tck
branch
from
September 11, 2026 13:34
44013f5 to
06a1d5c
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
3 times, most recently
from
September 11, 2026 18:01
f6f5473 to
355269f
Compare
aepfli
force-pushed
the
feat/provider-tck
branch
from
September 11, 2026 18:12
8e436f6 to
7ad62c7
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
3 times, most recently
from
September 12, 2026 10:43
b51c479 to
cd7ffbf
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
4 times, most recently
from
September 13, 2026 20:37
c0912a4 to
a37a8f5
Compare
aepfli
force-pushed
the
feat/provider-tck
branch
from
September 13, 2026 21:11
de17804 to
abe0f9a
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
2 times, most recently
from
September 14, 2026 05:12
0f10acf to
eac7a24
Compare
Adopts tools/provider-tck in the flagd provider, for the RPC and in-process resolvers, as two separate suites - they are separately conformant, and any difference between their results is a difference an application would see when it switches resolver. 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 existing container lifecycle in tests/flagd/testframework. The stack starts once per suite and is never restarted: scenario isolation comes from the control API, because container orchestrators cannot reliably preserve dynamically mapped host ports across a restart, and a restart there would look like a flaky provider. Adds tck.HTTPControl, the client for the control API in assets/openapi/control-api.yaml. It uses net/http only, so the TCK gains no container dependency; orchestrating the stack stays with the adopter, which is where the vendor-specific knowledge already lives. PrepareScenario prefers POST /reset and falls back to POST /start?config=... on 404 or 501, probing once per suite, and it uses /start rather than /reset for the scenario following a disconnect, since /reset resets flag state rather than starting a stopped backend. Finding: the RPC resolver never emits PROVIDER_STALE. Losing the stream sends of.ProviderError directly (pkg/service/rpc/service.go), whereas the in-process resolver emits PROVIDER_STALE on connection loss and only escalates to PROVIDER_ERROR once the retry grace period expires. So an application that switches from in-process to RPC silently stops receiving stale events. tck.Stale is withheld from the RPC suite, which reports the @Stale scenario as skipped with its reason rather than failing it; it needs its own issue against the provider and should be declared as soon as RPC emits PROVIDER_STALE. The in-process suite uses a longer readiness timeout, because it syncs the whole ruleset before reporting ready, and a retry grace period that outlasts the outage, so a scenario about staleness does not become one about failure. The unavailable provider keeps short deadlines in both suites so the initialisation-failure scenarios assert promptness rather than eventual failure. Part of open-feature/spec#417 Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…fails Running the conformance suite against Docker for the first time turned this up. Evaluating float-flag (0.5) through GetIntDetails returns 0 with no error code at all, rather than TYPE_MISMATCH with the code default. The application sees a plausible value and no indication anything went wrong. Both resolvers do it identically, which places the defect in the shared provider layer rather than in either transport. The Java flagd provider has the same defect (java-sdk-contrib#1830), so it is flagd-wide rather than Go-only. With the capability withheld the scenario is reported as skipped with its reason rather than failing, and both suites are green: flagd-rpc 29 scenarios, 29 passed flagd-in-process 29 scenarios, 29 passed Declared capability sets are now derived from observed behaviour rather than from reading the provider, which is what the suite is for. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…t properly The capability was renamed on the base branch, so the comments here named a symbol that no longer exists. Comments only -- both resolvers already withhold the capability, so nothing about what is declared changes. While correcting the name, the explanation is corrected too. It pointed at java-sdk-contrib#1830, which is the Java TCK's own pull request rather than an issue about the defect. The defect is tracked as open-feature/flagd#1996, which implements flagd's numeric coercion ADR: coercion is permitted when lossless, so 10.0 -> 10 keeps working, and must return TYPE_MISMATCH when information would be lost, which 0.5 does. It also no longer implies the specification requires this. OpenFeature has one numeric type, of "unspecified type or size", and differentiating integers from floats is an optional language idiom -- so the capability is tested against a rule borrowed from flagd rather than against a requirement. Someone reading this to decide whether flagd is non-conforming deserves to know that, and the gap in the provider contract is open-feature/spec#430. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The comment called this "a flagd-wide issue rather than a Go one", on the evidence that both Go resolvers do it and the Java provider does too. The Python flagd provider disproves that: it narrows nothing, in either resolver. Its RPC path asks flagd for an Int and receives INVALID_ARGUMENT for a float-valued flag; its in-process path admits only int for an integer request and deliberately maps float as (int, float), because widening towards float loses nothing. So the same server, asked the same question, answers correctly for one provider and wrongly for two. That places the defect in the Go and Java provider layers, not in flagd, and the comment now says so. Worth having on the record precisely because the earlier conclusion was drawn from two languages agreeing. Two implementations sharing a defect looks like evidence about the thing they both talk to, right up until a third one does not share it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The e2e module reaches the TCK through a replace to ../../../tools/provider-tck, and that package imports the conformance assets as a Go module rather than reading a vendored copy. A replace does not bring the replacement's requirements with it, so this module needs the assets module in its own go.mod, and without it the suite does not build: missing go.sum entry for module providing package github.com/open-feature/spec/specification/assets/provider-tck Worth recording why this was not caught earlier: the conformance tests are behind `//go:build e2e`, so `go build ./...` and `go vet ./...` never compile them and both report success on a module that cannot build its own tests. `go vet -tags=e2e` fails immediately. Any check of this package that omits the tag is checking nothing. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The TCK's own go.mod moved to fc99d5ac, but this module reaches the TCK through a replace to ../../../tools/provider-tck, and a replace does not bring the replacement's requirements with it. This module's own requirement is what selects the assets the suite actually runs, so leaving it at ba002ce8 would have run the previous revision's Gherkin against the new vocabulary -- the re-initialisation scenario would still have been untagged and mandatory, and would have failed, which is the exact outcome the pin move exists to correct. That is a nasty shape of drift: the suite is green on paper, the spec revision it reports is whatever this module pins, and nothing compares the two. Anything that moves the TCK's pin has to move this one in the same change. The e2e module declares go 1.25.9, so it needs a newer toolchain than tools/provider-tck's go 1.25.0 -- GOTOOLCHAIN=go1.25.8 refuses it outright rather than falling back. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…rest @lifecycle was withheld, and that was the expensive mistake in this adoption rather than a cautious one. The provider implements openfeature.StateHandler, Init builds the client and blocks until the event stream is up or the deadline expires, and Init can and does fail -- so the lifecycle scenarios assert something real here. Withholding the tag made Go blind to six scenarios Java was running, which is why Java ran 36 of the 40 and Go ran 29. Declaring it costs nothing now. Measured, per resolver: flagd-rpc 40 scenarios, 6 gated skips, 34 executed, 33 passed, 1 failed flagd-in-process 40 scenarios, 5 gated skips, 35 executed, 34 passed, 1 failed The one failure in each is "A large integer resolves without loss of precision", which fails with FLAG_NOT_FOUND because large-integer-flag is absent from flagd-testbed. open-feature/flagd-testbed#392 adds it. It says nothing about the provider and gets no deviation entry, because the gap is in the fixture and an entry there would attribute it to the provider. Two capabilities are withheld without a deviation entry, and the reason in both cases is that the specification permits the behaviour rather than requiring it. This is the correction this commit is really about: @reinitialization is undeclared. "A provider that was shut down can be initialized again" fails in both resolvers -- Shutdown clears the initialised flag, the second Init proceeds, and what it waits for never arrives, so it returns "provider initialization deadline exceeded". Requirement 2.5.2 says a provider SHOULD revert to its uninitialized state and its supporting text says "some providers MAY allow reinitialization from this state". Permitted, not required. The scenario is gated as of spec fc99d5ac and is now reported as skipped. An earlier draft of this file recorded it as a known deviation against tck.Lifecycle, which would have published a defect claim against a provider exercising a choice the specification offers it. @Stale stays undeclared on RPC, and that too is now a design choice rather than a defect. Requirement 5.1.1's supporting text offers both behaviours in one breath: a provider unable to evaluate flags "can" signal that with PROVIDER_ERROR, and one that caches rule-sets or evaluations "can" signal PROVIDER_STALE. The RPC resolver takes the first -- losing the stream sends of.ProviderError directly, and the SDK then short-circuits to the code default rather than serving cached values. Confirmed by running rather than by reading: declaring @Stale on RPC fails with "timed out after 15s waiting for a PROVIDER_STALE event". The in-process resolver does emit it and passes the scenario in 12.2s, so @Stale is declared there. That two resolvers of one provider report an outage differently is worth knowing and is written down in the file; it is not a conformance gap. JS and Java declare @Stale for both resolvers. On this evidence that is a vacuous declaration for RPC, which is a reason to leave it withheld here rather than to copy them. One capability keeps its deviation entry, and it is worth saying why it survives the same check: @numeric-coercion's rule is not in the specification at all (open-feature/spec#430), but flagd accepted it for itself in an ADR and has open-feature/flagd#1996 open to implement it. The summary now says so, so a consumer does not read the entry as a specification violation. Also fixed: Config.KnownDeviations was never populated. The per-suite knownDeviations field was collected and then dropped, because the call site passed Capabilities and not KnownDeviations -- the quietest possible way for a report to lose the one field that tells a defect apart from a design choice, and it read as wired because the struct field was filled in. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… run Follows the TCK's move to spec 26362f85. The e2e module reaches the TCK through a replace, and a replace does not bring the replacement's requirements with it, so this module's own pin is what selects the Gherkin the suite actually runs -- leaving it at fc99d5ac would have run the previous revision's assets against the new vocabulary and reported a spec revision this module does not use. Both capabilities are declared because the scenarios passed, not because flagd plausibly supports them. The run is the whole justification, so the numbers: each resolver goes from 40 canonical scenarios to 52, and each reports 50 passed and 2 failed. @Variants passes seven of its eight rows in both resolvers. The variant name survives the trip from the ruleset through the wire format into ResolutionDetail for booleans, strings, integers, floats and all three falsy flags. The eighth row asks large-integer-flag for its max-int32 variant and receives "", because that flag is absent from flagd-testbed -- the same fixture gap that already fails the untagged large-integer scenario, and the reason tck.LargeIntegers stays undeclared. Withholding the tag over it would skip seven working rows to hide one missing flag, so the comment on the capability list now says the suite carries two fixture failures rather than one. open-feature/flagd-testbed#392 adds the flags and both go away together; neither gets a knownDeviations entry, because the gap is in the fixture and an entry there would attribute it to the provider. @targeting was reserved rather than declarable until this spec revision, on the reasoning that asserting anything about the evaluation context needed an echo endpoint on the control API. It does not: targeting-key-flag carries one JsonLogic rule on the targeting key, so a matching context resolves to a different value than a non-matching one or none at all, and a provider that dropped the context is caught by the resolved value itself. All three scenarios pass in both resolvers, and running both was not a formality -- in-process evaluates the rule itself while RPC has flagd evaluate it, so agreement between them is evidence rather than duplication. The flag has been in flagd-testbed since flagd-testbed#103, released in v0.5.1 in February 2024, so unlike tck.LargeIntegers this needs no image bump. The new mandatory scenario that supplies an evaluation context to an untargeted flag passes in both resolvers too, which is worth stating because it is the half of requirement 2.2.1 that nothing here was testing: every resolve method takes a context, no scenario supplied one, and a provider that threw on any context or serialised it into a malformed request would have passed the whole suite. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Follows the assets pin to spec 009afe06 and declares the capability it adds. Anything that moves the TCK's pin has to move this module's go.mod too, because the e2e module carries the assets as an indirect dependency and a suite reading one revision of the Gherkin against another revision's flag set is the exact drift the module arrangement exists to prevent. Bumped by hand rather than with `go get`, which also wanted to take the go directive to 1.26.0 and flagd/core to v0.17.0 -- both unrelated to this change. @disabled-flags was expected to split the two resolvers and does not. The capability is gated because what a disabled flag resolves to depends on where the caller's default is substituted, which put the RPC resolver on the wrong side of the line by construction: it asks flagd to resolve every flag, and the caller's default never leaves the process. Both resolvers pass all four rows. Running it is what showed why, and it is not the reason the expectation assumed. flagd answers with reason DISABLED, an empty variant and a zero value, and isDefaultOrDisabledFallback in pkg/service/rpc/service.go recognises that pair and keeps defaultValue rather than taking the response's. The zero value is not what carries it: only the boolean row's default (false) coincides with its zero, and "bye" against "", 1 against 0 and 0.1 against 0.0 all fail if the response value is taken. So the line is "does the response distinguish a disabled flag from a resolved one" rather than "does the server see the caller's default" -- and an OFREP response does not, which is what keeps the tag worth having. The in-process resolver reads the state out of the ruleset it synced and arrives at the same answer by a different route, so running both was evidence rather than duplication. Both suites now run 56 scenarios: 54 pass and 2 fail. The two failures are the pre-existing pair in both resolvers and neither is about the provider -- large-integer-flag is absent from flagd-testbed, so the large-integer scenario fails with FLAG_NOT_FOUND and the last @Variants row has no variant to name. open-feature/flagd-testbed#392 adds the flag and both go green together. Neither gets a knownDeviations entry, because the gap is in the fixture. One caution is written down because it cost a wrong conclusion here: the first verification pass failed this outline with FLAG_NOT_FOUND and failed the object scenario with reason ERROR in the same run, and three subsequent runs of the RPC suite and a fourth of both showed neither. That is the launchpad reset race the OFREP suite documents -- POST /start returns before flagd's file source has loaded the regenerated flag file -- and not a property of these scenarios. Any single red run against this backend has to be re-run before it means anything. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
There is no container code left in this adoption. The suite owns the
stack now, so the file names a Compose file, names the container-internal
port each resolver connects to, and hands over a factory that builds a
provider from the endpoint the suite discovered:
tck.WithComposeFile("testdata/tck/docker-compose.yaml"),
tck.WithBackendPorts(suite.backendPort),
tck.WithProviderFromEndpoint(func(_ context.Context, e tck.BackendEndpoint) (...) {
return flagd.NewProvider(suite.resolver, flagd.WithHost(e.Host()),
flagd.WithPort(uint16(e.Port(suite.backendPort))), ...)
}),
Deleted with it: the temporary flags directory and its cleanup, the
tests/flagd/testframework.NewFlagdContainer call and its config, the
tck.NewHTTPControl construction, the host lookup, the port-by-name lookup
and its zero check. 462 lines to 461 -- but 149 code lines to 124, and
every one of the 45 that went was container orchestration that each
future adopter would otherwise write again. The rest of this file is the
evidence for what each capability declaration claims, which is the part
worth keeping.
The Compose file is this package's own rather than the testbed
submodule's, for two reasons. The submodule's bind-mounts ${FLAGS_DIR},
which defaults to its own directory, so an unset value has the launchpad
write into the checked-out submodule; and it runs an envoy sidecar that
exists for the TLS and permission-denied scenarios of the other e2e
suites here. Neither is wanted, and dropping both means the stack needs
no environment at all. Its service is called "backend", which is the
TCK's default and the name Java's flagd adoption uses, so the two
languages' stacks differ in nothing a reader has to reconcile. The cost
is the image tag pinned in a second place.
The other e2e suites in this package are untouched and keep using the
submodule's Compose file.
VERIFIED BY RUNNING, and the result is the same one recorded before:
56 scenarios, 54 passed, 2 failed, in both resolvers, the two failures
being the large-integer-flag pair the testbed cannot serve. The harness
handles the two-resolver, multi-port stack with no special-casing: two
suites, two stacks, each with its own discovered host ports.
WORTH READING BEFORE TRUSTING A RED RUN. Six consecutive runs of the RPC
suite produced 2, 2, 3, 3, 17 and 20 failures. The 20 was the old
hand-rolled wrapper and the 17 was this harness, so the flapping belongs
to the backend: the launchpad's POST /start returns once flagd answers
readyz, which happens before its file source has populated the store, and
every extra failure is FLAG_NOT_FOUND on a flag the testbed definitely
has. The README used to say a provider with an initialisation to block on
does not hit this; it hits it less often, not never, because flagd's RPC
Init waits for the event stream and flagd serves that as soon as it is
listening. No sleep is being added to compensate -- that belongs in the
testbed.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The base branch moved the specification assets pin to the appendix tip 93eb1a58; this module carries it as an indirect requirement through the local replace on tools/tck, so `go mod tidy` here is what propagates it. Tidy also raises the go directive from 1.25.9 to 1.26.0. That is not a choice made here: providers/flagd, which this module replaces locally, already declares 1.26.0, and CI builds with 1.26.0. The directive had simply been left behind because the previous pin bump was applied by hand. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…say so They were in one. `make e2e` runs `go test -tags=e2e` over every module, which is how this repository runs its containerised tests, and the CI job runs `make e2e` -- so every pull request was starting a Docker stack per resolver and going red on the two `large-integer-flag` scenarios that the testbed cannot serve and that say nothing about the provider. The settled policy across the four languages is exclusion from the default build, with a maintainer running the suites by hand before merge. Both suites therefore skip unless PROVIDER_TCK_RUN is set, and the skip message names the variable. An exclusion nobody wrote down is indistinguishable from a job somebody forgot to add, so the reason is in the file and in the README rather than in a review thread. A runtime skip rather than a second build tag, deliberately: the adoption stays compiled under -tags=e2e, so CI keeps typechecking it against tools/tck and a signature change in the harness cannot rot this file unnoticed. Only the container work is skipped. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… Appendix F The environment variable loses its PROVIDER_ prefix, following the package rename to `tck`: PROVIDER_TCK_RUN becomes TCK_RUN in the test and in the README command a maintainer copies. The README keeps the mechanism and drops the argument. Why an adoption suite is excluded from a default build rather than gating a merge is now Appendix F's "Running the suite in CI", settled for all four languages, and restating it here in different words is how four READMEs came to give three answers. What stays is Go's part of it, which the appendix cannot know: that `make e2e` applies -tags=e2e to every module in the workspace, so a build tag is not an exclusion in this repository but the opposite -- the first of the two mistakes the appendix names, and the one these two suites were making before the gate existed. The assets pin follows the base to v0.0.0-20260912211427-ccdb88790bb4, through `go mod tidy` on the local replace rather than by hand. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The assets pin brings reason.feature, whose six scenarios are gated as a whole on @standard-reasons -- a claim that the provider uses the standard resolution vocabulary with the standard meanings, not an exemption from anything. Declared for both resolvers, on evidence from running rather than from reading the provider: all nine executed rows pass in both. flagd reports STATIC for a rule-less flag, TARGETING_MATCH for a matching rule, DEFAULT for a rule that exists and did not match, DISABLED for a disabled flag and ERROR for a failed evaluation, which is Appendix F's mapping exactly. Both suites already declare @targeting and @disabled-flags, so all six scenarios run here rather than three of them skipping for a composed capability. Withholding it would have cost nothing in coverage of MUSTs -- values, variants and error codes are asserted elsewhere, ungated -- which is what makes declaring it a claim worth making rather than a default worth taking. Counts move with the pin: 56 scenarios to 65, and both resolvers still produce the identical result, 63 passed and 2 failed. The two failures are unchanged and are still the fixture gap, open-feature/flagd-testbed#392: large-integer-flag is absent from flagd-testbed:v3.8.0. This was an unusually quiet run of a suite whose launchpad race has produced 2 to 20 failures; the README's instruction to re-run a red result before reading anything into it still stands. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…rge-integers is Moves the e2e module's indirect pin to v0.0.0-20260913091110-89b1519a08d8 with the base. The Gherkin is byte-identical across the two versions -- only two $comment blocks in canonical-flags.json changed -- so the counts in this suite's README are unchanged at 65 scenarios and 63 passing, and nothing here needed remeasuring. The base now refuses a capability the language's SDK cannot express, which gives a reader of a report two named reasons a capability can be missing. This suite's @large-integers absence is neither of them, and the comment now says so: Go can ask for 2^53-1 and this provider would answer, the testbed simply has no flag to ask about. A Java suite is refused the declaration outright because its integer accessor is 32 bits; nothing like that applies here, and the difference matters because only one of the three says anything about flagd. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…viate
The adoption withheld @numeric-coercion and recorded a tracked deviation for
it at the same time. That is the one combination Appendix F's known-deviation
guidance singles out to avoid: a withheld capability plus a deviation asserts
that the provider is broken at something the suite never put to it, and the
three skips it produces cannot say which of "does not coerce" and "coerces,
and loses information one way round" is true.
Measured over three full runs, both resolvers, identically:
- "An integer requested as a float is widened without loss" PASSES.
integer-flag (10) through GetFloatDetails returns 10, reason STATIC, no
error code. So this provider does coerce, and the second shape does not
apply: shape 2 is legitimate only when the provider cannot attempt the
behaviour at all.
- "A float flag is not silently narrowed to an integer" FAILS. float-flag
(0.5) through GetIntDetails returns 0 with no error code, not
TYPE_MISMATCH with the code default. This is the deviation, and it stands
unchanged -- flagd measured against its own accepted numeric-coercion ADR,
tracked as open-feature/flagd#1996.
- "An integral float requested as an integer is coerced without loss" FAILS
with FLAG_NOT_FOUND, because flagd-testbed v3.8.0 does not serve
integral-float-flag. That is the backend's gap, not flagd's.
So both suites now declare the tag, keep the deviation, and run 65 scenarios
with 61 passing and 4 failing instead of 63 and 2. One of the four is the
provider's; three are the fixture's, and open-feature/flagd-testbed#392
retires all three together.
The second failure is the price of declaring and it is paid rather than
dodged: one red scenario belonging to flagd-testbed is a smaller loss than
three skips that misdescribe the provider. It is named in the deviation
summary so a consumer reading the report does not count it against flagd.
Also corrects a claim in the comment that was stated more strongly than the
evidence supports. It said the Python flagd provider gets this right "in
either resolver". It splits: the in-process resolver refuses 0.5 correctly,
and the RPC resolver narrows it to 0 exactly as this one does.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… the pin This suite declares @numeric-coercion and withholds @large-integers, and until now it argued each one out on its own. Appendix F settled the rule in spec 4cab0320 -- declare a capability when at least one scenario gating it can actually be put to the provider, withhold it only when none can, and the unit of the decision is the scenario rather than the tag -- so both answers now fall out of one cited rule instead of two local arguments. @numeric-coercion has three scenarios and flagd-testbed can still be asked two; @large-integers has one and the testbed serves no flag for it. That matters beyond tidiness. Four languages each reasoning from first principles to the same rule is how this effort keeps diverging, and this suite's own reasoning is what the rule was generalised from -- declaring the tag here is what found flagd's two resolvers disagreeing. The appendix's two consequences are cited in the same place rather than restated as judgements: a scenario failing for a missing fixture is not a provider defect and gets no knownDeviations entry, and a capability withheld for a backend gap is temporary, so flagd-testbed#392 is named with it or the withholding outlives its reason. Also follows the assets pin to spec 4cab0320. Prose-only in the assets module -- the two versions are byte-identical, same thirteen files, same tree digest -- so no count moves: both resolvers stay at 65 scenarios, 61 passed, 4 failed. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
`make tck` now runs every test matching "Conformance" and `make e2e` skips exactly those, so these two suites have the step of their own that Appendix F asks for and the TCK_RUN gate has nothing left to do. It is removed rather than kept alongside, because two mechanisms for one exclusion leave the next person to touch the pipeline guessing which is load-bearing. Nothing is lost on the point the gate was defending. The split is a test-name filter and not a build tag, so both targets still compile this file under -tags=e2e and a signature change in tools/tck still cannot rot it unnoticed -- which was the whole reason the gate was a runtime skip in the first place. testing.Short() stays. It is no longer the exclusion; it is the one guard left for a developer who names this package directly, and the README now says why neither it nor an environment variable is sufficient alone: -short defaults the wrong way round and a variable is invisible from the build. What a name filter costs is that a rename can move a suite from one target to the other in silence. conformance_naming_test.go pays that off: it parses this package and fails unless the tests that reach tck.Run are exactly the tests the pattern selects, in both directions, and fails too if it finds no suite at all so that it cannot pass vacuously. It carries no build tag, so `make test` runs it with neither Docker nor -tags=e2e -- a guard that only ran in the build it protects would be worth nothing. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The adoption was providers/flagd/e2e/tck_test.go, a package inside the e2e module. It is now providers/flagd/tck, a sibling module. Nesting it under e2e/ said it was a kind of e2e test, which is the conflation a step of its own exists to undo: the e2e suites test flagd against flagd's own harness and are expected green, while this one tests the provider against the OpenFeature provider contract and fails scenarios by design wherever a known deviation is declared. A reader who takes the second for the first reads a declared deviation as a regression. It is a module rather than a plain directory because a directory under providers/flagd would drag testcontainers, a Compose client and tools/tck into the dependency graph of every application that imports the provider. The e2e module was carrying them for the same reason and no longer does: it requires neither tools/tck nor the spec assets, and the spec pin it held indirectly now sits in the conformance module instead -- the same number of places, one of them moved. What follows from the move: - The build tag is //go:build tck, not //go:build e2e. The old tag was right while the file was a package inside the e2e suite and a leftover the moment it was not. It is not what selects the suite -- the module path is -- and it is not what keeps it out of `make e2e`, which builds this module under -tags=tck and runs nothing in it. Its job is the untagged build: without it, `make test` and a bare `go test ./...` here would start a Docker stack. - testdata/tck/docker-compose.yaml is testdata/docker-compose.yaml. The tck/ subdirectory existed to keep it apart from the e2e suites' fixtures, and there is nothing to keep it apart from here. - conformance_naming_test.go is guard_test.go, and it no longer asserts anything about names. `make tck` selects this module by its directory, so a rename can no longer move a suite from one target to the other and a guard against that defends nothing. What it keeps is the half a directory cannot check: that something in the module still reaches tck.Run, because a conformance module whose tests have stopped running the suite leaves `make tck` green by running nothing. It stays untagged and parses tck_test.go off disk, which is how it runs in the build the suite is absent from. - The test names are unchanged. Nothing selects on them any more, which is what makes keeping them cheap: TestFlagdRPCConformance and TestFlagdInProcessConformance stay readable in a failure line and usable with -run by hand, and leaving them alone keeps the before/after scenario counts comparable test by test. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
16504 bytes to 6157. An adoption README answers three questions: what
this provider declares and why each absence is what it is, what the
tally is and which failures are expected, and the one command that runs
it. Everything else here was the harness README's or Appendix F's.
Deleted, with a pointer in its place:
- the build-tag/module-path mechanism, ~70 lines arguing why a tag is
not an exclusion, why TCK_RUN was removed, why testing.Short is not
the gate and why the old -run 'Conformance' filter went. All of it
is identical in the OFREP adoption and in tools/tck/README.md, and
the reasoning above it is Appendix F's "Running the suite in CI".
- the argument for a conformance suite being a sibling of the e2e
suite rather than a package inside it -- Appendix F's, stated at
length there and now linked.
- the restatement of Appendix F's first rule for declaring, twice.
The per-capability reasoning is the one thing worth keeping at length,
and it already lives in the comments beside each declaration in
tck_test.go, which is the better home: the file a reader is in when they
change a declaration. The README now carries a one-line summary per
capability and says where the full version is.
Kept in full because it is this adoption's own measurement: the tally
and its four expected failures, the split between the provider's defect
and the fixture's three, and the six-run range that says to re-run a red
result before reading anything into it.
No behaviour change and no count moves.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Three changes to providers/flagd/e2e had no business in this PR. The conformance adoption moved *out* of that module, so the only correct diff there is none: - a README note saying the TCK used to live here, which is a tombstone in the documentation of a different suite; - go 1.25.9 -> 1.26.0, an unrelated toolchain bump; - flagd/core v0.16.0 -> v0.17.0 indirect, an unrelated dependency bump. The last two are go mod tidy drift that rode along on a module this branch has no other reason to touch. Restored to the base branch's content, which is main's. Also drops the Adopters table this branch added to tools/tck/README.md. An adoption branch editing the library's own README is the wrong direction across the seam, no other language's TCK has such a table, and a hand-maintained list of adopters in a library README has nothing keeping it honest. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Two things, both about where a fact lives. The Compose file moves to tests/flagd-testbed/docker-compose.yaml and is shared with the other conformance adoptions. Stripped of comments the two files were the same stack differing only in which ports they listed, and a copy per module is a mechanism for the image tag to drift between two suites whose results are only comparable if both answered the same backend -- JavaScript and the other three have already drifted that way. All four ports are published and each suite asks the harness for the subset it needs; there are no host bindings, so publishing one nothing connects to costs nothing. tests/ is where this repository already keeps shared test assets. The rest is comments. flagd-testbed#392 and the missing flags were stated five times across three files here, and the launchpad start race three; both are documented upstream, at greater length and with measurements, so each now gets a sentence and a link. Gone too: the account of the hand-rolled container wrapper this file replaced (PR #941's body), the restatements of Appendix F's rules for declaring and its known-deviation shapes, and the history of what earlier revisions of this file said. What stays, in full, is the evidence for every capability this provider declares or withholds: the two resolvers' event emission that decides @Stale, the three-run numeric-coercion measurements, the @disabled-flags result that contradicted the prediction, and which failures belong to the fixture rather than to flagd. tck_test.go 586 -> 404 lines, 424 comment lines to 243. No behaviour change: the only non-comment line that moved is the Compose path. Verified by running -- 65 scenarios in both resolvers, unchanged, in-process on its usual 61/4. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
September 14, 2026 06:27
bf04cfd to
ce90bfc
Compare
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 a new
providers/flagd/tckmodule.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 — and any
difference between the two results is a difference an application would see when it switches
resolver, which is a configuration change rather than a code change. That is exactly the class of
thing this suite exists to surface, and it surfaced one.
flagd-testbedis not modified and the existing flagd e2e suites are untouched. They stay inproviders/flagd/e2e— a sibling module, not a parent. They test flagd against flagd's ownharness and are expected green; this suite tests the provider against the OpenFeature provider
contract and fails scenarios by design wherever a deviation is declared, so filing one under the
other would put two different meanings of red on one signal.
What it declares
@events@lifecycle@configuration-change@object@variants@targeting@disabled-flags@standard-reasons@numeric-coercion@unavailable@stale@reinitialization@large-integersThree of the four absences are decisions the specification permits, and the code says so at each one
rather than leaving a reader to guess:
@staleon RPC. The RPC resolver emitsPROVIDER_ERRORon connection loss and neverPROVIDER_STALE; in-process emitsSTALEfirst and escalates toERRORonly when the retry graceperiod expires. So the two resolvers of one provider report an outage differently, and an
application moving from in-process to RPC stops receiving stale events. Worth knowing — but
requirement 5.1.1's supporting text offers both behaviours in the same breath ("can", twice), so it
is a design choice and gets no known-deviation entry.
@reinitialization. Requirement 2.5.2 is aSHOULDwhose supporting text says "some providersMAY allow reinitialization from this state". Permitted, not required. This provider does not offer
it. That scenario was mandatory until spec
fc99d5acand had been written down as a deviationagainst flagd before anyone read 2.5.2; gating it is the fix.
@large-integers.flagd-testbedserves no flag for it, so no scenario gating it can be put tothe provider at all. The withholding names
flagd-testbed#392 so it is revisited when
the backend gains the flag rather than outliving its reason.
The one known deviation
@numeric-coercionis declared, its scenario fails, and the failure is recorded beside it —which is the shape Appendix F asks for. flagd narrows
0.5to0through the integer accessor withno error code, where the lossy half of its own accepted
coercion ADR
requires
TYPE_MISMATCH. The lossless half works:10requested as a float widens cleanly. So theprovider coerces and gets one direction wrong, and withholding the tag could not have said that —
it would have asserted a defect at a question the suite never asked. Tracked against
flagd#1996.
The rule is flagd's own commitment rather than a specification requirement — the specification does
not define numeric coercion at all (spec#430) —
and the deviation's summary says so, so a consumer does not read it as a contract violation.
Running it, and the tally
Both suites run 65 scenarios: 61 pass, 4 fail. The four are the same set in both resolvers — the
coercion deviation above, plus three
FLAG_NOT_FOUNDfailures whereflagd-testbedv3.8.0 servesneither
integral-float-flagnorlarge-integer-flag. flagd-testbed#392 fixes all three at once.None of the three gets a deviation entry, because an entry there would attribute a fixture gap to the
provider.
A red run with more than four failures is the testbed's readiness window, not this provider:
POST /startreturns before flagd's file source has loaded the flags, so a stateless provider racesthat load on every scenario. No sleep is being added to compensate — the control API's promise is
that a command has taken effect when it returns, and a suite that sleeps instead of holding it to
that promise stops being able to detect when it breaks.
Module wiring
providers/flagd/tck/go.modis a module of its own, besideproviders/flagd/e2erather than insideit, requiring
tools/tckthrough a localreplace.providers/flagd/e2erequires neithertools/tcknor the spec assets any more — the spec pin moved with the files rather than multiplying.Two targets, doing two different jobs:
make tckruns this module. Nothing on a pull request does.make e2ebuilds it under-tags=tckwith an empty-run, so a signature change in the harnesscannot rot this adoption unnoticed — Appendix F requires the suite to keep compiling even when it
does not execute. Verified rather than asserted:
ok github.com/open-feature/go-sdk-contrib/providers/flagd/tck 0.175s [no tests to run].An untagged guard in the module fails if nothing in it reaches
tck.Run, because a conformance modulewhose tests stopped running the suite leaves
make tckgreen by running nothing.What is not here
different capability set. Left out to keep the reviewable surface small.