Skip to content

Commit b73e6ab

Browse files
committed
docs(tck): point at Appendix F for why a suite is excluded from CI
Re-pins the spec submodule to ccdb8879, which adds "Running the suite in CI" to Appendix F. The gherkin, flags and openapi assets are unchanged, so nothing in this module's behaviour moves. The CI-exclusion reasoning was promoted into the appendix precisely because four READMEs is where it drifted into three different answers. So this README now keeps only the mechanism -- the testExclusions property, the fact that the parent POM defines no default for it, that it is a Surefire exclusion and not a compiler one, what this repository's e2e profile does to it, and the help:evaluate command that resolves it -- and links to the appendix for the reasoning rather than restating it. The appendix names two mistakes, and both of them happened here: providers/ofrep never declared the property, and providers/flagd had an e2e profile clearing it while ci.yml's main job activated that profile on every push. Both are already fixed; what changes here is that the record points at the general statement instead of paraphrasing it. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
1 parent cfd84ce commit b73e6ab

2 files changed

Lines changed: 35 additions & 21 deletions

File tree

tools/tck/README.md

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,19 @@ it. That is a known SDK gap,
226226
which the suite reproduced from the outside — the gap was originally found by hand-comparing
227227
implementations against the js-sdk reference. Everything else survives delegation unchanged.
228228

229+
Two more tests are not suites at all, because what they guard is invisible from inside a scenario.
230+
[`InProcessBackendControlTest`](src/test/java/dev/openfeature/contrib/tools/tck/InProcessBackendControlTest.java)
231+
calls the unsupported operations directly, so a connection operation that quietly did nothing cannot
232+
pass as a skip.
233+
[`HttpBackendControlTest`](src/test/java/dev/openfeature/contrib/tools/tck/HttpBackendControlTest.java)
234+
stubs the control API with the JDK's own `com.sun.net.httpserver.HttpServer` — no Docker, nothing off
235+
loopback — and asserts the request sequence in order: that `/reset` is preferred and `/start` is the
236+
fallback, that an unimplemented `/reset` is probed **once per suite** and the answer cached, and that
237+
the scenario after a `disconnect()` uses `/start` rather than `/reset`. All three are normative in
238+
`openapi/control-api.yaml`, all three are decided in code no scenario can observe, and a control that
239+
got any of them wrong would let scenarios run against the previous one's backend state and report the
240+
results as conformance.
241+
229242
## Adopting it
230243

231244
This section describes a provider with an external backend — the common case.
@@ -718,39 +731,40 @@ extending `ContainerizedProviderTckTest` needs a working Docker daemon for its C
718731

719732
### Containerised suites are excluded from the default build, on purpose
720733

721-
A `ContainerizedProviderTckTest` subclass must be **excluded from the module's default test run**,
722-
and the adopting module says so in its own POM. This repository's convention is the
723-
`testExclusions` property the parent POM feeds to Surefire:
734+
**Why** an adoption suite is excluded rather than gating, and the two mistakes that exclusion
735+
invites, are written down once for all four languages in
736+
[Appendix F: Running the suite in CI](https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md#running-the-suite-in-ci).
737+
Read that first. What follows is only the Maven mechanism, which is this repository's and not the
738+
appendix's business.
739+
740+
A `ContainerizedProviderTckTest` subclass is **excluded from the module's default test run**, and
741+
the adopting module says so in its own POM. This repository's convention is the `testExclusions`
742+
property the parent POM feeds to Surefire:
724743

725744
```xml
726745
<properties>
727746
<testExclusions>**/e2e/*.java</testExclusions>
728747
</properties>
729748
```
730749

731-
This is a decision, not an omission. A default build that needs Docker fails on any machine or CI
732-
job without a daemon, and the failure reads as a broken provider rather than a missing prerequisite.
733-
The suites are instead **run locally by a maintainer before merge**, and a PR adopting the TCK is
734-
expected to quote the result. Adding a scheduled or path-filtered workflow to run them was
735-
considered and declined: a suite whose red is diagnosed by whoever happens to read the notification
736-
is worse than one whose red is diagnosed by the person who caused it.
750+
The parent POM defines no default for it, so a module that wants the gate must declare the property
751+
itself. It is a **Surefire** exclusion, not a compiler one: the suite still compiles against the
752+
harness in every build, which is what keeps an adoption from rotting unnoticed.
737753

738-
**Check what your profiles do to that property.** This is the part that is easy to get wrong. If
739-
your module has a profile that *clears* `testExclusions` in order to run some other Docker suite —
740-
`<testExclusions/>` — and a CI job activates that profile, the TCK suite runs there too. In this
741-
repository `ci.yml`'s `main` job activates `e2e` on every push, so `providers/flagd` narrows its
742-
`e2e` profile to `**/e2e/*TckTest.java` instead of clearing it: the module's legacy suites keep
743-
running and the TCK suites stay out. Resolve the property rather than reading the POM:
754+
**Then resolve the property under every profile your CI activates** — do not read the POM, which is
755+
the mistake the appendix names first. Here, `ci.yml`'s `main` job activates `e2e` on every push, and
756+
`providers/flagd` has an `e2e` profile for its legacy `Run*Test` suites; that profile therefore
757+
narrows the exclusion to `**/e2e/*TckTest.java` rather than clearing it to `<testExclusions/>`, so
758+
the legacy suites keep running and the TCK suites stay out. Both halves of the appendix's warning
759+
happened in this repository — one adoption never declared the property, the other had a profile
760+
putting it back — and both were found by running this, not by reading:
744761

745762
```bash
746763
mvn -Pe2e -pl providers/<your-provider> help:evaluate -Dexpression=testExclusions -DforceStdout
747764
```
748765

749-
Write the exclusion down where the adopter can find it. An exclusion nobody records is
750-
indistinguishable from an oversight, and both halves of that went wrong here: one of the two
751-
adoptions in this repository ran a Docker-dependent suite in its default build because it never
752-
declared the property, and the other was reported as excluded from every job when a profile was
753-
quietly putting it back.
766+
The single documented command that runs a suite deliberately, per the appendix, is the one in each
767+
adoption's own README: `-DtestExclusions=` on the command line overrides the property for one run.
754768

755769
Scenarios run **serially** and the suite enforces this, overriding any
756770
`cucumber.execution.parallel.enabled=true` in your module's `junit-platform.properties`. Control API

tools/tck/spec

0 commit comments

Comments
 (0)