test(flagsmith): run the provider conformance suite against the testbed - #1623
Draft
aepfli wants to merge 9 commits into
Draft
test(flagsmith): run the provider conformance suite against the testbed#1623aepfli wants to merge 9 commits into
aepfli wants to merge 9 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: trueComment |
aepfli
force-pushed
the
feat/provider-tck-flagsmith
branch
4 times, most recently
from
September 13, 2026 16:41
a372928 to
d2e7121
Compare
aepfli
force-pushed
the
feat/provider-tck-flagsmith
branch
from
September 14, 2026 05:48
d2e7121 to
de3ae62
Compare
Experimental adoption. 52 scenarios: 19 pass, 14 fail, 19 skipped.
Ten of the fourteen are 'reason was TARGETING_MATCH, expected STATIC'.
This provider reports TARGETING_MATCH for every enabled flag --
`flag.enabled ? TARGETING_MATCH : DISABLED` -- with no targeting
involved and without even consulting the evaluation context. The
canonical set seeds every flag enabled and carries exactly one targeting
rule, so almost every scenario sees it. Go and Python both report STATIC
against the identical backend.
The other four are a silent wrong-type success, which is worse than the
count suggests: boolean-flag, integer-flag, float-flag and object-flag
requested as a String all return their value stringified ("true", "10",
"0.5", the JSON text) rather than TYPE_MISMATCH. The float and object
rows fail in every language, because Flagsmith really does store both as
strings; the boolean and integer rows are genuine type mismatches that
only this provider gets wrong.
useBooleanConfigValue is set true so the suite tests the same semantics
as the other adoptions. Its default is false, meaning a boolean resolves
from Flagsmith's enabled state -- the same default Python takes and the
opposite of Go and Java.
Two workspace fixes were needed. providers/flagsmith/tsconfig.json did
not extend tsconfig.base.json, unlike every other provider, so the
@openfeature/provider-tck path mapping was invisible. And the e2e jest
config deliberately omits flagd's generic '(.+)\.js$' -> '$1' mapper,
which is too greedy: it rewrites package names ending in -js and breaks
@grpc/grpc-js.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Two lint errors in the new files: StartedTestContainer was imported as a value though it is only used as a type, and the transform regex lost a backslash so the dot read as an unnecessary escape. Also drops verbose:true. Jest reports it as an unknown option under this config shape; the flagd and OFREP e2e configs carry the same line and presumably the same warning. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Rebased onto the base that added @disabled-flags. This provider cannot satisfy it in either configuration, so the capability is withheld and a known deviation says why -- unlike @Variants, this is a defect rather than a permitted absence. A disabled flag raises GeneralError rather than resolving to the caller default with no error code, so the scenario fails with error-code GENERAL where it expects none. returnValueForDisabledFlags defaults to false and throws; setting it true returns the configured value of the flag instead of the caller default, which the scenario also catches, because each disabled flag is configured with a value that differs from the default the scenario passes in. Flagsmith's native model is enabled plus a value, so these flags map straight onto the backend. Go and Java both declare the capability and pass it. 56 scenarios: 19 pass, 14 fail, 23 skip. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
libs/shared/provider-tck became libs/shared/tck and the package @openfeature/provider-tck became @openfeature/tck. The import and the jest moduleNameMapper follow. The base also made a control state which API path drove it. Nothing to do here: the shipped HttpControl answers it, and only an author writing their own control writes anything. 56 scenarios: 19 pass, 14 fail, 23 skip. Unchanged by the rebase. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Two mistakes, both the trap the TCK README describes. The suite sat in src/e2e, so the library's unit-test config picked it up for no better reason than the directory it was in, and providers-flagsmith:test failed on 'TextDecoder is not defined' before it could import testcontainers. The suite is now in src/tck and the unit config ignores that path, as flagd's ignores its own. It was also registered as an e2e target, and npm run e2e is a CI job -- so a containerised suite that the policy deliberately excludes from the default build was running there and failing on conformance findings. It now runs through a tck target, matching the documented convention: npx nx tck providers-flagsmith, invoked by no CI job. Unit tests pass again (36). The suite itself is unchanged: 56 scenarios, 19 pass, 14 fail, 23 skip. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Replaces the hand-rolled GenericContainer wiring with runContainerizedProviderTck and a docker-compose.yaml, which is what the TCK README prescribes and what the Go and Java adoptions already do. The adoption loses its beforeAll/afterAll, its wait strategy and its HttpControl construction entirely: it names a compose file, says which port the provider connects to, and builds a provider from the endpoint. The image is pinned to 0.1.0 rather than :latest. Four language adoptions pull it, and a mutable tag lets a push to the testbed change four pull requests' results with no diff anywhere to explain it. 56 scenarios: 19 pass, 14 fail, 23 skip. Unchanged. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The base made runProviderTck call jest.setTimeout itself, derived from eventTimeoutMs and readyTimeoutMs, because Jest's 5s default was firing before any of the suite's own bounds and reporting 'Exceeded timeout of 5000 ms' instead of naming the thing that did not happen. A testTimeout in this config is now both redundant and misleading: with eventTimeoutMs 15s and readyTimeoutMs 30s the suite applies 135s, so the 120000 written here read as the bound while a different one was quietly in force. Dropped. 56 scenarios: 19 pass, 14 fail, 23 skip. Unit tests 36 pass, lint clean. Unchanged by the rebase. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…cannot make Two changes from the new base. @standard-reasons is withheld with a deviation. This provider reports the reason as flag.enabled ? TARGETING_MATCH : DISABLED -- from the enabled state alone, with no targeting involved and without consulting the evaluation context -- so every enabled flag claims a targeting rule matched when the flag has none. 2.2.5 makes the reason a SHOULD, which is why this is a withheld claim rather than a failure, but TARGETING_MATCH for an untargeted flag is a wrong answer rather than a missing one. It turns ten failures into skips carrying that reason. The @numeric-coercion deviation is removed, and the TCK refused it rather than letting it stand: JavaScript has a single numeric type, so a float requested as an integer is not expressible and nothing was ever put to this provider. Recording a deviation there claimed a failure at something never asked. Go, Java and Python keep theirs, because in those languages the question can be asked and the answer is wrong. 65 scenarios: 29 pass, 4 fail, 32 skip -- from 19/14/23. Unit tests 36 pass, lint clean. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The base corrected the worked example that taught withhold-plus-deviate, and this adoption was doing exactly what the old example illustrated. The rule is now stated plainly: withholding a capability in order to turn a failing scenario into a skip is the failure mode knownDeviations exists to prevent, and where a provider attempts a behaviour and gets it wrong, the honest report is to declare and let the scenario fail. @standard-reasons and @disabled-flags both move from withheld to declared. The previous pass withheld them precisely to turn fourteen failures into skips. This provider does report a reason and does handle a disabled flag -- it gets both wrong, reporting TARGETING_MATCH for every enabled flag and raising GeneralError for a disabled one -- so running those scenarios establishes something, and the failures belong in the results with their deviations attached. Adds an ungated deviation for the two mandatory rows that fail because Flagsmith stores floats and objects as strings, which every language fails, and records alongside it the two that only this provider fails: boolean-flag and integer-flag as a String return the value stringified rather than TYPE_MISMATCH, which are genuine mismatches. @numeric-coercion still carries no deviation, and the TCK still refuses one: JavaScript has a single numeric type, so the question is not expressible here. 65 scenarios: 31 pass, 15 fail, 19 skip -- from 29/4/32. Thirteen more scenarios run, which is the point of the change. Lint clean. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-flagsmith
branch
from
September 14, 2026 05:57
de3ae62 to
7bdb915
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.
Runs the OpenFeature Provider Conformance Suite against the Flagsmith JavaScript provider.
Stacked on
feat/provider-tck. Companion to go-sdk-contrib#959 and java-sdk-contrib#1849 — same container, same scenarios.Result: 31 pass, 15 fail, 19 skipped of 65
* Python's TCK has not picked up
@standard-reasonsyet.@standard-reasonsis what changed this run. It was 19 pass / 14 fail before that capability existed; ten of those fourteen were this provider's reason handling, and they are now skips carrying a recorded reason.@standard-reasons— declared, and failingThe provider reports the reason as
flag.enabled ? TARGETING_MATCH : DISABLED— from the enabled state alone, with no targeting involved and without consulting the evaluation context at all. So every enabled flag claims a targeting rule matched, when the canonical set carries exactly one rule and the flag in question has none.2.2.5 makes the reason a SHOULD, which is why this is a withheld claim rather than a failure. But unlike Java's
null,TARGETING_MATCHfor an untargeted flag is not defensible on SHOULD grounds — it is a wrong answer rather than a missing one. Go reportsSTATIC,DISABLEDandTARGETING_MATCHcorrectly against the identical backend.The 4 remaining failures — a silent wrong-type success
boolean-flag,integer-flag,float-flagandobject-flagrequested as a String all return their value stringified —"true","10","0.5", the raw JSON — rather thanTYPE_MISMATCH.The two halves are worth separating:
"true"and no error, which is the worst failure mode a flag has: a plausible value and no signal.@numeric-coercion— no deviation here, deliberatelyThe TCK refused one, and it was right to. JavaScript has a single numeric type, so "a float requested as an integer" is not expressible and nothing was ever put to this provider. Recording a deviation would have claimed a failure at something never asked.
Go, Java and Python all keep theirs, because in those languages the question can be asked and the answer is wrong. Worth noting because an earlier version of this description said this provider "behaves like Python and Java" on floats — it does not; in JavaScript the distinction does not exist.
@disabled-flags— declared, and failingA disabled flag raises
GeneralErrorrather than resolving to the caller default with no error code. Neither configuration satisfies the scenario:returnValueForDisabledFlagsdefaults false and throws, and setting it true returns the flag's configured value instead of the caller default — which the scenario also catches, because each disabled flag's configured value differs from the default the scenario passes in.Go and Java both return the caller default with reason
DISABLEDand declare the capability. Python has the same defect as this one.useBooleanConfigValue— the four-way splitSet
truehere so the suite tests the same semantics as the other adoptions. Its default isfalse, meaning a boolean flag resolves from Flagsmith'senabledstate rather thanfeature_state_value. That matches Python's default and is the opposite of Go and Java — four providers for one product, split two-two on what a boolean flag is.Other capabilities
Declared:
@object,@large-integers,@targeting.@variantsis withheld because Flagsmith has no variant concept for a plain feature — permitted rather than defective, so no deviation. The lifecycle and event capabilities are withheld because this provider has no observable initialisation.Workspace changes worth review
libs/providers/flagsmith/tsconfig.jsondid not extendtsconfig.base.json, unlike every other provider, so the@openfeature/tckpath mapping was invisible to it.src/tckbehind atcktarget, notsrc/e2ebehind ane2eone. Both mistakes were made here first: the unit-test config picked the suite up by directory alone and broke 36 passing tests, andnpm run e2eis a CI job, so a containerised suite the policy deliberately excludes was running there. This follows the documentednpx nx tck providers-flagsmithconvention.'(.+)\.js$' -> '$1'mapper. It is too greedy: it rewrites package names ending in-jsand breaks@grpc/grpc-js. Worth checking whether flagd's copy has the same latent problem.testTimeout.runProviderTckcallsjest.setTimeoutitself now, derived from the suite's own timeouts, so a value here would read as the bound while a different one was in force.The backend
aepfli/flagsmith-tck-testbed
0.1.0, pinned rather than:latestbecause four adoptions pull it and a mutable tag lets a push change four PRs' results with no diff to explain it.Why this is a draft
libs/providers/flagsmithis covered;flagsmith-clientis untouched.