Skip to content

Commit 88e4d98

Browse files
committed
feat(provider-tck): emit a machine-readable conformance report
Setting PROVIDER_TCK_REPORT_DIR makes each suite write its run to <dir>/<name>.json against the report schema in the specification repository (open-feature/spec#425, part of open-feature/spec#424). Go is the language that needs this first. godog counts a capability-gated skip in its passed tally, so a run that skipped five of twenty-nine scenarios prints "29 scenarios (29 passed)". Appendix F is unambiguous that a scenario skipped for an undeclared capability is reported as skipped with the reason and never as passed, and the harness does say so in a separate log line -- but the headline number still says something false, and the number is what gets read. pytest and jest-cucumber report skips correctly, so this is the runner's property rather than the suite's design. The report does not fix godog's summary. It makes the summary stop mattering, by recording every scenario's outcome individually so a consumer can check the rule instead of trusting the runner to have applied it. The same run now reports twenty-four passed and five not-declared, each with its reason. Identity comes from revision.go, generated by sync_assets.go beside the embedded artifacts. Generating both in one command is what keeps them honest: the CI check regenerates and fails on any difference, so a revision disagreeing with the artifacts beside it cannot be committed. The check is widened to cover the generated file, which it would otherwise have missed. Both the commit and the tree hash are recorded, the tree because it identifies the artifacts alone -- unchanged by unrelated edits elsewhere in the specification, so two runs of identical artifacts agree even when pinned to different commits, and checkable because `git rev-parse <commit>:specification/assets/provider-tck` reproduces it. Two smaller decisions. The provider is identified by the name it reports through its own metadata, with Config.Name recorded as the configuration, because Config.Name is chosen to read well in a failure message -- "flagd-rpc" -- and a provider with two materially different modes produces two reports that are not interchangeable. And how the backend was driven is read through an optional interface rather than a new BackendControl method, so that adding it breaks no existing implementation and a control that does not implement it simply omits the field. Emission is opt-in through the environment rather than through Config so that producing a report is a property of the run and not of the code: CI sets it, a local run does not, and no adopter changes a line to publish one. The tests assert the property that motivated the work -- that no scenario the capability gate stopped is ever reported as passed, and that every scenario is accounted for exactly once, since a report that silently omitted what it skipped would satisfy the first half while still misleading a reader. That test earned its place immediately: it caught this emitter recording every skipped scenario twice, the second time as passed, because godog does not deliver the before hook's ErrSkip to the after hook. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
1 parent ca75476 commit 88e4d98

11 files changed

Lines changed: 808 additions & 11 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ provider-tck-assets:
2121
cd tools/provider-tck && go run ./sync_assets.go
2222

2323
provider-tck-assets-check: provider-tck-assets
24-
git diff --exit-code -- tools/provider-tck/pkg/tck/assets
24+
git diff --exit-code -- tools/provider-tck/pkg/tck/assets tools/provider-tck/pkg/tck/revision.go
2525

2626
test:
2727
go list -f '{{.Dir}}/...' -m | xargs -I{} go test -v {}

tools/provider-tck/README.md

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,62 @@ provider in a domain replaces and shuts down the previous one; a fresh domain pe
275275
leave every provider of the suite registered and running, which for a provider holding a network
276276
connection means leaking one connection per scenario.
277277

278+
## Conformance reports
279+
280+
Set `PROVIDER_TCK_REPORT_DIR` and each suite writes a machine-readable report of its run to
281+
`<dir>/<name>.json`, conforming to the [report schema][report-schema] in the specification.
282+
283+
```console
284+
$ PROVIDER_TCK_REPORT_DIR=./reports go test ./...
285+
$ jq '.scenarios | group_by(.outcome) | map({(.[0].outcome): length}) | add' reports/in-memory.json
286+
{
287+
"passed": 24,
288+
"not-declared": 5
289+
}
290+
```
291+
292+
It is an environment variable rather than a `Config` field so that emitting a report is a property
293+
of the run and not of the code: CI sets it, a developer running the suite locally does not, and no
294+
adopter changes a line to publish one. Unset means no report, which is not an error. Several suites
295+
in one test binary each write their own file, so flagd's two resolvers do not collide.
296+
297+
### Why this exists in Go before the other languages
298+
299+
Because Go is the language that needs it most. godog counts a capability-gated skip in its **passed**
300+
tally:
301+
302+
```
303+
29 scenarios (29 passed)
304+
```
305+
306+
Five of those twenty-nine did not run. Appendix F is unambiguous that a scenario skipped for an
307+
undeclared capability is reported as skipped with the reason and *never* as passed, and the harness
308+
does say so in a separate log line — but the headline number still says something false, and a
309+
number is what gets read. pytest and jest-cucumber both report skips correctly, so this is a property
310+
of the runner rather than of the suite's design.
311+
312+
The report does not fix godog's summary. It makes the summary stop mattering, by recording the
313+
outcome of every scenario individually so that a consumer can check the rule instead of trusting the
314+
runner to have applied it. `reports/in-memory.json` above accounts for all twenty-nine scenarios and
315+
calls five of them `not-declared`, each with the reason.
316+
317+
### What identifies a report
318+
319+
`tck.specRevision` and `tck.assetsTree` come from [`revision.go`](./pkg/tck/revision.go), which
320+
`sync_assets.go` generates from the submodule alongside the embedded artifacts. Generating both in
321+
the same command is what keeps them honest: `make provider-tck-assets-check` regenerates and fails on
322+
any difference, so a revision that disagrees with the artifacts beside it cannot be committed.
323+
324+
The tree hash is carried as well as the commit because it identifies the artifacts alone. It is
325+
unchanged by unrelated edits elsewhere in the specification, so two runs that executed identical
326+
artifacts report the same value even when pinned to different commits — and it is checkable, since
327+
`git rev-parse <specRevision>:specification/assets/provider-tck` must reproduce it.
328+
329+
`provider.name` is what the provider reports through its own metadata, not `Config.Name`.
330+
`Config.Name` is chosen to read well in a failure message — `flagd-rpc` — which makes it the
331+
*configuration*, and it is reported as such. One provider with two materially different modes
332+
produces two reports that are not interchangeable.
333+
278334
## The self-tests
279335

280336
Three suites run against providers from the SDK itself. They need no Docker and finish in
@@ -345,5 +401,6 @@ the SDK's provider rather than reimplementing it — every resolution decision i
345401
[appendix-b]: https://github.com/open-feature/spec/blob/main/specification/appendix-b-gherkin-suites.md
346402
[appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md
347403
[control-api]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/openapi/control-api.yaml
404+
[report-schema]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/report/conformance-report.schema.json
348405
[spec]: https://github.com/open-feature/spec
349406
[tracking]: https://github.com/open-feature/spec/issues/417

tools/provider-tck/pkg/tck/capability.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,13 @@ func (c Capability) Tag() string { return string(c) }
204204
// String implements fmt.Stringer.
205205
func (c Capability) String() string { return string(c) }
206206

207-
// capabilityForTag maps a Gherkin tag onto the capability it gates, reporting
207+
// CapabilityForTag maps a Gherkin tag onto the capability it gates, reporting
208208
// whether the tag gates anything at all.
209-
func capabilityForTag(tag string) (Capability, bool) {
209+
//
210+
// Exported because a conformance report is read by things outside this package:
211+
// deciding whether a scenario was skipped legitimately means knowing which of
212+
// its tags gate a capability and which are merely organisational.
213+
func CapabilityForTag(tag string) (Capability, bool) {
210214
for _, c := range allCapabilities {
211215
if string(c) == tag {
212216
return c, true
@@ -230,7 +234,7 @@ type capabilitySet map[Capability]struct{}
230234
func newCapabilitySet(caps []Capability) (capabilitySet, error) {
231235
set := make(capabilitySet, len(caps))
232236
for _, c := range caps {
233-
if _, known := capabilityForTag(string(c)); !known {
237+
if _, known := CapabilityForTag(string(c)); !known {
234238
return nil, fmt.Errorf(
235239
"unknown capability %q: capabilities are the constants declared in this package, one of %s",
236240
c, formatCapabilities(AllCapabilities()))

tools/provider-tck/pkg/tck/inprocess.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,10 @@ func (c *InProcessControl) ChangeFlag(context.Context) error {
234234

235235
return c.current.UpdateFlag(ChangingFlagKey, changingFlag(c.changingVariant))
236236
}
237+
238+
// ControlAPI reports how this backend was driven.
239+
//
240+
// "in-process" is the narrow allowance the schema makes for a provider with no
241+
// backend. A report claiming it for a provider that has one should be treated
242+
// with suspicion, which is precisely why it is recorded rather than assumed.
243+
func (c *InProcessControl) ControlAPI() string { return "in-process" }

0 commit comments

Comments
 (0)