test(flagd): run the provider conformance suite against both resolvers - #1608
Draft
aepfli wants to merge 2 commits into
Draft
test(flagd): run the provider conformance suite against both resolvers#1608aepfli wants to merge 2 commits into
aepfli wants to merge 2 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 |
The suite's normative control path for any provider with a real backend, and the piece the README listed as the one blocking a containerised adopter. HttpControl implements both BackendControl and ConnectionControl over the global fetch, so it adds no dependency. It mirrors the Go client's behaviours: prepareScenario prefers POST /reset and falls back to POST /start?config=... on a 404 or 501, caching that decision once per suite; and because /reset restores flag state rather than starting a stopped backend, the scenario following a disconnect always uses /start. The fallback is the normal path rather than an edge case, since flagd-testbed's launchpad serves only /start, /restart, /stop and /change. The base URL may be a thunk. A control service's host port is mapped dynamically and does not exist until the stack is up, while runProviderTck has to be called at module load, so a plain string is usually impossible to supply. httpControl.spec.ts pins the request sequence against a stubbed fetch, with no container involved: the fallback rules are invisible from inside a scenario, so a client that gets them wrong produces failures that look like provider defects. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
flagd resolves flags two quite different ways, and they are separately conformant: any difference between the two results is a difference an application would see when it switches resolver. Two spec files rather than one, because jest-cucumber accumulates step definitions in module state and two runProviderTck calls in one file would make every step ambiguous. The TCK drives flagd-testbed's launchpad through the standardised control API, which the launchpad already implements, so neither the testbed nor the existing e2e suites in this directory change. The stack is started once per suite and never restarted; isolation comes from the control API instead. Both resolvers declare Events, Stale, ConfigurationChange, Object and UnavailableInit. The Stale declaration is the notable one: in Go the RPC resolver never emits PROVIDER_STALE while in-process does (open-feature/go-sdk-contrib#939), but here both report a lost connection through the same disconnectCallback seam -- grpc-service.ts:274 and grpc-fetch.ts:197 -- and the single handler behind it, flagd-provider.ts:130-148, emits PROVIDER_STALE before escalating to PROVIDER_ERROR. The contract is implemented once, in the provider, so it cannot differ between resolvers. StrictNumericTyping is left undeclared, as it must be for every JavaScript provider: the language has no integer type, so the scenario is unsatisfiable by construction rather than by defect. Targeting and Caching are undeclared because no scenario carries their tags yet. Closes #1607 Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
from
August 24, 2026 20:14
ee193f4 to
447e45c
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.
What
The flagd provider under the conformance suite, both resolvers, plus
HttpControl— the client for the backend control API — added to the TCK library.HttpControlimplementsBackendControlandConnectionControlover the globalfetch, so it adds no dependency. It mirrors the Go client:prepareScenario()prefersPOST /resetand falls back toPOST /start?config=defaulton 404/501, caching that decision for the suite; after a disconnect it always uses/start, because/resetrestores flag state and is not specified to start a stopped backend. flagd-testbed's launchpad serves only/start,/restart,/stopand/change, so the fallback is the normal path rather than an edge case.baseUrlaccepts a thunk, and that is necessary rather than decorative:runProviderTckis called at module load, but the launchpad's host port does not exist untilbeforeAllhas started the stack.Two spec files, because jest-cucumber accumulates step definitions in module state and two
runProviderTckcalls in one file would make every step ambiguous.The finding: no Go-style asymmetry here
Go's flagd RPC resolver never emits
PROVIDER_STALEwhile its in-process resolver does (go-sdk-contrib#939), so the Go adoption has to withhold the capability for RPC. JavaScript has no such split, and the reason is structural rather than lucky.Neither resolver decides anything about staleness. Both report a lost connection through a
disconnectCallback—service/grpc/grpc-service.ts:274andservice/in-process/grpc/grpc-fetch.ts:197— and the provider owns the whole transition in one place,flagd-provider.ts:130-148: emitStale, arm a timer, escalate toErrorwhenretryGracePeriodexpires.Because the contract exists once, the two resolvers cannot diverge. That is the shape Go needs, and it has been noted on go-sdk-contrib#939.
Both suites therefore declare
Events,Stale,ConfigurationChange,Object,UnavailableInit.StrictNumericTypingis withheld from both, as it must be for every JavaScript provider — the language has no integer type, so the scenario is unsatisfiable by construction rather than through any defect. See the TCK README.Verification
FlagdComposeContainer'sstart/stop/getLaunchpadUrl/getPort, the provider options used by the neighbouring e2e suite, and the disconnect seams cited aboveflagd-testbed/launchpad/main.go— only/start,/restart,/stop,/change; the/resetfallback is livehttpControl.spec.tsfetch— the/reset→/startfallback and its caching, and the start-after-disconnect rule — so the control client has coverage that needs no containerNo CI will run on this PR while it is stacked: the workflow triggers on
pull_request: branches: [main].One deliberate omission: I did not add
jest.retryTimesto the TCK specs, though the neighbouring flagd suites use 3. A conformance result that only holds on the third attempt is not a conformance result, so contention will surface as a failure rather than being retried away.Worth filing against the JS flagd provider
service/in-process/file/file-fetch.tsnever receives or callsdisconnectCallback— itsconnectignores the reconnect callback — so a provider usingofflineFlagSourcePathemits neitherPROVIDER_STALEnor a reconnectPROVIDER_READY. That may be intended for a file source, but it is undocumented and invisible.retryGracePeriod:configuration.ts:118-123documents@default 5;constants.ts:6setsDEFAULT_RETRY_GRACE_PERIOD = 10. Small, but it is the knob deciding how long a provider stays STALE before claiming ERROR.