feat: add Appendix F, provider conformance (TCK) - #423
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 |
…pec submodule
The Gherkin, the canonical flag set and the control API document are not Java
artifacts. They are language-agnostic definitions of the provider contract that
every language's TCK must agree on byte for byte, and they only lived in this
module because the proof of concept had to start somewhere.
They now live in open-feature/spec as Appendix F, under
specification/assets/provider-tck/, and are copied in from the `spec` git
submodule at generate-resources — the same mechanism tools/flagd-api-testkit
already uses for the flagd test harness. The copies are git-ignored and carry a
do-not-edit note; changes belong in the spec repo and arrive here by bumping the
submodule.
Consumers are unaffected: the artifacts are still packaged into the release JAR,
@SelectClasspathResource("features") still resolves, and nobody needs a submodule
of their own. Verified byte-identical after the round trip.
The in-memory CI job now checks out submodules, since without them there is no
suite to run.
DEPENDS ON open-feature/spec#423. The submodule is pinned to that PR's branch
commit rather than to a commit on the spec repo's main branch. That is reachable,
so CI can fetch it, but it must be re-pinned to main once #423 merges and before
this lands.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…pec submodule
The Gherkin, the canonical flag set and the control API document are not Java
artifacts. They are language-agnostic definitions of the provider contract that
every language's TCK must agree on byte for byte, and they only lived in this
module because the proof of concept had to start somewhere.
They now live in open-feature/spec as Appendix F, under
specification/assets/provider-tck/, and are copied in from the `spec` git
submodule at generate-resources — the same mechanism tools/flagd-api-testkit
already uses for the flagd test harness. The copies are git-ignored and carry a
do-not-edit note; changes belong in the spec repo and arrive here by bumping the
submodule.
Consumers are unaffected: the artifacts are still packaged into the release JAR,
@SelectClasspathResource("features") still resolves, and nobody needs a submodule
of their own. Verified byte-identical after the round trip.
The in-memory CI job now checks out submodules, since without them there is no
suite to run.
DEPENDS ON open-feature/spec#423. The submodule is pinned to that PR's branch
commit rather than to a commit on the spec repo's main branch. That is reachable,
so CI can fetch it, but it must be re-pinned to main once #423 merges and before
this lands.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
lifecycle.feature was tagged @events, which is wrong in both directions.
Too strict: a stateless provider such as OFREP emits no events of its own
and cannot declare @events, yet it still initialises against a backend and
still owes the lifecycle contract.
Too lax: FeatureProviderStateManager emits PROVIDER_READY/PROVIDER_ERROR
around initialize for any provider, whether or not it is an EventProvider.
So a provider that does no initialisation of its own reaches READY exactly
as NoOpProvider would, and the readiness scenario passes vacuously.
Adds Capability.LIFECYCLE ("@lifecycle") -- performs an initialisation that
reaches its backend, with an observable outcome -- and re-vendors
lifecycle.feature verbatim from the spec assets, where the feature-level tag
is now @lifecycle (spec dfa16586, PR open-feature/spec#423).
flagd declares LIFECYCLE in both resolver modes: RPC does a round trip and
in-process syncs the whole ruleset during initialisation, so the scenarios
assert something real there.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…odule The feature files, canonical flag set and control-API document are owned by open-feature/spec. Vendoring them here made this repository a second place the definition of conformance could drift, which is precisely what the suite exists to prevent. They are now a pinned submodule at libs/shared/provider-tck/spec and the copies are gone. Adopters are unaffected, and that is the constraint the change had to respect: the rollup asset globs copy the artifacts out of the submodule and into the published package, so installing @openfeature/provider-tck from npm still needs no submodule and no particular repository layout. resolveAssetDir therefore has to satisfy two layouts -- the packaged copy next to the bundle, and the submodule under the library root -- and tries both. The spec calls the feature directory `gherkin`; the package keeps the name the API talks about. Contributors do need the submodule: without it no feature file loads at all. `nx test` and `nx package` depend on a pullSpec target that initialises it, and CI already checks out with `submodules: recursive`. Prettier is pointed at the submodule instead of the old vendored paths so it never rewrites artifacts that are consumed byte for byte by every language's TCK. The .gitattributes normalising those files to LF goes with them; the equivalent lives upstream, where the files now do. Pinned to dfa1658 (open-feature/spec#423). Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
4661068 to
36392a9
Compare
| @@ -0,0 +1,82 @@ | |||
| { | |||
There was a problem hiding this comment.
I wonder if the canonical flags should also be gherkin? They could be a very large "given", I guess.
There was a problem hiding this comment.
i can see the motivition behind it. but i am not sure it will be easier to read or understand or parse. Especially a json is easily transformed into another JSON for eg. CLI's to prefill databases etc. Not so sure about the gherkin for this purpose.
| Scenario: An integer flag resolves as an integer | ||
| # Paired with the float scenario below and with the narrowing scenario in errors.feature. | ||
| # Together they pin down that the two numeric types stay distinct rather than both being | ||
| # funnelled through one numeric representation. | ||
| Given a Integer-flag with key "integer-flag" and a default value "1" | ||
| When the flag was evaluated with details | ||
| Then the resolved details value should be "10" | ||
| And the error-code should be "" | ||
| And no exception should have been thrown | ||
|
|
||
| Scenario: A float flag resolves as a float | ||
| Given a Float-flag with key "float-flag" and a default value "0.1" | ||
| When the flag was evaluated with details | ||
| Then the resolved details value should be "0.5" | ||
| And the error-code should be "" | ||
| And no exception should have been thrown |
There was a problem hiding this comment.
What about integer precision? I think a lot of our SDKs support resolutions above 32 bit (though some like JS are practically capped at 2⁵³... it might be worth testing at least up to 2⁵³ -1
There was a problem hiding this comment.
fully agree, but for now, i would love to focus on the method, rather than the tests. We should agree and finalize the basic process. Afterwards we can add more tests to ensure compatibility
There was a problem hiding this comment.
added more cases on the side
|
I have to say, the Java impl is remarkably small and clean in Java. I think there's a lot of value here. |
|
I really like the idea, and the impl is clean (though TBH the issue and PR description is very wordy and maybe could be compressed and made less verbose) it took me a long time to get through 😅 . I have 2 things I think I'd want to understand additionally:
|
|
I am in total favor for extensibility. This would even help our providers. We could try to define defaults in java, like a default resource path and a default step path, add those to the test, and have a simply way for the beginning. But also I am not sure, if this would be something which we should delay to the next version. For now, there is the idea, but do we need it immediately to ensure conformance. It sounds like a nice to have feature. A good iteration on the tck, i feel like it could bloat the current efforts. wdyt? I measured what the Java classpath actually does here — the defaults idea works, with one catchFour measurements against the real module:
The third is the catch. If the default resource path is just plus one extra glue package on the base suite ( Net cost: two lines in If it does land, I'd word it in Appendix F as an outcome rather than a mechanism — an adopter can add scenarios and steps that run in the same backend phase without redeclaring the canonical set — since it's pure convention in Java and Python, while Go and JS have no runtime scanning and need one registration callback. Worth separating from the above either way: the suite should verify its canonical scenarios actually ran. That is not about extensibility — the same footgun already exists as // edit: flagd specifically is actually not a good reference example, as we are testing the evaluation engine seperately now. with really good tests. not sure if we want to have all of those tests executed during TCK evaluation. I see the benefit and the ease of running. But the more i think about, the TCK might be best to kept seperate as a verifiable unit, with whom nobody can mingle. Still torn, by both sides |
4c88e46 to
2acd860
Compare
…ouse rule The suite asserted an exact reason in thirteen places across three feature files, which narrowed 2.2.5 into a MUST for every adopter. 2.2.5 is a SHOULD that goes further than the others: a provider may use one of the listed values "or some other string indicating the semantic reason for the returned flag value". A provider whose backend reports vendor-specific reasons was therefore being failed for something the specification permits, and the appendix recorded that as a deliberate exception. It bought very little. Every canonical flag resolves to a value distinct from the caller's default, so a provider that silently falls back is already caught by the value assertion -- the reason only said why it failed rather than that it did. And of the thirteen assertions, five sat beside an error-code assertion that already carries the MUST, while the remaining eight asserted STATIC, which is the one reason the specification genuinely leaves open: types.md types DEFAULT as "no dynamic evaluation occurred or dynamic evaluation yielded no result", and a rule-less flag satisfies that as readily as STATIC does. So the reasons move to reason.feature, gated as a whole on @standard-reasons. The gate is a claim rather than an exemption: a provider declares that it uses the standard vocabulary with the standard meanings, and that file checks it. One that does not declare it loses nothing, since values, variants and error codes are asserted everywhere else on MUSTs. What the declaration buys is something a report's reader can act on -- anyone building telemetry or debugging on reason can see the vocabulary was verified rather than assumed. This also lets the appendix state what each reason means without asking the specification to close the set, because the mapping is the content of an opt-in claim and constrains nobody who does not make it. TARGETING_MATCH and DISABLED compose with @targeting and @disabled-flags, since neither reason can be observed without the behaviour that produces it. CACHED and STALE are left out: both need a scenario shape the suite does not have yet. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The four reasons a provider produces in normal execution rest on Requirement 1.4.7, which makes the SDK propagate the provider's reason -- but 1.4.7 is scoped to "cases of normal execution". Abnormal execution is 1.4.9, a SHOULD on the SDK to "indicate an error", and nothing requires the provider's reason to survive it. So an ERROR scenario in a provider suite can pass on a value the SDK wrote, which is the vacuous shape this appendix warns about everywhere else. Kept rather than dropped, because the assertion is the pair and not the field. The error code on its own is already covered for every provider by 2.2.7 and 1.4.8, both MUSTs on a closed enumeration, asserted ungated in errors.feature. The reason on its own could be the SDK's. Their agreement is what neither can satisfy alone, and an evaluation reporting FLAG_NOT_FOUND with reason STATIC is incoherent whoever wrote it. Recorded in both places a reader might look: beside the scenarios, and in the appendix's reason mapping. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…ation Two capabilities say nothing about any provider: @large-integers where the integer accessor is 32-bit, and @numeric-coercion where the language has a single numeric type. No provider in those languages can satisfy them and none ever will until the SDK changes. Until now that was documentation, and adopters were expected to act on it. They did, repeatedly: in one implementation three separate suites each left the same capability undeclared, each with its own comment restating the same property of the language. A fact about a language was being remembered in three places per language and in every future adoption, and a single wrong one would put a claim in a report that no scenario could have verified -- the failure the reserved-capability rules exist to prevent, reached by another route. So the implementation refuses it at configuration time, using the machinery that already refuses a reserved capability. The two refusals are deliberately kept distinguishable in the skip reason, because they are different facts: a reserved capability is global and expires when the specification adds scenarios, while an inexpressible one is a single language's and permanent. A reader seeing a capability absent from a report has to be able to tell "this provider declined" from "no provider in this language can be asked", since only the first describes the provider. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…s STATIC Two $comment blocks survived the reason change: "Every scenario expects reason STATIC" and "which is what lets every other scenario expect reason STATIC". The appendix was corrected in c342461 and this asset was not, which matters more than the usual stale comment -- the flag set is copied verbatim into four languages' packaged artifacts, so it is what a backend author reads when seeding a harness, and it now described a suite that no longer exists. Both now say what is actually true: only targeting-key-flag has a rule, and the absence of rules elsewhere is what lets a provider declaring @standard-reasons expect STATIC there rather than TARGETING_MATCH. The load-bearing property is unchanged -- it was never really about the reason, it was about not seeding targeting onto anything else. Found by the Java implementation while re-pinning, which is the argument for four implementations in one sentence: the asset is consumed by all of them and was wrong in a way no single language's tests could notice. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…rbids "A provider withholding this capability should say which it is -- a deliberate choice, or a tracked defect -- and a conformance report has knownDeviations for the second." That is withhold-plus-deviate, and a few sections above, the known-deviation rule says to prefer declaring the capability and letting the scenario fail, because a withheld capability plus a deviation asserts a defect at something nobody asked. Two parts of one document disagreeing is bad enough; this was also the part an implementer would read to learn the shape, and two of the four adoptions followed it there. Measurement settles which is right: flagd widens 10 to 10.0 correctly and narrows 0.5 to 0 with no error code, in both resolvers and in more than one language. It does coerce and gets one direction wrong, which is exactly what a skip cannot express. Withholding is still right for a provider that cannot attempt the behaviour at all -- a single numeric type, where the distinction does not exist to get wrong. Also adds the carve-out a TCK implementation's own self-tests need, which the rule did not cover. Those suites run the scenarios against an SDK's in-memory provider as a fixture, produce no report about a third party, and run in the ordinary build where a permanently failing scenario is a broken build rather than a finding -- nobody downstream can act on it, because the fix is an SDK release away. They may withhold, on the condition that the defect is pinned by a test of its own so the skip is not the only record. An adoption has no such licence. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Three implementations answered one question three ways, and the question was never written down: what do you declare when the backend under test cannot serve a flag some scenario needs? Go and JavaScript withheld @large-integers; Python declared it and reported a failure that was the backend's. Python also withheld @numeric-coercion on the same reasoning, and Go withheld it while recording a deviation against it. The rule that resolves all of them: declare when at least one scenario gating the tag can actually be put to the provider, withhold only when none can. @large-integers has one scenario and the reference backend serves no flag for it, so nothing can be established. @numeric-coercion has three and a backend missing one flag can still be asked the other two. That difference is not academic. Declaring @numeric-coercion is how one provider's two resolvers were found to disagree with each other -- one coercing correctly, one silently narrowing 0.5 to 0 -- which six passes of reading the source had not revealed. Withholding it to avoid one fixture failure would have cost both answers. Two consequences are stated with it. A scenario failing because its fixture is absent is not a provider defect and must not be recorded as one, or the report accuses the provider of the backend's gap. And a capability withheld for a backend gap is temporary in a way one withheld by choice is not -- it needs a note saying why, or it outlives its reason. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…s owed As written the rule said "declare when at least one scenario gating it can be put to the provider", with no condition on it. Read literally that forces a declaration wherever the scenarios are reachable -- including where the specification permits a provider to decline outright. @numeric-coercion is defined by no requirement, so a provider that simply does not coerce is entitled to withhold it, and the rule as written would have turned that permitted choice into a manufactured failure. That is the mirror image of the mistake the rule exists to prevent. Found by the Go implementation, whose self-tests withhold @numeric-coercion for exactly that reason while its flagd adoption declares it: memprovider does not coerce by design, flagd does coerce and gets one direction wrong. Two bullets four lines apart, resting on different grounds, and only one of them is rule six's business. So the rule now opens with the condition it always had implicitly: it applies once a provider is attempting the capability. Whether the provider owes an answer at all is Decision 4's question and comes first. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…vice provisional Two changes to the CI section, deliberately separated because one is settled and the other is not. Settled: a conformance suite gets a step of its own rather than being folded into a provider's existing end-to-end suite. The reason is what a failure says, not how long the run takes. A dedicated step going red reports that conformance failed; the same scenarios inside an e2e suite report that a test failed, and a reader has to go and find out which kind. The two also mean different things by failure -- an e2e suite is expected green, so a failure is a regression, while a conformance suite fails scenarios by design wherever a knownDeviation is declared, and that failure is correct output until the defect ships a fix. Sharing one signal between "you broke something" and "this is the known state" ends with somebody silencing the informative half. Four implementations answered this differently and none of them decided it; each landed where its build system pushed it. Not settled: whether the suite can be a required gate. That advice rests on the premise that a conformance run is unavoidably red, which is only true because we have been judging it by "zero failures". Judged instead by whether its results match its declaration, a healthy adoption is green in its steady state with its deviations intact. That is under discussion in #417 and the paragraph now says so, so nobody builds on it before it is decided. The separation advice holds either way. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…s not The paragraph justified a dedicated step with "a dedicated step that goes red reports that conformance failed" -- a property that only exists if something invokes the step. The same section says, provisionally, that nothing should: the suite is excluded from the default build pending the gating discussion. So the argument depended on a condition the surrounding advice forbids, and an audit of the two implementations that already have a separate target found exactly that hole -- both satisfy "do not fold it into the e2e suite", neither has a conformance job, so nothing goes red because nothing runs. Rewritten to argue from what a result means rather than from a failure being observed, and to say plainly that this is about a build target rather than a CI job. The two are easy to conflate. A step no pipeline invokes still earns its place: a maintainer running it by hand gets an unambiguous answer instead of a mixed one, and if the gating question resolves towards running it, the thing to gate on already exists and is already scoped to the right scenarios. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The appendix asked for a step of its own and stopped there, so four implementations put the files in four places and three of them selected the suite by test name: a skip filter on "Conformance", a Surefire pattern on *TckTest.java, a nested directory under the end-to-end suite. Only one filed it as a sibling. Stated as a property rather than a layout, because where files live is idiomatic per language and per repository, while the thing that goes wrong is not. A naming convention works until a test is renamed, and then it fails in the direction that hides the problem: the suite stops being selected, the step goes green having run nothing, and a pull request keeps quoting a tally from the last run that happened. That is the same shape as the CI exclusions every implementation believed it had and did not. The evidence is what each approach needed to be trustworthy. Selecting by name required an AST parser asserting that the set of tests calling the runner equals the set matching the pattern, mutation-tested in both directions -- and a third mutation, because such a guard otherwise passes vacuously the day it goes blind. Selecting by directory required nothing. Nesting inside the end-to-end directory gets its own sentence because it is the same mistake one level down: it says the conformance suite is a kind of end-to-end test, which is precisely what a separate step exists to deny, and where the end-to-end suite is its own module it also drags container libraries into tests that never use them. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Three of the four implementations could, by three different routes, and one of them did: a full adoption suite ran against the previous pin's feature files and reported a tally byte-identical to the run before it. Nothing failed, nothing warned, and it was caught only because someone compared two numbers that should have differed. The cause is the same wherever a copy is involved. Moving a pin updates the recorded revision, not the working tree the build copies from, so the two disagree silently and the copy wins. A rebase does the same thing: it moves the gitlink and leaves the checkout behind. Recorded here because it was recorded nowhere durable -- it surfaced in two languages' working notes during a rewrite and would have been lost with them, which is a poor fate for the only evidence that the failure mode is real. It is also the same shape as the CI exclusions every implementation believed it had and did not: a structural property everyone assumed held, in four places, none of them checked. Says to wire the fetch into the build rather than trusting whoever moves the pin to run a second command, notes that an immutable checksummed dependency avoids the problem outright, and warns that a guard catching one symptom is not a substitute -- a pin that changes only a scenario's content passes every such guard and still tests the wrong thing. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…ation Every other capability is covered somewhere without containers, by an in-memory or controllable provider standing in for the backend, so a regression in the step definitions or the capability gate shows up in an ordinary build. @Stale is not, in any of the four implementations -- faking a provider that loses its backend and regains it needs a control that can disconnect, and the in-process control paths refuse connection operations by design. The consequence is the part worth recording: the stale/ready transition is exercised only by a containerised adoption, which is also the suite most likely to be excluded from a default build. So the one capability with no cheap coverage is the one whose only coverage is the easiest to switch off. Surfaced by the JavaScript implementation while rewriting its README, which had the observation in a code comment and nowhere else -- exactly the case this appendix asked implementers to report rather than let vanish. Naming a way to close it, since a gap with no route out reads as a permanent limitation: a controllable provider that can fake a disconnect, written carefully enough not to become a mock that passes whatever the provider does. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The requirement said the suite must keep compiling in the default build. In at least one language that is impossible and always will be: where the ordinary build compiles what gets published, it is configured for library code -- no test globals, a different module target -- and an adoption that calls the test framework's own functions can never join it. Stated that way the requirement was unmeetable, which makes it a permanent known gap rather than a rule. What was actually wanted is that the adoption is typechecked against the harness by something that runs ordinarily, so a signature change in the harness cannot rot an adoption unnoticed. A typecheck scoped to the adoption satisfies that, and is a further argument for the adoption having a directory of its own -- a directory nothing else occupies is something a typecheck can be pointed at. Both ways of failing it are now named, because they are opposites and both were observed within an hour of each other. Excluding the adoption by path can remove it from the build as well as from the run, silently, since nothing fails when nothing is compiled. Removing an exclusion can pull the adoption into a build that cannot compile it, which at least fails loudly. The cheap check is the same either way and is worth stating: introduce a deliberate compile error in the adoption and confirm the ordinary build rejects it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The appendix said the step vocabulary was inherited from the flagd test harness "wherever it was already provider-neutral", which implies exceptions and named none. There are two, and they are the entire diff: a stable provider and an unavailable one both carried "flagd" in the step text. Recording them here is what lets four language READMEs stop recording them separately -- it was the one piece of provenance the Java implementation could not find an upstream home for -- and it turns an unfalsifiable claim into a checkable one. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…idation The appendix already said CACHED is unasserted because no scenario repeats an evaluation without a configuration change in between. That clause is load-bearing in a second way it did not state: against a provider with a client-side cache -- flagd's RPC resolver runs an LRU by default -- the evaluation after the change answers from the cache unless invalidation works, so the configuration-change scenarios depend on it and nothing tests it directly. An adoption that does not disable the cache is therefore resting on an untested dependency, and a failure there reads as a configuration-change defect rather than an invalidation one. Not requiring adoptions to disable the cache, because a provider measured as it ships is the more useful measurement -- but the dependency should be visible to whoever reads the failure. Found by the Go flagd adoption, which does not disable the cache. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A property was added without the heading following it. Noticed independently by the Go and Java implementations, both of which read this file while deciding what their READMEs could stop restating -- which is the use the count matters for: an implementer checking they have covered all of them counts the heading, not the bullets. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…said so Requirement 5.1.1 offers two responses to a provider losing its backend, in consecutive sentences with the same modal: one unable to evaluate flags "can signal this by emitting a PROVIDER_ERROR event", and one that caches "can signal this by emitting a PROVIDER_STALE event". Can, twice. Neither is required. The appendix gated @Stale without recording why, so the only thing telling an adopter that withholding it is legitimate was the tag's absence from a list -- and the tag's name reads like an obligation. @reinitialization already has this treatment for the same shape of requirement; @Stale did not, and the omission had a cost: a provider going straight to ERROR was on its way to being written down as deviating from a contract that explicitly permits it. flagd is the worked example, and it differs by transport rather than by defect, which makes it a portability finding rather than a conformance failure. Found by the Go implementation while cutting restatements out of its adoption -- this was the one fact it could not find an owner for upstream. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… not the adoption's This appendix said a state-changing control endpoint must not return until the state is being served, and then, twice, told an adopter stuck with a backend that breaks it to put the wait in the adoption. Stating a rule and licensing a local workaround for violations of it undercuts the rule. It is also measurably the wrong instrument. One backend defect becomes every adoption's problem, solved once per language; and the adoptions that implement the workaround report stable results while the ones that do not report flapping ones against the same backend -- so the suite reports a difference in harness behaviour as a difference in provider behaviour. Observed directly: of the adoptions run against one testbed today, the one carrying a settle-wait read a clean floor while the others bounced between 2 and 28 failures, and the difference was the wait rather than the provider. The wait belongs in the backend. Until it is fixed there the suite fails, the failures are read against a documented floor, and a run is repeated before an extra failure is attributed to the provider -- a race moves between scenarios, a defect does not. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The previous commit argued the case from an observation it did not check: that the adoption carrying a settle-wait read a clean floor while the others bounced, and that the wait was the difference. It was not. Removing that wait left its suite on exactly the same tally over four consecutive runs, and faster, because it had stopped polling for a condition that was already true. The correlation was real and the causal claim was mine to verify before writing it into a normative document. The principled argument is unchanged and does not need it -- and the measurement, correctly stated, is the stronger case: a compensating wait is hard to show is load-bearing, easy to leave in long after its defect is fixed, and certain to make two adoptions incommensurable meanwhile. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…ne wrote down
The vocabulary and the artifacts decide what is asked. Nothing decided whether
two languages' answers mean the same thing, and three things do: what the steps
this suite adds actually assert, how long it waits, and how it registers the
provider. All four implementations agree on every one of them -- because one
author wrote them in parallel, which is not a mechanism a fifth can rely on.
Each fact here was read out of the four branches rather than asserted:
- Seven steps are this suite's own and four assert something other than their
wording suggests. "no exception should have been thrown" is about a failure
escaping to the caller, not about the evaluation succeeding. "the resolved
details value should have changed" asserts a delta, because POST /change
promises only that the value differs and not what it becomes. The split
connection-lost/restored steps exist because the inherited self-healing form
cannot express "assert stale, then reconnect" -- which is also why no scenario
reaches POST /restart. The shutdown bound asserts that shutdown returns at all,
not that it is fast.
- Event 12s, ready 30s, startup 60s, identical in Go, Java, JavaScript and
Python, and an explicit `within {int}ms` in a step always wins over the event
default. The appendix previously mentioned timeouts once, in passing, and gave
no values -- so "promptly" and "timed out" meant whatever each implementation
chose.
- A fresh provider per scenario under a domain derived from the suite name.
Both halves matter: registering once per suite changes what the lifecycle
scenarios establish, and a fresh domain per scenario leaks a provider each
time, since registering into a domain replaces whatever was there.
Also requires the extension point to expose the client under test -- without it
an extension step can only build a second client, which resolves against a
different provider and reports success having asked the provider under test
nothing -- and requires the two run-integrity checks all four already implement.
Alongside, three stale facts: the assets README listed five feature files and
there are six (reason.feature), the reference implementation is tools/tck rather
than tools/provider-tck, and the control API's known-gap paragraph said context
passthrough was out of scope when the @targeting scenarios now cover the
targeting key. What remains uncovered is every other attribute, which is what it
now says.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Asked for directly on spec#423: an explicit Appendix F requirement that the single-init lifecycle be open to an adopter's own scenarios and steps running in the same phase, so vendors extend the suite rather than maintaining a parallel harness for provider-specific features. The section already described the mechanism and the four properties that keep an extension from becoming a conformance claim, but only as something an implementation MAY offer. That is the wrong strength for exactly the reason given: it is nearly free in a runner that resolves steps dynamically and not free in one driven by declarative suite annotations, so the implementations that would skip it are precisely the ones whose adopters most need it. Adopters who cannot extend do not go without; they build the parallel harness this section exists to prevent. Also states the lifecycle requirement outright -- one backend start and teardown covering canonical and extension scenarios alike -- which was implied by "the same lifecycle" and is the part an implementation could satisfy in letter while missing in substance. All four implementations already offer one, so this records what they do rather than asking for new work. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…tyle Appendix F had grown real obligations on a TCK implementation -- offer an extension point, use these timeouts, fail the run on a reserved tag -- and stated every one of them inside rationale. A fifth implementer could not enumerate what they had to do without reading the whole document, which is the substance of the review feedback that it "took a long time to get through". Twenty obligations are now blockquoted statements with the RFC 2119 keyword bolded, each followed by the reasoning it used to be buried in. Nothing changed about what is required; the statements were already there as prose. Deliberately not numbered, and the reason is worth recording since numbering was the first instinct. No appendix in this specification uses numbered requirements -- A states five as blockquotes, B, D and E have none -- and specification.json builds its 135 rules from the numbered sections only, referencing no appendix at all. Numbers here would be inert: nothing extracts them and no SDK suite cites them. Worse, this appendix *tests* numbered requirements (2.2.1, 2.4.1, 5.1.1), so a second scheme beside them would put provider obligations and harness obligations in one namespace, which is a trap for exactly the implementer the numbering would be for. Adds "What an implementation must do" near the top: every obligation in one place, grouped and linked to the section that explains it, with a note saying why it carries no numbers. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The @standard-reasons section had grown to 158 lines and half of it was about other capabilities: @numeric-coercion, the SDK-inexpressible rule and accessor width had all accumulated underneath it, where a reader looking for the coercion rule would never find them. They are now three sections with their own headings, and a misfiled paragraph about empty knownDeviations arrays has moved beside the other deviation rules. Compression alongside, mostly in @standard-reasons and the CI guidance: the thirteen-place provenance for why reasons became a capability is one parenthesis rather than a paragraph, the four-mechanism CI story is two clauses rather than five lines, and the compile-exclusion discussion loses a restatement. Also corrects the normative-status open question, which said nothing here is expressed normatively. That stopped being true in the previous commit. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
lifecycle.feature was tagged @events, which is wrong in both directions.
Too strict: a stateless provider such as OFREP emits no events of its own
and cannot declare @events, yet it still initialises against a backend and
still owes the lifecycle contract.
Too lax: FeatureProviderStateManager emits PROVIDER_READY/PROVIDER_ERROR
around initialize for any provider, whether or not it is an EventProvider.
So a provider that does no initialisation of its own reaches READY exactly
as NoOpProvider would, and the readiness scenario passes vacuously.
Adds Capability.LIFECYCLE ("@lifecycle") -- performs an initialisation that
reaches its backend, with an observable outcome -- and re-vendors
lifecycle.feature verbatim from the spec assets, where the feature-level tag
is now @lifecycle (spec dfa16586, PR open-feature/spec#423).
flagd declares LIFECYCLE in both resolver modes: RPC does a round trip and
in-process syncs the whole ruleset during initialisation, so the scenarios
assert something real there.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…pec submodule
The Gherkin, the canonical flag set and the control API document are not Java
artifacts. They are language-agnostic definitions of the provider contract that
every language's TCK must agree on byte for byte, and they only lived in this
module because the proof of concept had to start somewhere.
They now live in open-feature/spec as Appendix F, under
specification/assets/provider-tck/, and are copied in from the `spec` git
submodule at generate-resources — the same mechanism tools/flagd-api-testkit
already uses for the flagd test harness. The copies are git-ignored and carry a
do-not-edit note; changes belong in the spec repo and arrive here by bumping the
submodule.
Consumers are unaffected: the artifacts are still packaged into the release JAR,
@SelectClasspathResource("features") still resolves, and nobody needs a submodule
of their own. Verified byte-identical after the round trip.
The in-memory CI job now checks out submodules, since without them there is no
suite to run.
DEPENDS ON open-feature/spec#423. The submodule is pinned to that PR's branch
commit rather than to a commit on the spec repo's main branch. That is reachable,
so CI can fetch it, but it must be re-pinned to main once #423 merges and before
this lands.
The pin is the branch tip rather than the first commit of that PR, so the copied
assets carry the `@numeric-coercion` vocabulary and the reserved-capability
control API this branch already uses. Verified byte-identical against the
artifacts this commit deletes.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A vendor with provider-specific features -- flagd's fractional targeting, a proprietary evaluation mode -- had no way to test them inside this suite. The only option was a second Cucumber runner of their own, which means a second backend lifecycle to start and a second copy of this suite's configuration to keep in step with it. Answers @toddbaert's review request on open-feature/spec#423. The suite now also selects the classpath directory tck-extensions/ and the glue package openfeature.tck.extensions. An adopter writes two files and no annotations: src/test/resources/tck-extensions/fractional.feature src/test/java/openfeature/tck/extensions/FractionalSteps.java Their scenarios are discovered into the same suite and the same Cucumber engine, and therefore the same @BeforeAll -- one backend lifecycle, one BackendControl. The canonical steps are on the glue path too, so an extension scenario can open with `Given a stable provider` and continue with whatever is specific to that provider. The extension directory is deliberately not features/ and not a subdirectory of it. Measured on this module: two classpath roots holding the same directory are scanned additively, but two holding the same directory *and* the same file name are not -- one wins silently and the other file is never read, with test-classes beating the jar. An adopter who put features/errors.feature in their test resources would replace a canonical file with their own and watch the suite report success having run theirs. A distinct name makes that collision unreachable rather than documented. The directory ships inside the jar holding nothing but a README, because a @SelectClasspathResource naming a resource that exists on no classpath root is a hard discovery error rather than an empty selection -- so an adopter who extends nothing must still resolve it. Cucumber ignores files that are not .feature, and tolerates a glue package that does not exist, so the unused extension point costs an adopter nothing. Also adds ProviderTck, which names every value the suite's annotations carry. An annotation value has to be a compile-time constant, so an adopter who writes a @ConfigurationParameter of their own cannot compute one; without the constants they would restate our package name or our object factory as a string literal that nothing keeps in step. Constant concatenation is legal in an annotation value, so ProviderTck.ALL_GLUE + ",com.vendor.steps" is what they write instead. The TCK's own fixture -- tck-extensions/extension-selftest.feature and a step class in openfeature.tck.extensions -- is test-scoped, so it is not in the released jar and cannot reach an adopter's run. It sits exactly where an adopter's would, which is the only way to check the convention rather than assert it about a path no build uses. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
What
Adds Appendix F: Provider Conformance (TCK) — a language-agnostic conformance suite for validating OpenFeature providers, plus the three artifacts it rests on.
Marked experimental and explicitly non-normative. This is a proof of concept and the scenario set is a representative subset, not exhaustive coverage.
Why
OpenFeature's central promise is that swapping providers does not change application behaviour, and nothing currently verifies it. Every provider tests itself differently, against its own backend, with its own harness — so "implements the provider contract" is an unverified claim, and a behavioural difference between two providers gets discovered by the application that trips over it.
Why a new appendix rather than extending Appendix B
They answer different questions:
Appendix F also needs more than Gherkin. The scenarios, the flag set they assume, and the control API that produces outages are three co-dependent artifacts — a feature file that evaluates
boolean-flagis meaningless without the flag definition, and a disconnect scenario is meaningless without the endpoint that produces the disconnect. An OpenAPI document has no home under "Gherkin Suites".Both appendices now cross-reference each other.
Contents
specification/appendix-f-provider-conformance.md— the appendix, written for implementers in other languages: what the artifacts are, the control-API contract and its two invariants, the capability tag vocabulary, the six things a language implementation owns, and the rules for extending the suitespecification/assets/provider-tck/gherkin/— four feature files (evaluation, errors, events, lifecycle)specification/assets/provider-tck/flags/canonical-flags.json— the flag set every scenario assumesspecification/assets/provider-tck/openapi/control-api.yaml— what a backend under test must exposespecification/assets/provider-tck/README.md— a landing page for the directory, matching the convention ofassets/gherkin/README.mdspecification/assets/provider-tck/go.modandembed.go— the directory is also a nested Go module exposing the artifacts as anembed.FS. A Go module zip carries a git submodule only as a gitlink, so the Go TCK depends on this module instead of vendoring a copy. The other languages build from a working tree and keep using the submodule; these two files are inert for them. Precedent:open-feature/flagd-schemas.Assets live under
assets/provider-tck/rather than alongside the existing SDK Gherkin, becauseassets/gherkin/evaluation.featurealready exists and means something different.A
.gitattributesnormalises these files to LF. They are consumed byte for byte by every language's TCK and copied verbatim into published build artifacts, so a Windows checkout must not produce a different packaged file than a Linux one.Design decisions worth reviewing
Capabilities. Not every provider implements every optional part of the contract. Scenarios exercising an optional capability carry a tag; a provider declares what it supports and the rest are reported as skipped with a reason, never as passed. The rule behind it: a conformance suite that quietly goes green on scenarios it did not run is worse than no suite at all. Reserved tags (
@targeting,@caching) have no scenarios yet and must not be declarable, so an adopter declaring "everything" cannot pick up a vacuous claim.@lifecycleand@eventsare separate. A provider can perform an initialisation that reaches its backend without ever emitting events, and vice versa. An earlier draft conflated them, which forced providers to declare a capability they did not have in order to run scenarios they could satisfy.@numeric-coercionis genuinely optional. The specification does not require a provider to reportTYPE_MISMATCHwhen a float flag is requested as an integer; that rule comes from a flagd ADR and the tag borrows its name. An earlier draft of this PR claimed the opposite and called not declaring the tag "an admission of a known bug". That was wrong, and the appendix now says so. What remains true is that silently narrowing0.5to0is bad for users, and a provider withholding the capability should say whether that is a choice or a tracked defect. The report envelope in #425 hasknownDeviationsfor the second.No container restarts. Unavailability is simulated inside the running stack via the control API. Orchestrators assign host ports dynamically and cannot reliably preserve them across a restart, so restarting silently invalidates every provider pointed at the old port — and the failure looks like a flaky provider.
In-process control is a narrow carve-out. Providers with no backend (in-memory, environment-variable, file-based) may be driven in-process. Providers with a backend must use the control API; reaching an external backend through a side channel produces a suite that passes while proving nothing.
Extensions may add questions but never replace one. An adopter can ship its own feature files alongside the canonical set, but an extension file cannot occupy a canonical path, an extension scenario can never satisfy a canonical one, and a run that did not execute the canonical set in full must fail. The last rule was found by accident: a test selector matching one scenario produced a green suite and a well-formed report describing one scenario out of twenty-nine.
Open questions
GET /last-evaluationor similar). Until then a provider that silently drops the context passes. The@targetingtag is reserved for those scenarios.@numeric-coerciontests only the lossy half. A provider that wrongly rejects10.0as an integer passes. Closing that needs an integral float in the canonical flag set, which changes the flag set for every language at once. Accessor width (32-bit versus 64-bit integer accessors) is not modelled either.Implementations
tools/provider-tckin open-feature/java-sdk-contrib#1830, adopted by the flagd provider for both its RPC and in-process resolvers. It consumes these files from this repo via submodule, and the artifacts were verified to round-trip byte-identical.Which of #417's open questions this answers
Q1, directory layout — proposes
specification/assets/provider-tck/{gherkin,flags,openapi}/. The subdirectory is forced rather than chosen:assets/gherkin/evaluation.featurealready exists and tests an SDK, not a provider.Q4, numeric coercion as capability vs hard requirement — keeps it a capability, and the appendix now states plainly that the specification does not require the behaviour, so a provider that coerces is not violating the spec. The distinction between "chose not to" and "known defect, tracked here" is left to the report format in #425 rather than to the tag vocabulary.
Left open, unchanged by this PR: Q2 versioning, Q3 canonical flag-set format, Q5 context passthrough, Q6 reason values, Q7 conformance claims. Q5 is also recorded in the appendix's own "Open questions" section; Q7 is split out into #424.
What the suite has found so far
Two real defects, from the outside, which is the argument for the whole exercise:
MultiProviderswallows child provider events — it extendsEventProviderbut never subscribes to its children, soPROVIDER_CONFIGURATION_CHANGED,PROVIDER_ERRORandPROVIDER_STALEnever reach the client. Independently reproduced by running the suite against it; already tracked as open-feature/java-sdk#1882 (gap 1, High), where it had been found by hand-comparing against the js-sdk reference.