feat(provider-tck): let an adopter add scenarios and steps to the run - #1622
Closed
aepfli wants to merge 7 commits into
Closed
feat(provider-tck): let an adopter add scenarios and steps to the run#1622aepfli wants to merge 7 commits into
aepfli wants to merge 7 commits into
Conversation
Set PROVIDER_TCK_REPORT_DIR and each suite writes <dir>/<name>.json, conforming to the report schema in open-feature/spec (spec#424, schema in spec#425). Unset means no report, which is not an error. An environment variable rather than a TckOptions field, so that emitting 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. Every scenario appears exactly once, whatever its outcome. That is what makes Appendix F's rule -- a scenario skipped for an undeclared capability is reported as skipped and never as passed -- checkable by a consumer rather than dependent on the runner's summary. The harness verifies the accounting itself at the end of every run, report or no report, and fails the suite if a scenario is missing or recorded twice; that double-record is the bug that bit the Go implementation. Outcomes are recorded where the decision is made rather than scraped out of a Jest reporter. The harness already supplies jest-cucumber with the describe/test pair it calls, and already plans every scenario ahead of the run, so recording an outcome is a matter of wrapping the calls it was making anyway and asserting each definition against the plan. A scenario is registered when it is defined, so one Jest never finished still appears, as a failure that says so. JavaScript is the language that needs the fourth outcome. @strict-numeric-typing is unsatisfiable here because there is no integer type, so reporting it as not-declared would show every JavaScript provider as missing something none of them can have. TckOptions gains notApplicable, which gates identically to an undeclared capability but reports not-applicable. tck.specRevision and tck.assetsTree are captured at build time by scripts/write-revision.js, run from the pullSpec target, because the spec submodule is not part of the published npm package. Git or the submodule being unavailable leaves the committed values alone rather than guessing. BackendControl gains an optional controlApi member, so adding it breaks no existing implementation; a control that omits it omits the field. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
scenarios[] entries were identified by feature and name. Every row of a Scenario Outline shares one name, so errors.feature's type-mismatch matrix produced eleven entries a consumer cannot tell apart: if one row failed and ten passed, the report could not say which, and anything keying on feature and name kept whichever row it saw last. Each entry now carries the Examples row it came from, keyed by column header, with the cell contents verbatim as strings -- Gherkin has no types, so 1 stays "1". It is present only for a scenario originating from an outline, including one the capability gate skipped: a skipped row is still a row, and dropping its example would make the four @object rows indistinguishable in exactly the report that is supposed to explain them. name is now the scenario name as the feature file writes it, placeholders and all, rather than jest-cucumber's expanded title. The expanded title is the runner's string, and Go's and Python's runners produce different ones for the same row, which defeats the cross-language comparison the format exists for. 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 would put a separator, an ordering and an escaping rule into normative text that four languages must reproduce byte for byte, and drift there is invisible until two reports quietly fail to line up. The three implementations had already diverged on precisely that point. jest-cucumber substitutes each row into the outline and keeps only the result, so the Examples tables are read from the feature file separately with the same Gherkin parser it uses. Pairing the two parses positionally is sound only while both derive from the same file, so it is checked -- outline titles and row counts must agree -- and the suite fails rather than reporting a row it cannot identify. @cucumber/gherkin becomes a peer dependency; jest-cucumber already depends on it directly, so no adopter gains an install. The run's own accounting is keyed on the example too. Keyed on the name alone, dropping one of eleven rows and recording another twice tallied as eleven expected and eleven recorded, and the check that exists to catch a dropped scenario would have passed. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…ng untested ones
Three defects in the capability rollup, all of them ways the report could report
a result the suite never obtained.
A failed capability was emitted as {"state": "failed"} with no reason. The schema
requires a reason for any outcome other than passed, so that entry does not
validate -- and it appears only when a provider is actually failing, which is
precisely when the report matters. It now says how many of how many scenarios
carrying the tag failed, and points at the per-scenario results for which and why.
No test caught it because every self-test suite passes, so nothing that runs end
to end reaches that branch. The new test drives the report builder directly with
synthetic records, which is the only way to exercise a failure without breaking a
provider on purpose.
A declared capability that no scenario carries was reported as passed. @targeting
is reserved -- it is in the vocabulary and nothing tests it, because asserting
that an evaluation context reached the backend needs an echo operation the control
API does not have -- so a provider declaring it got a green result for a claim
nothing had examined.
A declared capability whose every scenario was skipped for a *different* one was
likewise reported as passed, because the rollup counted a capability as exercised
by tag presence rather than by execution. Both scenarios in events.feature carry
@events as well as @Stale or @configuration-change, so the in-memory suite
reported @events as passed while neither scenario ran.
Both are the vacuous pass the capability vocabulary was introduced to eliminate,
arriving through the report rather than through the suite. A scenario now counts
towards a capability only if it actually ran, and a declared capability with
nothing to show is omitted. The suite asked no question of it, so it has no answer
to report, and a consumer sees the tag is absent rather than a pass it cannot rely
on. Omitting is preferred to inventing a fifth outcome: the four in the schema are
about what the provider did, and "this run demonstrated nothing" is a fact about
the run.
An undeclared capability is still reported as not-declared with its reason, and
not-applicable is unaffected -- @strict-numeric-typing being unanswerable in a
language with no integer type is a different statement from a capability nothing
exercised, and both need saying.
Matches the same fix in Go, open-feature/go-sdk-contrib#944.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The report defined a result format of its own: a scenarios[] list, a four-value
outcome enum, an example field for Scenario Outline row identity, and a git tree
hash for the executed assets. A maintainer pushed back on inventing one, and was
right to. All of it is specified by Cucumber Messages already, and a second
definition is a second thing to version and two places for the same fact to
disagree.
A run now writes two files. <dir>/<name>.ndjson is a Cucumber Messages stream --
Meta, Source, GherkinDocument, Pickle, TestCase, TestRunStarted, then
TestCaseStarted/TestStepFinished/TestCaseFinished per scenario and TestRunFinished.
<dir>/<name>.json is the envelope from the reshaped schema: what was tested, what
the provider declares, and where the results are with a sha256 digest over them.
jest-cucumber has no output layer at all -- no reporter, no JSON, nothing -- so the
stream is built by the harness. Two things it already did make that cheap rather
than painful. It plans every scenario before the run, which is where the Pickle and
TestCase messages come from; and it owns the test/test.skip calls, which is where
the outcomes are recorded, at the point the decision is made rather than scraped
back out of a reporter afterwards. @cucumber/gherkin compiles Source,
GherkinDocument and Pickle from the feature files, so no message here is hand-rolled
and no id invented. @cucumber/messages joins it as a peer dependency; jest-cucumber
already pulls both in, so no adopter gains an install.
Four things are deleted, each because Messages carries the fact better:
- scenarios[] and the outcome enum. A scenario is a Pickle with a TestCase, and
its outcome is TestStepResult.status. A capability-gated scenario is SKIPPED
with the reason on the result, whichever kind of skip it was: not-applicable
moves out of the per-scenario outcome and into declaration.notApplicable, where
it is stated once with a reason instead of repeated on every scenario carrying
the tag. @strict-numeric-typing is unsatisfiable in JavaScript -- a fact about
the language, not about a run -- so the suites now pass the reason with it, and
NO_INTEGER_TYPE_IN_JAVASCRIPT is exported so every JS report words it the same.
- example. Pickle.astNodeIds identifies an outline row exactly: the second id is
the TableRow in the GherkinDocument. Four implementations had each reinvented
this and diverged -- a bare scenario name, a runner's example id, jest-cucumber's
expanded title. errors.feature's eleven type-mismatch rows share a name, and
share their expanded title too, so the AST node is the only thing that tells
them apart. examples.ts, which re-parsed each feature file to recover the rows,
goes with it; the rows now come from the same parse that produced the pickles,
and are kept only to name a row in a diagnostic.
- assetsTree. The stream carries every executed feature file verbatim as a Source,
which identifies the assets by content, covers only what actually ran, and sits
under the report's digest. write-revision.js stops computing the tree hash.
- the per-capability roll-up, which the reshaped schema does not have. It is
derivable from the declaration and the stream, and the vacuous passes it guarded
against -- a reserved tag no scenario carries, a capability whose every scenario
was skipped for a different one -- are avoided by counting test cases that ran
rather than tags present. The README says so for a consumer computing one.
The properties that mattered are kept, and still checked on every run rather than
promised: the harness refuses outright to run a scenario whose capabilities are
unmet, and verifies at the end that the stream accounts for every planned scenario
exactly once -- now keyed on the pickle rather than on feature, name and example.
One TestStep per test case, not one per Gherkin step. jest-cucumber runs a whole
scenario as a single Jest test and reports one outcome for it; it never says which
step failed. A step per Gherkin step would mean inventing per-step results --
marking them all failed over-claims, marking one of them failed picks a step at
random -- so the stream carries the granularity the runner has. The steps are in the
stream on the Pickle with the outline row already substituted, and jest-cucumber's
failure message names the step it was on.
Verified end to end. The in-memory suite writes 29 pickles, 29 test cases and 29
outcomes: 24 PASSED and 5 SKIPPED, every gated scenario SKIPPED with its capability
named and none reported as passed. The envelope validates against the reshaped
schema under a Draft 2020-12 validator and its digest matches the stream. Every
envelope in the stream validates against the Envelope.json schema @cucumber/messages
ships, which messages.spec.ts now asserts on every run. The multi-provider suite,
run with its exclusion overridden to exercise the failure path, reports 16 FAILED,
8 PASSED and 5 SKIPPED, matching Jest's own tally.
Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
…he stream The envelope named the results format but not its version, and Messages is versioned. This implementation is on 24.1.0 while the Go TCK builds against v21 and the Python one on 34.2.0, so a consumer holding two reports cannot assume one schema validates both. Guessing is worse than not validating. A later schema accepts messages this producer could not have emitted, and an earlier one rejects messages that are perfectly valid, so a check against the wrong version reports a result that has nothing to do with the stream. The value comes from the same `version` export the stream already reports in its own Meta message, so the envelope and the stream cannot disagree about which release produced it, and a dependency bump cannot leave the report claiming the release it used to build against. Two existing assertions compared the whole results object with toEqual and so failed on the new field, which is the behaviour worth having -- a field added to a published envelope should not slip in unnoticed. Both now name it. The fixture takes the real library version rather than a literal for the same reason. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
A vendor with provider-specific behaviour -- flagd's `fractional` targeting is the motivating case -- has scenarios the canonical set cannot carry. The alternative to taking them here is a parallel harness that reimplements the provider lifecycle, the per-scenario backend reset and the report, and then drifts from them. `extensionFeatures` names feature files loaded alongside the packaged ones, and `extensionSteps` contributes step definitions into the same `autoBindSteps` call. Both are optional; absent is today's run exactly. One `describe`, one provider lifecycle, one capability gate, one report, and an extension scenario may use the canonical vocabulary freely. An extension cannot shadow a canonical scenario. A feature named after a canonical one is refused, as is one inside the canonical asset directory or a second extension reusing a name -- the substitution the Java prototype allowed, where a same-named file in a second root replaced the canonical scenario and the suite went green having run the adopter's version, is not representable. The canonical features are also loaded unconditionally and first, so no extension wiring can prevent one from being planned. Extension scenarios are named under an `extensions/` URI prefix in the results stream rather than a path in open-feature/spec, which is how a report consumer tells them from canonical ones. They take part in the existing per-scenario accounting like any other scenario. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
The existing accounting proves the report has an entry for every scenario. It does not prove the run produced those entries: a scenario is registered when it is defined, so one the runner declines to run carries a placeholder failure and satisfies the accounting anyway. A partial run therefore goes green -- a `-t` filter, a `testPathIgnorePatterns` entry, a mistake in the extension wiring -- while its report supports nothing. The recorder now separates a scenario it has *recorded* from one the run has *settled*, and the suite fails unless every canonical scenario reached a decision. A capability skip is a decision and passes: declaring fewer capabilities narrows what the suite asks, says so in the declaration and in each skipped result's reason, and is a claim the provider is making rather than a hole in the run. Extension scenarios are excluded -- which of their own an adopter runs is the adopter's business. The limit is Jest's: a suite whose tests are all filtered out, or whose file is excluded, never reaches `afterAll`, so nothing runs to complain. That case produces no report at all rather than a green partial one. Working on one scenario with `-t` now ends in a failed suite. That is the cost of not being able to mistake a partial run for a complete one. Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
aepfli
force-pushed
the
feat/provider-tck-report
branch
2 times, most recently
from
September 11, 2026 11:17
8e88c9b to
14191cb
Compare
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lets an adopter run their own scenarios and steps inside the TCK's suite, in the same backend lifecycle phase.
Stacked on #1610. Answers @toddbaert's review request on open-feature/spec#423: a vendor with provider-specific behaviour (flagd's
fractional, say) should extend the TCK rather than maintain a parallel harness that reimplements the lifecycle and then drifts from it.What an adopter writes
JavaScript has no runtime scanning, so this is explicit configuration — unlike Java and Python, which get zero-config conventions. Both options are optional; absent means today's behaviour exactly.
Extension steps are bound in the same
autoBindStepscall as the canonical ones, so an extension scenario can use the whole canonical vocabulary and only needs new steps for what it adds.Extensions cannot shadow canonical scenarios
Enforced, not documented: extension features must live in their own directory and a feature named after a canonical one is refused. This is deliberate — the equivalent hazard was measured in Java, where a same-named file in a second classpath root silently replaced the canonical one and the suite went green having run the vendor's version instead. That is the worst failure available to a conformance suite, so it is made impossible rather than discouraged.
Extension scenarios are counted and reported, and stay distinguishable from canonical ones.
Second commit is separable
feat(provider-tck): fail a run that did not execute the canonical setcan be dropped independently if a reviewer considers it out of scope for an extensibility PR. It exists because a partial run currently produces a well-formed, valid report: in the Go implementation,-runon a single scenario passed green and emitted a report covering 1 of 29 canonical scenarios. Extension scenarios can never close a gap in the canonical set.Verified
tsc --noEmitandnx lint provider-tckclean.Verification was completed by the coordinating session after the implementing agent stalled; the commits are its work, the checks above are mine.