feat(provider-tck): add the JavaScript conformance suite for OpenFeature providers - #1606
Draft
aepfli wants to merge 11 commits into
Draft
feat(provider-tck): add the JavaScript conformance suite for OpenFeature providers#1606aepfli wants to merge 11 commits into
aepfli wants to merge 11 commits into
Conversation
…ure providers
A conformance suite any JavaScript provider can adopt to verify it implements
the provider contract, and the JS implementation of the cross-language suite
defined in Appendix F. It runs the same Gherkin, the same canonical flag set
and the same control API as the Go, Java and Python implementations.
It uses jest-cucumber, the runner the flagd e2e suite already uses, so an
adopting library gains no new test framework. Adoption is a single
runProviderTck() call in a spec file; the TCK owns registration, event
awaiting, per-scenario reset and teardown.
Capability gating goes through jest-cucumber's tagFilter, which marks
non-matching scenarios skippedViaTagFilter rather than dropping them, and
jest-cucumber turns that into test.skip. The reason travels in the scenario
name via scenarioNameTemplate, since Jest has nowhere else to put it, so an
undeclared capability is reported as a visible skip rather than silently
omitted.
Three self-tests: the SDK's InMemoryProvider, MultiProvider wrapping exactly
one child so any difference is attributable to delegation alone, and unit tests
pinning what the Gherkin cannot assert about itself.
Two language-specific notes, both documented:
* JavaScript has no integer type. typeof 10 and typeof 0.5 are both 'number',
the Evaluation API exposes only getNumberDetails, and the in-memory provider
type-checks with typeof value != typeof defaultValue. Requesting float-flag
as an Integer is therefore indistinguishable from requesting it as a Float,
so no provider in this language can satisfy @strict-numeric-typing. Every
JS suite leaves that capability undeclared. This is a language property
rather than a defect, but it means the capability's meaning is
language-dependent in a way the specification does not yet acknowledge -
raised on spec#417.
* In JavaScript's favour: its in-memory provider has putConfiguration and
emits PROVIDER_CONFIGURATION_CHANGED, which the Go and Python SDKs' do not
(go-sdk#530, python-sdk#620). No wrapper class is needed here, and the
in-memory suite declares ConfigurationChange directly.
Authored without a Node toolchain on the machine, so every jest-cucumber and
SDK API was pinned against the source of the pinned versions rather than
against a build. CI is the first execution.
Part of open-feature/spec#417
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
|
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 |
…nt config Two failures from the first CI run, which was this library's first ever build. @openfeature/server-sdk declares FlagConfiguration in its type definitions but does not re-export it from the package entry point, so importing it is a TS2459 and every suite failed at load. Deriving it from InMemoryProvider's constructor avoids the unexported name and keeps it in lockstep with whatever the SDK actually accepts. It is re-exported here because it is the return type of the public canonicalFlagSet. The lint target reported the whole project as ignored. The root .eslintrc.json sets ignorePatterns to everything and each project un-ignores itself with its own .eslintrc.json; this was the only lib under libs/shared without one. Copied from ofrep-core. Worth noting the SDK gap for its own report: FlagConfiguration is part of the in-memory provider's public constructor signature, so consumers writing a flag set in TypeScript cannot name its type. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
… off the spec assets Three things from the second CI run. ConstructorParameters<typeof InMemoryProvider>[0] resolves to `FlagConfiguration | undefined`, because the constructor parameter is optional, and the undefined propagated into every use of the type. NonNullable is load-bearing rather than decorative. The vendored conformance artifacts are now in .prettierignore. They are copies of open-feature/spec's assets and are consumed byte for byte by every language's TCK, so letting a formatter near them would silently fork the definition of conformance -- the single thing this suite exists to prevent. release-please-config.json is restored to its original formatting. The entry was previously added by a JSON round-trip, which reflowed every single-line array in the file into a multi-line one and buried a seven-line change in a whole-file diff. Still outstanding and not fixable here: `nx format:check` also wants libs/shared/provider-tck/README.md and two source files reformatted. There is no Node toolchain on the machine this was written on, so prettier cannot be run to produce its exact output; that step is non-blocking in CI (the workflow swallows its exit code) and needs one `npx nx format:write` from someone with node installed. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…he workspace The conformance assets ship inside the library so that adopting the TCK never requires a git submodule or a particular repository layout. A workspace-relative glob broke that promise: it resolves against whatever directory the test runner started in, which is the workspace root here and something else entirely for anyone consuming the published package. They are now located relative to this module, trying the two layouts that actually occur -- next to the bundle in the published package, and at the library root in this repository -- and failing with a message naming both if neither is present. This brings JavaScript in line with the other three implementations, which already ship the assets inside the artifact: Go embeds them with go:embed, Python packages them in the wheel and reads them via importlib.resources, and Java packages them in the JAR and selects them from the classpath. None of those require anything of a consumer either. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The error message in resolveAssetDir was written through a shell heredoc that
ate a backslash, turning every \n escape into a real newline. Template
literals survive that, but .join('\n') became a single-quoted string spanning
a line break, which is an unterminated string literal and failed the lint
parse.
Rewritten without any escapes: the candidate paths are joined with a comma into
a single-line message, which reads better in a thrown error anyway.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…nds us
The suite failed to compile. Two distinct causes, both in how the recorder is
typed:
- the SDK declares an event handler's payload optional, so the parameter is
`EventDetails | undefined` and the recorder would not take it. It is now
queued as-is rather than dropped, because an event that arrives without
details has still arrived and every "should have been executed" assertion
is about arrival -- dropping it would turn a delivered event into a
timeout, the most misleading failure this could produce;
- `flagsChanged` is declared only on the configuration-change payload, so on
the union of every payload it resolves through their `Record<string,
unknown>` index signature and widens to `unknown`, which has no `includes`,
`length` or `join`. The change assertion narrows to the change payload,
which is sound because that recorder is by construction the change
recorder.
`EventDetails` also defaults to both the server and the web event unions, which
is wider than a suite running on the server SDK can ever see, so the parameter
is pinned.
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>
lifecycle.feature was gated by @events, which was wrong in both directions. A stateless provider -- OFREP, or anything evaluating over a request-scoped call -- cannot declare @events, yet may still have a real initialisation whose outcome is worth asserting. It was locked out of scenarios that were never about events. The reverse case is worse, because it goes green. Every SDK synthesises PROVIDER_READY for a provider with no initialisation step, so a provider that declares @events but does nothing on startup passes the readiness scenario without demonstrating anything -- a NoOpProvider passes it identically. @lifecycle asserts the stronger thing: that initialisation genuinely reaches the backend and that both terminal outcomes are observable. Neither self-test declares it, and that is the point rather than an oversight. InMemoryProvider has no backend to reach and no initialisation step, and the multi-provider wrapping it has neither either, so both were passing the readiness scenario vacuously on a synthesised event. The scenarios now report as skipped with the reason, which is the honest answer. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…inst a real bug The suite fails 24 of 29 scenarios, and the failures are genuine. MultiProvider keys the evaluation context by object identity, so an ordinary context-free evaluation misses the lookup and returns the code default with GENERAL; and it flattens TYPE_MISMATCH to GENERAL across the whole type-mismatch matrix. Both are filed as #1609 with reproductions. The suite did exactly what it was written to do. The identical scenarios pass 29 of 29 against the unwrapped InMemoryProvider, so the wrapper is the only variable and every failure is attributable to it. Go's multi-provider passes the same suite, so this is not inherent to the pattern. Excluded rather than deleted, in jest.config.ts with the reasoning recorded there and in a header on the spec file itself: a defect in another library should not block the conformance suite's own adoption, and this file is now the regression test for #1609. Re-enabling it is a one-line change. Verified on Node 24 with the suite running for real: 2 suites passed, 29 passed, 5 skipped by capability, 0 failed. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…mentation The suite was testing `@openfeature/multi-provider` from this repository, which is deprecated in favour of the Multi-Provider now shipped inside `@openfeature/server-sdk`. Conformance-testing a package nobody should adopt proves little, and the other languages already make the other choice: Go tests `go-sdk/openfeature/multi`, Java tests `dev.openfeature.sdk.multiprovider`, and Python has no multi-provider in either place so has no such suite. Reaching it needs the workspace's server-sdk devDependency at ^1.23.0; 1.19.0 predates the export. That is a minor bump and every provider's peer range (^1.17.0) already admits it. Retargeting changes the result substantially. The deprecated package failed 24 of 29, largely because it keyed evaluation context by object identity, so an ordinary context-free evaluation missed the lookup and fell through to the code default. The SDK's does not have that defect: evaluation, variants, reasons and configuration-change events all survive delegation intact. What remains is 16 failures with one root cause -- the multi-provider replaces the child's error code with GENERAL, 15 times for TYPE_MISMATCH and once for FLAG_NOT_FOUND. It is discarded rather than lost: collectProviderErrors builds an ErrorWithCode carrying the child's real code, and constructAggregateError then wraps it in `AggregateError extends GeneralError`, leaving the true code reachable only via originalErrors[].error.code, which nothing reads. An application can no longer tell "you asked for the wrong type" from "something went wrong", which is the distinction the error code exists to carry. So the suite is excluded for now, but against the right subject and for a defensible reason: it is the regression test, and re-enabling it is deleting one line of jest.config.ts. This is what the suite is for -- wrapping exactly one child means the correct answer is already known, being whatever that child scores alone, so any difference is the wrapper's. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck
branch
from
August 24, 2026 14:00
c93be33 to
c86c7d2
Compare
This was referenced Aug 24, 2026
Appendix F requires a skipped scenario to be reported with the reason it was skipped. Jest has nowhere to put that but the test name, and jest-cucumber's scenarioNameTemplate does not reach far enough: it is applied to a Scenario Outline's own title, and each example row is then defined under its expanded title instead. Every skipped example row therefore showed no reason at all -- the four rows of errors.feature gated by @object whenever a provider does not declare it, which is the common case. jest-cucumber accepts the describe/test pair it calls, so the harness supplies one per feature and names the skip at the point the test.skip call is made, which is the only seam an example row passes through. Naming a skip needs to know which capabilities gated that particular row, so the harness works out ahead of the run exactly which scenarios jest-cucumber will define and which of them the gate will skip. The plan is positional, not keyed on the scenario name: Gherkin permits tags on an individual Examples block, so two rows of one outline can differ in whether they are gated, and a name-keyed plan would gate all of them together. The runner asserts each title it is handed against the plan, so a change in jest-cucumber's behaviour surfaces as a loud failure rather than a wrong reason. 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 #1607
Part of open-feature/spec#417 — the cross-language tracking issue for the provider conformance suite. Java is the reference (java-sdk-contrib#1830); Go (go-sdk-contrib#940) and Python (python-sdk-contrib#409) are in review.
What this is
A conformance suite any JavaScript provider can adopt to verify it implements the provider contract — the JS implementation of Appendix F, running the same Gherkin scenarios, against the same canonical flag set, driven through the same control API as every other language's TCK.
It uses jest-cucumber, which
libs/providers/flagdalready uses for its e2e suite, so an adopting library gains no new test framework.The adoption surface
One call in one spec file:
The TCK owns registration under a suite-scoped domain, event awaiting, per-scenario backend reset and teardown.
One constraint worth knowing: one suite per file. jest-cucumber accumulates step definitions in module state, so two calls in one file would register the vocabulary twice and every step would report as ambiguous. Two resolvers means two spec files.
Capability gating without silent omission
This was the interesting design problem. Appendix F requires that an undeclared capability be reported as skipped with a reason, never silently dropped — but
tagFiltersounds like it just filters scenarios out.It does not. jest-cucumber marks non-matching scenarios
skippedViaTagFilterandgetTestFunctionmaps that tojestLike.test.skip:So they stay in the report as skipped. The reason travels in the scenario name, because Jest has nowhere else to put it:
Why the harness composes that name itself
jest-cucumber offers
scenarioNameTemplatefor exactly this, and it does not reach a Scenario Outline's example rows. The template is applied to the outline's own title, and each row is then defined under its expanded title instead:Every skipped example row therefore showed no reason at all — the four rows of
errors.featuregated by@objectwhenever a provider does not declare it, which is the common case:That is the rule this suite exists to enforce, broken in the suite itself, so it is fixed here rather than in the stacked report PR where it was first written.
jest-cucumber also accepts the
describe/testpair it calls, and that seam does reach the example rows, because it is the one everytest.skipcall passes through. The harness supplies one per feature and names the skip at the point the call is made (src/lib/scenarioRunner.ts).Naming a skip needs to know which capabilities gated that particular row, so the harness works out ahead of the run exactly which scenarios jest-cucumber will define and which of them the gate will skip. The plan is positional, not keyed on the scenario name: Gherkin permits tags on an individual
Examplesblock, so two rows of one outline can differ in whether they are gated, and a name-keyed plan would gate all of them together. The runner asserts each title it is handed against the plan, so a change in jest-cucumber's behaviour surfaces as a loud failure rather than a wrong reason.The one place JavaScript cannot answer the shared question
@strict-numeric-typingasserts a provider reportsTYPE_MISMATCHrather than narrowing0.5to0when a float flag is requested as an integer. In Go, Java and Python that has a right answer.JavaScript has no integer type.
typeof 10andtypeof 0.5are both'number', the Evaluation API exposes onlygetNumberDetails, andInMemoryProvidertype-checks withtypeof resolutionResult?.value != typeof defaultValue. Requestingfloat-flagas an Integer is indistinguishable from requesting it as a Float, so no provider in this language can satisfy that scenario — not through a defect, but because the distinction does not exist.Every JS suite therefore leaves the capability undeclared and the scenario is reported as skipped. That is the honest outcome, but it means the capability's meaning is language-dependent in a way the specification does not currently acknowledge, which is worth a decision upstream rather than a quiet omission in one implementation. Raised on spec#417.
A note in JavaScript's favour
The Go and Python SDKs' in-memory providers cannot update their flag set or emit
PROVIDER_CONFIGURATION_CHANGED, which Appendix A requires (go-sdk#530, python-sdk#620). Both TCKs had to ship a wrapper class to get any coverage of the configuration-change path.JavaScript's has
putConfigurationand emits the event, so this implementation needs no wrapper and the in-memory suite declaresConfigurationChangedirectly. It is the reference behaviour the other two should grow — worth saying out loud, since the findings so far have all pointed the other way.Self-tests
inMemory.spec.tsInMemoryProvidermultiProvider.spec.tsMultiProviderwrapping one childinProcessControl.spec.tsInProcessControlmultiProvider.spec.tswraps exactly one child deliberately: the correct answer is precisely what the in-memory suite already asserts about the child alone, so any difference is attributable to the multi-provider and nothing else. The Java equivalent found a real bug this way (java-sdk#1882); whether this one forwards child events is exactly what this suite is here to find out.It found one, and the suite is excluded until it is fixed. The subject is
@openfeature/server-sdk'sMultiProvider, not this repository's@openfeature/multi-provider, which is deprecated in favour of it — testing a package nobody should adopt would prove little, and Go and Java already test their SDKs' implementations rather than their contrib ones. Reaching it needs the workspace devDependency at^1.23.0; 1.19.0 predates the export, and every provider's peer range (^1.17.0) already admits the bump.It fails 16 of 29 scenarios, all one cause: the multi-provider replaces the child's error code with
GENERAL— 15 ×TYPE_MISMATCHand 1 ×FLAG_NOT_FOUND. The code is discarded rather than lost, sincecollectProviderErrorsbuilds anErrorWithCodecarrying the child's real code andconstructAggregateErrorthen wraps it in anAggregateError extends GeneralError. Filed as js-sdk#1452 with a four-line reproduction that needs no test framework.Everything else passes, which is the useful half: evaluation, variants, reasons and configuration-change events all survive delegation intact. So the spec file is kept and excluded via one line of
jest.config.ts— it is the regression test for that fix. Go's equivalent suite passes against its SDK's multi-provider, which is the outcome to expect here once js-sdk#1452 lands.Verification
nx test provider-tck/nx lint/ buildautoBindSteps,loadFeatures,Options.tagFilter/scenarioNameTemplate,ParsedScenario.skippedViaTagFilter→test.skip, data tables parsed toArray<Record<string,string>>, tags parsed lowercased and with@, step args = capture groups then the step argument lastOpenFeature.setProviderAndWait/getClient/close/clearProviders,client.addHandler,client.providerStatus,ServerProviderEvents,ProviderStatus, the typedget*DetailscallsMultiProviderconstructor@openfeature/server-sdk—ProviderEntryInput[].gitattributesOne thing I deliberately did not write: a unit test asserting the configuration-change event payload via the SDK's event emitter, because I could not verify that emitter's API without a toolchain. The payload is already asserted end-to-end by the
@configuration-changescenario, which runs in the in-memory suite.What I most expect CI to find: a TypeScript strictness error, and the exact arity jest-cucumber expects for step functions (it treats a surplus parameter as a
donecallback, which would hang rather than fail loudly — I kept every arity exact, but that is the sort of thing a build catches and reading does not).Known gaps
getGherkinTestPathin@openfeature/flagd-core. Correct for consumers inside this Nx workspace — every current adopter — but an external npm consumer would need the packaged copy. Worth revisiting when one exists.Open questions
libs/shared/provider-tckthe right home, alongsideflagd-coreandofrep-core?flagd-core's precedent, but bakes a path into a published package. Keep the precedent, or resolve from__dirnameagainst the packaged assets?@strict-numeric-typingbeing unsatisfiable in this language — handle it per-implementation as here, or does Appendix F need to say something about capabilities that are language-dependent?