feat(provider-tck): in-process control path and in-memory/multi-provider self-tests - #1837
Draft
aepfli wants to merge 3 commits into
Draft
feat(provider-tck): in-process control path and in-memory/multi-provider self-tests#1837aepfli wants to merge 3 commits into
aepfli wants to merge 3 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
Step definitions reached the Compose stack and the HTTP control API directly, through TckRuntime. That made the suite unrunnable for any provider without a containerised backend, and it put transport knowledge in the one layer that should have none. Introduce BackendControl as the single seam between the step definitions and whatever manipulates the backend. All nine touchpoints — scenario reset, flag change, disconnect, reconnect, bounded outage, provider creation and the suite lifecycle — now go through it. ControlApiClient becomes HttpBackendControl, one implementation of that seam; nothing about the HTTP control API spec changes and it remains the normative contract for external backends. Split the base class along the same line. ProviderTckTest (renamed from AbstractProviderTckTest) keeps only what every provider needs: capability declaration, timeouts, awaiting and step wiring. ContainerizedProviderTckTest extends it with the Compose lifecycle, port discovery and HttpBackendControl construction, and carries the compose-specific configuration that used to sit on ProviderTckHarness. Adopters with an external backend keep an unchanged surface — the flagd suites need only the superclass name. Behaviour is unchanged: same control API calls in the same order, same once-per-suite Compose lifecycle, same no-container-restart invariant. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…test Providers without an external backend — in-memory, environment-variable, file-based — could not run the TCK: every path to the backend went through Docker, Compose and HTTP. Add the in-process control path so they can, and use it to give the TCK a self-test. InProcessBackendControl manipulates the SDK's InMemoryProvider directly. Flag operations are map updates and a configuration change is updateFlag(), so the event the suite awaits is the provider's own PROVIDER_CONFIGURATION_CHANGED rather than one the TCK synthesised. It is deliberately bound to InMemoryProvider and deliberately not a general-purpose escape hatch: an external backend driven through a side channel bypasses the HTTP control API, which is the only thing that makes a conformance claim portable across languages. The README and the BackendControl javadoc say so explicitly. Connection control is modelled through the existing capability mechanism rather than no-op stubs. disconnect(), reconnect() and disconnectFor() are left at their throwing defaults, and the harness leaves STALE and UNAVAILABLE_INIT undeclared, so those scenarios are reported as skipped-with-reason. Over-declaring a capability the control cannot back fails loudly with a message naming the fix — an UnsupportedOperationException reached from a live scenario is a test-configuration bug, never a skip. InProcessBackendControlTest pins that, because a scenario that never runs cannot prove it would have failed. InMemoryProviderTckTest runs the full applicable suite against InMemoryProvider: 26 passed, 3 skipped by capability, no Docker, under a second. It is both the reference adoption for a backend-less provider and a CI canary that reports a broken step definition or capability gate in seconds — wired as its own Docker-free job alongside the existing matrix, which is unchanged. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-in-process-control
branch
from
August 24, 2026 09:42
2a559b1 to
61309e3
Compare
A provider that delegates is still a provider, and delegation is where the contract is easiest to drop on the floor: a variant that does not survive the hop, a reason rewritten, an error code flattened, an event that never arrives. MultiProviderTckTest runs the suite against the SDK's MultiProvider wrapping exactly one InMemoryProvider. One child is the interesting configuration rather than a degenerate one — the correct answer is then precisely what InMemoryProviderTckTest already asserts, so any difference between the two suites is attributable to MultiProvider and nothing else. This is not a test of aggregation; it is a test that delegation is transparent. It found something on the first run. MultiProvider extends EventProvider but never subscribes to its children, so a child's PROVIDER_CONFIGURATION_CHANGED — along with its PROVIDER_ERROR and PROVIDER_STALE — is swallowed and never reaches the client. Wrapping a provider in a multi-provider silently costs you those events, with nothing in the API to hint at it. That is a known gap, open-feature/java-sdk#1882 (gap 1, "child provider event aggregation and status tracking", High), originally found by hand-comparing implementations against the js-sdk reference. Reproducing it from the outside, without knowing it was there, is a fair advertisement for what the TCK is for. CONFIGURATION_CHANGE is therefore left undeclared, so the scenario is reported as skipped-with-reason rather than passing on a provider that cannot satisfy it — the same treatment flagd's STRICT_NUMERIC_TYPING gets. Delete the omission once #1882 is fixed. Everything else survives delegation unchanged: 25 passed, 4 skipped. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
This was referenced Aug 24, 2026
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.
What
Makes the TCK runnable without Docker, a compose stack, or HTTP, so providers with no external backend (in-memory, environment-variable, file-based) can adopt it — and uses that to give the TCK a self-test.
Three commits, deliberately separable, each building standalone:
refactor— extractBackendControl, split the base class. No behaviour change.feat— the in-process control path and the in-memory self-test.test— a second self-test againstMultiProvider, which found a real gap.1. The refactoring
Step definitions reached the Compose stack and the HTTP control API directly through
TckRuntime. That made the suite unrunnable for any provider without a containerised backend, and put transport knowledge in the one layer that should have none.BackendControlis now the single seam between step definitions and backend manipulation:All nine places where a step touched HTTP or container state now go through it.
ControlApiClientbecameHttpBackendControl, one implementation of that seam. Nothing about the HTTP control API spec changes — it remains the normative contract for external backends.The base class splits along the same line:
ProviderTckTest(renamed fromAbstractProviderTckTest) — capability declaration, timeouts, awaiting, step wiringContainerizedProviderTckTest extends ProviderTckTest— Compose lifecycle, port discovery,HttpBackendControlconstruction, and the compose-specific config that used to sit onProviderTckHarnessflagd needed exactly the superclass rename — one import, one
extends, nothing else.Where the abstraction leaked
Two places beyond the step layer had to change, both unavoidable and both the point of the split:
ProviderTckHarnessmixed core config (capabilities,eventTimeout,readyTimeout) with Compose config (composeFile,backendPorts,controlPort,defaultConfig,startupTimeout,settleTime) in one SPITckRuntimehard-codedComposeContainer, andBackendEndpointwraps it directlyNothing else moved.
FlagSteps,EventStepsandContextStepswere already clean.2. The feature
InProcessBackendControlmanipulates the SDK'sInMemoryProviderdirectly. Flag operations are map updates;changeFlag()isupdateFlag(), so the event the suite awaits is the provider's ownPROVIDER_CONFIGURATION_CHANGEDcarryingchanging-flaginflagsChanged— not something the TCK synthesised.InMemoryProviderTckTestruns the full applicable suite:No Docker, no network, under a second. It doubles as the reference adoption for a backend-less provider (three methods) and as a Docker-free CI canary.
Connection control is modelled by capability, not by no-op stubs
disconnect(),reconnect()anddisconnectFor()are left at their throwing defaults, and the harness leavesSTALEandUNAVAILABLE_INITundeclared. The three skipped scenarios are exactly those, each reported with its reason:I verified this cannot go silently green. Temporarily declaring
STALEmakes the scenario fail, with a message naming the fix:InProcessBackendControlTestpins that permanently, along with two things the Gherkin cannot assert about itself: thatchangeFlag()actually changes the resolved value, and that it does not leak into the next scenario.InMemoryProviderdoes declareSTRICT_NUMERIC_TYPING— itsisAssignableToonly widensInteger→Long, sofloat-flag(0.5) requested as an integer is aTYPE_MISMATCHrather than a silent0. It is the reference behaviour that capability describes.3. A second self-test: MultiProvider
MultiProviderTckTestruns the same suite against the SDK'sMultiProviderwrapping exactly oneInMemoryProvider. One child is the interesting configuration rather than a degenerate one: the correct answer is then precisely whatInMemoryProviderTckTestalready asserts, so any difference between the two suites is attributable toMultiProviderand nothing else. This is not a test of aggregation across backends — it is a test that delegation is transparent, which is where the contract is easiest to drop (a variant that does not survive the hop, a reason rewritten, an error code flattened, an event that never arrives).It found something on the first run:
MultiProviderextendsEventProviderbut never subscribes to its children, so a child'sPROVIDER_CONFIGURATION_CHANGED— along withPROVIDER_ERRORandPROVIDER_STALE— is swallowed and never reaches the client. Wrapping a provider in a multi-provider silently costs you those events, with nothing in the API to hint at it.This is a known gap: open-feature/java-sdk#1882, gap 1 ("child provider event aggregation and status tracking", High), which states verbatim that it "does not listen to or forward events from child providers". That gap was originally found by hand-comparing implementations against the js-sdk reference; the TCK reproduced it from the outside without knowing it was there, which is a fair advertisement for the whole exercise.
CONFIGURATION_CHANGEis therefore left undeclared, so the scenario is reported as skipped-with-reason rather than passing on a provider that cannot satisfy it — the same treatment flagd'sSTRICT_NUMERIC_TYPINGgets. Delete the omission once #1882 is fixed. Everything else survives delegation unchanged: values, variants, reasons, the full type-mismatch matrix,FLAG_NOT_FOUND, structured values, strict numeric typing and reachingREADY.CI
New
provider-tckjob running both in-process suites with no Docker and noe2eprofile. It runs in parallel with, not as a gate on, the existing matrix, so a green run is not delayed. The containerised flagd suite is unchanged.Docs
New README section on which base class to extend, and an explicit statement that in-process control is for backend-less providers only — an external backend driven through a custom in-JVM
BackendControlbypasses the control API and proves nothing. Same statement in theBackendControljavadoc, where someone is more likely to hit it.Scope decision
The design sketch listed
setFlag/removeFlagonBackendControl. No Gherkin step needs them and the control API has no endpoints for them, so adding them would create methodsHttpBackendControlcannot implement. Left out, and recorded in the README's known gaps as needing a control-API revision first.Verification
InProcessBackendControlTest: 4 passedmvn -Pcodequality,deploy verifygreen — checkstyle, PMD, SpotBugs, javadoc, spotlessNot verified: the containerised flagd suite has not been run — no Docker on the machine this was developed on. It compiles, and every control-API call is the same endpoint in the same order as before, but CI is the first real execution.