Repository navigation
test(flagd): adopt the OpenFeature Provider TCK - #1847
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Compose environment configuration tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ContainerizedProviderTckTest.java, tools/tck/README.md |
ContainerizedProviderTckTest provides a Compose environment map and applies its entries before starting the stack. The README documents this option. |
Provider TCK setup providers/flagd/pom.xml, providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/ConfigCucumberTest.java, providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/AbstractResolverTest.java |
The flagd module adds the TCK test dependency and shared resolver test configuration. The Cucumber suite explicitly selects PicoFactory. |
Resolver suites and documented results providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/tck/{RpcTest,InProcessTest,FileTest}.java, providers/flagd/README.md |
Adds TCK suites for RPC, in-process, and file resolvers. The README documents commands, profiles, CI behavior, and results against flagd-testbed v3.11.0. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Other
Sequence Diagram(s)
sequenceDiagram
participant FileTest
participant ContainerizedProviderTckTest
participant ComposeStack
participant flagdFile
FileTest->>ContainerizedProviderTckTest: Provide FLAGS_DIR through composeEnvironment()
ContainerizedProviderTckTest->>ComposeStack: Apply environment entries with withEnv
ContainerizedProviderTckTest->>ComposeStack: Start stack
ComposeStack->>flagdFile: Provide FLAGS_DIR to file resolver
Possibly related PRs
- open-feature/java-sdk-contrib#1959: Changes flagd numeric coercion and the testbed version used by the documented TCK results.
- open-feature/java-sdk-contrib#1957: Changes in-process numeric narrowing covered by the in-process TCK suite.
- open-feature/java-sdk-contrib#1953: Changes shared TCK execution and flagd suite settings used by these suites.
Merge Risk: 
8b661
-
The TCK integration is mergeable with owner awareness, but its documentation should qualify the claim that every failure is a provider regression; testbed readiness can also affect the result.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely identifies the main change: adopting the OpenFeature Provider TCK for flagd. Description check The description directly explains the TCK adoption, supported resolver modes, configuration changes, exclusions, and reported test results. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 6 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
github-actions
Bot
requested review from
Kavindu-Dodan,
beeme1mr,
thisthat and
toddbaert
September 11, 2026 10:34
aepfli
marked this pull request as draft
September 11, 2026 10:40
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
2 times, most recently
from
September 11, 2026 13:44
20cf419 to
177ebe0
Compare
aepfli
force-pushed
the
feat/provider-tck-flagd
branch
15 times, most recently
from
September 13, 2026 21:04
1e3c1dc to
be8ed17
Compare
The TCK gained @standard-reasons, and this suite declares every declarable capability except @reinitialization and @large-integers -- so it picked the new tag up by default and started running reason.feature without anyone deciding that it should. That is the right answer here, but it was measured before it was written down rather than after. All nine scenarios pass in both modes, including the two that compose with @targeting and @disabled-flags: STATIC for the rule-less flags, TARGETING_MATCH and DEFAULT either side of targeting-key-flag's rule, DISABLED for a disabled flag, and ERROR beside FLAG_NOT_FOUND and TYPE_MISMATCH. So the claim the tag makes -- the standard vocabulary with the standard meanings -- holds for the RPC resolver and the in-process one alike. Each mode is now 65 scenarios, 59 passing, 2 skipped and 4 failing, up from 56 and 50. The four failures are the same four as before and none of them is new: the lossy numeric coercion that flagd#1996 tracks, and the three flags flagd-testbed v3.8.0 does not serve. The cold-start initialisation error recorded on CONNECTED_DEADLINE_MS did not reproduce in this run; it is intermittent and host-dependent, so the note stays and the count says which run it comes from. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
capabilities() is now declarableExcept(REINITIALIZATION). @large-integers goes, and its going is the whole point: it was never a decision this suite took. flagd does not decline to resolve 2^53 - 1 -- Client.getIntegerDetails is a 32-bit Integer, so no Java provider can be asked, and the TCK refuses the capability centrally rather than having each adoption remember. The paragraph that used to explain it here was one of four saying the same thing about the same language. @reinitialization stays withheld, and stays this suite's call. That one is a fact about flagd: FlagdProviderSyncResources keeps isInitialized and isShutDown as separate flags and refuses initialize() when either is set, which Requirement 2.5.2 permits, so withholding is the honest report and no KnownDeviation is owed. Nothing about the SDK's accessor has any bearing on it, which is why one moved and the other did not. Measured, both resolvers, on the pinned testbed image: 65 scenarios, 59 passed, 2 skipped, 4 failed -- identical to the previous pass, because this changes why a scenario is skipped rather than whether it is. The two skips now read differently in the results, which is the observable part: Skipped: provider does not declare capability REINITIALIZATION (tag @reinitialization). Declared capabilities: [...] Skipped: the Java SDK cannot express capability LARGE_INTEGERS (tag @large-integers) - Client.getIntegerDetails takes and returns a 32-bit Integer ... not the provider under test declining A reader of the report can now tell which of the two absences describes flagd. The four failures are unchanged: flagd#1996's lossy coercion plus the three flags testbed v3.8.0 does not serve. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…hand The javadoc beside knownDeviations() argues from first principles that declaring @numeric-coercion is right even though one of its three scenarios asks for a flag the pinned testbed does not serve. Appendix F now states that as a rule, so the argument belongs upstream and the citation belongs here. Both halves of it apply in order, which is worth spelling out because the first half is what keeps the rule from over-reaching: flagd is attempting the coercion -- the widening scenario passes, which is the evidence -- and two of the three scenarios can be put to it. A provider that does not coerce at all stops at the first half and withholds, as the SDK's in-memory provider does. Also names the appendix's other consequence as the reason the deviation summary names the testbed: a scenario that fails because the backend cannot serve its fixture is not a provider defect, and a summary that did not say so would leave the report accusing flagd of the stack's gap. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
`mvn --projects providers/flagd -P e2e test` does not run the conformance suites. The `e2e` profile is the thing that keeps them out: it narrows the exclusion to **/e2e/*TckTest.java so the legacy Run*Test suites run and the TCK ones do not. Verified by running it rather than reading the POM - 788 tests in 12:18, RunFileTest, RunInProcessTest and RunRpcTest, and not one mention of either TckTest class anywhere in the log. Appendix F now asks for a step of its own rather than a corner of an existing e2e suite, because of what a red build says: `-Pe2e` red means the provider's own end-to-end suites regressed, while `-Ptck` red means conformance failed, and a conformance run carries failures by design wherever AbstractFlagdTckTest declares a knownDeviation. The new `tck` profile clears the exclusion and narrows Surefire's includes to **/e2e/*TckTest.java in the same breath, which is what makes it a conformance step and not a wider one: `mvn -Ptck -pl providers/flagd test` runs FlagdRpcTckTest and FlagdInProcessTckTest and nothing else - 130 tests, 65 scenarios per resolver, 59 passing, 2 skipped and 4 failing in both. Nothing activates the profile in CI, for the reason the `e2e` profile's comment already gives. The exclusion stays Surefire's and not the compiler's, so both suites still compile in the default build. The README says not to add `-am` to that run, and why: it pulls tools/tck and tools/flagd-core into the reactor and runs their suites too, so a failure in either comes out as a `-Ptck` failure. A one-off `install` is what `-am` was there for. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The two conformance suites lived under e2e/ and were selected out of it by
filename: `**/e2e/*TckTest.java`, in three places. They now live in a sibling
package, `src/test/java/.../flagd/tck/`, and every selector names the directory
instead.
Nesting them under e2e/ said "this is a kind of e2e test", which is the
conflation the separate `tck` step exists to undo. The two suites answer
different questions -- e2e tests this provider against flagd's own test harness,
tck tests it against the OpenFeature provider contract -- and they mean
different things by a red run: an e2e suite is expected green, while a
conformance suite fails scenarios by design wherever a knownDeviation is
declared. A file is now in the conformance directory or it is not, and no
convention about class names holds that line.
So the names drop what the directory now says. FlagdRpcTckTest in package
...flagd.tck said "tck" twice and "flagd" twice; it is RpcTest, beside
InProcessTest, over AbstractResolverTest. The `*Test` suffix stays because
Surefire's default includes need it -- that is not the selector being removed.
The three selectors, all directory-shaped now:
* the module's default testExclusions gains tck, so both Docker-dependent
packages stay out of `mvn verify`;
* the `e2e` profile drops e2e from the exclusion and keeps tck, where it used
to narrow to a filename pattern;
* the `tck` profile is its mirror image -- drops tck, keeps e2e, and includes
`**/tck/*.java`.
Both halves of the `tck` profile are still needed, for the reason they always
were: the include alone leaves the exclusion in force and runs nothing, and
dropping the exclusion alone runs the module's unit tests alongside the suites.
testExclusions is still a Surefire and not a compiler exclusion, so both
packages still compile in the default build.
RpcTest and InProcessTest now state their configuration() rather than deriving
it. The default derivation reads the class name, so the rename would have filed
their runs as "rpc" and "in-process" instead of "flagd-rpc" and
"flagd-in-process" -- a report is read away from this repository, where the
provider's name is the half that matters. The suites are otherwise unchanged.
Unchanged too: 65 scenarios per resolver, 59 passed, 2 skipped, 4 failed, in
both modes. Same scenarios, same results, a different directory.
The profile comment documenting the run had kept `-am` on it, which the READMEs
already warn against; it says the command that works now.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The TCK section was 5.5 KB of a 28 KB provider README, and most of it was the base README's or Appendix F's: why an adoption suite is excluded rather than gating, why it gets a step of its own, what a red conformance build says, why both halves of the tck profile are needed, and why the -am the command must not carry would mix signals. What is left answers the three questions an adoption README owes a reader. What this provider declares and why each absence is what it is - by pointing at AbstractResolverTest, where the reasoning sits next to the declaration and is measured rather than asserted. What the tally is and which failures are expected - 65 scenarios, 59/2/4 per resolver, three testbed gaps and one real defect. And the command that runs it. One local fact stays at length because it is a trap and is recorded nowhere else: -Pe2e does not run these suites, it is the profile that keeps them out, and it used to clear the exclusion outright and run them red in CI on every unrelated pull request. 2.4 KB, from 5.5. The adoption now adds 38 lines to this README rather than 85. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…round it
Stripped of comments the two TCK Compose files in this repository were
the same stack: same image, same service name, same absence of host
bindings, differing only in which ports each listed. Two hand-maintained
copies is the mechanism by which they drift onto different images while
both claiming to ask the same questions, so there is now one file at
tools/flagd-testbed/docker-compose.yaml exposing 8013, 8015, 8016 and
8080. Extra ports cost nothing: with no host bindings the TCK maps each
container port dynamically and resolves only the ones a suite asks for,
so the flagd suites never look up 8016.
Placed outside both provider modules rather than in either, so neither
reaches into the other's test tree and a third adoption against the same
backend reads the same path. That costs a tools/ directory which is not
a Maven module -- no pom, no CHANGELOG, nothing released -- and a
composeFile() that climbs out of its own module, which both suites now
explain rather than leaving to look like a mistake.
The comment is down from 29 lines across two files to 13 in one, and
keeps only what is a trap here: that it is NOT the legacy e2e stack at
providers/flagd/test-harness, which bind-mounts ${FLAGS_DIR}, names its
service "flagd" and runs an envoy sidecar. Which flags the image does
not serve belongs to open-feature/flagd-testbed#392, and which scenarios
that costs is in AbstractResolverTest.
AbstractResolverTest loses the testbed-gap essay, the falsy-flag rename
history and two paragraphs about what an earlier revision of the file
declared. Every measured probe stays: the @numeric-coercion three-way
result, the @reinitialization and @Stale source-line evidence, the
@disabled-flags and @standard-reasons runs, and both findings recorded
on CONNECTED_DEADLINE_MS.
Comments and YAML comments only. No behaviour change.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Capability.DISABLED_FLAGS no longer frames the gate as where evaluation happens, so the sentence here that echoed it says what was actually measured instead: both resolvers are told the flag is disabled. Comments only. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Measured this pass: an RPC run came back with a fifth failure, FLAG_NOT_FOUND on an evaluation.feature row expecting no error code, and the next run of the same tree was clean. Same shape the OFREP adoption already records against open-feature/flagd-testbed#394, so it is named here rather than left for the next reader to diagnose as a regression. Comments only. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Four languages pinned this image by hand and had drifted: JavaScript was already on v3.10.1 while Go, Java and Python sat on v3.8.0, so the suites whose results are only comparable if they asked the same backend were asking two. v3.10.1 is the current release, so aligning up rather than down. What this does NOT fix, stated because the tag is easy to mistake for a fix: open-feature/flagd-testbed#392 and #394 are both still open, so v3.10.1 carries neither the three missing precision flags nor the /start readiness fix. The fixture failures and the readiness race are unchanged. The one behavioural change in range is open-feature/flagd-testbed#390, which increases the simulated downtime -- and that is exactly the timing the @Stale and @unavailable scenarios depend on. Tallies recorded against v3.8.0 have not been re-measured on this image, including the failure ranges the adoption READMEs cite by tag; those figures stay as they are because they are a record of what v3.8.0 did, and re-taking them is follow-up work rather than a rewrite. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The four scenarios the new tag gates were mandatory and passing before it existed, so the "everything except @reinitialization" default already declares it. Recorded here anyway, because this file's standard is that each declaration rests on evidence from a run and not on inheriting the default. Measured in both resolver modes: 65 scenarios, 2 skipped, and the same four failures as before -- the lossy numeric coercion (open-feature/flagd#1996) and the three flags the pinned testbed image does not serve (open-feature/flagd-testbed#392). None of the four @string-typing scenarios is among them. flagd's flag definitions carry a JSON type per flag and both resolvers preserve it, so a non-string flag asked through the String accessor is a real mismatch here and is reported as one. RPC additionally showed the intermittent "half" variant failure this branch already records -- surefire's reruns had it pass four times in five, which is the testbed readiness window of open-feature/flagd-testbed#394 and not a property of any assertion. The clean-run tally is unchanged. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Appendix F split @string-typing at bda599f1: the outline keeps boolean-flag and integer-flag, and float-flag and object-flag moved behind a new @fully-typed-values that asks whether the store records a native type for those two as well. flagd is the case the split was not written for, and declaring both is how that shows. Its flag definitions carry a JSON type per flag and both resolvers preserve it, so all four questions have the same answer here. A partially typed backend declares the first tag and withholds the second; there is nothing partial about this one. declarableExcept(REINITIALIZATION) already picks the new tag up, so this is javadoc rather than a declaration change — but the tag was measured rather than inherited, which is this file's standing rule. Both modes after the re-pin: 65 scenarios, 59 passing, 2 skipped, 4 failing, identical to the run before it. The two skips are still the withheld @reinitialization and @large-integers; the surefire report has "A float flag is not returned as its string representation" and "A structured flag is not returned as its JSON text" as executed and passing, which is the part a tally alone would not have shown — an undeclared @fully-typed-values would have turned both into skips and left the failure count untouched. The four failures are unchanged and none is a string-typing scenario: the lossy numeric coercion (open-feature/flagd#1996) and the three flags the pinned testbed image does not serve (open-feature/flagd-testbed#392). Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
v3.10.3 is the first testbed release carrying flagd v0.17.0, which includes "wait for flag state before reporting ready" (open-feature/flagd#2050, closing open-feature/flagd#2047). That defect returned 200 from /readyz while the flag store was still empty, on 100% of starts, and is the cause behind the intermittent FLAG_NOT_FOUND runs recorded in these suites. v3.10.2 looked current but shipped flagd v0.16.3; the base-image bump landed after that tag was cut. This also picks up the TLS SANs fix (flagd-testbed#421). Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Cut specification rationale, TCK-library vocabulary, testbed behaviour and PR-review narrative: each of those facts belongs in the repository that owns the thing it describes, and a restatement here only drifts. Kept why each capability is declared or withheld, the deviation claim with its tracking issue, the measured tallies, and why the two gRPC deadlines are the values they are. AbstractResolverTest goes from 180 comment lines to 107. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Adopting the TCK puts a second Cucumber ObjectFactory on this module's test classpath: the TCK registers its own through META-INF/services, and cucumber-picocontainer, which the TCK brings transitively, registers PicoFactory. Cucumber refuses to choose between them. The three e2e suites already name PicoFactory for this reason; ConfigCucumberTest did not, so it failed with "More than one Cucumber ObjectFactory was found on the classpath" once this branch rebased onto a base that ships the TCK's factory. 96 scenarios, all passing. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The root pom now excludes and selects every provider's tck package, so this module drops its own tck profile and keeps only the e2e exclusion and its tck.testFailureIgnore setting. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The suite drove a compose file kept beside it in tools/flagd-testbed, which named the image itself. That is a second pin for the same backend, and it drifted: it still said v3.10.3 after the submodule had moved to v3.10.4, so bumping the submodule changed nothing for these suites while appearing to. Three of the four expected failures were waiting on flags the image already served. The testbed ships a compose file. Driving that one, from the submodule the legacy e2e suites already run, leaves one pin -- the submodule -- and the suites follow it. Its flagd service is named flagd rather than backend, hence the backendService override. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Measured on v3.11.0, 65 scenarios and 2 skipped in both modes: RPC passes all 63, in-process passes 62. The three scenarios that failed on flags the testbed did not serve now pass, and so does the lossy coercion case on RPC -- flagd has enforced that half of the rule since v0.18.1, and RPC evaluates in flagd. In-process evaluates here and still truncates: FlagdCore turns a Double into an int with intValue() and no losslessness check, so float-flag (0.5) resolves as 0 rather than reporting TYPE_MISMATCH. So the deviation is no longer shared, and it moves from the base class to InProcessTest. It also is not flagd's any more; the issue stays as the URL because that is where the rule is settled, and the summary now names the line that breaks it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
Member
|
Rebasing and cleaning this up. |
…eclaring it FlagdCore no longer narrows a Double with intValue() (#1957), so in-process reports TYPE_MISMATCH where it used to discard the fraction. The declaration said to delete it once the conversion checked for loss; both modes now pass all 63 scenarios, and the README and javadoc said 62. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The measured tallies, the per-capability justifications and the full history of how the deadline was arrived at were a PR description living in the file. What is left is what a reader of the code cannot derive from it: why the deadline is generous, why the port is not one of the stack's, and why reinitialization is withheld. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A stack whose state the test JVM has to read, and not only the containers -- a flag file on a bind mount, a dump directory -- needs to be told where on the host to put it, and only the test knows. Nothing in the suite could reach the Compose invocation's environment, so the first such adopter had no way to say it. composeEnvironment() returns an empty map by default, so every existing suite builds its stack exactly as before. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
flagd has three resolvers and the TCK adoption covered two. The third one reads its ruleset off the disk rather than over gRPC, so the testbed has to write it somewhere this JVM can see: composeEnvironment() names a directory under the module's gitignored tmp/, the compose file mounts it at /flags, and the launchpad's allFlags.json lands there. The legacy e2e suite has reached the same arrangement through ContainerEntry all along. AbstractResolverTest keeps the deadlines, the customization seam and the rest, and gains two overridable methods for where the flags come from -- the gRPC pair's defaults are the expressions that were inline before, so RpcTest and InProcessTest build the same providers they did. Measured on flagd-testbed v3.11.0, not assumed. Declaring everything but @reinitialization and deselecting nothing left 19 failures, and every one was either a scenario written for a socket or the single @Stale scenario: - @ext-connection is excluded. Those scenarios turn on a gRPC connection -- a custom certificate, a lost stream, a 403 from envoy -- and a file reader has none, so five timed out and one passed without using the certificate for anything. - @metadata-provider is excluded: it names a selector- flag file, which the launchpad deliberately keeps out of the combined allFlags.json. - @fractional-v2 is excluded as in the other two suites. - STALE is withheld. The provider can go stale, from the same onError that serves the other two, but the only outage the control API can stage is stopping flagd's listeners and a file on disk does not notice. @configuration-change holds, which was the one worth checking: the control API rewrites the mounted file and a 100ms poll interval picks it up, well inside the event timeout that the 5s default would have blown. 353 scenarios, 290 passing, 63 skipped, nothing failing. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The call was hand-written while resolving a rebase, so palantir wrapped it the other way round and spotless:check failed the Java 21 build. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
toddbaert
force-pushed
the
feat/provider-tck-flagd
branch
from
November 2, 2026 15:59
42a43d5 to
368ad4b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert
approved these changes
Nov 2, 2026
Member
|
🥳 🥳 🥳 |
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.
Adopts the conformance suite in the flagd provider, in all three resolver modes. Split out of
#1830 so the suite and its first adopter are reviewed as separate questions: whether the TCK is the
right contract, and whether flagd satisfies it.
What it is
A shared abstract base and three subclasses. The TCK brings its own Gherkin, step definitions and
Compose lifecycle, so a mode needs no registration and no build configuration.
tck/AbstractResolverTest— shared configuration and the capability declarationtck/RpcTest,tck/InProcessTest— the two gRPC modes, differing only in resolver and porttck/FileTest— reads the ruleset off the disk, so it needs the stack's flag file visible to thisJVM.
ContainerizedProviderTckTest.composeEnvironment()is added for that: the suite names a hostdirectory, the testbed's Compose file bind-mounts it at
/flags, and the launchpad writesallFlags.jsonthere. It is the only new API, and it defaults to empty.pom.xml— the TCK test dependency, the e2e exclusion, andtck.testFailureIgnore(leftfalse).The backend is the flagd-testbed's own
docker-compose.yaml, from thetest-harnesssubmodulethe legacy e2e suites already run, so the image tag follows the submodule pin instead of a second copy
that can drift. The ~18 other files in
providers/flagd/.../e2e/are untouched.The adoption is a directory, not a naming convention. It lives in
…/flagd/tck/beside the e2epackage rather than inside it, and every selector is a path: the root pom's
**/providers/*/tck/*.java, and**/e2e/*.javafor this module's own exclusion.testExclusionsisa Surefire and not a compiler exclusion, so the package still compiles in the default build — verified
by referencing a
Capabilityconstant that does not exist and watching the default build fail.What it declares
Capability.declarableExcept(REINITIALIZATION)in all three modes, andFileTestwithholdsSTALEon top of that.@reinitializationis a choice the specification offers, not a gap. Requirement 2.5.2 says aprovider SHOULD revert to its uninitialized state after shutdown, and its supporting text says "some
providers MAY allow reinitialization from this state". Permitted, not required.
shutdown()neverclears
isInitialized, so a shut-down flagd provider is terminally shut down.STALEis withheld for the file resolver only, and it is a fact about the backend. The providercan go stale, but the only outage the control API can stage is stopping flagd's listeners, and a file
already on disk survives that — so nothing would cause the event the scenario waits for.
@large-integersis not in the list to remove: Java's integer accessor is a 32-bitInteger, so2^53 − 1 cannot be asked for by any Java provider. That is a property of the SDK rather than of flagd,
and the TCK refuses the capability outright instead of expecting each adopter to know it. v3.11.0
added that scenario to both of the testbed's Gherkin trees, so this PR also excludes the tag from
the three legacy e2e suites and from
FlagdCoreEvaluatorTest.No known deviations. The
@numeric-coercionone is gone: #1957 fixed the narrowing it recorded,and the
knownDeviations()override is deleted rather than left asserting a defect that no longerexists.
Running it, and the tally
mvn -Ptck -pl providers/flagd -am testOn
flagd-testbedv3.11.0, 65 scenarios per suite, nothing failing: 63 pass in each gRPC mode(2 skipped) and 62 in file mode (3 skipped) —
Tests run: 195, Failures: 0, Errors: 0, Skipped: 7.Any failure is a regression.
If a run reports far more failures than expected, check for a leftover testbed stack first.