feat(provider-tck): fail a run that did not execute the canonical set

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>

Author: aepfli

libs/shared/provider-tck/README.md

@@ -288,6 +288,18 @@ is reported as skipped and *never* as passed — checkable by a consumer rather
 runner's summary being trustworthy. The harness checks 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.
 
+**The canonical scenarios must also have actually run.** The accounting above proves the report has
+an entry per scenario; it does not prove the run produced those entries, because a scenario is
+registered when it is *defined* and one the runner declines to run carries a placeholder failure
+instead. So a filtered run — `jest -t`, a `testPathIgnorePatterns` entry, a mistake in the extension
+wiring — satisfies the accounting and goes green while its report supports nothing. The harness
+therefore fails the suite unless every canonical scenario reached a decision. A capability skip is a
+decision and passes the check; extension scenarios are excluded from it, because which of their own
+scenarios an adopter runs is the adopter's business.
+
+Working on one scenario with `-t` therefore ends in a failed suite. That is the intended cost: the
+alternative is a green run that cannot be told apart from a complete one.
+
 ### Reading the stream
 
 | Question | Where the answer is |
@@ -326,7 +338,8 @@ hand-rolled and no id is invented.
 
 A scenario is registered when it is *defined*, so one Jest never finished — a timeout, or a `-t`
 filter — still appears, as a failure that says so. **A report from a filtered run is partial by
-construction; do not publish one.**
+construction**, and the canonical-coverage check above fails the suite rather than leaving that to
+be noticed.
 
 **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

libs/shared/provider-tck/src/lib/report.spec.ts

@@ -12,6 +12,7 @@ import {
   REPORT_DIR_ENV,
   coverageProblems,
   reportBaseName,
+  unexecutedScenarios,
   writeConformanceReport,
 } from './report';
 import { loadTckFeatures } from './runProviderTck';
@@ -179,15 +180,19 @@ describe('scenario accounting', () => {
     ]);
   });
 
-  it('accounts for every planned scenario exactly once', () => {
-    const recorder = new ConformanceRecorder({
+  /** A recorder writing into the stream the canonical features above were registered with. */
+  const recorderInto = () =>
+    new ConformanceRecorder({
       suiteName: 'unit',
       control,
       declared: new Set(declared),
       notApplicable: new Map(),
       messages,
     });
 
+  it('accounts for every planned scenario exactly once', () => {
+    const recorder = recorderInto();
+
     for (const feature of plans) {
       for (const { name, pickleId, example, missing } of feature.scenarios) {
         const identity = { feature: feature.feature, name, pickleId, example };
@@ -233,6 +238,50 @@ describe('scenario accounting', () => {
         '[key=string-flag requested=Integer default=1]: expected 1 outcome(s), recorded 2',
     ]);
   });
+
+  it('does not call a scenario executed until the runner has decided about it', () => {
+    // The distinction the canonical guard turns on, and the reason `coverageProblems` alone is not
+    // enough. A scenario is registered when it is *defined*, so one Jest then declines to run is
+    // present in the report -- carrying the placeholder failure -- and satisfies the accounting
+    // above while having proved nothing.
+    const recorder = recorderInto();
+    const held = planned.find((scenario) => !scenario.example) as ScenarioIdentity;
+
+    const complete = recorder.started(held);
+    for (const scenario of planned.filter((entry) => entry !== held)) {
+      recorder.started(scenario)({ durationMs: 0 });
+    }
+
+    expect(unexecutedScenarios(planned, recorder.settled)).toEqual([`${held.feature}.feature: ${held.name}`]);
+
+    complete({ durationMs: 1 });
+    expect(unexecutedScenarios(planned, recorder.settled)).toEqual([]);
+  });
+
+  it('treats a capability skip as a decision rather than as a scenario that did not run', () => {
+    // Declaring fewer capabilities narrows what the suite asks, and says so in the declaration and
+    // in every skipped result's reason. It is a claim the provider is making, not a hole in the run,
+    // so the guard must not fire on it -- otherwise no provider could adopt the suite selectively.
+    const recorder = recorderInto();
+    const gated = plans.flatMap((feature) =>
+      feature.scenarios
+        .filter((scenario) => scenario.missing.length)
+        .map(({ name, pickleId, example, missing }) => ({
+          identity: { feature: feature.feature, name, pickleId, example },
+          missing,
+        })),
+    );
+
+    expect(gated.length).toBeGreaterThan(0);
+    gated.forEach(({ identity, missing }) => recorder.skipped(identity, missing));
+
+    expect(
+      unexecutedScenarios(
+        gated.map(({ identity }) => identity),
+        recorder.settled,
+      ),
+    ).toEqual([]);
+  });
 });
 
 describe('writing the report', () => {

libs/shared/provider-tck/src/lib/report.ts

@@ -152,6 +152,7 @@ const UNREPORTED =
  */
 export class ConformanceRecorder {
   private readonly recorded: ScenarioIdentity[] = [];
+  private readonly settledIds = new Set<string>();
 
   constructor(private readonly context: RecorderContext) {}
 
@@ -164,6 +165,10 @@ export class ConformanceRecorder {
    */
   skipped(scenario: ScenarioIdentity, missing: readonly Capability[]): void {
     this.register(scenario, { status: TestStepResultStatus.SKIPPED, message: this.skipReason(missing) });
+    // A gate skip is a settled outcome, not an absent one. The question was put to the suite and
+    // answered: this provider does not claim the capability. That is exactly what a conformance
+    // report is for, so it must not read as a scenario the run failed to reach.
+    this.settledIds.add(scenario.pickleId);
   }
 
   /**
@@ -176,7 +181,12 @@ export class ConformanceRecorder {
   started(scenario: ScenarioIdentity): (completion: { durationMs: number; error?: unknown }) => void {
     const complete = this.register(scenario, { status: TestStepResultStatus.FAILED, message: UNREPORTED });
 
-    return ({ durationMs, error }) =>
+    return ({ durationMs, error }) => {
+      // Settled only here, when the scenario's body has actually run. Registration happens when the
+      // scenario is *defined*, which Jest does even for a scenario it then declines to run, so
+      // registration alone says nothing about execution.
+      this.settledIds.add(scenario.pickleId);
+
       complete(
         error === undefined
           ? { status: TestStepResultStatus.PASSED, durationMs }
@@ -186,6 +196,7 @@ export class ConformanceRecorder {
               message: error instanceof Error ? error.message : String(error),
             },
       );
+    };
   }
 
   /**
@@ -196,13 +207,27 @@ export class ConformanceRecorder {
    */
   skippedUnexpectedly(scenario: ScenarioIdentity, reason: string): void {
     this.register(scenario, { status: TestStepResultStatus.FAILED, message: reason });
+    // Settled: the run reached this scenario and decided about it. The decision was wrong, and it is
+    // recorded as a failure, which is a different complaint from the one the coverage guard makes.
+    this.settledIds.add(scenario.pickleId);
   }
 
   /** Every scenario recorded so far, in the order it was recorded. */
   get results(): readonly ScenarioIdentity[] {
     return this.recorded;
   }
 
+  /**
+   * The scenarios whose outcome the run actually determined, by pickle id.
+   *
+   * Narrower than {@link results}, and the difference is the point: a scenario is *recorded* when it
+   * is defined and *settled* when the runner has decided about it. Everything recorded but not
+   * settled is a scenario the report carries a placeholder failure for rather than a result.
+   */
+  get settled(): ReadonlySet<string> {
+    return this.settledIds;
+  }
+
   /** How many scenarios ended in each Cucumber status, for a line a human reads. */
   get statusCounts(): Record<string, number> {
     return this.context.messages.statusCounts;
@@ -343,6 +368,28 @@ export function coverageProblems(
   return problems.sort();
 }
 
+/**
+ * The scenarios that were planned but whose outcome the run never determined.
+ *
+ * {@link coverageProblems} asks whether the report accounts for every scenario; this asks the
+ * separate question of whether the run *answered* for them. The two differ because a scenario is
+ * registered when it is defined, so one Jest declined to run is present in the report -- carrying
+ * the placeholder failure {@link ConformanceRecorder.started} wrote -- and therefore satisfies the
+ * accounting while proving nothing.
+ *
+ * A capability skip is settled and does not appear here. Declaring fewer capabilities narrows what
+ * the suite asks; it is visible in the report's declaration and in each skipped result's reason, and
+ * it is a claim the provider is making rather than a hole in the run.
+ *
+ * Give this the canonical scenarios only. An adopter's own scenarios are theirs to filter.
+ */
+export function unexecutedScenarios(planned: readonly ScenarioIdentity[], settled: ReadonlySet<string>): string[] {
+  return planned
+    .filter((scenario) => !settled.has(scenario.pickleId))
+    .map(scenarioName)
+    .sort();
+}
+
 /**
  * A scenario's identity as one printable string, for a diagnostic a person has to act on.
  *

libs/shared/provider-tck/src/lib/runProviderTck.ts

@@ -10,7 +10,9 @@ import type { FeatureMessages } from './messages';
 import { ConformanceMessages, readFeatureMessages } from './messages';
 import type { TckOptions } from './options';
 import { resolveCapabilities } from './options';
-import { ConformanceRecorder, coverageProblems, writeConformanceReport } from './report';
+import type { ScenarioIdentity } from './report';
+import { ConformanceRecorder, coverageProblems, unexecutedScenarios, writeConformanceReport } from './report';
+import { SPEC_REVISION } from './revision';
 import type { FeaturePlan } from './scenarioRunner';
 import { planFeature, scenarioRunner } from './scenarioRunner';
 import { TckState } from './state';
@@ -311,17 +313,42 @@ export function runProviderTck(options: TckOptions): void {
       // skipped and never as passed. That is only checkable if the report accounts for every
       // scenario, so the accounting is verified here rather than assumed -- in every run, not only
       // when a report is being written.
-      const planned = plans.flatMap((plan) =>
-        plan.scenarios.map(({ name, pickleId, example }) => ({ feature: plan.feature, name, pickleId, example })),
-      );
-      const problems = coverageProblems(recorder.results, planned);
+      const plannedIn = (wanted: boolean): ScenarioIdentity[] =>
+        plans.flatMap((plan, position) =>
+          features[position].canonical === wanted
+            ? plan.scenarios.map(({ name, pickleId, example }) => ({ feature: plan.feature, name, pickleId, example }))
+            : [],
+        );
+
+      const canonicalPlanned = plannedIn(true);
+      const problems = coverageProblems(recorder.results, [...canonicalPlanned, ...plannedIn(false)]);
       if (problems.length) {
         throw new Error(
           `provider-tck [${options.name}]: the conformance report does not account for every ` +
             `scenario exactly once, so it cannot be trusted:\n  ${problems.join('\n  ')}`,
         );
       }
 
+      // The accounting above 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 Jest
+      // then declined to run carries a placeholder failure and satisfies the accounting anyway. A
+      // partial run therefore goes green today -- a `-t` filter, a `testPathIgnorePatterns` entry,
+      // or a mistake in the extension wiring -- while its report claims nothing it can support.
+      //
+      // The canonical set is the entire content of a conformance claim, so it is checked. Extension
+      // scenarios are excluded: they are the adopter's, and filtering them is the adopter's business.
+      const unexecuted = unexecutedScenarios(canonicalPlanned, recorder.settled);
+      if (unexecuted.length) {
+        throw new Error(
+          `provider-tck [${options.name}]: ${unexecuted.length} of ${canonicalPlanned.length} ` +
+            `canonical scenarios did not run, so this run cannot support a conformance claim and ` +
+            `its report must not be published. The canonical set is the one in open-feature/spec ` +
+            `at ${SPEC_REVISION || 'an unknown revision'}; it is fixed, and running less of it is ` +
+            `not a configuration. If you filtered deliberately -- 'jest -t' while working on a ` +
+            `single scenario -- this failure is the expected consequence.\n  ${unexecuted.join('\n  ')}`,
+        );
+      }
+
       const written = writeConformanceReport(recorder, options.name);
       if (written) {
         const counts = Object.entries(recorder.statusCounts)