diff --git a/Makefile b/Makefile index fa1455997..1886d40a0 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ provider-tck-assets: cd tools/provider-tck && go run ./sync_assets.go provider-tck-assets-check: provider-tck-assets - git diff --exit-code -- tools/provider-tck/pkg/tck/assets + git diff --exit-code -- tools/provider-tck/pkg/tck/assets tools/provider-tck/pkg/tck/revision.go test: go list -f '{{.Dir}}/...' -m | xargs -I{} go test -v {} diff --git a/tools/provider-tck/README.md b/tools/provider-tck/README.md index 526f1922c..9a2ea60cd 100644 --- a/tools/provider-tck/README.md +++ b/tools/provider-tck/README.md @@ -204,6 +204,100 @@ provider in a domain replaces and shuts down the previous one; a fresh domain pe leave every provider of the suite registered and running, which for a provider holding a network connection means leaking one connection per scenario. +## Conformance reports + +Set `PROVIDER_TCK_REPORT_DIR` and each suite writes a machine-readable report of its run to +`/.json`, conforming to the [report schema][report-schema] in the specification. + +```console +$ PROVIDER_TCK_REPORT_DIR=./reports go test ./... +$ jq '.scenarios | group_by(.outcome) | map({(.[0].outcome): length}) | add' reports/in-memory.json +{ + "passed": 24, + "not-declared": 5 +} +``` + +It is an environment variable rather than a `Config` field so that emitting a report is a property +of the run and not of the code: CI sets it, a developer running the suite locally does not, and no +adopter changes a line to publish one. Unset means no report, which is not an error. Several suites +in one test binary each write their own file, so flagd's two resolvers do not collide. + +### Why this exists in Go before the other languages + +Because Go is the language that needs it most. godog counts a capability-gated skip in its **passed** +tally: + +``` +29 scenarios (29 passed) +``` + +Five of those twenty-nine did not run. 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 a +number is what gets read. pytest and jest-cucumber both report skips correctly, so this is a property +of the runner rather than of the suite's design. + +The report does not fix godog's summary. It makes the summary stop mattering, by recording the +outcome of every scenario individually so that a consumer can check the rule instead of trusting the +runner to have applied it. `reports/in-memory.json` above accounts for all twenty-nine scenarios and +calls five of them `not-declared`, each with the reason. + +### What identifies a report + +`tck.specRevision` and `tck.assetsTree` come from [`revision.go`](./pkg/tck/revision.go), which +`sync_assets.go` generates from the submodule alongside the embedded artifacts. Generating both in +the same command is what keeps them honest: `make provider-tck-assets-check` regenerates and fails on +any difference, so a revision that disagrees with the artifacts beside it cannot be committed. + +The tree hash is carried as well as the commit because it identifies the artifacts alone. It is +unchanged by unrelated edits elsewhere in the specification, so two runs that executed identical +artifacts report the same value even when pinned to different commits — and it is checkable, since +`git rev-parse :specification/assets/provider-tck` must reproduce it. + +`provider.name` is what the provider reports through its own metadata, not `Config.Name`. +`Config.Name` is chosen to read well in a failure message — `flagd-rpc` — which makes it the +*configuration*, and it is reported as such. One provider with two materially different modes +produces two reports that are not interchangeable. + +### What identifies a scenario + +`feature` and `name` together do not. Every row of a Scenario Outline shares one name, and the +type-mismatch matrix in `errors.feature` is eleven rows, so eleven entries carry the same feature and +the same name. A report that stopped there could not say which row failed, and a consumer keying on +the pair would keep whichever row it read last. + +A row is identified by its parameters, which the report carries in `example` — the Examples row it +came from, keyed by column header: + +```console +$ jq -c '.scenarios[] | select(.name | startswith("Requesting the wrong type")) | .example' reports/in-memory.json +{"default":"false","key":"string-flag","requested":"Boolean"} +{"default":"1","key":"string-flag","requested":"Integer"} +{"default":"0.1","key":"string-flag","requested":"Float"} +... +``` + +The values are the cells verbatim, as strings. Gherkin has no types, so `"1"` stays `"1"`: coercing +it would be this implementation inventing a fact the feature file did not state, and four +implementations would each invent a different one. + +It is a field rather than a naming convention because the parameters *are* the identity, and they +come from the feature file rather than from any runner. Mandating a mangled name instead would put a +separator, an ordering and an escaping rule into normative text that every implementation has to +reproduce byte for byte, and drift there is invisible until two reports silently fail to line up. + +A skipped row carries it too. The capability gate records its outcome before the scenario starts, so +the four rows of the `@object` outline would otherwise be four `not-declared` entries differing in +nothing — exactly as ambiguous as four failures. + +godog's hooks receive an already-expanded scenario, whose step text has the parameters substituted +into it and whose row is otherwise gone. What survives is `AstNodeIds`, whose last entry is the id of +the Examples `TableRow`, so the row is recovered by parsing the embedded feature files a second time +and indexing every row by that id. Those ids come from a counter godog shares across the files it +parses, which means reproducing them means reproducing godog's parse; a run that cannot resolve a row +it knows came from an outline fails rather than quietly emitting the ambiguity again. + ## The self-tests Three suites run against providers from the SDK itself. They need no Docker and finish in @@ -258,11 +352,21 @@ the SDK's provider rather than reimplementing it — every resolution decision i then a provider that silently drops the context passes. `@targeting` is reserved for these. - **`POST /restart` is unused.** No current scenario needs a bounded outage — the stale scenario uses an explicit disconnect and reconnect — so `tck.ConnectionControl` has no `DisconnectFor`. -- **Caching, hooks and flag metadata** are not covered. +- **Hooks and flag metadata** are not covered. +- **Caching is not covered, and the suite is quietly exposed to it.** `@caching` is reserved and no + scenario carries it, but flagd's RPC resolver enables an LRU cache *by default* and rewrites the + reason to `CACHED` on a hit. The adoption does not turn it off, so the suite already runs against a + caching provider while asserting `STATIC` everywhere. It passes only because no scenario evaluates + the same flag twice in a way that hits the cache — so a scenario added later that does will fail + against flagd RPC with `CACHED`, and the failure will look like a provider defect rather than a + test-design one. Note also that the configuration-change scenario already depends on cache + invalidation working without saying so: against flagd RPC it reads `changing-flag`, changes it, and + reads again, which only gives the right answer because the change event evicts the entry. [appendix-a]: https://github.com/open-feature/spec/blob/main/specification/appendix-a-included-utilities.md [appendix-b]: https://github.com/open-feature/spec/blob/main/specification/appendix-b-gherkin-suites.md [appendix-f]: https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md [control-api]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/openapi/control-api.yaml +[report-schema]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/report/conformance-report.schema.json [spec]: https://github.com/open-feature/spec [tracking]: https://github.com/open-feature/spec/issues/417 diff --git a/tools/provider-tck/go.mod b/tools/provider-tck/go.mod index ea5692471..29b18a5fd 100644 --- a/tools/provider-tck/go.mod +++ b/tools/provider-tck/go.mod @@ -3,13 +3,13 @@ module github.com/open-feature/go-sdk-contrib/tools/provider-tck go 1.25.0 require ( + github.com/cucumber/gherkin/go/v26 v26.2.0 github.com/cucumber/godog v0.15.1 github.com/cucumber/messages/go/v21 v21.0.1 github.com/open-feature/go-sdk v1.18.0 ) require ( - github.com/cucumber/gherkin/go/v26 v26.2.0 // indirect github.com/gofrs/uuid v4.4.0+incompatible // indirect github.com/hashicorp/go-immutable-radix v1.3.1 // indirect github.com/hashicorp/go-memdb v1.3.5 // indirect diff --git a/tools/provider-tck/pkg/tck/capability.go b/tools/provider-tck/pkg/tck/capability.go index 46829a8f2..026a5d997 100644 --- a/tools/provider-tck/pkg/tck/capability.go +++ b/tools/provider-tck/pkg/tck/capability.go @@ -139,9 +139,13 @@ func (c Capability) Tag() string { return string(c) } // String implements fmt.Stringer. func (c Capability) String() string { return string(c) } -// capabilityForTag maps a Gherkin tag onto the capability it gates, reporting +// CapabilityForTag maps a Gherkin tag onto the capability it gates, reporting // whether the tag gates anything at all. -func capabilityForTag(tag string) (Capability, bool) { +// +// Exported because a conformance report is read by things outside this package: +// deciding whether a scenario was skipped legitimately means knowing which of +// its tags gate a capability and which are merely organisational. +func CapabilityForTag(tag string) (Capability, bool) { for _, c := range allCapabilities { if string(c) == tag { return c, true @@ -156,7 +160,7 @@ type capabilitySet map[Capability]struct{} func newCapabilitySet(caps []Capability) (capabilitySet, error) { set := make(capabilitySet, len(caps)) for _, c := range caps { - if _, known := capabilityForTag(string(c)); !known { + if _, known := CapabilityForTag(string(c)); !known { return nil, fmt.Errorf( "unknown capability %q: capabilities are the constants declared in this package, one of %s", c, formatCapabilities(allCapabilities)) diff --git a/tools/provider-tck/pkg/tck/examples.go b/tools/provider-tck/pkg/tck/examples.go new file mode 100644 index 000000000..4a53f2b2d --- /dev/null +++ b/tools/provider-tck/pkg/tck/examples.go @@ -0,0 +1,231 @@ +package tck + +import ( + "fmt" + "io/fs" + "sort" + "strings" + "sync" + + "github.com/cucumber/gherkin/go/v26" + "github.com/cucumber/godog" + messages "github.com/cucumber/messages/go/v21" +) + +// WHY THIS FILE EXISTS +// +// A row of a Scenario Outline is identified by its parameters. Every row shares +// the outline's name, so a report that identifies a scenario by feature and name +// gives eleven identical entries for the eleven rows of the type-mismatch matrix +// in errors.feature. If one row fails and ten pass, that report cannot say which +// failed, and a consumer keying on feature and name keeps whichever row it read +// last. +// +// godog hands a hook a *godog.Scenario, which is a messages.Pickle. A pickle is +// the already-expanded scenario: its step text has the parameters substituted +// into it, and the row they came from is gone. What survives is AstNodeIds, +// whose last entry, for a pickle compiled from an outline, is the id of the +// Examples TableRow the pickle was expanded from. +// +// So the row is recovered by parsing the same feature files a second time and +// indexing every Examples TableRow by that id. + +// exampleRow is one row of an Examples table. +type exampleRow struct { + // values are the cells keyed by column header, verbatim as strings. Gherkin + // has no types, so "1" stays "1": coercing it to a number would be this + // implementation inventing a fact the feature file did not state, and the + // four language implementations would each invent a different one. + values map[string]string + // order is where the row sits among all the rows of its scenario, counting + // across every Examples block the scenario has. It is carried so the report + // can list the rows in the order the table declares them rather than in + // whatever order sorting the parameter values happens to produce. + order int +} + +// exampleIndex resolves a pickle to the Examples row it was expanded from. +type exampleIndex struct { + // rows is keyed by the id of the TableRow AST node. + rows map[string]exampleRow + // outlines names every scenario that is an outline, keyed by feature URI and + // scenario name. It exists so a failure to resolve a row can be told apart + // from a scenario that legitimately has none, and reported rather than + // silently emitting a report with the ambiguity this field exists to remove. + outlines map[string]bool +} + +var ( + exampleIndexOnce sync.Once + exampleIndexVal *exampleIndex + exampleIndexErr error +) + +// scenarioExamples returns the index, building it once per process. +func scenarioExamples() (*exampleIndex, error) { + exampleIndexOnce.Do(func() { + exampleIndexVal, exampleIndexErr = buildExampleIndex() + }) + return exampleIndexVal, exampleIndexErr +} + +// buildExampleIndex parses the embedded feature files and indexes their +// Examples rows by AST node id. +// +// The id has to agree with the one godog will report, and godog's ids are not +// intrinsic to a document: they come from a counter (messages.Incrementing) that +// godog creates once per run and shares across every file it parses, so an id +// depends on how many nodes were numbered before it. Reproducing them therefore +// means reproducing godog's whole parse — the same files, in the same order, +// with pickle compilation in between, because compiling pickles draws from the +// same counter. +// +// That is a coupling to godog's internals, and it is a deliberate one: the +// alternative is to fork the pickle compiler. It is not left to be trusted. +// A pickle that comes from an outline and does not resolve is reported as a +// failure by the run, so a godog release that renumbers nodes breaks the build +// loudly instead of quietly emitting reports with the ambiguity removed again. +func buildExampleIndex() (*exampleIndex, error) { + index := &exampleIndex{ + rows: map[string]exampleRow{}, + outlines: map[string]bool{}, + } + + paths, err := featureFilePaths() + if err != nil { + return nil, err + } + + newID := (&messages.Incrementing{}).NewId + for _, path := range paths { + file, err := assets.Open(path) + if err != nil { + return nil, fmt.Errorf("opening the embedded feature file %s: %w", path, err) + } + document, err := gherkin.ParseGherkinDocumentForLanguage(file, gherkin.DefaultDialect, newID) + closeErr := file.Close() + if err != nil { + return nil, fmt.Errorf("parsing the embedded feature file %s: %w", path, err) + } + if closeErr != nil { + return nil, fmt.Errorf("closing the embedded feature file %s: %w", path, closeErr) + } + + document.Uri = path + index.collectDocument(document) + + // The result is discarded; the side effect is the point. Compiling the + // pickles advances the id counter exactly as godog's parse advances it, + // so the next document's AST nodes are numbered the way godog numbers + // them. + _ = gherkin.Pickles(*document, path, newID) + } + + return index, nil +} + +// featureFilePaths lists the embedded feature files in the order godog walks +// them, which is the lexical order fs.WalkDir yields. +func featureFilePaths() ([]string, error) { + var paths []string + err := fs.WalkDir(assets, featuresPath, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() || !strings.HasSuffix(path, ".feature") { + return nil + } + paths = append(paths, path) + return nil + }) + if err != nil { + return nil, fmt.Errorf("listing the embedded feature files: %w", err) + } + // WalkDir already yields lexical order; sorting says so rather than relying + // on it, since the ids only line up if this order matches godog's. + sort.Strings(paths) + return paths, nil +} + +func (index *exampleIndex) collectDocument(document *messages.GherkinDocument) { + if document == nil || document.Feature == nil { + return + } + for _, child := range document.Feature.Children { + if child == nil { + continue + } + if child.Scenario != nil { + index.collectScenario(document.Uri, child.Scenario) + } + if child.Rule == nil { + continue + } + for _, ruleChild := range child.Rule.Children { + if ruleChild != nil && ruleChild.Scenario != nil { + index.collectScenario(document.Uri, ruleChild.Scenario) + } + } + } +} + +func (index *exampleIndex) collectScenario(uri string, scenario *messages.Scenario) { + if len(scenario.Examples) == 0 { + return + } + index.outlines[outlineKey(uri, scenario.Name)] = true + + order := 0 + for _, examples := range scenario.Examples { + if examples == nil || examples.TableHeader == nil { + continue + } + headers := make([]string, 0, len(examples.TableHeader.Cells)) + for _, cell := range examples.TableHeader.Cells { + headers = append(headers, cell.Value) + } + + for _, row := range examples.TableBody { + if row == nil { + continue + } + values := make(map[string]string, len(headers)) + for i, cell := range row.Cells { + // A row with more cells than headers is malformed Gherkin the + // parser would have rejected; guarding costs nothing and keeps a + // future parser change from panicking here. + if i >= len(headers) { + break + } + values[headers[i]] = cell.Value + } + if len(values) > 0 { + index.rows[row.Id] = exampleRow{values: values, order: order} + } + order++ + } + } +} + +// rowFor resolves the Examples row a pickle was expanded from. +// +// The last AST node id is the TableRow for an outline pickle and the Scenario +// node for an ordinary one, so a lookup that misses is the ordinary case rather +// than an error. +func (index *exampleIndex) rowFor(sc *godog.Scenario) (exampleRow, bool) { + if sc == nil || len(sc.AstNodeIds) == 0 { + return exampleRow{}, false + } + row, ok := index.rows[sc.AstNodeIds[len(sc.AstNodeIds)-1]] + return row, ok +} + +// isOutline reports whether a scenario name in a feature belongs to a Scenario +// Outline, and therefore must carry an example. +func (index *exampleIndex) isOutline(uri, name string) bool { + return index.outlines[outlineKey(uri, name)] +} + +func outlineKey(uri, name string) string { + return uri + "\n" + name +} diff --git a/tools/provider-tck/pkg/tck/inprocess.go b/tools/provider-tck/pkg/tck/inprocess.go index 49cb968fd..0fe14d75c 100644 --- a/tools/provider-tck/pkg/tck/inprocess.go +++ b/tools/provider-tck/pkg/tck/inprocess.go @@ -234,3 +234,10 @@ func (c *InProcessControl) ChangeFlag(context.Context) error { return c.current.UpdateFlag(ChangingFlagKey, changingFlag(c.changingVariant)) } + +// ControlAPI reports how this backend was driven. +// +// "in-process" is the narrow allowance the schema makes for a provider with no +// backend. A report claiming it for a provider that has one should be treated +// with suspicion, which is precisely why it is recorded rather than assumed. +func (c *InProcessControl) ControlAPI() string { return "in-process" } diff --git a/tools/provider-tck/pkg/tck/report.go b/tools/provider-tck/pkg/tck/report.go new file mode 100644 index 000000000..dcb888849 --- /dev/null +++ b/tools/provider-tck/pkg/tck/report.go @@ -0,0 +1,385 @@ +package tck + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "runtime/debug" + "sort" + "strings" + "time" +) + +// ReportDirEnv names the directory a conformance report is written to. +// +// It is an environment variable rather than a Config field so that emitting a +// report is a property of the run and not of the code: CI sets it, a developer +// running the suite locally does not, and no adopter has to change a line to +// publish one. A suite writes /.json, so several suites in one test +// binary — flagd's RPC and in-process resolvers, say — each produce their own +// file without colliding. +// +// Unset means no report, which is the default and is not an error. +const ReportDirEnv = "PROVIDER_TCK_REPORT_DIR" + +// reportSchemaVersion is the major version of the report schema this emitter +// produces. See specification/assets/provider-tck/report/. +const reportSchemaVersion = "1" + +// Outcome is the result of one scenario, or of one capability. +// +// There are four rather than two because "did not run" is not one thing. +// A capability the provider chose not to declare is a different statement from +// one the language makes impossible — @strict-numeric-typing cannot hold in a +// language with no integer type — and reporting both as "not declared" would +// show a whole language as missing something none of its providers can have. +type Outcome string + +const ( + OutcomePassed Outcome = "passed" + OutcomeFailed Outcome = "failed" + OutcomeNotDeclared Outcome = "not-declared" + OutcomeNotApplicable Outcome = "not-applicable" +) + +// Report is one run of the suite against one provider in one configuration. +// +// The field names and shape are fixed by the schema in the specification +// repository; this type is deliberately a transcription of it rather than a +// convenient Go representation, because the point of the format is that four +// languages emit the same thing. +type Report struct { + SchemaVersion string `json:"schemaVersion"` + Provider ReportProvider `json:"provider"` + SDK ReportSDK `json:"sdk"` + TCK ReportTCK `json:"tck"` + Backend *ReportBackend `json:"backend,omitempty"` + Capabilities map[string]ReportCapability `json:"capabilities"` + Scenarios []ReportScenario `json:"scenarios"` +} + +type ReportProvider struct { + Name string `json:"name"` + Version string `json:"version,omitempty"` + Language string `json:"language"` + Configuration string `json:"configuration,omitempty"` +} + +type ReportSDK struct { + Name string `json:"name"` + Version string `json:"version"` +} + +type ReportTCK struct { + Implementation string `json:"implementation"` + Version string `json:"version"` + SpecRevision string `json:"specRevision"` + SpecRelease string `json:"specRelease,omitempty"` + AssetsTree string `json:"assetsTree,omitempty"` +} + +type ReportBackend struct { + Description string `json:"description,omitempty"` + ControlAPI string `json:"controlApi,omitempty"` +} + +type ReportCapability struct { + State Outcome `json:"state"` + Reason string `json:"reason,omitempty"` +} + +type ReportScenario struct { + Feature string `json:"feature"` + Name string `json:"name"` + // Example is the Examples row this entry came from, keyed by column header, + // present only for a scenario expanded from a Scenario Outline. + // + // It is what makes such an entry identifiable. Feature and name are shared by + // every row of an outline -- eleven rows of the type-mismatch matrix in + // errors.feature produce eleven otherwise identical entries -- so without it + // a report cannot say which row failed. + // + // The values are the cells verbatim, as strings. Gherkin has no types, so the + // cell "1" is reported as "1" and not as 1; the report says what the table + // said and leaves the interpretation to whoever reads it. + Example map[string]string `json:"example,omitempty"` + Tags []string `json:"tags,omitempty"` + Outcome Outcome `json:"outcome"` + Reason string `json:"reason,omitempty"` + DurationMs float64 `json:"durationMs,omitempty"` +} + +// scenarioRecord is what the runner accumulates as scenarios execute. +type scenarioRecord struct { + feature string + name string + example map[string]string + // exampleOrder is the row's position in its scenario's Examples tables, kept + // only to sort the report. Ordering by the parameter values would list the + // rows of a matrix in an order the feature file never mentions, which makes a + // report needlessly hard to read next to the table it came from. + exampleOrder int + tags []string + outcome Outcome + reason string + duration time.Duration +} + +// buildReport assembles the report from what the run observed. +// +// The per-scenario list is the load-bearing part. Appendix F requires that a +// scenario skipped for an undeclared capability is never reported as passed, +// and godog's own summary does exactly that — it counts capability skips in its +// passed tally, so the headline number says something false. Emitting the +// outcome of every scenario individually makes the rule checkable by a consumer +// instead of dependent on each runner's summary being trustworthy. +func (r *runner) buildReport() Report { + providerName := r.observedProviderName() + r.mu.Lock() + records := make([]scenarioRecord, len(r.records)) + copy(records, r.records) + r.mu.Unlock() + + sort.Slice(records, func(i, j int) bool { + if records[i].feature != records[j].feature { + return records[i].feature < records[j].feature + } + if records[i].name != records[j].name { + return records[i].name < records[j].name + } + return records[i].exampleOrder < records[j].exampleOrder + }) + + scenarios := make([]ReportScenario, 0, len(records)) + // failed counts, per capability, the scenarios gating on it that failed, so a + // capability is reported as passed only when everything gating on it passed and + // a failure can say how much failed. + failed := map[Capability]int{} + // exercised counts the scenarios gating on each capability at all. A capability + // no scenario carries cannot have been demonstrated, and reporting it as passed + // would claim conformance the suite never tested -- which is the same vacuous + // green the capability vocabulary exists to prevent. + exercised := map[Capability]int{} + + for _, rec := range records { + scenarios = append(scenarios, ReportScenario{ + Feature: rec.feature, + Name: rec.name, + Example: rec.example, + Tags: rec.tags, + Outcome: rec.outcome, + Reason: rec.reason, + DurationMs: float64(rec.duration.Microseconds()) / 1000.0, + }) + // Only a scenario that actually ran exercises anything. A scenario + // skipped for one undeclared capability still carries its other tags, + // and counting those would report a capability as passed on the + // strength of a scenario that never executed: events.feature's + // scenarios carry @events alongside @stale and @configuration-change, + // so withholding either left @events reading "passed" while both of its + // scenarios were skipped. + if rec.outcome != OutcomePassed && rec.outcome != OutcomeFailed { + continue + } + for _, tag := range rec.tags { + capability, gates := CapabilityForTag(tag) + if !gates { + continue + } + exercised[capability]++ + if rec.outcome == OutcomeFailed { + failed[capability]++ + } + } + } + + capabilities := map[string]ReportCapability{} + for _, capability := range AllCapabilities() { + switch { + case !r.caps.has(capability): + capabilities[capability.Tag()] = ReportCapability{ + State: OutcomeNotDeclared, + Reason: fmt.Sprintf( + "not declared by this provider's configuration; the %s scenarios were skipped and did not contribute to this result", + capability.Tag()), + } + case exercised[capability] == 0: + // Declared, but no scenario in the suite gates on it. Saying nothing is + // the only honest answer: the suite asked no question, so it has none + // to report. Claiming passed would be a green result for an untested + // claim, which is precisely what this suite exists to make impossible. + case failed[capability] > 0: + capabilities[capability.Tag()] = ReportCapability{ + State: OutcomeFailed, + Reason: fmt.Sprintf( + "%d of %d scenarios carrying %s failed; the per-scenario results say which, and why", + failed[capability], exercised[capability], capability.Tag()), + } + default: + capabilities[capability.Tag()] = ReportCapability{State: OutcomePassed} + } + } + + return Report{ + SchemaVersion: reportSchemaVersion, + Provider: ReportProvider{ + Name: providerName, + Language: "go", + Configuration: r.cfg.Name, + }, + SDK: ReportSDK{Name: goSDKModule, Version: sdkVersion()}, + TCK: ReportTCK{ + Implementation: tckImplementation, + Version: tckVersion(), + SpecRevision: SpecRevision, + AssetsTree: AssetsTree, + }, + Backend: &ReportBackend{ + Description: r.cfg.Control.Description(), + ControlAPI: controlAPIOf(r.cfg.Control), + }, + Capabilities: capabilities, + Scenarios: scenarios, + } +} + +// observedProviderName is what the provider called itself, falling back to the +// suite name when no scenario ever registered one -- which happens when every +// scenario was skipped, and is worth reporting as the suite name rather than as +// an empty string the schema would reject. +func (r *runner) observedProviderName() string { + r.mu.Lock() + defer r.mu.Unlock() + if r.providerName != "" { + return r.providerName + } + return r.cfg.Name +} + +// controlAPIReporter is implemented by a BackendControl that knows which kind +// of control the schema should record. +// +// It is an optional interface rather than a method on BackendControl because +// adding a method would break every existing implementation for the sake of one +// string, and a control that does not implement it simply omits the field. +type controlAPIReporter interface { + // ControlAPI reports "http" for the normative control API or "in-process" + // for the narrow allowance made for providers with no backend. + ControlAPI() string +} + +func controlAPIOf(control BackendControl) string { + if reporter, ok := control.(controlAPIReporter); ok { + return reporter.ControlAPI() + } + return "" +} + +const ( + goSDKModule = "github.com/open-feature/go-sdk" + tckImplementation = "go-sdk-contrib/tools/provider-tck" + tckModule = "github.com/open-feature/go-sdk-contrib/tools/provider-tck" +) + +// writeReport emits the report if ReportDirEnv is set. +// +// A failure to write is reported as a test failure rather than logged and +// ignored. CI that asked for a report and silently did not get one is how a +// publishing pipeline ends up serving a stale result forever. +func (r *runner) writeReport() { + dir := strings.TrimSpace(os.Getenv(ReportDirEnv)) + if dir == "" { + return + } + + report := r.buildReport() + + data, err := json.MarshalIndent(report, "", " ") + if err != nil { + r.t.Errorf("provider-tck [%s]: could not encode the conformance report: %v", r.cfg.Name, err) + return + } + data = append(data, '\n') + + if err := os.MkdirAll(dir, 0o755); err != nil { + r.t.Errorf("provider-tck [%s]: could not create the report directory %s: %v", r.cfg.Name, dir, err) + return + } + + path := filepath.Join(dir, reportFileName(r.cfg.Name)) + if err := os.WriteFile(path, data, 0o644); err != nil { + r.t.Errorf("provider-tck [%s]: could not write the conformance report to %s: %v", r.cfg.Name, path, err) + return + } + + r.t.Logf("provider-tck [%s]: conformance report written to %s", r.cfg.Name, path) +} + +// reportFileName turns a suite name into a filename. +// +// Suite names are chosen to read well in failure messages rather than to be +// path-safe, so anything that is not obviously safe becomes a hyphen. Without +// this a suite named "flagd/rpc" would silently write outside the directory it +// was given. +func reportFileName(name string) string { + var b strings.Builder + for _, r := range name { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-', r == '_', r == '.': + b.WriteRune(r) + default: + b.WriteRune('-') + } + } + cleaned := strings.Trim(b.String(), "-.") + if cleaned == "" { + cleaned = "report" + } + return cleaned + ".json" +} + +// sdkVersion reports the go-sdk version this binary was built against. +// +// Read from the build info rather than declared, because a declared version is +// a second place to be wrong: the report would keep claiming 1.14.0 after a +// dependency bump moved the actual code underneath it. +func sdkVersion() string { + return moduleVersion(goSDKModule) +} + +func tckVersion() string { + return moduleVersion(tckModule) +} + +func moduleVersion(path string) string { + info, ok := debug.ReadBuildInfo() + if !ok { + return "unknown" + } + if info.Main.Path == path && info.Main.Version != "" { + return info.Main.Version + } + for _, dep := range info.Deps { + if dep == nil || dep.Path != path { + continue + } + // A replace directive means the code being run is not the version the + // requirement names, and saying so is more useful than either version + // alone. + if dep.Replace != nil && dep.Replace.Version != "" { + return dep.Replace.Version + } + if dep.Version != "" { + return dep.Version + } + } + return "unknown" +} + +// featureName turns a Gherkin document's URI into the bare feature name the +// schema asks for: "errors", not "features/errors.feature". +func featureName(uri string) string { + base := filepath.Base(filepath.ToSlash(uri)) + return strings.TrimSuffix(base, filepath.Ext(base)) +} diff --git a/tools/provider-tck/pkg/tck/report_internal_test.go b/tools/provider-tck/pkg/tck/report_internal_test.go new file mode 100644 index 000000000..094b026f0 --- /dev/null +++ b/tools/provider-tck/pkg/tck/report_internal_test.go @@ -0,0 +1,68 @@ +package tck + +import ( + "strings" + "testing" +) + +// TestFailedCapabilityCarriesAReason exercises the branch no passing suite can. +// +// The schema requires a reason whenever an outcome is not "passed", and the +// self-test suites all pass, so nothing that runs end to end ever builds a failed +// capability entry. That branch was schema-invalid for a while and no test +// noticed, because the only way to reach it is to fail a scenario on purpose -- +// which is what this does, by handing the report builder the records directly +// rather than by breaking a provider. +func TestFailedCapabilityCarriesAReason(t *testing.T) { + caps, err := newCapabilitySet([]Capability{Object, Events}) + if err != nil { + t.Fatalf("building the capability set: %v", err) + } + + r := &runner{cfg: Config{Name: "synthetic", Control: stubControl{}}, caps: caps} + r.records = []scenarioRecord{ + {feature: "errors", name: "a structured flag fails", tags: []string{"@object"}, + outcome: OutcomeFailed, reason: "resolved to nil, expected an object"}, + {feature: "errors", name: "a structured flag succeeds", tags: []string{"@object"}, + outcome: OutcomePassed}, + {feature: "events", name: "ready fires", tags: []string{"@events"}, + outcome: OutcomePassed}, + } + + report := r.buildReport() + + object, present := report.Capabilities[Object.Tag()] + if !present { + t.Fatalf("%s is missing from the report", Object.Tag()) + } + if object.State != OutcomeFailed { + t.Errorf("%s reported as %q, want %q", Object.Tag(), object.State, OutcomeFailed) + } + if object.Reason == "" { + t.Fatalf("%s failed but carries no reason; the schema rejects a non-passed outcome "+ + "without one, so a report built this way would not validate", Object.Tag()) + } + // The reason has to be usable, not merely present: a consumer reading a + // comparison page wants to know how much failed before opening the detail. + if !strings.Contains(object.Reason, "1 of 2") { + t.Errorf("%s reason %q does not say how many of how many failed", Object.Tag(), object.Reason) + } + + events, present := report.Capabilities[Events.Tag()] + if !present { + t.Fatalf("%s is missing from the report", Events.Tag()) + } + if events.State != OutcomePassed { + t.Errorf("%s reported as %q, want %q; one capability failing must not drag down another", + Events.Tag(), events.State, OutcomePassed) + } + + // Every capability the report does mention, other than a pass, must carry a + // reason -- the same rule the schema enforces, checked here so a change to the + // builder fails in this package rather than in a validator downstream. + for tag, result := range report.Capabilities { + if result.State != OutcomePassed && result.Reason == "" { + t.Errorf("capability %s is %q with no reason", tag, result.State) + } + } +} diff --git a/tools/provider-tck/pkg/tck/report_test.go b/tools/provider-tck/pkg/tck/report_test.go new file mode 100644 index 000000000..765284b6f --- /dev/null +++ b/tools/provider-tck/pkg/tck/report_test.go @@ -0,0 +1,393 @@ +package tck_test + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/open-feature/go-sdk-contrib/tools/provider-tck/pkg/tck" + "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk/openfeature/memprovider" +) + +// TestReportNeverCallsASkippedScenarioPassed is the reason the report exists. +// +// Appendix F requires that a scenario skipped for an undeclared capability is +// reported as skipped with the reason, never as passed. Go's runner does not +// honour that in its own summary: godog counts capability-gated skips in its +// passed tally, so the headline number the suite prints says something false and +// only a separate log line reveals it. +// +// The report is what makes the rule checkable rather than aspirational, so this +// test asserts the property directly: every scenario carrying a tag the suite +// did not declare appears in the report as not-declared, with a reason, and +// none of them appears as passed. +func TestReportNeverCallsASkippedScenarioPassed(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, dir) + + // Deliberately narrow: declaring only Object leaves every event, lifecycle, + // stale, unavailable and strict-numeric-typing scenario ungated and skipped, + // which is precisely the situation the rule governs. + tck.Run(t, tck.Config{ + Name: "report-selftest", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + Capabilities: []tck.Capability{tck.Object}, + }) + + report := readReport(t, filepath.Join(dir, "report-selftest.json")) + + declared := map[string]bool{tck.Object.Tag(): true} + + var skipped, passed int + for _, scenario := range report.Scenarios { + needsUndeclared := false + for _, tag := range scenario.Tags { + // Only tags that gate a capability matter; the feature files are + // free to carry organisational tags that gate nothing. + if _, gates := tck.CapabilityForTag(tag); gates && !declared[tag] { + needsUndeclared = true + } + } + + switch { + case needsUndeclared: + skipped++ + if scenario.Outcome != tck.OutcomeNotDeclared { + t.Errorf("scenario %q needs an undeclared capability but was reported as %q; "+ + "Appendix F requires it be reported as %q and never as passed", + scenario.Name, scenario.Outcome, tck.OutcomeNotDeclared) + } + if scenario.Reason == "" { + t.Errorf("scenario %q was skipped without a reason; the reason is what makes a "+ + "skip readable to someone comparing providers", scenario.Name) + } + case scenario.Outcome == tck.OutcomePassed: + passed++ + default: + t.Errorf("scenario %q needs no undeclared capability but was reported as %q: %s", + scenario.Name, scenario.Outcome, scenario.Reason) + } + } + + if skipped == 0 { + t.Fatal("no scenario was skipped, so this test asserted nothing; either the capability " + + "gate stopped working or the feature files no longer carry capability tags") + } + if passed == 0 { + t.Fatal("no scenario passed, so the suite did not really run") + } + + // The count is the other half of the property. A report that simply omitted + // the scenarios it did not run would satisfy every assertion above while + // still misleading a consumer, who has no way to know how many questions + // went unasked. + if total := len(report.Scenarios); total != skipped+passed { + t.Errorf("report accounts for %d scenarios but %d passed and %d were skipped; "+ + "every scenario in the suite must appear exactly once", total, passed, skipped) + } +} + +// TestReportRecordsUndeclaredCapabilities checks the capability summary agrees +// with the per-scenario detail, since a consumer may read either. +func TestReportRecordsUndeclaredCapabilities(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, dir) + + tck.Run(t, tck.Config{ + Name: "capability-selftest", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + Capabilities: []tck.Capability{tck.Object}, + }) + + report := readReport(t, filepath.Join(dir, "capability-selftest.json")) + + for _, capability := range tck.AllCapabilities() { + result, ok := report.Capabilities[capability.Tag()] + if !ok { + t.Errorf("capability %s is missing from the report; a capability is omitted only when "+ + "it is declared and no scenario exercises it, which is not the case here", + capability.Tag()) + continue + } + + want := tck.OutcomeNotDeclared + if capability == tck.Object { + want = tck.OutcomePassed + } + if result.State != want { + t.Errorf("capability %s reported as %q, want %q", capability.Tag(), result.State, want) + } + if want == tck.OutcomeNotDeclared && result.Reason == "" { + t.Errorf("capability %s is not declared but carries no reason", capability.Tag()) + } + } +} + +// TestReportNotWrittenByDefault keeps report emission opt-in. +// +// A suite that wrote files into the working directory of every developer who +// ran it would be a nuisance, and worse, a report written by accident is a +// report nobody checked. +func TestReportNotWrittenByDefault(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, "") + + tck.Run(t, tck.Config{ + Name: "no-report", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + Capabilities: []tck.Capability{tck.Object}, + }) + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("reading %s: %v", dir, err) + } + if len(entries) != 0 { + t.Errorf("no report directory was configured but %d file(s) were written", len(entries)) + } +} + +// TestSpecRevisionIsRecorded guards the generated constants. +// +// They are what lets a consumer know which questions a report answers, and they +// are generated rather than written, so the failure mode is silence: a +// regenerate that stopped emitting them would leave the report structurally +// valid and semantically useless. +func TestSpecRevisionIsRecorded(t *testing.T) { + if len(tck.SpecRevision) != 40 { + t.Errorf("SpecRevision = %q, want a 40-character commit SHA; run `make provider-tck-assets`", + tck.SpecRevision) + } + if len(tck.AssetsTree) != 40 { + t.Errorf("AssetsTree = %q, want a 40-character tree SHA; run `make provider-tck-assets`", + tck.AssetsTree) + } +} + +// TestOutlineRowsAreDistinguishable is the property the example field exists to +// establish. +// +// Every row of a Scenario Outline shares one feature and one name. The +// type-mismatch matrix in errors.feature is eleven rows, so before the field +// existed the report held eleven entries differing only in how long each took; +// if one row failed and ten passed, nothing in the report said which. The +// identity of a row is its parameters, so this asserts that feature, name and +// example together are unique across the whole report -- which is exactly the +// key a consumer needs to be able to use. +func TestOutlineRowsAreDistinguishable(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, dir) + + tck.Run(t, tck.Config{ + Name: "outline-identity", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + Capabilities: []tck.Capability{tck.Object}, + }) + + report := readReport(t, filepath.Join(dir, "outline-identity.json")) + + seen := map[string]bool{} + for _, scenario := range report.Scenarios { + key := scenarioKey(scenario) + if seen[key] { + t.Errorf("two entries share the identity %s; a Scenario Outline row is identified by "+ + "its parameters, so a consumer keying on feature, name and example would keep "+ + "only one of them", key) + } + seen[key] = true + } + + // A report where nothing came from an outline would satisfy the loop above + // while asserting nothing, so the matrix itself is pinned: eleven rows, + // eleven different parameter sets. + const matrix = "Requesting the wrong type returns the code default" + examples := map[string]bool{} + rows := 0 + for _, scenario := range report.Scenarios { + if scenario.Name != matrix { + continue + } + rows++ + if len(scenario.Example) == 0 { + t.Errorf("row %d of %q carries no example, so it is indistinguishable from the others", + rows, matrix) + continue + } + examples[canonicalExample(scenario.Example)] = true + } + if rows == 0 { + t.Fatalf("no entry for %q; either the feature files changed or the suite did not run", matrix) + } + if len(examples) != rows { + t.Errorf("%d rows of %q produced %d distinct examples", rows, matrix, len(examples)) + } + + // The field is present only for outline rows. Emitting an empty object for an + // ordinary scenario would be a second thing for four implementations to agree + // on, and the schema asks for omission instead. + const ordinary = "An unknown flag key returns the code default" + found := false + for _, scenario := range report.Scenarios { + if scenario.Name != ordinary { + continue + } + found = true + if scenario.Example != nil { + t.Errorf("%q is not a Scenario Outline but carries example %v", ordinary, scenario.Example) + } + } + if !found { + t.Errorf("no entry for %q, so the omission of example was not checked", ordinary) + } +} + +// TestSkippedOutlineRowsCarryTheirExample covers the capability gate, which +// records its outcome before the scenario runs and so is a second place the +// example has to be filled in. +// +// A skipped outline row is exactly as ambiguous as a failed one: the four rows +// of the @object outline are four entries that differ in nothing without it. +func TestSkippedOutlineRowsCarryTheirExample(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, dir) + + tck.Run(t, tck.Config{ + Name: "skipped-outline", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + // Empty rather than nil: a nil Capabilities means "declare everything", + // and what this test needs is a provider that declares nothing, so the + // @object outline is gated in the Before hook and never starts. + Capabilities: []tck.Capability{}, + }) + + report := readReport(t, filepath.Join(dir, "skipped-outline.json")) + + const outline = "Requesting a structured flag as a scalar returns the code default" + examples := map[string]bool{} + rows := 0 + for _, scenario := range report.Scenarios { + if scenario.Name != outline { + continue + } + rows++ + if scenario.Outcome != tck.OutcomeNotDeclared { + t.Errorf("%q was reported as %q; @object was not declared", outline, scenario.Outcome) + } + if len(scenario.Example) == 0 { + t.Errorf("a skipped row of %q carries no example, so the report cannot say which row "+ + "was skipped", outline) + continue + } + examples[canonicalExample(scenario.Example)] = true + } + if rows == 0 { + t.Fatalf("no entry for %q; the capability gate did not record it at all", outline) + } + if len(examples) != rows { + t.Errorf("%d skipped rows of %q produced %d distinct examples", rows, outline, len(examples)) + } +} + +// scenarioKey is the identity of a report entry: feature, name and example. +func scenarioKey(scenario tck.ReportScenario) string { + return scenario.Feature + "/" + scenario.Name + "/" + canonicalExample(scenario.Example) +} + +// canonicalExample renders an example so two of them compare equal exactly when +// their parameters do, independently of map iteration order. +func canonicalExample(example map[string]string) string { + keys := make([]string, 0, len(example)) + for key := range example { + keys = append(keys, key) + } + sort.Strings(keys) + + parts := make([]string, 0, len(keys)) + for _, key := range keys { + parts = append(parts, key+"="+example[key]) + } + return strings.Join(parts, "\x00") +} + +func readReport(t *testing.T, path string) tck.Report { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("no conformance report at %s: %v", path, err) + } + + var report tck.Report + if err := json.Unmarshal(data, &report); err != nil { + t.Fatalf("report at %s is not valid JSON: %v", path, err) + } + if report.SchemaVersion == "" { + t.Fatalf("report at %s has no schemaVersion", path) + } + return report +} + +// TestReservedCapabilityIsNotReportedAsPassed covers the capability a provider +// declares and the suite never tests. +// +// @targeting is reserved: it exists in the vocabulary but no scenario carries +// it, because asserting that an evaluation context reached the backend needs an +// echo operation the control API does not have yet. Reporting it as passed would +// be a green result for a claim nothing tested -- the same vacuous pass the +// capability vocabulary was introduced to eliminate, arriving through the report +// instead of through the suite. +// +// Omitting it is the honest answer: the suite asked no question, so it has none +// to report. A consumer sees the tag is absent rather than a pass it cannot rely +// on. +func TestReservedCapabilityIsNotReportedAsPassed(t *testing.T) { + dir := t.TempDir() + t.Setenv(tck.ReportDirEnv, dir) + + tck.Run(t, tck.Config{ + Name: "reserved-capability", + Control: plainMemoryControl{}, + NewProvider: func(context.Context) (openfeature.FeatureProvider, error) { + return memprovider.NewInMemoryProvider(tck.CanonicalFlagSet()), nil + }, + // Targeting is declared and no scenario carries it. Object is declared so + // the suite still does something. + Capabilities: []tck.Capability{tck.Object, tck.Targeting}, + }) + + report := readReport(t, filepath.Join(dir, "reserved-capability.json")) + + if result, present := report.Capabilities[tck.Targeting.Tag()]; present { + t.Errorf("%s was declared and no scenario exercises it, but the report states %q; "+ + "a capability the suite never tested must not be reported as a result", + tck.Targeting.Tag(), result.State) + } + + // The declared capability that is exercised must still be reported, so the + // omission above is specific rather than a general failure to report. + if result, present := report.Capabilities[tck.Object.Tag()]; !present { + t.Errorf("%s was declared and exercised but is missing from the report", tck.Object.Tag()) + } else if result.State != tck.OutcomePassed { + t.Errorf("%s reported as %q, want %q", tck.Object.Tag(), result.State, tck.OutcomePassed) + } +} diff --git a/tools/provider-tck/pkg/tck/revision.go b/tools/provider-tck/pkg/tck/revision.go new file mode 100644 index 000000000..db290bee3 --- /dev/null +++ b/tools/provider-tck/pkg/tck/revision.go @@ -0,0 +1,15 @@ +// Code generated by sync_assets.go. DO NOT EDIT. + +package tck + +// SpecRevision is the open-feature/spec commit the embedded conformance +// artifacts were taken from. +const SpecRevision = "dfa16586d91ca020ef1b3b82a7c972d833ff8f29" + +// AssetsTree is the git tree object ID of specification/assets/provider-tck at +// SpecRevision. +// +// It identifies the artifacts rather than the commit, so an unrelated change +// elsewhere in the specification leaves it untouched, and +// `git rev-parse SpecRevision:specification/assets/provider-tck` reproduces it. +const AssetsTree = "904aa7d5fd7a856a4f92ace24355bd1987143abc" diff --git a/tools/provider-tck/pkg/tck/run.go b/tools/provider-tck/pkg/tck/run.go index fc6ac9aed..196fc4976 100644 --- a/tools/provider-tck/pkg/tck/run.go +++ b/tools/provider-tck/pkg/tck/run.go @@ -8,6 +8,7 @@ import ( "strings" "sync" "testing" + "time" "github.com/cucumber/godog" "github.com/open-feature/go-sdk/openfeature" @@ -67,6 +68,8 @@ func Run(t *testing.T, cfg Config) { }.Run() r.reportSkips() + r.reportExampleGaps() + r.writeReport() if status != 0 && !t.Failed() { t.Fatalf("provider-tck [%s]: suite failed with exit status %d", cfg.Name, status) @@ -81,6 +84,41 @@ type runner struct { mu sync.Mutex skips []skippedScenario + + // records is the per-scenario outcome list the conformance report is built + // from. It is kept even when no report is requested, because it costs + // nothing and the alternative is a code path that only ever runs in CI. + records []scenarioRecord + + // started times scenarios by pickle id. godog gives no scenario-scoped place + // to hang this, so a map is the only option; the key is the pickle id rather + // than the scenario name because every row of a Scenario Outline shares one + // name and would otherwise share one entry. + started map[string]time.Time + + // gated names the scenarios the capability gate stopped before their first + // step, so the after hook does not record them a second time. + // + // It exists because godog does not deliver the before hook's ErrSkip to the + // after hook -- err arrives nil, indistinguishable from a scenario that ran + // and passed. Testing for ErrSkip there silently recorded every skipped + // scenario twice, once correctly and once as passed, which is the exact + // failure Appendix F forbids. + // + // Keyed by pickle id for a second reason as well as the shared name: Gherkin + // allows an Examples block to carry its own tags, so two rows of one outline + // can differ in whether the gate stops them. Keyed by name, gating one row + // would suppress the after hook for every row, and the rows that did run + // would vanish from the report entirely. + gated map[string]bool + + // exampleGaps collects outline rows whose Examples row could not be + // resolved. See reportExampleGaps. + exampleGaps []string + + // providerName is what the provider called itself, observed from the last + // scenario that registered one. + providerName string } // skippedScenario records a scenario that did not run because the provider did @@ -108,11 +146,16 @@ func (r *runner) initializeScenario(ctx *godog.ScenarioContext) { func (r *runner) beforeScenario(ctx context.Context, sc *godog.Scenario) (context.Context, error) { if capability, missing := r.missingCapability(sc); missing { r.recordSkip(sc.Name, capability) + r.recordOutcome(sc, OutcomeNotDeclared, fmt.Sprintf( + "requires capability %s, which this provider does not declare", capability.Tag()), 0) + r.markGated(sc.Id) return ctx, fmt.Errorf( "%w: scenario requires capability %s (Gherkin tag %s), which this provider does not declare. Declared capabilities: %s", godog.ErrSkip, capability, capability.Tag(), formatCapabilities(r.caps.sorted())) } + r.markStarted(sc.Id) + ctx = withState(ctx, newScenarioState(&r.cfg)) if err := r.cfg.Control.PrepareScenario(ctx); err != nil { @@ -125,13 +168,161 @@ func (r *runner) beforeScenario(ctx context.Context, sc *godog.Scenario) (contex // afterScenario detaches the scenario's event handlers. A skipped scenario has // no state, which is not an error. -func (r *runner) afterScenario(ctx context.Context, _ *godog.Scenario, err error) (context.Context, error) { +func (r *runner) afterScenario(ctx context.Context, sc *godog.Scenario, err error) (context.Context, error) { if state, stateErr := stateFrom(ctx); stateErr == nil { + if state.providerName != "" { + r.mu.Lock() + r.providerName = state.providerName + r.mu.Unlock() + } state.teardown() } + + // A capability skip was already recorded before the scenario started. + // Recording it again here would put it in the report twice, the second time + // as passed. + if !r.wasGated(sc.Id) { + outcome, reason := OutcomePassed, "" + if err != nil { + outcome, reason = OutcomeFailed, err.Error() + } + r.recordOutcome(sc, outcome, reason, r.elapsed(sc.Id)) + } + return ctx, err } +// recordOutcome appends one scenario's result. +func (r *runner) recordOutcome(sc *godog.Scenario, outcome Outcome, reason string, duration time.Duration) { + tags := make([]string, 0, len(sc.Tags)) + for _, tag := range sc.Tags { + tags = append(tags, tag.Name) + } + + // Resolved before the lock, because a failure to resolve takes the lock + // itself. + example, order := r.exampleFor(sc) + + r.mu.Lock() + defer r.mu.Unlock() + r.records = append(r.records, scenarioRecord{ + feature: featureName(sc.Uri), + name: sc.Name, + example: example, + exampleOrder: order, + tags: tags, + outcome: outcome, + reason: reason, + duration: duration, + }) +} + +// exampleFor resolves the Examples row a scenario came from, or nil for a +// scenario that is not an outline row. +// +// It is called from both the gate path and the pass/fail path, because a +// skipped outline row is exactly as ambiguous as a failed one: four skipped +// rows of the @object outline are four entries that differ in nothing without +// it. +func (r *runner) exampleFor(sc *godog.Scenario) (map[string]string, int) { + index, err := scenarioExamples() + if err != nil { + r.recordExampleGap(fmt.Sprintf( + "the embedded feature files could not be parsed for their Examples tables: %v", err)) + return nil, 0 + } + + if row, ok := index.rowFor(sc); ok { + return row.values, row.order + } + + if index.isOutline(sc.Uri, sc.Name) { + r.recordExampleGap(fmt.Sprintf( + "scenario %q in %s comes from a Scenario Outline, but no Examples row matched AST node ids %v", + sc.Name, sc.Uri, sc.AstNodeIds)) + } + + return nil, 0 +} + +func (r *runner) recordExampleGap(detail string) { + r.mu.Lock() + defer r.mu.Unlock() + for _, existing := range r.exampleGaps { + if existing == detail { + return + } + } + r.exampleGaps = append(r.exampleGaps, detail) +} + +// reportExampleGaps fails the run if any outline row went unidentified. +// +// Recovering the Examples row depends on reproducing the AST node ids godog +// assigns, which is a coupling to how godog numbers nodes. This is what keeps +// that coupling honest: if a godog release changes the numbering, the report +// would quietly go back to emitting rows that differ in nothing, and a quiet +// return to the ambiguity this field exists to remove is worse than a build +// failure. +func (r *runner) reportExampleGaps() { + r.mu.Lock() + gaps := make([]string, len(r.exampleGaps)) + copy(gaps, r.exampleGaps) + r.mu.Unlock() + + if len(gaps) == 0 { + return + } + + message := []string{fmt.Sprintf( + "provider-tck [%s]: %d Scenario Outline row(s) could not be identified by their Examples parameters, "+ + "so the report cannot distinguish them:", r.cfg.Name, len(gaps))} + for _, gap := range gaps { + message = append(message, " - "+gap) + } + r.t.Error(strings.Join(message, "\n")) +} + +// markGated and markStarted create their maps on first use. +// +// The runner has to work as a zero value: it is constructed as a struct literal +// in tests that exercise the gate directly, and a nil map assignment there is a +// panic rather than a helpful failure. +func (r *runner) markGated(name string) { + r.mu.Lock() + defer r.mu.Unlock() + if r.gated == nil { + r.gated = map[string]bool{} + } + r.gated[name] = true +} + +func (r *runner) markStarted(name string) { + r.mu.Lock() + defer r.mu.Unlock() + if r.started == nil { + r.started = map[string]time.Time{} + } + r.started[name] = time.Now() +} + +// wasGated reports whether the capability gate stopped this scenario. +func (r *runner) wasGated(name string) bool { + r.mu.Lock() + defer r.mu.Unlock() + return r.gated[name] +} + +func (r *runner) elapsed(name string) time.Duration { + r.mu.Lock() + defer r.mu.Unlock() + start, ok := r.started[name] + if !ok { + return 0 + } + return time.Since(start) +} + // missingCapability reports the first capability a scenario needs that the // provider did not declare. // @@ -139,7 +330,7 @@ func (r *runner) afterScenario(ctx context.Context, _ *godog.Scenario, err error // files stay free to carry organisational tags. func (r *runner) missingCapability(sc *godog.Scenario) (Capability, bool) { for _, tag := range sc.Tags { - capability, gates := capabilityForTag(tag.Name) + capability, gates := CapabilityForTag(tag.Name) if !gates { continue } diff --git a/tools/provider-tck/pkg/tck/state.go b/tools/provider-tck/pkg/tck/state.go index cb3c1cebb..3671d0b9d 100644 --- a/tools/provider-tck/pkg/tck/state.go +++ b/tools/provider-tck/pkg/tck/state.go @@ -47,6 +47,12 @@ type evaluation struct { type scenarioState struct { cfg *Config + // providerName is the name the provider reports through its own metadata, + // captured so the conformance report identifies the provider rather than the + // suite. Config.Name is chosen to read well in failure messages -- "flagd-rpc" + // -- and is the configuration, not the provider. + providerName string + client *openfeature.Client flag *flagUnderTest diff --git a/tools/provider-tck/pkg/tck/steps_provider.go b/tools/provider-tck/pkg/tck/steps_provider.go index 0c9bc0c6b..c834864bd 100644 --- a/tools/provider-tck/pkg/tck/steps_provider.go +++ b/tools/provider-tck/pkg/tck/steps_provider.go @@ -49,6 +49,7 @@ func aStableProvider(ctx context.Context) (context.Context, error) { state.cfg.readyTimeout(), regErr) } + state.providerName = provider.Metadata().Name state.client = openfeature.NewClient(state.cfg.domain()) return ctx, nil } diff --git a/tools/provider-tck/sync_assets.go b/tools/provider-tck/sync_assets.go index 03a88b3d0..1889ff690 100644 --- a/tools/provider-tck/sync_assets.go +++ b/tools/provider-tck/sync_assets.go @@ -39,23 +39,35 @@ import ( "fmt" "io/fs" "os" + "os/exec" "path/filepath" + "strings" ) const ( - specRoot = "pkg/tck/spec/specification/assets/provider-tck" + // specRoot is the checked-out submodule. + specRoot = "pkg/tck/spec" + + // assetsPathInSpec is where the artifacts live inside the specification + // repository. It names both the directory to copy from and the tree whose + // hash is recorded alongside the commit. + assetsPathInSpec = "specification/assets/provider-tck" + destRoot = "pkg/tck/assets" ) +// specAssets is the directory the artifacts are copied from. +var specAssets = filepath.Join(specRoot, filepath.FromSlash(assetsPathInSpec)) + // copies maps a directory in the spec repository to its destination in the // package. The layout is preserved rather than renamed, so a reader comparing the // two trees sees the same shape. var copies = []string{"gherkin", "flags", "openapi"} func main() { - if _, err := os.Stat(specRoot); err != nil { + if _, err := os.Stat(specAssets); err != nil { fail("the spec submodule is not checked out at %s: %v\n\n"+ - "Run: git submodule update --init tools/provider-tck/pkg/tck/spec", specRoot, err) + "Run: git submodule update --init tools/provider-tck/pkg/tck/spec", specAssets, err) } if err := os.RemoveAll(destRoot); err != nil { @@ -68,14 +80,18 @@ func main() { total := 0 for _, dir := range copies { - n, err := copyTree(filepath.Join(specRoot, dir), filepath.Join(destRoot, dir)) + n, err := copyTree(filepath.Join(specAssets, dir), filepath.Join(destRoot, dir)) if err != nil { fail("copying %s: %v", dir, err) } total += n } - fmt.Printf("synced %d conformance artifacts from %s\n", total, specRoot) + if err := writeRevision(); err != nil { + fail("recording the spec revision: %v", err) + } + + fmt.Printf("synced %d conformance artifacts from %s\n", total, specAssets) } // gitattributes pins the line endings of everything generated here. @@ -146,3 +162,57 @@ func fail(format string, args ...any) { fmt.Fprintf(os.Stderr, "sync_assets: "+format+"\n", args...) os.Exit(1) } + +// writeRevision records which revision of the specification these artifacts came +// from, as Go source. +// +// A conformance report has to name the revision it ran against or two reports +// cannot be compared, and for Go the submodule is absent from the published +// module, so the answer is baked in at sync time rather than read at run time. +// Generating it from the same command that copies the artifacts is what keeps it +// honest: the CI check regenerates both and fails on any difference, so a +// revision that disagrees with the artifacts beside it cannot be committed. +// +// The tree hash is recorded as well as the commit because it identifies the +// artifacts alone. It does not change when an unrelated part of the +// specification does, so two runs that executed identical artifacts report the +// same value even when pinned to different commits. It is also checkable, since +// `git rev-parse :specification/assets/provider-tck` must reproduce it. +func writeRevision() error { + commit, err := gitOutput("-C", specRoot, "rev-parse", "HEAD") + if err != nil { + return err + } + tree, err := gitOutput("-C", specRoot, "rev-parse", "HEAD:"+assetsPathInSpec) + if err != nil { + return err + } + + return os.WriteFile(filepath.Join("pkg", "tck", "revision.go"), + []byte(fmt.Sprintf(revisionTemplate, commit, tree)), 0o644) +} + +func gitOutput(args ...string) (string, error) { + out, err := exec.Command("git", args...).Output() + if err != nil { + return "", fmt.Errorf("git %s: %w", strings.Join(args, " "), err) + } + return strings.TrimSpace(string(out)), nil +} + +const revisionTemplate = `// Code generated by sync_assets.go. DO NOT EDIT. + +package tck + +// SpecRevision is the open-feature/spec commit the embedded conformance +// artifacts were taken from. +const SpecRevision = %q + +// AssetsTree is the git tree object ID of specification/assets/provider-tck at +// SpecRevision. +// +// It identifies the artifacts rather than the commit, so an unrelated change +// elsewhere in the specification leaves it untouched, and +// ` + "`git rev-parse SpecRevision:specification/assets/provider-tck`" + ` reproduces it. +const AssetsTree = %q +`