feat(provider-tck): add Java conformance suite for OpenFeature providers - #1830
Draft
aepfli wants to merge 2 commits into
Draft
feat(provider-tck): add Java conformance suite for OpenFeature providers#1830aepfli wants to merge 2 commits into
aepfli wants to merge 2 commits into
Conversation
OpenFeature promises that swapping providers does not change application
behaviour, but nothing verifies that today. Every provider tests
differently, so "implements the provider contract" is an unverified
claim.
Adds tools/provider-tck: the canonical Gherkin, the Cucumber step
definitions, and an abstract JUnit Platform Suite that owns the whole
test lifecycle. A provider author implements a four-method factory
interface and supplies a docker-compose stack; the TCK owns container
lifecycle, dynamic port discovery, control API calls, provider
registration and event awaiting.
Also adds a standardised backend control API (OpenAPI), derived from the
endpoints flagd-testbed's launchpad already implements, and the canonical
flag set the feature files assume. Both are packaged in the JAR alongside
the features so consumers need no git submodule.
Two normative requirements are documented in the control API spec:
* Backend unavailability MUST be simulated inside the running stack,
never by stopping or restarting a container. Testcontainers cannot
reliably preserve dynamically mapped host ports across a container
restart, and which bindings preserve them differs by language.
* /start resets flag state; /restart preserves it. An outage must be
observable as a change in availability, never in flag values.
flagd is the first adopter, wrapping the unmodified flagd-testbed image.
The adoption is 48 lines of code plus a compose file and a one-line
ServiceLoader registration; flagd-testbed is not modified and the
existing flagd e2e suites are untouched.
Scenario coverage is a representative subset covering each architectural
mechanism once: typed evaluation with value/variant/reason, the
integer/float distinction, TYPE_MISMATCH and FLAG_NOT_FOUND returning
code defaults without throwing, provider init success and failure, and
configuration-change and stale/ready event transitions.
Two findings from the first run against flagd:
* The flagd provider silently narrows a float flag to an integer:
evaluating float-flag (0.5) as an integer returns 0 with no error
code, rather than TYPE_MISMATCH with the code default. Reported as a
visible skip via the STRICT_NUMERIC_TYPING capability pending a fix.
* Cucumber parallelism inherited from a consuming module's
junit-platform.properties silently corrupts the suite, because
control API state is global to the stack. The base suite now pins
serial execution rather than relying on documentation.
Refs #1829
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
requested review from
Kavindu-Dodan,
beeme1mr,
thisthat and
toddbaert
July 27, 2026 09:13
…atch Adds flagd in-process alongside RPC, and broadens type-mismatch coverage from a single case to the full non-numeric matrix. Covering two modes exposed a leak in the adoption surface. Harness discovery went through ServiceLoader, which becomes ambiguous the moment a provider registers a second harness, and resolving it needed a system property plus one Surefire execution per mode in every adopter's POM. Fixed in the base class rather than absorbed as boilerplate: a concrete suite class already IS a ProviderTckHarness, so TckSuiteListener (a TestExecutionListener auto-registered from this JAR) reports which suite the JUnit Platform is running and TckRuntime instantiates that class. Adding a mode is now one class and nothing else — no registration file, no system property, no build configuration. Adoption drops to a single file; the META-INF/services registration is gone, retained only as a documented fallback for launchers that disable listener auto-registration. flagd's two modes therefore become a shared AbstractFlagdTckTest plus a four-line subclass each. In-process needs a longer initialisation deadline than RPC because it syncs the whole ruleset before reporting ready, while the unavailable provider keeps a short deadline so the init-failure scenarios still assert promptness. Type-mismatch coverage now spans every non-numeric combination — string, boolean, integer, float and object requested as each incompatible type, 15 cases — each asserting the full three-part contract: the code default is returned, TYPE_MISMATCH is reported, and nothing is thrown. All pass in both modes. Numeric coercion stays separate under @strict-numeric-typing, because "is 0.5 an integer" has a defensible wrong answer whereas "is a string a boolean" does not. Both resolvers narrow float to int identically (0, no error code), which places that defect in the shared provider layer rather than in either transport, so the capability is withheld on the shared base class. Verified: 29 scenarios per mode, 28 passed and 1 visibly skipped in each; existing flagd e2e suites unaffected (RunFileTest 151, RunInProcessTest 223, RunRpcTest 213). Refs #1829 Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1829
What this is
A conformance suite any OpenFeature provider can adopt to verify it implements the provider contract of the specification. OpenFeature's central promise is that swapping providers does not change application behaviour — nothing verifies that today, and every provider tests differently.
New module
tools/provider-tckcarries the canonical Gherkin, the Cucumber step definitions, and an abstract JUnit Platform Suite that owns the entire test lifecycle. It also carries a standardised backend control API (OpenAPI) and the canonical flag set, both derived from whatflagd-testbed's launchpad already implements.flagd is the first adopter.
flagd-testbedis not modified, and the existing flagd e2e suites are untouched.Full design rationale is in #1829; this description covers what a reviewer needs.
The adoption surface
Four methods with no default, everything else a convention. The flagd adoption in full — 48 lines of code:
Plus a 15-line compose file and a one-line
META-INF/servicesregistration.createProvideris a factory rather than a field because external ports are dynamically mapped and unknown until the stack is running.The structure follows
tools/flagd-api-testkit, which already solved this adoption problem: features packaged in the JAR and selected with@SelectClasspathResource, so consumers need no git submodule; normally test-scoped deps promoted tocompile;ServiceLoaderdiscovery.The two normative requirements
Both are in
openapi/control-api.yamland both are easy to get wrong.Containers are never stopped or restarted mid-suite. Backend unavailability is simulated inside the running stack — process kill, proxy toxic, socket block. This is portability, not preference: Testcontainers cannot reliably preserve dynamically mapped host ports across a container restart, and which bindings preserve them differs by language. A TCK that restarts containers works in one language and mysteriously fails in another. flagd's testbed already does the right thing by killing the flagd process inside a container that keeps running.
/startresets flag state;/restartpreserves it. An outage must be observable as a change in availability, never as a change in flag values. Scenario isolation depends on that split.Findings from the first real run
1. The flagd provider silently narrows a float flag to an integer. Evaluating
float-flag(0.5) throughgetIntegerDetailsreturns0with no error code at all — notTYPE_MISMATCHwith the code default. The application sees a plausible value and no indication anything went wrong.Handled by not declaring
Capability.STRICT_NUMERIC_TYPINGinFlagdTckTest, so the scenario reports as skipped with the reason printed rather than silently passing. Needs its own issue against the flagd provider.2. Cucumber parallelism silently corrupts the suite.
providers/flagd'sjunit-platform.propertiessetscucumber.execution.parallel.enabled=true, which the TCK inherited just by being on that classpath. Scenarios raced each other's control-API calls — one scenario's/startrestarted the backend underneath another's disconnect assertion — and the symptom looked like a flaky provider, not a broken test.AbstractProviderTckTestnow pins serial execution via@ConfigurationParameter, which overrides the consuming module's properties file. Enforced rather than documented, because the failure mode is so misleading.Verification
mvn --projects tools/provider-tck -P codequality,deploy clean verifyfailOnWarnings=truemaven.compiler.release=11mvn --projects providers/flagd -P e2e test -Dtest=FlagdTckTestRunFileTest151,RunInProcessTest223,RunRpcTest213 — all greenTestcontainers is pinned to
2.0.4, matching whatproviders/flagdalready uses, so two Testcontainers majors never share a test classpath.Open questions
Where I would most value pushback:
@strict-numeric-typingacceptable as a capability? It models a spec violation as an optional feature, which puts it on the same footing as "this provider has no streaming transport". It is pragmatic — a provider with the defect can adopt today and see the gap reported explicitly instead of being unable to adopt at all — but a separate "known deviations" concept might be more honest.open-feature/spec? They are language-agnostic contract definitions, not Java artifacts, and every language's TCK must agree on them byte for byte or "conformance" means nothing. The POM carries a comment describing the migration; consumers would see no difference. Cross-language tracking issue to follow.STATICthe right expected reason for a flag with no targeting rules, universally across providers, or does it need to be a per-provider expectation?Known gaps
TckRuntimeis static, so one TCK suite runs per JVM fork at a time.FlagdTckTestwith a different resolver, left out to keep the reviewable surface small.Update: both flagd resolver modes, expanded type-mismatch matrix
Both modes now covered
flagd resolves flags two quite different ways — RPC evaluates remotely over gRPC, in-process syncs the ruleset and evaluates locally. Both are now under the TCK.
This exposed a leak in the adoption surface and I fixed the base class rather than absorbing the boilerplate. Harness discovery went through
ServiceLoader, which becomes ambiguous the moment a provider registers a second harness; resolving it needed a system property plus one Surefire execution per mode in every adopter's POM. That is exactly the kind of leak this design is supposed to avoid.A concrete suite class already is a
ProviderTckHarness, so the TCK now asks the JUnit Platform which suite is running:TckSuiteListener, aTestExecutionListenerauto-registered from this JAR, reports the executing suite class andTckRuntimeinstantiates it.Consequences:
META-INF/servicesregistration is gone, retained only as a documented fallback for launchers that disable listener auto-registration.AbstractFlagdTckTestplus a four-line subclass each.One genuinely per-mode difference worth noting for other providers: in-process needs a longer initialisation deadline than RPC, because it syncs the whole ruleset before reporting ready. The unavailable provider keeps a short deadline so the init-failure scenarios still assert promptness rather than just eventual failure.
Type-mismatch coverage expanded
From one case to the full non-numeric matrix — string, boolean, integer, float and object each requested as every incompatible type, 15 cases. Each asserts the full three-part contract: the code default is returned,
TYPE_MISMATCHis reported, and nothing is thrown.All 15 pass in both modes, including structured-flag-as-scalar.
Numeric coercion stays separate under
@strict-numeric-typing, because "is0.5an integer?" has a defensible wrong answer whereas "is a string a boolean?" does not.Finding refined
Both resolvers narrow float to int identically —
0, no error code. That places the defect in the shared provider layer rather than in either transport, so the capability is withheld on the shared base class rather than per mode. It is the only scenario either mode does not pass.Verification
FlagdRpcTckTestFlagdInProcessTckTestRunFileTest/RunInProcessTest/RunRpcTestmvn -P codequality,deploy clean verifyon the moduleBoth suites run in one JVM fork, sequentially, each with its own Compose stack, with correct harness selection and no configuration.