test(flagsmith): run the provider conformance suite against the testbed - #1849
Draft
aepfli wants to merge 5 commits into
Draft
test(flagsmith): run the provider conformance suite against the testbed#1849aepfli wants to merge 5 commits into
aepfli wants to merge 5 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
5 times, most recently
from
September 13, 2026 07:21
50b5ba8 to
a97a728
Compare
Experimental adoption. 52 scenarios: 20 pass, 12 fail, 20 skipped. Eight of the twelve failures are 'expected STATIC but was null' -- this provider never populates the resolution reason. The values are all correct; only the reason is missing. 2.2.5 makes it a SHOULD, so null is arguably permitted, but the Go and Python Flagsmith providers both populate it against the identical backend. Two more are float resolution. Flagsmith stores floats as strings, because feature_state_value is natively boolean, integer or string only. Go's provider parses the string back; this one type-checks and falls back to the code default, so GetFloatValue never works against Flagsmith. The last two are shared with every other language: float-flag and object-flag requested as a String succeed, because on this backend both really are strings. @large-integers is withheld deliberately: Java's accessor is a 32-bit Integer, so 2^53-1 cannot be asked for. Same reason Java withholds it for flagd, and it accounts for the extra skip against Go's 19. The Jackson pin is a workaround for a TCK-introduced conflict, not a provider defect: provider-tck exports jackson-databind 2.22.1 while flagsmith-java-client pins jackson-annotations 2.15.2, and the client's ObjectMapper then dies on JsonSerializeAs. Verified by removing the TCK dependency, after which the provider's own 35 tests pass. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Flagsmith's native model is `enabled` plus a value, so the canonical set's four disabled-* flags map straight onto it. The testbed grew the flags in the same pass. 56 scenarios: 24 pass, 12 fail, 20 skip. The four new scenarios pass; the twelve failures are unchanged. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The tool moved from tools/provider-tck to tools/tck and its package from dev.openfeature.contrib.tools.providertck to dev.openfeature.contrib.tools.tck. Dependency and imports follow. The explicit testcontainers dependency is now load-bearing rather than redundant: the TCK made its own provided and optional, so an adopter declares it. The Jackson pin stays, and was re-tested rather than assumed -- removing it still fails with ClassNotFoundException on JsonSerializeAs, so the TCK still exports databind 2.22.1 against the client's annotations 2.15.2. The comment is corrected: it is compile scope and has to be, because the provider's own main source imports com.fasterxml.jackson.databind. Saying 'test scope only' was wrong. 56 scenarios: 24 pass, 12 fail, 20 skip. Unchanged by the rebase. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…onvention The compose file moves to src/test/resources/tck/docker-compose.yaml, the path the TCK javadoc uses, and the image is pinned to 0.1.0 rather than :latest. Pinning is the substantive half. Four language adoptions pull this image, and a mutable tag lets a push to the testbed change four pull requests' results with no diff anywhere to explain it. The tag stays overridable through FLAGSMITH_TESTBED_IMAGE. 56 scenarios: 24 pass, 12 fail, 20 skip. Unchanged. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The base added @standard-reasons, and it is the capability that turned eight of this provider's twelve failures into skips: it never populates the resolution reason, so every evaluation returns null and not one of the reasons the capability claims is reported. The resolved values were correct throughout -- only the reason was missing. Withheld with a deviation rather than simply skipped, because 2.2.5 makes the reason a SHOULD and null is arguably permitted. What makes it a defect worth recording is that the Go Flagsmith provider reports STATIC, DISABLED and TARGETING_MATCH against the identical backend, so this is a gap rather than a considered choice. The numeric-coercion deviation is now measured rather than predicted: this provider behaves like Python, not like Go. float-flag resolves to the caller default, so reading a float back as a float does not work against Flagsmith at all. 65 scenarios: 31 pass, 5 fail, 29 skip -- up from 24/12/20. The five remaining are two unreadable floats, the shared type-system pair, and object-flag not resolving as a structure. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
aepfli
force-pushed
the
feat/provider-tck-flagsmith
branch
from
September 13, 2026 16:17
a97a728 to
0069480
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 Java provider.
Stacked on
feat/provider-tck. Companion to go-sdk-contrib#959 and js-sdk-contrib#1623 — same container, same scenarios.Result: 31 pass, 5 fail, 29 skipped of 65
* Python's TCK has not picked up
@standard-reasonsyet.@standard-reasonsis what changed this run. It was 24 pass / 12 fail before that capability existed; eight of those twelve failures were this provider not populating the resolution reason, and they are now skips carrying a recorded reason rather than bare failures.@standard-reasons— withheld, with a deviationThis provider never populates the resolution reason. Every evaluation returns
null, so not one of the reasons the capability claims is reported. The resolved values are correct throughout; only the reason is missing.2.2.5 makes the reason a SHOULD, which is exactly why this is a withheld claim rather than a failure. What makes it a defect worth recording is that the Go Flagsmith provider reports
STATIC,DISABLEDandTARGETING_MATCHagainst the identical backend — so this is a gap rather than a considered choice.The 5 remaining failures
feature_state_valueis natively boolean, integer or string — so every float is stored as a string. Go's provider parses it back ("Because We store floats as string"); this one type-checks and falls through to the caller default, sogetFloatValuenever works against Flagsmith. Python has the same gap; the@numeric-coerciondeviation now records this as measured rather than predicted.float-flagandobject-flagrequested as a String succeed rather than reportingTYPE_MISMATCH, because on this backend both genuinely are strings. Not a provider bug in any of the four languages.object-flagnot resolving as a structure.@large-integers— refused by the TCK, and correctlyThe base now refuses this capability in Java rather than leaving it to the adopter: Java's integer accessor is a 32-bit
Integer, so 2^53−1 cannot be asked for at all. That is a property of the SDK, not of the provider, and it is the same reason Java withholds it for flagd.Worth knowing alongside it:
Value.asInteger()silently truncates aLongto its low 32 bits —4294967301becomes5,2^53−1becomes−1— with no error. That is a defect in the Java SDK rather than anything this PR touches, but it is why the refusal matters: declaring the capability would not have errored, it would have returned a plausible wrong number.Other capabilities
Declared:
@object,@targeting,@disabled-flags.@disabled-flagsholds because Flagsmith models a feature state asenabledplus a value, so the canonical set's fourdisabled-*flags map straight onto it, and this provider returns the caller default with reasonDISABLED. Go agrees; Python and JS both raiseGENERALand withhold it.@variantsis withheld: Flagsmith has no variant concept for a plain feature, and the evaluation response carries no variant key. Permitted rather than defective — 2.2.4 makes it a SHOULD — so no deviation.The lifecycle and event capabilities are withheld because this provider has no observable initialisation for the suite to assert against.
A finding about the TCK, not the provider
tools/tckexportsjackson-databind 2.22.1, whileflagsmith-java-client7.4.3 pinsjackson-annotationsandjackson-coreat2.15.2. Maven's nearest-wins assembles databind 2.22 against annotations 2.15, and the Flagsmith client'sObjectMapperdies withClassNotFoundException: com.fasterxml.jackson.annotation.JsonSerializeAs.Verified this is the TCK's doing: with the TCK dependency removed the provider's own 35 tests pass; with it added, 22 error. Re-tested after the TCK moved testcontainers to
provided/optional— still present, so it wants its own fix upstream: shade Jackson, or stop exporting a hard databind version.The pin here is the workaround. It is compile scope and has to be, because the provider's own main source imports
com.fasterxml.jackson.databind. It cannot be aligned upward —jackson-annotationshas no 2.22.1 release, it tracks its own version line — so that route needs the Jackson BOM.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