Skip to content

Commit bb3a394

Browse files
committed
feat(provider-tck): identify a Scenario Outline row by its parameters
A scenario entry in the conformance report was identified by feature and name. Every row of a Scenario Outline shares one name, so the type-mismatch matrix in errors.feature produced eleven entries that differed only in durationMs. If one row failed and ten passed, the report could not say which failed, and a consumer keying on feature and name kept whichever row it read last. Per the report schema, a scenarioResult now carries `example`: the Examples row it came from, keyed by column header, with the cells verbatim as strings. Gherkin has no types, so "1" stays "1" rather than becoming 1 -- the report says what the table said. It is present only for outline rows and omitted otherwise. godog hands a hook an already-expanded pickle, whose step text has the parameters substituted in and whose row is otherwise gone. What survives is AstNodeIds, whose last entry is the id of the Examples TableRow. The row is therefore recovered by parsing the embedded feature files a second time and indexing every TableRow by that id. Those ids come from a counter godog creates once per run and shares across the files it parses, so reproducing them means reproducing godog's parse -- same files, same order, pickle compilation in between. That coupling is not left to be trusted: a pickle that came from an outline and did not resolve fails the run, because quietly returning to the ambiguity this field exists to remove is worse than a build failure. The capability gate records its outcome before a scenario starts, so it fills the field in too. Four skipped rows of the @object outline are as ambiguous as four failed ones. The gate's own bookkeeping is keyed by pickle id rather than by scenario name for the same reason. 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 suppressed the after hook for every row and the rows that did run would have vanished from the report. gherkin/go/v26 moves from an indirect requirement to a direct one. It is the same module and version godog already builds against, so no dependency is added and no go.sum entry changes. Verified against the schema on open-feature/spec#425 with a Draft 2020-12 validator: every report the self-tests emit validates, the eleven matrix rows carry eleven distinct examples matching the feature file, and (feature, name, example) is unique across every scenario in a report. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
1 parent 99033ab commit bb3a394

6 files changed

Lines changed: 555 additions & 26 deletions

File tree

tools/provider-tck/README.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,44 @@ artifacts report the same value even when pinned to different commits — and it
260260
*configuration*, and it is reported as such. One provider with two materially different modes
261261
produces two reports that are not interchangeable.
262262

263+
### What identifies a scenario
264+
265+
`feature` and `name` together do not. Every row of a Scenario Outline shares one name, and the
266+
type-mismatch matrix in `errors.feature` is eleven rows, so eleven entries carry the same feature and
267+
the same name. A report that stopped there could not say which row failed, and a consumer keying on
268+
the pair would keep whichever row it read last.
269+
270+
A row is identified by its parameters, which the report carries in `example` — the Examples row it
271+
came from, keyed by column header:
272+
273+
```console
274+
$ jq -c '.scenarios[] | select(.name | startswith("Requesting the wrong type")) | .example' reports/in-memory.json
275+
{"default":"false","key":"string-flag","requested":"Boolean"}
276+
{"default":"1","key":"string-flag","requested":"Integer"}
277+
{"default":"0.1","key":"string-flag","requested":"Float"}
278+
...
279+
```
280+
281+
The values are the cells verbatim, as strings. Gherkin has no types, so `"1"` stays `"1"`: coercing
282+
it would be this implementation inventing a fact the feature file did not state, and four
283+
implementations would each invent a different one.
284+
285+
It is a field rather than a naming convention because the parameters *are* the identity, and they
286+
come from the feature file rather than from any runner. Mandating a mangled name instead would put a
287+
separator, an ordering and an escaping rule into normative text that every implementation has to
288+
reproduce byte for byte, and drift there is invisible until two reports silently fail to line up.
289+
290+
A skipped row carries it too. The capability gate records its outcome before the scenario starts, so
291+
the four rows of the `@object` outline would otherwise be four `not-declared` entries differing in
292+
nothing — exactly as ambiguous as four failures.
293+
294+
godog's hooks receive an already-expanded scenario, whose step text has the parameters substituted
295+
into it and whose row is otherwise gone. What survives is `AstNodeIds`, whose last entry is the id of
296+
the Examples `TableRow`, so the row is recovered by parsing the embedded feature files a second time
297+
and indexing every row by that id. Those ids come from a counter godog shares across the files it
298+
parses, which means reproducing them means reproducing godog's parse; a run that cannot resolve a row
299+
it knows came from an outline fails rather than quietly emitting the ambiguity again.
300+
263301
## The self-tests
264302

265303
Three suites run against providers from the SDK itself. They need no Docker and finish in

tools/provider-tck/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ module github.com/open-feature/go-sdk-contrib/tools/provider-tck
33
go 1.25.0
44

55
require (
6+
github.com/cucumber/gherkin/go/v26 v26.2.0
67
github.com/cucumber/godog v0.15.1
78
github.com/cucumber/messages/go/v21 v21.0.1
89
github.com/open-feature/go-sdk v1.18.0
910
)
1011

1112
require (
12-
github.com/cucumber/gherkin/go/v26 v26.2.0 // indirect
1313
github.com/gofrs/uuid v4.4.0+incompatible // indirect
1414
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
1515
github.com/hashicorp/go-memdb v1.3.5 // indirect
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
package tck
2+
3+
import (
4+
"fmt"
5+
"io/fs"
6+
"sort"
7+
"strings"
8+
"sync"
9+
10+
"github.com/cucumber/gherkin/go/v26"
11+
"github.com/cucumber/godog"
12+
messages "github.com/cucumber/messages/go/v21"
13+
)
14+
15+
// WHY THIS FILE EXISTS
16+
//
17+
// A row of a Scenario Outline is identified by its parameters. Every row shares
18+
// the outline's name, so a report that identifies a scenario by feature and name
19+
// gives eleven identical entries for the eleven rows of the type-mismatch matrix
20+
// in errors.feature. If one row fails and ten pass, that report cannot say which
21+
// failed, and a consumer keying on feature and name keeps whichever row it read
22+
// last.
23+
//
24+
// godog hands a hook a *godog.Scenario, which is a messages.Pickle. A pickle is
25+
// the already-expanded scenario: its step text has the parameters substituted
26+
// into it, and the row they came from is gone. What survives is AstNodeIds,
27+
// whose last entry, for a pickle compiled from an outline, is the id of the
28+
// Examples TableRow the pickle was expanded from.
29+
//
30+
// So the row is recovered by parsing the same feature files a second time and
31+
// indexing every Examples TableRow by that id.
32+
33+
// exampleRow is one row of an Examples table.
34+
type exampleRow struct {
35+
// values are the cells keyed by column header, verbatim as strings. Gherkin
36+
// has no types, so "1" stays "1": coercing it to a number would be this
37+
// implementation inventing a fact the feature file did not state, and the
38+
// four language implementations would each invent a different one.
39+
values map[string]string
40+
// order is where the row sits among all the rows of its scenario, counting
41+
// across every Examples block the scenario has. It is carried so the report
42+
// can list the rows in the order the table declares them rather than in
43+
// whatever order sorting the parameter values happens to produce.
44+
order int
45+
}
46+
47+
// exampleIndex resolves a pickle to the Examples row it was expanded from.
48+
type exampleIndex struct {
49+
// rows is keyed by the id of the TableRow AST node.
50+
rows map[string]exampleRow
51+
// outlines names every scenario that is an outline, keyed by feature URI and
52+
// scenario name. It exists so a failure to resolve a row can be told apart
53+
// from a scenario that legitimately has none, and reported rather than
54+
// silently emitting a report with the ambiguity this field exists to remove.
55+
outlines map[string]bool
56+
}
57+
58+
var (
59+
exampleIndexOnce sync.Once
60+
exampleIndexVal *exampleIndex
61+
exampleIndexErr error
62+
)
63+
64+
// scenarioExamples returns the index, building it once per process.
65+
func scenarioExamples() (*exampleIndex, error) {
66+
exampleIndexOnce.Do(func() {
67+
exampleIndexVal, exampleIndexErr = buildExampleIndex()
68+
})
69+
return exampleIndexVal, exampleIndexErr
70+
}
71+
72+
// buildExampleIndex parses the embedded feature files and indexes their
73+
// Examples rows by AST node id.
74+
//
75+
// The id has to agree with the one godog will report, and godog's ids are not
76+
// intrinsic to a document: they come from a counter (messages.Incrementing) that
77+
// godog creates once per run and shares across every file it parses, so an id
78+
// depends on how many nodes were numbered before it. Reproducing them therefore
79+
// means reproducing godog's whole parse — the same files, in the same order,
80+
// with pickle compilation in between, because compiling pickles draws from the
81+
// same counter.
82+
//
83+
// That is a coupling to godog's internals, and it is a deliberate one: the
84+
// alternative is to fork the pickle compiler. It is not left to be trusted.
85+
// A pickle that comes from an outline and does not resolve is reported as a
86+
// failure by the run, so a godog release that renumbers nodes breaks the build
87+
// loudly instead of quietly emitting reports with the ambiguity removed again.
88+
func buildExampleIndex() (*exampleIndex, error) {
89+
index := &exampleIndex{
90+
rows: map[string]exampleRow{},
91+
outlines: map[string]bool{},
92+
}
93+
94+
paths, err := featureFilePaths()
95+
if err != nil {
96+
return nil, err
97+
}
98+
99+
newID := (&messages.Incrementing{}).NewId
100+
for _, path := range paths {
101+
file, err := assets.Open(path)
102+
if err != nil {
103+
return nil, fmt.Errorf("opening the embedded feature file %s: %w", path, err)
104+
}
105+
document, err := gherkin.ParseGherkinDocumentForLanguage(file, gherkin.DefaultDialect, newID)
106+
closeErr := file.Close()
107+
if err != nil {
108+
return nil, fmt.Errorf("parsing the embedded feature file %s: %w", path, err)
109+
}
110+
if closeErr != nil {
111+
return nil, fmt.Errorf("closing the embedded feature file %s: %w", path, closeErr)
112+
}
113+
114+
document.Uri = path
115+
index.collectDocument(document)
116+
117+
// The result is discarded; the side effect is the point. Compiling the
118+
// pickles advances the id counter exactly as godog's parse advances it,
119+
// so the next document's AST nodes are numbered the way godog numbers
120+
// them.
121+
_ = gherkin.Pickles(*document, path, newID)
122+
}
123+
124+
return index, nil
125+
}
126+
127+
// featureFilePaths lists the embedded feature files in the order godog walks
128+
// them, which is the lexical order fs.WalkDir yields.
129+
func featureFilePaths() ([]string, error) {
130+
var paths []string
131+
err := fs.WalkDir(assets, featuresPath, func(path string, entry fs.DirEntry, err error) error {
132+
if err != nil {
133+
return err
134+
}
135+
if entry.IsDir() || !strings.HasSuffix(path, ".feature") {
136+
return nil
137+
}
138+
paths = append(paths, path)
139+
return nil
140+
})
141+
if err != nil {
142+
return nil, fmt.Errorf("listing the embedded feature files: %w", err)
143+
}
144+
// WalkDir already yields lexical order; sorting says so rather than relying
145+
// on it, since the ids only line up if this order matches godog's.
146+
sort.Strings(paths)
147+
return paths, nil
148+
}
149+
150+
func (index *exampleIndex) collectDocument(document *messages.GherkinDocument) {
151+
if document == nil || document.Feature == nil {
152+
return
153+
}
154+
for _, child := range document.Feature.Children {
155+
if child == nil {
156+
continue
157+
}
158+
if child.Scenario != nil {
159+
index.collectScenario(document.Uri, child.Scenario)
160+
}
161+
if child.Rule == nil {
162+
continue
163+
}
164+
for _, ruleChild := range child.Rule.Children {
165+
if ruleChild != nil && ruleChild.Scenario != nil {
166+
index.collectScenario(document.Uri, ruleChild.Scenario)
167+
}
168+
}
169+
}
170+
}
171+
172+
func (index *exampleIndex) collectScenario(uri string, scenario *messages.Scenario) {
173+
if len(scenario.Examples) == 0 {
174+
return
175+
}
176+
index.outlines[outlineKey(uri, scenario.Name)] = true
177+
178+
order := 0
179+
for _, examples := range scenario.Examples {
180+
if examples == nil || examples.TableHeader == nil {
181+
continue
182+
}
183+
headers := make([]string, 0, len(examples.TableHeader.Cells))
184+
for _, cell := range examples.TableHeader.Cells {
185+
headers = append(headers, cell.Value)
186+
}
187+
188+
for _, row := range examples.TableBody {
189+
if row == nil {
190+
continue
191+
}
192+
values := make(map[string]string, len(headers))
193+
for i, cell := range row.Cells {
194+
// A row with more cells than headers is malformed Gherkin the
195+
// parser would have rejected; guarding costs nothing and keeps a
196+
// future parser change from panicking here.
197+
if i >= len(headers) {
198+
break
199+
}
200+
values[headers[i]] = cell.Value
201+
}
202+
if len(values) > 0 {
203+
index.rows[row.Id] = exampleRow{values: values, order: order}
204+
}
205+
order++
206+
}
207+
}
208+
}
209+
210+
// rowFor resolves the Examples row a pickle was expanded from.
211+
//
212+
// The last AST node id is the TableRow for an outline pickle and the Scenario
213+
// node for an ordinary one, so a lookup that misses is the ordinary case rather
214+
// than an error.
215+
func (index *exampleIndex) rowFor(sc *godog.Scenario) (exampleRow, bool) {
216+
if sc == nil || len(sc.AstNodeIds) == 0 {
217+
return exampleRow{}, false
218+
}
219+
row, ok := index.rows[sc.AstNodeIds[len(sc.AstNodeIds)-1]]
220+
return row, ok
221+
}
222+
223+
// isOutline reports whether a scenario name in a feature belongs to a Scenario
224+
// Outline, and therefore must carry an example.
225+
func (index *exampleIndex) isOutline(uri, name string) bool {
226+
return index.outlines[outlineKey(uri, name)]
227+
}
228+
229+
func outlineKey(uri, name string) string {
230+
return uri + "\n" + name
231+
}

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

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,40 @@ type ReportCapability struct {
9090
}
9191

9292
type ReportScenario struct {
93-
Feature string `json:"feature"`
94-
Name string `json:"name"`
95-
Tags []string `json:"tags,omitempty"`
96-
Outcome Outcome `json:"outcome"`
97-
Reason string `json:"reason,omitempty"`
98-
DurationMs float64 `json:"durationMs,omitempty"`
93+
Feature string `json:"feature"`
94+
Name string `json:"name"`
95+
// Example is the Examples row this entry came from, keyed by column header,
96+
// present only for a scenario expanded from a Scenario Outline.
97+
//
98+
// It is what makes such an entry identifiable. Feature and name are shared by
99+
// every row of an outline -- eleven rows of the type-mismatch matrix in
100+
// errors.feature produce eleven otherwise identical entries -- so without it
101+
// a report cannot say which row failed.
102+
//
103+
// The values are the cells verbatim, as strings. Gherkin has no types, so the
104+
// cell "1" is reported as "1" and not as 1; the report says what the table
105+
// said and leaves the interpretation to whoever reads it.
106+
Example map[string]string `json:"example,omitempty"`
107+
Tags []string `json:"tags,omitempty"`
108+
Outcome Outcome `json:"outcome"`
109+
Reason string `json:"reason,omitempty"`
110+
DurationMs float64 `json:"durationMs,omitempty"`
99111
}
100112

101113
// scenarioRecord is what the runner accumulates as scenarios execute.
102114
type scenarioRecord struct {
103-
feature string
104-
name string
105-
tags []string
106-
outcome Outcome
107-
reason string
108-
duration time.Duration
115+
feature string
116+
name string
117+
example map[string]string
118+
// exampleOrder is the row's position in its scenario's Examples tables, kept
119+
// only to sort the report. Ordering by the parameter values would list the
120+
// rows of a matrix in an order the feature file never mentions, which makes a
121+
// report needlessly hard to read next to the table it came from.
122+
exampleOrder int
123+
tags []string
124+
outcome Outcome
125+
reason string
126+
duration time.Duration
109127
}
110128

111129
// buildReport assembles the report from what the run observed.
@@ -127,7 +145,10 @@ func (r *runner) buildReport() Report {
127145
if records[i].feature != records[j].feature {
128146
return records[i].feature < records[j].feature
129147
}
130-
return records[i].name < records[j].name
148+
if records[i].name != records[j].name {
149+
return records[i].name < records[j].name
150+
}
151+
return records[i].exampleOrder < records[j].exampleOrder
131152
})
132153

133154
scenarios := make([]ReportScenario, 0, len(records))
@@ -145,6 +166,7 @@ func (r *runner) buildReport() Report {
145166
scenarios = append(scenarios, ReportScenario{
146167
Feature: rec.feature,
147168
Name: rec.name,
169+
Example: rec.example,
148170
Tags: rec.tags,
149171
Outcome: rec.outcome,
150172
Reason: rec.reason,

0 commit comments

Comments
 (0)