fix(provider-tck): give a failed capability a reason, and stop claiming untested ones

Two defects in the capability rollup, both found by the Java implementation
reviewing this one.

A failed capability was emitted as {"state": "failed"} with no reason. The schema
now requires a reason for any outcome other than passed, so that entry does not
validate -- and it would have appeared only when a provider was 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 ever reaches that branch. The new internal 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 exists in the vocabulary but 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. That is the vacuous pass the capability vocabulary was
introduced to eliminate, arriving through the report rather than through the
suite.

Such a capability is now omitted. The suite asked no question, 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 "the suite does not test this" is a
fact about the suite.

Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>

Author: aepfli

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

@@ -131,10 +131,15 @@ func (r *runner) buildReport() Report {
 	})
 
 	scenarios := make([]ReportScenario, 0, len(records))
-	// failedCapabilities tracks which declared capabilities had a scenario fail,
-	// so a capability is only reported as passed when everything gating on it
-	// actually passed.
-	failedCapabilities := map[Capability]bool{}
+	// 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{
@@ -145,12 +150,14 @@ func (r *runner) buildReport() Report {
 			Reason:     rec.reason,
 			DurationMs: float64(rec.duration.Microseconds()) / 1000.0,
 		})
-		if rec.outcome != OutcomeFailed {
-			continue
-		}
 		for _, tag := range rec.tags {
-			if capability, gates := CapabilityForTag(tag); gates {
-				failedCapabilities[capability] = true
+			capability, gates := CapabilityForTag(tag)
+			if !gates {
+				continue
+			}
+			exercised[capability]++
+			if rec.outcome == OutcomeFailed {
+				failed[capability]++
 			}
 		}
 	}
@@ -165,8 +172,18 @@ func (r *runner) buildReport() Report {
 					"not declared by this provider's configuration; the %s scenarios were skipped and did not contribute to this result",
 					capability.Tag()),
 			}
-		case failedCapabilities[capability]:
-			capabilities[capability.Tag()] = ReportCapability{State: OutcomeFailed}
+		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}
 		}

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)
+		}
+	}
+}

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

@@ -113,8 +113,8 @@ func TestReportRecordsUndeclaredCapabilities(t *testing.T) {
 	for _, capability := range tck.AllCapabilities() {
 		result, ok := report.Capabilities[capability.Tag()]
 		if !ok {
-			t.Errorf("capability %s is missing from the report; every capability is reported, "+
-				"because an absent one is indistinguishable from one that was forgotten",
+			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
 		}
@@ -193,3 +193,48 @@ func readReport(t *testing.T, path string) tck.Report {
 	}
 	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)
+	}
+}