From e9b49a909f6cf801e2ae43f0e262aff50bb6cd80 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 2 Nov 2026 11:10:16 +0100 Subject: [PATCH 1/3] feat(tck): emit a machine-readable conformance report Collapsed from 29 commits when this branch was replayed onto main, where the base suite landed squashed and reworked: TckRuntime is gone, the steps moved to internal.steps behind TckLifecycle, and the comments were culled. Most of the commits followed spec pins and base revisions that no longer exist, so one squashed replay meant one conflict pass, following the js-sdk-contrib replay of the same branch. What moved with the base: - The run metadata TckRuntime used to hold now lives on TckRunMetadata. TckLifecycle records it when a suite starts, and ProviderSteps records the provider's own name. Its two static entry points are public only because the lifecycle sits in the internal package, the pattern AdopterTags and CanonicalScenarioGuard.observe follow. - The report writes backend.backendType from BackendControl.backendType(), so a control that states none is reported as "custom", matching the schema change in spec#425. - The report self-test gates through Capability.requireDeclared, which replaced CapabilityGate. - The README sections are cut to main's style. The original subjects, oldest first: - feat(provider-tck): emit a machine-readable conformance report - feat(provider-tck): fail a run that executes less than the canonical set - chore(provider-tck): follow the spec submodule bump in the reported revision - fix(provider-tck): follow the capability model in the declaration test - chore(provider-tck): follow the spec submodule bump in the reported revision - test(provider-tck): follow the capability change in the declaration test - chore(provider-tck): follow the disabled-flag capability in the report - refactor(tck): follow the rename into the reporting machinery - refactor(tck): always write the backend block, and type controlApi - refactor(tck)!: TCK knobs lose the PROVIDER_ prefix - feat(tck): report the revision that gated the reasons - feat(tck): close the loop from the gitlink to the revision a report names - chore(tck): follow the re-pin to aa2ad24f in the revision a report names - test(tck): stop the report fixture depicting the shape the guidance discourages - docs(tck): stop naming an adoption's class in the report's own docs - docs(tck): follow the base README rewrite into the reporting section - docs(tck): stop naming a canonical count the suite has already outgrown - docs(tck): drop two notes about what a previous revision said - chore(tck): follow the re-pin to d47a66eb in the revision a report names - chore(tck): follow the re-pin to bda599f1 in the revision a report names - chore(tck): follow the spec pin in the revision a report names - chore(tck): pin the assets branch in the revision a report names - chore(tck): follow the assets pin in the revision a report names - chore(tck): move the recorded spec revision with the re-pin - chore(tck): move the recorded spec revision with the re-pin - chore(tck): follow the spec pin to the current #423 tip - chore(tck): pin the spec assets to main now that #423 has merged - chore(tck): follow the spec assets to their first release, v0.1.0 - style(provider-tck): wrap the tag assertion the way spotless wants Signed-off-by: Simon Schrottner --- tools/tck/README.md | 50 ++ tools/tck/pom.xml | 51 ++ .../tools/tck/CanonicalScenarioGuard.java | 294 +++++++ .../contrib/tools/tck/CanonicalScenarios.java | 260 +++++++ .../contrib/tools/tck/ConformanceReport.java | 253 ++++++ .../tools/tck/ConformanceReportPlugin.java | 300 +++++++ .../contrib/tools/tck/ProviderTck.java | 4 +- .../contrib/tools/tck/ProviderTckTest.java | 6 +- .../contrib/tools/tck/ReportNames.java | 42 + .../contrib/tools/tck/TckBuildInfo.java | 124 +++ .../contrib/tools/tck/TckRunMetadata.java | 154 ++++ .../tools/tck/internal/TckSuiteListener.java | 17 +- .../tck/internal/steps/ProviderSteps.java | 13 +- .../tck/internal/steps/TckLifecycle.java | 2 + .../contrib/tools/tck/tck-build.properties | 7 + .../tools/tck/CanonicalScenarioGuardTest.java | 241 ++++++ .../tck/ConformanceReportPluginTest.java | 731 ++++++++++++++++++ .../tck/selftest/ReportSelfTestSteps.java | 74 ++ .../resources/report-selftest/report.feature | 46 ++ 19 files changed, 2659 insertions(+), 10 deletions(-) create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java create mode 100644 tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java create mode 100644 tools/tck/src/main/resources-filtered/dev/openfeature/contrib/tools/tck/tck-build.properties create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java create mode 100644 tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java create mode 100644 tools/tck/src/test/resources/report-selftest/report.feature diff --git a/tools/tck/README.md b/tools/tck/README.md index ec1851827d..871d7b7f42 100644 --- a/tools/tck/README.md +++ b/tools/tck/README.md @@ -231,6 +231,56 @@ provider's pom (or pass `-Dtck.testFailureIgnore=true`): ``` +## The canonical set cannot be reduced + +`CanonicalScenarioGuard` is a JUnit test the suite selects. It fails the build when the run is set up +to execute less than the canonical set: a feature file added to or shadowing one in `gherkin/`, +`cucumber.filter.tags` or `cucumber.filter.name`, or selectors or glue overridden in +`junit-platform.properties`. It checks the plan and the filter configuration before the first +scenario, so it needs no backend. Extension scenarios are ignored. + +Narrow a run with `capabilities()`, whose skips are reported with a reason. To filter anyway while +debugging, set `-Dtck.partial=true` (or `TCK_PARTIAL`); the guard then reports itself as skipped +rather than passed. + +## Conformance reports + +Set `TCK_REPORT_DIR` (or `-Dtck.report.dir=...`) and each suite writes an envelope conforming to the +[report schema][report-schema] and its results as a [Cucumber Messages][messages] stream. Unset means +no report. Several suites in one JVM each write their own pair. + +```console +$ TCK_REPORT_DIR=./reports mvn -Ptck -pl providers/flagd test +$ ls reports/ +flagd-in-process.json flagd-in-process.ndjson flagd-rpc.json flagd-rpc.ndjson +``` + +The `.ndjson` is written by Cucumber's own `MessageFormatter`, the class behind the built-in +`message:` plugin; a plugin of its own exists only so the path can follow the run. A scenario's +outcome is the most severe result among its steps, hooks included, which is what makes a +capability-gated skip read as `SKIPPED`. A scenario is identified by `pickle.astNodeIds`, whose +second entry names the `Examples` row of an outline. + +The envelope carries what the stream cannot: + +- **`provider`**: the provider's own metadata name, and the suite name as its `configuration` + (`ProviderTckHarness.configuration()` overrides the derived one). +- **`sdk`**: the SDK version on the classpath at runtime. +- **`tck`**: the implementation and `specRevision`, the open-feature/spec commit the packaged assets + came from, baked in from this module's POM. +- **`backend`**: the control's description and `backendType`, which the control states through + `BackendControl.backendType()`: `http`, `in-process`, or `custom` for one that states nothing. + Always present, never inferred. +- **`declaration`**: the declared capabilities. A skip for a capability this SDK cannot express + carries a reason worded unlike an undeclared one, so the two are distinguishable in the results. + +`knownDeviations` says whether a withheld capability is a limitation or a bug; `results.digest` +covers the `.ndjson`. Every scenario appears exactly once, which `ConformanceReportPluginTest` +asserts over a real Cucumber run. + +[report-schema]: https://github.com/open-feature/spec/blob/main/specification/assets/provider-tck/report/conformance-report.schema.json +[messages]: https://github.com/cucumber/messages + ## Extensions Provider-specific scenarios (e.g. flagd's `fractional`) run inside the same suite: diff --git a/tools/tck/pom.xml b/tools/tck/pom.xml index 3d1906a0e1..434cf7feaa 100644 --- a/tools/tck/pom.xml +++ b/tools/tck/pom.xml @@ -18,6 +18,12 @@ false + + 10b1a610ed0afe6bf0e38b441d7362f5a76cb47d 3.27.7 4.3.0 2.22.3 @@ -66,6 +72,39 @@ cucumber-junit-platform-engine + + + io.cucumber + cucumber-core + + + + io.cucumber + messages + + + + + + io.cucumber + gherkin + compile + + + + + org.junit.jupiter + junit-jupiter-api + compile + + + + + src/main/resources + false + + + src/main/resources-filtered + true + + + diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java new file mode 100644 index 0000000000..4d0fc92e74 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java @@ -0,0 +1,294 @@ +package dev.openfeature.contrib.tools.tck; + +import dev.openfeature.contrib.tools.tck.internal.TckSuiteListener; +import io.cucumber.junit.platform.engine.Constants; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.platform.engine.TestSource; +import org.junit.platform.engine.support.descriptor.ClasspathResourceSource; +import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; + +/** + * Fails a suite that is set up to run less than the whole canonical scenario set. + * + *

Everything else in this module makes the canonical set easy to extend safely. Nothing makes it + * hard to shrink, and shrinking it is the failure that matters: a run that asks fifty-four of the + * fifty-six questions and reports success is indistinguishable, in every artifact it produces, from + * one that asked all fifty-six. The known ways to get there are a feature file dropped into + * {@code gherkin/}, a {@code cucumber.filter.tags} or {@code cucumber.filter.name} expression, and + * selectors or glue overridden in a consuming module's {@code junit-platform.properties}. + * + *

Selected by {@link ProviderTckTest} as an ordinary JUnit Jupiter test, so a reduced set + * fails the build the way any other failing test does. A {@link + * org.junit.platform.launcher.TestExecutionListener} cannot do that job: the JUnit Platform catches + * and logs whatever a listener throws, which is exactly the silent pass being guarded against. + * + *

Two kinds of evidence, both available before a single scenario has run: + * + *

    + *
  • the discovered test plan, captured by {@link TckSuiteListener}. Which + * scenarios a suite selected is settled at discovery, so a canonical file that was shadowed, + * replaced or added to is visible there. + *
  • the effective filter configuration, read through this test's own + * {@link ExtensionContext}. Cucumber applies {@code cucumber.filter.tags} as a skip at + * execution rather than as a discovery filter — a filtered scenario is in the plan and never + * runs — so the plan cannot show it and the configured expression has to be read directly. The + * Jupiter engine inside the suite resolves configuration from the same sources the Cucumber + * engine does, whether the value came from a system property, a + * {@code junit-platform.properties} or an annotation on the suite. + *
+ * + *

Checking the setup rather than counting afterwards is not a compromise made for convenience. A + * count is only complete once the last scenario has finished, and the only hook that runs there is a + * listener, which cannot fail anything. Both kinds of evidence are settled before the first scenario + * runs, so the check itself takes no measurable time and needs no Compose stack — though where its + * result appears in a run depends on the order the JUnit Platform happens to execute the two engines + * in, which is not specified. + * + *

Extension scenarios are ignored entirely. The check is defined over {@code gherkin/}, so an + * adopter's {@code extensions/} scenarios can neither stand in for a canonical scenario nor look + * like a spurious one. + * + *

Separable from the rest of the extension work by design: it guards a bypass rather than enabling + * anything, and dropping it leaves the extension point unaffected. + */ +@ExtendWith(CanonicalScenarioGuard.CaptureConfiguration.class) +public final class CanonicalScenarioGuard { + + /** + * System property that downgrades this check to a skip. + * + *

An escape hatch is necessary rather than a weakness. Running one scenario with + * {@code -Dcucumber.filter.tags=@events} is routine while debugging a provider, and a check that + * made that impossible would be switched off permanently instead of temporarily. It produces a + * skip rather than a pass, so the run says out loud that its canonical set was not verified. + */ + public static final String PARTIAL_PROPERTY = "tck.partial"; + + /** Environment variable equivalent of {@link #PARTIAL_PROPERTY}. */ + public static final String PARTIAL_ENV = "TCK_PARTIAL"; + + /** Cucumber configuration keys that stop a discovered scenario from running. */ + private static final String[] FILTER_KEYS = { + Constants.FILTER_TAGS_PROPERTY_NAME, Constants.FILTER_NAME_PROPERTY_NAME + }; + + /** + * What each TCK suite in a discovered plan selected under {@code gherkin/}. + * + *

Keyed by suite class name and accumulated rather than replaced. A suite discovers its + * children through a launcher of its own, so this is called more than once per build with plans + * of differing scope, and a plan containing no TCK suite says nothing about the ones already + * recorded. + */ + private static final Map> DISCOVERED = new ConcurrentHashMap<>(); + + private static final String CANONICAL_PREFIX = ProviderTck.FEATURES + "/"; + + /** + * Records what the suites in a discovered plan will run. Public only for the suite listener in + * the internal package. + * + * @param plan the plan about to be executed + */ + public static void observe(TestPlan plan) { + for (TestIdentifier root : plan.getRoots()) { + collectSuites(plan, root); + } + } + + /** Forgets what has been observed, so a test can drive the guard over a plan of its own. */ + static void forget() { + DISCOVERED.clear(); + } + + /** + * Returns what each suite in the observed plans selected under {@code gherkin/}. + * + * @return canonical scenarios by suite class name + */ + static Map> discovered() { + return Collections.unmodifiableMap(new LinkedHashMap<>(DISCOVERED)); + } + + /** Asserts that this run will execute the whole canonical scenario set. */ + @Test + @DisplayName("the canonical scenario set was not reduced") + void theCanonicalScenarioSetWasNotReduced() { + if (partialRunAllowed()) { + Assumptions.abort("Skipped: " + PARTIAL_PROPERTY + " is set, so the canonical scenario set was not " + + "verified. This run is not a conformance run."); + } + + String problems = check(CanonicalScenarios.shipped(), discovered(), CaptureConfiguration.filters()); + if (problems != null) { + throw new AssertionError(problems); + } + } + + /** + * Reports every way this run falls short of the canonical set. + * + * @param canonical the canonical scenario set, as the TCK ships it + * @param discovered what each suite selected under {@code gherkin/} + * @param filters configured Cucumber filters, by configuration key + * @return a report of every problem found, or {@code null} when there is none + */ + static String check( + Set canonical, + Map> discovered, + Map filters) { + List problems = new ArrayList<>(); + + for (Map.Entry filter : filters.entrySet()) { + problems.add(filter.getKey() + " is set to '" + filter.getValue() + + "'. Cucumber applies it by skipping scenarios that would otherwise have run, so a " + + "conformance run cannot be filtered. Decline capabilities your provider does not have " + + "through capabilities() instead — those scenarios are reported as skipped with a reason, " + + "which a filtered one is not. To filter anyway while debugging, set -D" + PARTIAL_PROPERTY + + "=true and accept that the run is not a conformance run."); + } + + if (discovered.isEmpty()) { + problems.add("No TCK suite was found in the JUnit test plan, so it cannot be shown that the canonical " + + "scenarios will run. This normally means TckSuiteListener was not auto-registered — the same " + + "condition that makes harness discovery fall back to ServiceLoader. Enable JUnit Platform " + + "listener auto-registration, or set -D" + PARTIAL_PROPERTY + "=true to accept a run whose " + + "canonical set is unverified."); + } + + for (Map.Entry> suite : discovered.entrySet()) { + Set missing = new TreeSet<>(canonical); + missing.removeAll(suite.getValue()); + if (!missing.isEmpty()) { + problems.add(suite.getKey() + " left out " + missing.size() + " of " + canonical.size() + + " canonical scenarios: " + missing + + ". A conformance run executes the canonical set in full. Look for a selector or glue " + + "override in junit-platform.properties, a cucumber.features property, or a feature file " + + "of your own in " + CANONICAL_PREFIX + " shadowing a canonical one. Scenarios your " + + "provider cannot support are declined through capabilities(), which reports them as " + + "skipped rather than removing them."); + } + + Set unexpected = new TreeSet<>(suite.getValue()); + unexpected.removeAll(canonical); + if (!unexpected.isEmpty()) { + problems.add(suite.getKey() + " selected " + unexpected.size() + " scenario(s) under " + + CANONICAL_PREFIX + " that this TCK does not ship: " + unexpected + + ". That directory is the canonical set, and adding to it changes what conformance means. " + + "Put your own scenarios in " + ProviderTck.EXTENSIONS + "/ instead, where they run in the " + + "same suite and the same backend lifecycle."); + } + } + + return problems.isEmpty() ? null : String.join(System.lineSeparator() + System.lineSeparator(), problems); + } + + /** + * Returns the Cucumber filters this run is configured with. + * + * @param context the executing test's context, which resolves configuration the way the Cucumber + * engine beside it does + * @return configured filters by configuration key, empty when the run is unfiltered + */ + static Map filtersIn(ExtensionContext context) { + Map configured = new LinkedHashMap<>(); + for (String key : FILTER_KEYS) { + context.getConfigurationParameter(key) + .map(String::trim) + .filter(value -> !value.isEmpty()) + .ifPresent(value -> configured.put(key, value)); + } + return configured; + } + + /** Returns whether the run has declared itself partial. */ + private static boolean partialRunAllowed() { + return isTrue(System.getProperty(PARTIAL_PROPERTY)) || isTrue(System.getenv(PARTIAL_ENV)); + } + + private static boolean isTrue(String value) { + return value != null && ("".equals(value.trim()) || Boolean.parseBoolean(value.trim())); + } + + private static void collectSuites(TestPlan plan, TestIdentifier identifier) { + Optional> suite = TckSuiteListener.harnessClassOf(identifier); + if (suite.isPresent()) { + Set canonical = new LinkedHashSet<>(); + collectCanonical(plan, identifier, canonical); + DISCOVERED.put(suite.get().getName(), Collections.unmodifiableSet(canonical)); + return; + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectSuites(plan, child); + } + } + + private static void collectCanonical(TestPlan plan, TestIdentifier identifier, Set into) { + if (identifier.isTest()) { + canonicalRefOf(identifier).ifPresent(into::add); + } + for (TestIdentifier child : plan.getChildren(identifier)) { + collectCanonical(plan, child, into); + } + } + + /** + * Returns the canonical scenario a test identifier stands for, if it is one. + * + *

A Cucumber scenario discovered from a classpath resource carries a + * {@link ClasspathResourceSource} naming the feature resource and the scenario's line — for a + * Scenario Outline, the line of the {@code Examples} row it was compiled from, which is what makes + * each row count separately. + */ + private static Optional canonicalRefOf(TestIdentifier identifier) { + Optional source = identifier.getSource(); + if (!source.isPresent() || !(source.get() instanceof ClasspathResourceSource)) { + return Optional.empty(); + } + ClasspathResourceSource resource = (ClasspathResourceSource) source.get(); + String name = resource.getClasspathResourceName(); + if (!name.startsWith(CANONICAL_PREFIX)) { + return Optional.empty(); + } + return resource.getPosition() + .map(position -> new CanonicalScenarios.Ref(name, position.getLine(), identifier.getDisplayName())); + } + + /** + * Hands the guard the configuration of the engine it is running in. + * + *

Jupiter resolves configuration parameters for an extension, not for a test method, so the + * context is captured here rather than injected. Inside a suite it carries the suite's own + * {@code @ConfigurationParameter} values as well as the system properties and + * {@code junit-platform.properties} the Cucumber engine beside it reads. + */ + static final class CaptureConfiguration implements BeforeEachCallback { + + private static volatile Map filters = Collections.emptyMap(); + + @Override + public void beforeEach(ExtensionContext context) { + filters = Collections.unmodifiableMap(filtersIn(context)); + } + + static Map filters() { + return filters; + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java new file mode 100644 index 0000000000..d417f045fa --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java @@ -0,0 +1,260 @@ +package dev.openfeature.contrib.tools.tck; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.cucumber.gherkin.GherkinParser; +import io.cucumber.messages.types.Envelope; +import io.cucumber.messages.types.Examples; +import io.cucumber.messages.types.Feature; +import io.cucumber.messages.types.FeatureChild; +import io.cucumber.messages.types.GherkinDocument; +import io.cucumber.messages.types.Rule; +import io.cucumber.messages.types.RuleChild; +import io.cucumber.messages.types.Scenario; +import io.cucumber.messages.types.TableRow; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.CodeSource; +import java.util.Collections; +import java.util.Enumeration; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.stream.Stream; + +/** + * The canonical scenario set, as this artifact ships it. + * + *

Read from this artifact's own code source — the JAR or {@code target/classes} that + * {@link ProviderTck} was loaded from — rather than through the classloader. That is the point of the + * class: a feature file placed in {@code gherkin/} on another classpath root shadows the canonical + * one of the same name, and a check that read the shadowed copy back through + * {@code getResource("gherkin/errors.feature")} would be checking the replacement against itself. + * The code source is the only view of {@code gherkin/} that an adopter's classpath cannot alter. + * + *

A scenario is identified by its resource name and its line, which is what a Cucumber + * {@code ClasspathResourceSource} in the JUnit test plan carries. For a Scenario Outline that line is + * the {@code Examples} row, so each row counts individually — which is necessary, because eleven rows + * of the type-mismatch matrix share one name. + * + *

What this does not establish is that the canonical files contain what they + * should. A replacement file that happened to place its scenarios on the same lines would satisfy the + * comparison. Content is covered elsewhere and better: the results stream carries the {@code source} + * of every feature that executed, and the report's {@code tck.specRevision} says which revision it + * should match. + */ +final class CanonicalScenarios { + + private static final String FEATURE_SUFFIX = ".feature"; + + private static volatile Set shipped; + + private CanonicalScenarios() {} + + /** + * Returns every scenario the canonical feature files in this artifact compile to. + * + * @return the canonical scenario set, never empty + * @throws IllegalStateException if this artifact's own feature files cannot be read + */ + static Set shipped() { + Set cached = shipped; + if (cached == null) { + synchronized (CanonicalScenarios.class) { + if (shipped == null) { + shipped = Collections.unmodifiableSet(read()); + } + cached = shipped; + } + } + return cached; + } + + /** Reads and compiles the canonical feature files out of the code source this class came from. */ + @SuppressFBWarnings( + value = "PATH_TRAVERSAL_IN", + justification = "The path is this class's own code source, reported by the JVM. Nothing outside the " + + "running artifact can influence it, and reading it from anywhere else would defeat the point " + + "of the class") + private static Set read() { + CodeSource codeSource = ProviderTck.class.getProtectionDomain().getCodeSource(); + URL location = codeSource == null ? null : codeSource.getLocation(); + if (location == null) { + throw new IllegalStateException("The tck code source is not visible to this JVM, so the " + + "canonical scenario set cannot be established. Set -D" + CanonicalScenarioGuard.PARTIAL_PROPERTY + + "=true to run without the canonical-set check, understanding that the run is then not a " + + "conformance run."); + } + + Path path; + try { + path = Paths.get(location.toURI()); + } catch (URISyntaxException | IllegalArgumentException e) { + throw new IllegalStateException( + "The tck code source " + location + " is not a file, so the " + + "canonical scenario set cannot be established.", + e); + } + + Set refs = new LinkedHashSet<>(); + try { + if (Files.isDirectory(path)) { + fromDirectory(path, refs); + } else { + fromJar(path, refs); + } + } catch (IOException e) { + throw new UncheckedIOException("Could not read the canonical feature files from " + path, e); + } + + if (refs.isEmpty()) { + throw new IllegalStateException("No canonical scenarios found in " + ProviderTck.FEATURES + "/ of the " + + "tck artifact at " + path + ". The artifact is not intact."); + } + return refs; + } + + private static void fromDirectory(Path root, Set refs) throws IOException { + Path features = root.resolve(ProviderTck.FEATURES); + if (!Files.isDirectory(features)) { + return; + } + try (DirectoryStream entries = Files.newDirectoryStream(features, "*" + FEATURE_SUFFIX)) { + for (Path entry : entries) { + String name = ProviderTck.FEATURES + "/" + entry.getFileName(); + collect(name, Files.readAllBytes(entry), refs); + } + } + } + + private static void fromJar(Path jar, Set refs) throws IOException { + try (JarFile file = new JarFile(jar.toFile())) { + Enumeration entries = file.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + String name = entry.getName(); + if (!entry.isDirectory() + && name.startsWith(ProviderTck.FEATURES + "/") + && name.endsWith(FEATURE_SUFFIX)) { + try (InputStream in = file.getInputStream(entry)) { + collect(name, readAll(in), refs); + } + } + } + } + } + + private static byte[] readAll(InputStream in) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int read; + while ((read = in.read(buffer)) != -1) { + out.write(buffer, 0, read); + } + return out.toByteArray(); + } + + /** + * Compiles one feature file to the scenarios Cucumber would run from it. + * + *

The Gherkin document is walked rather than the pickles it produces, because a pickle carries + * no line number — and the line is what the JUnit test plan identifies a scenario by. The walk is + * the same rule Gherkin's own compiler applies: a Scenario Outline yields one scenario per + * {@code Examples} body row, anything else yields one at its own line. + */ + private static void collect(String resource, byte[] content, Set refs) { + GherkinParser parser = GherkinParser.builder() + .includeSource(false) + .includePickles(false) + .includeGherkinDocument(true) + .build(); + + try (Stream envelopes = parser.parse(resource, content)) { + envelopes.forEach(envelope -> envelope.getGherkinDocument() + .flatMap(GherkinDocument::getFeature) + .ifPresent(feature -> collectFeature(resource, feature, refs))); + } + } + + private static void collectFeature(String resource, Feature feature, Set refs) { + for (FeatureChild child : feature.getChildren()) { + child.getScenario().ifPresent(scenario -> collectScenario(resource, scenario, refs)); + child.getRule().ifPresent(rule -> collectRule(resource, rule, refs)); + } + } + + private static void collectRule(String resource, Rule rule, Set refs) { + for (RuleChild child : rule.getChildren()) { + child.getScenario().ifPresent(scenario -> collectScenario(resource, scenario, refs)); + } + } + + private static void collectScenario(String resource, Scenario scenario, Set refs) { + List examples = scenario.getExamples(); + if (examples.isEmpty()) { + refs.add(new Ref(resource, scenario.getLocation().getLine(), scenario.getName())); + return; + } + for (Examples block : examples) { + for (TableRow row : block.getTableBody()) { + refs.add(new Ref(resource, row.getLocation().getLine(), scenario.getName())); + } + } + } + + /** + * One scenario, identified the way the JUnit test plan identifies it. + * + *

The name is carried for the failure message only; two scenarios are the same scenario when + * they are at the same line of the same classpath resource. + */ + static final class Ref implements Comparable { + + private final String resource; + private final long line; + private final String name; + + Ref(String resource, long line, String name) { + this.resource = resource; + this.line = line; + this.name = name; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof Ref)) { + return false; + } + Ref that = (Ref) other; + return line == that.line && resource.equals(that.resource); + } + + @Override + public int hashCode() { + return 31 * resource.hashCode() + Long.hashCode(line); + } + + @Override + public int compareTo(Ref other) { + int byResource = resource.compareTo(other.resource); + return byResource != 0 ? byResource : Long.compare(line, other.line); + } + + @Override + public String toString() { + return resource + ":" + line + (name == null || name.isEmpty() ? "" : " (" + name + ")"); + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java new file mode 100644 index 0000000000..42b331462b --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java @@ -0,0 +1,253 @@ +package dev.openfeature.contrib.tools.tck; + +import com.fasterxml.jackson.annotation.JsonInclude; +import java.util.List; + +/** + * The envelope of one conformance run: what was tested, what the provider claims, and where the + * results are. + * + *

This document deliberately carries no per-scenario outcomes. The results + * themselves are a standard format — a Cucumber Messages stream, referenced by {@link Results} — + * because per-scenario outcomes, tags, Scenario Outline row identity and the executed feature source + * are all already specified there. Defining them a second time here would create a format to + * maintain and version and two places for the same fact to disagree. + * + *

The field names and nesting are fixed by the report schema in the OpenFeature specification + * repository. This class is a transcription of that schema rather than a shape that would be + * convenient in Java, because the point of the format is that every language's TCK emits the same + * document. + * + *

Fields left {@code null} are omitted from the JSON. The schema sets + * {@code additionalProperties: false} throughout, so an unexpected field is a validation failure + * rather than something a consumer ignores. + * + * @see open-feature/spec#424 + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public final class ConformanceReport { + + /** + * The major version of the report schema this document conforms to. + * + *

An integer as a string, so a consumer can reject a report it does not understand rather + * than guessing at it. + */ + public static final String SCHEMA_VERSION = "1"; + + /** The schema version this document claims. */ + public final String schemaVersion; + + /** What was tested. */ + public final Provider provider; + + /** Which OpenFeature SDK the provider was exercised through. */ + public final Sdk sdk; + + /** What asked the questions, and which questions. */ + public final Tck tck; + + /** + * What the provider was pointed at. + * + *

Always present: {@code backend} is in the schema's top-level {@code required} array, + * including for a provider with no backend — that is what {@code backendType}'s + * {@code in-process} member is for. + */ + public final Backend backend; + + /** The capability set this provider claims. */ + public final Declaration declaration; + + /** Where the executed results live, and in what format. */ + public final Results results; + + /** Deviations the provider acknowledges, or {@code null} to say nothing. */ + public final List knownDeviations; + + ConformanceReport( + Provider provider, + Sdk sdk, + Tck tck, + Backend backend, + Declaration declaration, + Results results, + List knownDeviations) { + this.schemaVersion = SCHEMA_VERSION; + this.provider = provider; + this.sdk = sdk; + this.tck = tck; + this.backend = backend; + this.declaration = declaration; + this.results = results; + this.knownDeviations = knownDeviations; + } + + /** Identifies the provider under test. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Provider { + + /** + * What the provider calls itself, through its own metadata. + * + *

Not the name of the suite. A suite name is chosen to read well in a failure message — + * {@code flagd-rpc} — which makes it the {@link #configuration}, not the identity. + */ + public final String name; + + /** The language the provider is written in, always {@code java} here. */ + public final String language; + + /** + * Which configuration of the provider was tested. + * + *

A provider with more than one materially different mode produces one report per mode, + * and they are not interchangeable: flagd's RPC and in-process resolvers differ in whether + * they emit {@code PROVIDER_STALE}, so a report keyed on the provider name alone would have + * to pick one and misrepresent the other. + */ + public final String configuration; + + Provider(String name, String language, String configuration) { + this.name = name; + this.language = language; + this.configuration = configuration; + } + } + + /** Identifies the OpenFeature SDK the run went through. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Sdk { + + /** Maven coordinates of the SDK, without a version. */ + public final String name; + + /** The resolved SDK version. */ + public final String version; + + Sdk(String name, String version) { + this.name = name; + this.version = version; + } + } + + /** Identifies the TCK implementation and the conformance artifacts it executed. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Tck { + + /** Which TCK implementation produced this report. */ + public final String implementation; + + /** The version of that implementation. */ + public final String version; + + /** + * The open-feature/spec commit the executed artifacts came from. + * + *

The executed Gherkin no longer has to be taken on trust: the results stream carries the + * {@code source} of every feature it ran, so a consumer can diff what executed against what + * this revision contains. The revision still identifies the two artifacts the stream does + * not carry — the canonical flag set and the control API definition. + */ + public final String specRevision; + + Tck(String implementation, String version, String specRevision) { + this.implementation = implementation; + this.version = version; + this.specRevision = specRevision; + } + } + + /** Describes what the provider was pointed at. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Backend { + + /** A short human-readable description of the stack under test. */ + public final String description; + + /** + * How the backend was driven: {@code http}, {@code in-process} or {@code custom}. + * + *

{@code in-process} is the narrow allowance made for providers with no backend; a report + * claiming it for a provider that has one should be treated with suspicion. {@code custom} + * is a control the TCK does not ship, which the suite cannot vouch for. + * + *

Always written, and never inferred: it is what the control states through + * {@link BackendControl#backendType()}. + */ + public final BackendType backendType; + + Backend(String description, BackendType backendType) { + this.description = description; + this.backendType = backendType; + } + } + + /** The capability set this provider claims, as the Gherkin tags that gate them. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Declaration { + + /** + * Capabilities the provider declares, including the leading at-sign. + * + *

An input to reading the results rather than a summary of them, which is + * why it cannot be derived from the results payload and has to be stated here. A skipped + * scenario in the stream says the question was not put to this provider; only the declaration + * says whether that is because the provider declines the capability. Given the declaration + * and a scenario's tags — both of which the stream carries — the reason for a skip follows + * without being transported per scenario. + */ + public final List declared; + + Declaration(List declared) { + this.declared = declared; + } + } + + /** Where the executed results live, and in what format. */ + @JsonInclude(JsonInclude.Include.NON_NULL) + public static final class Results { + + /** The ndjson protocol at {@code https://github.com/cucumber/messages}. */ + public static final String CUCUMBER_MESSAGES = "cucumber-messages"; + + /** The results format, always {@link #CUCUMBER_MESSAGES} here. */ + public final String format; + + /** + * The Cucumber Messages release the stream was produced against. + * + *

Messages is versioned and the TCK implementations pin different releases -- this one + * takes whatever cucumber-jvm bundles -- 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. + * + *

Read back out of the stream's own {@code meta.protocolVersion} rather than from a + * constant or the artifact version, so the envelope and the stream cannot disagree about + * which release produced it. + */ + public final String formatVersion; + + /** + * Where to fetch the results: a path relative to this document. + * + *

Referenced rather than inlined because a Messages stream carries the feature sources and + * so is far larger than this envelope, and because a consumer deciding whether it cares about + * a report should not have to fetch the whole run to find out. + */ + public final String location; + + /** Digest over the results payload as {@code sha256:}. */ + public final String digest; + + Results(String format, String formatVersion, String location, String digest) { + this.format = format; + this.formatVersion = formatVersion; + this.location = location; + this.digest = digest; + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java new file mode 100644 index 0000000000..481c6aad52 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java @@ -0,0 +1,300 @@ +package dev.openfeature.contrib.tools.tck; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.cucumber.core.plugin.MessageFormatter; +import io.cucumber.messages.types.Envelope; +import io.cucumber.plugin.ConcurrentEventListener; +import io.cucumber.plugin.event.EventPublisher; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Writes a machine-readable conformance report for the suite that just ran. + * + *

Registered automatically by {@link ProviderTckTest}, so an adopter changes nothing to + * get one. It is opt-in per run: set {@value #REPORT_DIR_ENV} (or the + * {@value #REPORT_DIR_PROPERTY} system property) and each suite writes two files, + * {@code

/.json} and {@code /.ndjson}. Emitting a report is + * a property of the run rather than of the code — CI asks for one, a developer running the suite + * locally does not — and unset means no report, which is not an error. Several suites in one JVM each + * write their own pair of files, so flagd's two resolvers do not collide. + * + *

The results are not a format this project defines. The {@code .ndjson} file is + * a Cucumber Messages stream, produced by + * Cucumber's own {@link MessageFormatter} — the same class the built-in {@code message:} plugin + * instantiates, so the bytes are what {@code --plugin message:...} would have written. It already + * carries everything a per-scenario report would have had to invent: the outcome of every scenario, + * its tags including any on an individual {@code Examples} block, an exact Scenario Outline row + * identity through pickle AST node ids, and the {@code source} of every feature that executed. + * + *

The {@code .json} file is the envelope, and it exists because a Messages stream cannot say what + * it was a test of. No standard format identifies the provider, the SDK it was driven + * through, the TCK build, or — most importantly — the capability set the provider declared. That + * declaration is an input to reading the results rather than a summary of them: the stream says a + * scenario was skipped, and only the declaration says whether that is because the provider declines + * the capability it needed. + * + *

Why the stream is buffered rather than streamed to the file. The file name is + * derived from the provider configuration, which is not known when Cucumber wires plugins up — the + * suite reports it once its runtime has started, which is after the first messages have already been + * emitted. Buffering keeps one file per suite correctly named, and has the side benefit that + * {@code results.digest} is computed over exactly the bytes that were written. The stream for this + * suite is well under a megabyte. + * + * @see open-feature/spec#424 + */ +public final class ConformanceReportPlugin implements ConcurrentEventListener { + + /** Environment variable naming the directory reports are written to. */ + public static final String REPORT_DIR_ENV = "TCK_REPORT_DIR"; + + /** + * System property naming the directory reports are written to, taking precedence over the + * environment variable. + * + *

Accepted in addition to {@value #REPORT_DIR_ENV} because {@code -D} is how a Maven or + * Gradle invocation is usually parameterised. The environment variable is the portable spelling + * and is what every other language's TCK reads, so a cross-language CI job can set one thing. + */ + public static final String REPORT_DIR_PROPERTY = "tck.report.dir"; + + /** Extension of the envelope, which is what a consumer reads first. */ + static final String ENVELOPE_EXTENSION = ".json"; + + /** Extension of the Cucumber Messages stream the envelope points at. */ + static final String RESULTS_EXTENSION = ".ndjson"; + + private static final Logger log = LoggerFactory.getLogger(ConformanceReportPlugin.class); + + private static final String LANGUAGE = "java"; + + private final Supplier reportDir; + private final Supplier> metadata; + + /** Creates the plugin Cucumber instantiates by name. */ + public ConformanceReportPlugin() { + this(ConformanceReportPlugin::configuredReportDir, TckRunMetadata::last); + } + + ConformanceReportPlugin(Supplier reportDir, Supplier> metadata) { + this.reportDir = reportDir; + this.metadata = metadata; + } + + @Override + public void setEventPublisher(EventPublisher publisher) { + String dir = reportDir.get(); + if (dir == null) { + // Nothing asked for a report, so nothing is collected either. Registering the message + // formatter regardless would buffer a stream for every local test run. + return; + } + + ByteArrayOutputStream results = new ByteArrayOutputStream(); + new MessageFormatter(results).setEventPublisher(publisher); + + // Registered *after* the formatter, and deliberately so. Cucumber invokes the handlers for + // one event type in registration order, and the formatter closes its writer when it sees + // the run-finished message; going second is what guarantees the buffer is complete and + // flushed before the digest is taken over it. + publisher.registerHandlerFor(Envelope.class, envelope -> { + if (envelope.getTestRunFinished().isPresent()) { + write(dir, results.toByteArray()); + } + }); + } + + /** + * Returns the directory reports are written to, or {@code null} when none was configured. + * + * @return the configured report directory, trimmed, or {@code null} + */ + static String configuredReportDir() { + String property = System.getProperty(REPORT_DIR_PROPERTY); + if (property != null && !property.trim().isEmpty()) { + return property.trim(); + } + String environment = System.getenv(REPORT_DIR_ENV); + if (environment != null && !environment.trim().isEmpty()) { + return environment.trim(); + } + return null; + } + + /** + * Assembles the envelope for what the run observed. + * + * @param run what the runtime recorded about this suite + * @param location where the results stream sits, relative to the envelope + * @param digest digest over the results stream + * @return the envelope, ready to serialise + */ + ConformanceReport build(TckRunMetadata run, String location, String digest, String formatVersion) { + return new ConformanceReport( + new ConformanceReport.Provider(run.providerName(), LANGUAGE, run.configuration()), + new ConformanceReport.Sdk(TckBuildInfo.SDK_NAME, TckBuildInfo.sdkVersion()), + new ConformanceReport.Tck( + TckBuildInfo.IMPLEMENTATION, TckBuildInfo.tckVersion(), TckBuildInfo.specRevision()), + backendOf(run), + new ConformanceReport.Declaration(declaredTags(run.capabilities())), + new ConformanceReport.Results( + ConformanceReport.Results.CUCUMBER_MESSAGES, formatVersion, location, digest), + run.knownDeviations().isEmpty() ? null : run.knownDeviations()); + } + + /** + * Lists the declared capabilities as Gherkin tags, in the order the vocabulary declares them. + * + *

Ordered by the enum rather than by the set so that two runs of the same configuration + * produce byte-identical declarations, which is what makes the envelopes diffable. + * + *

{@linkplain Capability#reserved() Reserved} capabilities are skipped. A declaration that + * names one is already rejected where it enters the run, in {@link TckRunMetadata}, so this is + * not the thing that tells the adopter; it is what makes "no reserved tag in the emitted + * declaration" a property of the code that writes the document rather than a consequence of a + * check somewhere upstream of it. + */ + private static List declaredTags(Set declared) { + List tags = new ArrayList<>(declared.size()); + for (Capability capability : Capability.values()) { + if (!capability.reserved() && declared.contains(capability)) { + tags.add(capability.tag()); + } + } + return Collections.unmodifiableList(tags); + } + + /** + * Assembles the {@code backend} block, which is always emitted. + * + *

The schema requires it at the top level, and {@code backendType} within it, so there is no + * conditional emission here: a control that does not say how it drove the backend is + * {@code custom}. + */ + private static ConformanceReport.Backend backendOf(TckRunMetadata run) { + return new ConformanceReport.Backend(run.backendDescription().orElse(null), run.backendType()); + } + + /** + * Reads the Messages release out of the stream's own {@code meta} message. + * + *

Taken from the stream rather than from a constant or the {@code io.cucumber:messages} + * artifact version, because the envelope and the stream disagreeing about which release produced + * them would be worse than either being absent. {@code meta} is the first envelope cucumber + * writes, so only the first line is parsed. + * + * @param results the buffered results stream + * @return the protocol version, or {@code null} when the stream carries none + */ + private static String protocolVersionOf(byte[] results) { + String stream = new String(results, StandardCharsets.UTF_8); + int newline = stream.indexOf('\n'); + String first = newline < 0 ? stream : stream.substring(0, newline); + if (first.trim().isEmpty()) { + return null; + } + try { + JsonNode version = new ObjectMapper().readTree(first).path("meta").path("protocolVersion"); + return version.isTextual() ? version.asText() : null; + } catch (JsonProcessingException e) { + // A stream whose first line will not parse is a bug worth surfacing, but not here: + // omitting an optional field is better than failing a run that otherwise succeeded. + log.warn("tck: could not read the Messages protocol version from the stream", e); + return null; + } + } + + @SuppressFBWarnings( + value = "PATH_TRAVERSAL_IN", + justification = "The directory is supplied by whoever started the test run, which is the " + + "whole point of the setting; the file names within it are sanitised by ReportNames") + private void write(String dir, byte[] results) { + Optional observed = metadata.get(); + if (!observed.isPresent()) { + // Nothing observed the suite, which means it never got as far as starting the runtime. + // The run has failed for some other reason by now; adding a report with an invented + // provider name on top of that would only mislead. + log.error( + "{} is set but no TCK suite ran, so there is nothing to report on. " + + "This normally means the suite failed before its backend stack started.", + REPORT_DIR_ENV); + return; + } + + TckRunMetadata run = observed.get(); + String configuration = run.configuration(); + Path directory; + try { + directory = Paths.get(dir); + } catch (InvalidPathException e) { + throw new IllegalStateException( + "tck [" + configuration + "]: " + REPORT_DIR_ENV + " is not a usable path: " + dir, e); + } + + String base = ReportNames.baseNameOf(configuration); + String location = base + RESULTS_EXTENSION; + Path resultsPath = directory.resolve(location); + Path envelopePath = directory.resolve(base + ENVELOPE_EXTENSION); + + String json; + try { + json = new ObjectMapper() + .writerWithDefaultPrettyPrinter() + .writeValueAsString(build(run, location, digestOf(results), protocolVersionOf(results))) + + "\n"; + } catch (JsonProcessingException e) { + throw new IllegalStateException("tck [" + configuration + "]: could not encode the conformance report", e); + } + + // A failure to write is raised rather than logged and swallowed. CI that asked for a report + // and silently did not get one is how a publishing pipeline serves a stale result forever. + // The results go first: an envelope naming a stream that is not there is worse than neither. + try { + Files.createDirectories(directory); + Files.write(resultsPath, results); + Files.write(envelopePath, json.getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new UncheckedIOException( + "tck [" + configuration + "]: could not write the conformance report to " + directory, e); + } + + log.info("tck [{}]: conformance report written to {}, results to {}", configuration, envelopePath, resultsPath); + } + + /** Digests the results stream in the {@code sha256:} form the schema asks for. */ + private static String digestOf(byte[] results) { + MessageDigest sha256; + try { + sha256 = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + // SHA-256 is required of every Java platform, so this cannot happen on a working JVM. + throw new IllegalStateException("SHA-256 is not available", e); + } + byte[] digest = sha256.digest(results); + StringBuilder hex = new StringBuilder(7 + digest.length * 2); + hex.append("sha256:"); + for (byte b : digest) { + hex.append(Character.forDigit((b >> 4) & 0xf, 16)).append(Character.forDigit(b & 0xf, 16)); + } + return hex.toString(); + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java index 1c14985cd2..2364271c5d 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTck.java @@ -26,8 +26,8 @@ public final class ProviderTck { /** The glue path: canonical steps plus the extension package. */ public static final String ALL_GLUE = GLUE + "," + EXTENSION_GLUE; - /** The Cucumber plugins the suite registers. */ - static final String PLUGINS = "summary"; + /** The Cucumber plugins the suite registers: the summary and the conformance report. */ + static final String PLUGINS = "summary,dev.openfeature.contrib.tools.tck.ConformanceReportPlugin"; /** Parallel execution off; control API state is global to the backend. */ static final String PARALLEL = "false"; diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java index 31da2bda55..9c3a9f8e4b 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ProviderTckTest.java @@ -3,6 +3,7 @@ import io.cucumber.junit.platform.engine.Constants; import org.junit.platform.suite.api.ConfigurationParameter; import org.junit.platform.suite.api.IncludeEngines; +import org.junit.platform.suite.api.SelectClasses; import org.junit.platform.suite.api.SelectClasspathResource; import org.junit.platform.suite.api.Suite; @@ -18,11 +19,14 @@ * @see ProviderTckHarness * @see ContainerizedProviderTckTest * @see ProviderTck + * @see ConformanceReportPlugin + * @see CanonicalScenarioGuard */ @Suite -@IncludeEngines("cucumber") +@IncludeEngines({"cucumber", "junit-jupiter"}) @SelectClasspathResource(ProviderTck.FEATURES) @SelectClasspathResource(ProviderTck.EXTENSIONS) +@SelectClasses(CanonicalScenarioGuard.class) @ConfigurationParameter(key = Constants.PLUGIN_PROPERTY_NAME, value = ProviderTck.PLUGINS) @ConfigurationParameter( key = Constants.EXECUTION_MODE_FEATURE_PROPERTY_NAME, diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java index d2a12cd061..b2b275ba61 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java @@ -33,4 +33,46 @@ static String configurationOf(Class suite) { .toLowerCase(Locale.ROOT); return hyphenated.isEmpty() ? FALLBACK : hyphenated; } + + /** + * Turns a configuration name into the base name both of a run's files share. + * + *

A run writes an envelope and a results stream, and they are matched by name — the envelope's + * {@code results.location} is this base name plus the stream's extension — so one derivation + * produces both. + * + *

Configuration names are chosen to read well in a failure message rather than to be + * path-safe, so anything that is not obviously safe becomes a hyphen. Without this a + * configuration named {@code flagd/rpc} would silently write outside the directory it was given. + * + * @param configuration the configuration name + * @return a file name stem, with no extension + */ + static String baseNameOf(String configuration) { + StringBuilder safe = new StringBuilder(configuration.length()); + for (int i = 0; i < configuration.length(); i++) { + char c = configuration.charAt(i); + boolean allowed = (c >= 'a' && c <= 'z') + || (c >= 'A' && c <= 'Z') + || (c >= '0' && c <= '9') + || c == '-' + || c == '_' + || c == '.'; + safe.append(allowed ? c : '-'); + } + String trimmed = trim(safe.toString()); + return trimmed.isEmpty() ? FALLBACK : trimmed; + } + + private static String trim(String value) { + int start = 0; + int end = value.length(); + while (start < end && (value.charAt(start) == '-' || value.charAt(start) == '.')) { + start++; + } + while (end > start && (value.charAt(end - 1) == '-' || value.charAt(end - 1) == '.')) { + end--; + } + return value.substring(start, end); + } } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java new file mode 100644 index 0000000000..7346db6192 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java @@ -0,0 +1,124 @@ +package dev.openfeature.contrib.tools.tck; + +import dev.openfeature.sdk.OpenFeatureAPI; +import java.io.IOException; +import java.io.InputStream; +import java.util.Properties; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * What this TCK build is, and which conformance artifacts it carries. + * + *

A conformance report has to say which questions were asked, not only what the answers were, and + * neither fact is available from the running code. The spec revision the packaged Gherkin came from + * is a property of the build — the artifacts are in the JAR, the repository they came from is not — + * so it is baked into a generated resource at build time and read back here. The SDK version is the + * opposite case: the TCK depends on a version range, so what a consumer actually ran + * against is only knowable at runtime. + * + *

Everything here degrades to {@code unknown} rather than throwing. A report that is slightly + * less identifiable is worth more than a test run that fails while tidying up after itself. + */ +final class TckBuildInfo { + + /** Which TCK implementation this is, in the form the report schema asks for. */ + static final String IMPLEMENTATION = "java-sdk-contrib/tools/tck"; + + /** Maven coordinates of the SDK, reported without a version. */ + static final String SDK_NAME = "dev.openfeature:sdk"; + + /** Stands in for anything the build or the classpath could not tell us. */ + static final String UNKNOWN = "unknown"; + + private static final Logger log = LoggerFactory.getLogger(TckBuildInfo.class); + + /** Generated by the build from the module POM; see {@code src/main/resources-filtered}. */ + private static final String BUILD_PROPERTIES = "tck-build.properties"; + + /** Written into every JAR by maven-archiver, and the most direct statement of what resolved. */ + private static final String SDK_POM_PROPERTIES = "META-INF/maven/dev.openfeature/sdk/pom.properties"; + + private static final Properties BUILD = loadBuildProperties(); + + private TckBuildInfo() {} + + /** Returns the version of this TCK module. */ + static String tckVersion() { + return property("tck.version"); + } + + /** Returns the open-feature/spec commit the packaged conformance artifacts came from. */ + static String specRevision() { + return property("spec.revision"); + } + + /** + * Returns the OpenFeature SDK version that was actually on the classpath. + * + *

Read rather than declared. The TCK depends on an SDK version range so that adopting it can + * never force an upgrade, which means the version is a property of the consumer's build; a + * hardcoded one would be a second place to be wrong. + */ + static String sdkVersion() { + Properties pom = load(SDK_POM_PROPERTIES); + if (pom != null) { + String version = pom.getProperty("version", "").trim(); + if (!version.isEmpty()) { + return version; + } + } + + // The Maven descriptor can be stripped, or the SDK can arrive from somewhere that is not a + // JAR at all — an IDE's output directory, say — so fall back to the manifest. + Package sdkPackage = OpenFeatureAPI.class.getPackage(); + String implementation = sdkPackage == null ? null : sdkPackage.getImplementationVersion(); + if (implementation != null && !implementation.trim().isEmpty()) { + return implementation.trim(); + } + + log.warn( + "Could not determine the OpenFeature SDK version from {} or from the package manifest; " + + "the conformance report will say '{}'", + SDK_POM_PROPERTIES, + UNKNOWN); + return UNKNOWN; + } + + private static String property(String key) { + String value = BUILD.getProperty(key, "").trim(); + return value.isEmpty() ? UNKNOWN : value; + } + + private static Properties loadBuildProperties() { + Properties properties = load(packagePath() + BUILD_PROPERTIES); + if (properties == null) { + log.warn( + "{} is missing from the TCK JAR, so a conformance report cannot identify the build " + + "that produced it. This means the resource filtering configured in the " + + "tck POM did not run.", + BUILD_PROPERTIES); + return new Properties(); + } + return properties; + } + + private static String packagePath() { + return TckBuildInfo.class.getPackage().getName().replace('.', '/') + "/"; + } + + private static Properties load(String resource) { + ClassLoader loader = TckBuildInfo.class.getClassLoader(); + try (InputStream in = loader.getResourceAsStream(resource)) { + if (in == null) { + return null; + } + Properties properties = new Properties(); + properties.load(in); + return properties; + } catch (IOException e) { + log.warn("Could not read {} from the classpath", resource, e); + return null; + } + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java new file mode 100644 index 0000000000..cbd3d06703 --- /dev/null +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java @@ -0,0 +1,154 @@ +package dev.openfeature.contrib.tools.tck; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +/** + * What a conformance report needs to know about the run, gathered as the run proceeds. + * + *

It exists because the two facts a report needs most are known at different times and are gone + * by the time it is written. The declared capabilities and the backend description come from the + * harness when the suite starts; the provider's own metadata name is only observable once a scenario + * has built a provider. Meanwhile Cucumber emits {@code TestRunFinished} — where the file is written + * — after {@code @AfterAll} has already torn the suite's backend down. + * + *

So the suite lifecycle records one of these when a suite starts and it outlives the suite, and + * the report plugin reads it at the end. Public only for that lifecycle, which lives in the internal + * steps package: its two static entry points are not for adopters. + * + *

It is also the one place every path to a report passes through, which makes it where the + * declaration is checked: a {@linkplain Capability#reserved() reserved} capability is rejected here + * rather than being carried into a document that would then claim it. + */ +public final class TckRunMetadata { + + /** The most recent suite to start, kept after it stops for the report written at run end. */ + private static final AtomicReference last = new AtomicReference<>(); + + private final String configuration; + private final Set capabilities; + private final String backendDescription; + private final BackendType backendType; + private final List knownDeviations; + + private volatile String providerName; + + TckRunMetadata( + String configuration, + Set capabilities, + String backendDescription, + BackendType backendType, + List knownDeviations) { + Capability.requireDeclarable(capabilities); + this.configuration = configuration; + this.capabilities = capabilities.isEmpty() + ? Collections.emptySet() + : Collections.unmodifiableSet(EnumSet.copyOf(capabilities)); + this.backendDescription = backendDescription; + this.backendType = backendType; + this.knownDeviations = Collections.unmodifiableList(new ArrayList<>(knownDeviations)); + } + + /** Returns the suite name, which is the provider configuration under test. */ + String configuration() { + return configuration; + } + + /** Returns the capabilities the harness declared. */ + Set capabilities() { + return capabilities; + } + + /** Returns a short description of the backend stack, or empty when there is none. */ + Optional backendDescription() { + return Optional.ofNullable(backendDescription); + } + + /** + * Records the suite that is starting, as the run the conformance report describes. + * + * @param harness the suite's harness + * @param control the control its backend is driven through + */ + public static void begin(ProviderTckHarness harness, BackendControl control) { + record(new TckRunMetadata( + harness.configuration(), + harness.capabilities(), + control.description(), + control.backendType(), + harness.knownDeviations())); + } + + /** + * Records what the provider under test calls itself, against the suite that is running. + * + *

A conformance report identifies the provider by its own metadata name rather than by the + * suite's, and only a scenario that has built one can say what that is. + * + * @param name the provider's metadata name + */ + public static void observeProviderName(String name) { + TckRunMetadata run = last.get(); + if (run != null) { + run.recordProviderName(name); + } + } + + /** + * Replaces the recorded run. Also the seam the report tests use to describe a run without a + * backend. + * + * @param metadata what the run is, or {@code null} to forget the last one + */ + static void record(TckRunMetadata metadata) { + last.set(metadata); + } + + /** Returns the most recent suite to start, or empty if none has. */ + static Optional last() { + return Optional.ofNullable(last.get()); + } + + /** + * Returns how the backend was driven, as the control stated it; {@link BackendType#CUSTOM} for + * a control that states nothing. + */ + BackendType backendType() { + return backendType; + } + + /** Returns the deviations the provider author acknowledged, empty when there are none. */ + List knownDeviations() { + return knownDeviations; + } + + /** + * Records what the provider called itself, as observed from a scenario that built one. + * + * @param name the provider's own metadata name + */ + void recordProviderName(String name) { + if (name != null && !name.trim().isEmpty()) { + providerName = name; + } + } + + /** + * Returns the provider's own metadata name, falling back to the configuration name. + * + *

The fallback covers a run in which no scenario ever built a provider — every scenario + * skipped, or the stack never came up. Reporting the suite name there is more useful than the + * empty string the schema would reject. + * + * @return the name to report as the provider's identity + */ + String providerName() { + String observed = providerName; + return observed == null ? configuration : observed; + } +} diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckSuiteListener.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckSuiteListener.java index f8e88d0ca9..8d91d959b1 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckSuiteListener.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/TckSuiteListener.java @@ -1,5 +1,6 @@ package dev.openfeature.contrib.tools.tck.internal; +import dev.openfeature.contrib.tools.tck.CanonicalScenarioGuard; import dev.openfeature.contrib.tools.tck.ProviderTckHarness; import java.lang.reflect.Modifier; import java.util.Optional; @@ -7,6 +8,7 @@ import org.junit.platform.engine.support.descriptor.ClassSource; import org.junit.platform.launcher.TestExecutionListener; import org.junit.platform.launcher.TestIdentifier; +import org.junit.platform.launcher.TestPlan; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -21,6 +23,12 @@ public class TckSuiteListener implements TestExecutionListener { private static volatile Class current; + /** Hands the plan to {@link CanonicalScenarioGuard} before any of it executes. */ + @Override + public void testPlanExecutionStarted(TestPlan testPlan) { + CanonicalScenarioGuard.observe(testPlan); + } + @Override public void executionStarted(TestIdentifier testIdentifier) { harnessClassOf(testIdentifier).ifPresent(suite -> { @@ -48,8 +56,13 @@ public static Optional> currentSuite() { return Optional.ofNullable(current); } - /** The TCK suite class an identifier stands for, if it is one. */ - private static Optional> harnessClassOf(TestIdentifier testIdentifier) { + /** + * The TCK suite class an identifier stands for, if it is one. + * + * @param testIdentifier a node of the test plan + * @return the harness class, or empty when the node is not a TCK suite + */ + public static Optional> harnessClassOf(TestIdentifier testIdentifier) { return testIdentifier .getSource() .filter(ClassSource.class::isInstance) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java index 8bae27b35c..bab59c4b88 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/ProviderSteps.java @@ -9,6 +9,7 @@ import dev.openfeature.contrib.tools.tck.Capability; import dev.openfeature.contrib.tools.tck.ProviderTck; import dev.openfeature.contrib.tools.tck.ProviderTckHarness; +import dev.openfeature.contrib.tools.tck.TckRunMetadata; import dev.openfeature.contrib.tools.tck.TckState; import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.Metadata; @@ -131,11 +132,13 @@ private void createProvider(String flavour, String configuration) { state.provider = provider; state.domain = domain; state.client = api.getClient(domain); - log.info( - "Registered {} provider {} under domain {}", - flavour, - provider.getMetadata().getName(), - domain); + + // A conformance report identifies the provider by what it calls itself, not by the suite + // name, and this is the only place that knows it. + String providerName = provider.getMetadata().getName(); + TckRunMetadata.observeProviderName(providerName); + + log.info("Registered {} provider {} under domain {}", flavour, providerName, domain); } /** diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java index b3f094f80d..5ae1b3b9bb 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/internal/steps/TckLifecycle.java @@ -4,6 +4,7 @@ import dev.openfeature.contrib.tools.tck.BackendControl; import dev.openfeature.contrib.tools.tck.Capability; import dev.openfeature.contrib.tools.tck.ProviderTckHarness; +import dev.openfeature.contrib.tools.tck.TckRunMetadata; import dev.openfeature.contrib.tools.tck.internal.TckSuiteListener; import java.util.ArrayList; import java.util.Collections; @@ -116,6 +117,7 @@ static TckLifecycle start(ProviderTckHarness harness) { + ".backendControl() returned null; supply overHttp(..) or inProcess()."); } log.info("Backend control: {}", control.description()); + TckRunMetadata.begin(harness, control); TckLifecycle lifecycle = new TckLifecycle(harness, control); started.add(lifecycle); return lifecycle; diff --git a/tools/tck/src/main/resources-filtered/dev/openfeature/contrib/tools/tck/tck-build.properties b/tools/tck/src/main/resources-filtered/dev/openfeature/contrib/tools/tck/tck-build.properties new file mode 100644 index 0000000000..eabdcecc11 --- /dev/null +++ b/tools/tck/src/main/resources-filtered/dev/openfeature/contrib/tools/tck/tck-build.properties @@ -0,0 +1,7 @@ +# Generated at build time by Maven resource filtering. Do not edit the copy in target/. +# +# Identifies the TCK build and the conformance artifacts it carries, for the +# tck section of a conformance report. The values come from the tck +# POM; see the comment there for how the spec revision is maintained. +tck.version=${project.version} +spec.revision=${tck.spec.revision} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java new file mode 100644 index 0000000000..d95bb943b7 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java @@ -0,0 +1,241 @@ +package dev.openfeature.contrib.tools.tck; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.cucumber.junit.platform.engine.Constants; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.EngineFilter; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; +import org.junit.platform.launcher.listeners.SummaryGeneratingListener; +import org.junit.platform.launcher.listeners.TestExecutionSummary; + +/** + * A run that quietly asks fewer questions must not be able to report success. + * + *

Two levels here, deliberately. The comparison itself is checked directly, because a failure + * message is the whole product of a guard and it has to name the scenario that went missing. The + * wiring is checked by running the guard through the JUnit Platform under a real filter + * configuration, because "the guard sees what the Cucumber engine beside it sees" is a claim about + * the platform rather than about this code. + * + *

No scenario is executed anywhere in this file. The guard works from the discovered plan and the + * run's configuration, which is what lets it be checked — and, in a real build, fail — without a + * backend. + */ +class CanonicalScenarioGuardTest { + + private static final Map UNFILTERED = Collections.emptyMap(); + + @BeforeEach + void forgetPreviousPlans() { + CanonicalScenarioGuard.forget(); + } + + @Test + @DisplayName("the canonical set is read from this artifact, outline row by outline row") + void theCanonicalSetIsReadFromTheArtifact() { + Set canonical = CanonicalScenarios.shipped(); + + assertThat(canonical).isNotEmpty(); + assertThat(canonical) + .as("every canonical scenario comes from a packaged feature file") + .allSatisfy(ref -> assertThat(ref.toString()).startsWith(ProviderTck.FEATURES + "/")); + + // The eight rows of the type-mismatch matrix share one name, so a set that counted scenarios + // by name would see one of them. Each Examples row is its own line and its own entry. + // Eleven until specification revision d47a66eb moved the three "requested as a String" rows + // onto @string-typing, where a backend that stores every value as a string can withhold them. + assertThat(canonical) + .filteredOn(ref -> ref.toString().contains("Requesting the wrong type returns the code default")) + .hasSize(8); + } + + @Test + @DisplayName("a suite that selects the whole canonical set, unfiltered, passes") + void anIntactSuitePasses() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + assertThat(CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), CanonicalScenarioGuard.discovered(), UNFILTERED)) + .isNull(); + } + + @Test + @DisplayName("extension scenarios neither count towards the canonical set nor disturb it") + void extensionScenariosAreIgnored() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + Set observed = + CanonicalScenarioGuard.discovered().get(TckSuiteFixture.class.getName()); + + // The fixture suite does select an extension feature — ExtensionPointTest checks that it does + // — and none of it reaches the guard. + assertThat(observed) + .as("the guard is defined over %s/ alone", ProviderTck.FEATURES) + .isEqualTo(CanonicalScenarios.shipped()) + .allSatisfy(ref -> assertThat(ref.toString()).doesNotContain(ProviderTck.EXTENSIONS + "/")); + } + + @Test + @DisplayName("a scenario filter fails the run, whatever it would have excluded") + void aScenarioFilterFails() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + CanonicalScenarioGuard.discovered(), + Collections.singletonMap(Constants.FILTER_TAGS_PROPERTY_NAME, "not @events")); + + // Cucumber applies a tag filter by skipping scenarios during execution, so the plan still + // contains them and only the configured expression shows what will be left out. The guard + // therefore rejects the filter itself rather than trying to predict what it matches. + assertThat(problems) + .isNotNull() + .contains(Constants.FILTER_TAGS_PROPERTY_NAME) + .contains("not @events") + .as("and it points at the mechanism that legitimately narrows a run") + .contains("capabilities()"); + } + + @Test + @DisplayName("the guard reads the filter the Cucumber engine beside it would read") + void theGuardReadsTheRunsFilterConfiguration() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + TestExecutionSummary filtered = runGuard(Constants.FILTER_TAGS_PROPERTY_NAME, "@events"); + + assertThat(filtered.getTestsFailedCount()) + .as("a filtered run fails at the guard, before a Compose stack is worth starting") + .isEqualTo(1); + assertThat(filtered.getFailures().get(0).getException()) + .hasMessageContaining(Constants.FILTER_TAGS_PROPERTY_NAME); + } + + @Test + @DisplayName("an unfiltered run of an intact suite passes the guard as executed") + void anIntactRunPassesTheGuard() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + assertThat(runGuard(null, null).getTestsSucceededCount()).isEqualTo(1); + } + + @Test + @DisplayName("a run that declares itself partial is skipped rather than passed") + void aPartialRunIsSkipped() { + CanonicalScenarioGuard.observe(discoverFixtureSuite()); + + String previous = System.getProperty(CanonicalScenarioGuard.PARTIAL_PROPERTY); + System.setProperty(CanonicalScenarioGuard.PARTIAL_PROPERTY, "true"); + try { + TestExecutionSummary summary = runGuard(Constants.FILTER_TAGS_PROPERTY_NAME, "@events"); + + // Aborted rather than skipped: the test ran and declined to conclude, which is what an + // assumption produces and what Surefire and the JUnit reports show as skipped. + assertThat(summary.getTestsAbortedCount()) + .as("a skip says the canonical set was not verified; a pass would claim it was") + .isEqualTo(1); + assertThat(summary.getTestsSucceededCount()).isZero(); + assertThat(summary.getTestsFailedCount()).isZero(); + } finally { + restore(CanonicalScenarioGuard.PARTIAL_PROPERTY, previous); + } + } + + @Test + @DisplayName("a feature file added to the canonical directory fails the run") + void anAddedCanonicalFeatureFails() { + Set withExtra = new LinkedHashSet<>(CanonicalScenarios.shipped()); + withExtra.add(new CanonicalScenarios.Ref(ProviderTck.FEATURES + "/vendor.feature", 7, "A vendor scenario")); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + Collections.singletonMap(TckSuiteFixture.class.getName(), withExtra), + UNFILTERED); + + assertThat(problems) + .isNotNull() + .contains(ProviderTck.FEATURES + "/vendor.feature") + .as("and it says where the scenario should have gone") + .contains(ProviderTck.EXTENSIONS + "/"); + } + + @Test + @DisplayName("a canonical file replaced by another of the same name fails the run") + void aShadowedCanonicalFileFails() { + // What the separate extension directory exists to prevent, expressed as what the guard sees + // if it happens anyway: the replacement compiles to different scenarios, so entries of the + // canonical set go missing. + Set shadowed = new LinkedHashSet<>(CanonicalScenarios.shipped()); + Iterator entries = shadowed.iterator(); + CanonicalScenarios.Ref removed = entries.next(); + entries.remove(); + + String problems = CanonicalScenarioGuard.check( + CanonicalScenarios.shipped(), + Collections.singletonMap(TckSuiteFixture.class.getName(), shadowed), + UNFILTERED); + + assertThat(problems).isNotNull().contains(removed.toString()).contains("shadowing"); + } + + @Test + @DisplayName("a plan with no TCK suite in it is a failure, not a pass") + void anUnobservedRunFails() { + Map> nothing = Collections.emptyMap(); + + assertThat(CanonicalScenarioGuard.check(CanonicalScenarios.shipped(), nothing, UNFILTERED)) + .as("an unverifiable conformance run is not a conformance run") + .isNotNull() + .contains("TckSuiteListener") + .contains(CanonicalScenarioGuard.PARTIAL_PROPERTY); + } + + @Test + @DisplayName("the partial-run escape hatch carries no PROVIDER_ prefix either") + void thePartialKnobIsSpelledWithoutThePrefix() { + // Literals on purpose: every TCK knob an adopter or a CI job sets is spelled TCK_*, and one + // of them keeping the old PROVIDER_TCK_* prefix is the half-rename that makes the set + // unguessable. + assertThat(CanonicalScenarioGuard.PARTIAL_ENV).isEqualTo("TCK_PARTIAL"); + assertThat(CanonicalScenarioGuard.PARTIAL_PROPERTY).isEqualTo("tck.partial"); + } + + /** Discovers the real suite configuration, without executing anything. */ + private static org.junit.platform.launcher.TestPlan discoverFixtureSuite() { + return LauncherFactory.create() + .discover(LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(TckSuiteFixture.class)) + .build()); + } + + /** Runs the guard itself through the JUnit Platform, optionally under a Cucumber filter. */ + private static TestExecutionSummary runGuard(String key, String value) { + LauncherDiscoveryRequestBuilder request = LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClass(CanonicalScenarioGuard.class)) + .filters(EngineFilter.includeEngines("junit-jupiter")); + if (key != null) { + request.configurationParameter(key, value); + } + + SummaryGeneratingListener summary = new SummaryGeneratingListener(); + LauncherFactory.create().execute(request.build(), summary); + return summary.getSummary(); + } + + private static void restore(String key, String previous) { + if (previous == null) { + System.clearProperty(key); + } else { + System.setProperty(key, previous); + } + } +} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java new file mode 100644 index 0000000000..def029138f --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java @@ -0,0 +1,731 @@ +package dev.openfeature.contrib.tools.tck; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.openfeature.contrib.tools.tck.selftest.ReportSelfTestSteps; +import io.cucumber.junit.platform.engine.Constants; +import io.cucumber.plugin.event.EventHandler; +import io.cucumber.plugin.event.EventPublisher; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.TreeMap; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.platform.engine.discovery.DiscoverySelectors; +import org.junit.platform.launcher.EngineFilter; +import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder; +import org.junit.platform.launcher.core.LauncherFactory; + +/** + * The conformance report exists to make one rule checkable, so these tests check it. + * + *

Appendix F requires that a scenario skipped for an undeclared capability is reported as skipped + * with the reason and never as passed. The Go TCK shipped an emitter that broke the rule silently — + * its runner did not deliver the capability-skip signal to the after-hook, so every skipped scenario + * was recorded twice, once correctly and once as passed. + * + *

Since the results are now a Cucumber Messages stream rather than a format this project defines, + * the rule is a property of what Cucumber emits, and only a real Cucumber run can demonstrate it. + * These tests therefore execute a fixture suite through the JUnit Platform — same engine, same gate, + * same plugin — and read the stream back as a consumer would: outcome per scenario derived from the + * most severe step result, exactly as the {@code cucumber-query} helpers do it. + * + *

What is not asserted here is the serialisation of the stream. That is Cucumber's own + * {@code MessageFormatter}, and re-checking it would only be checking Cucumber. + */ +class ConformanceReportPluginTest { + + /** + * The fixture's {@code @stale} tag is deliberately absent, so two scenarios must be skipped — and + * a third that no declaration could have covered, because {@code @large-integers} is + * {@linkplain Capability#inexpressible() inexpressible} in Java. + */ + private static final Set DECLARED = EnumSet.of(Capability.OBJECT, Capability.EVENTS); + + private static final String CONFIGURATION = "my-provider-rpc"; + + private static final String FEATURE_URI = "classpath:report-selftest/report.feature"; + + private static final String OUTLINE = "Requesting the wrong type returns the code default"; + + /** + * Severity order of {@code TestStepResultStatus}, least to most severe. + * + *

A {@code testCaseFinished} message carries no status: a scenario's outcome is the most + * severe result among its steps, hooks included. That is not incidental — it is the mechanism + * that makes a gated skip truthful, because the aborted {@code @Before} hook contributes a + * {@code SKIPPED} result that outranks every {@code PASSED} step it prevented from running. + */ + private static final List SEVERITY = + Arrays.asList("UNKNOWN", "PASSED", "SKIPPED", "PENDING", "UNDEFINED", "AMBIGUOUS", "FAILED"); + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @TempDir + static Path reportDir; + + private static JsonNode envelope; + private static byte[] resultsBytes; + private static Results results; + + @BeforeAll + static void runTheFixtureSuite() throws IOException { + runSuite(reportDir, DECLARED); + + Path envelopePath = reportDir.resolve(CONFIGURATION + ".json"); + Path resultsPath = reportDir.resolve(CONFIGURATION + ".ndjson"); + assertThat(envelopePath).exists(); + assertThat(resultsPath).exists(); + + envelope = MAPPER.readTree(Files.readAllBytes(envelopePath)); + resultsBytes = Files.readAllBytes(resultsPath); + results = Results.parse(new String(resultsBytes, StandardCharsets.UTF_8)); + } + + @Test + @DisplayName("every scenario appears exactly once, whatever happened to it") + void everyScenarioAppearsExactlyOnce() { + // Eight pickles: five plain scenarios and three outline rows. A report that quietly omitted + // the ones it did not run would satisfy every other rule here and still mislead, because a + // reader would have no way to know how many questions went unasked. + assertThat(results.pickles).hasSize(8); + + assertThat(results.pickleOfTestCase.values()) + .as("one test case per pickle, and no pickle executed twice") + .containsExactlyInAnyOrderElementsOf(results.pickles.keySet()); + + assertThat(results.testCaseOfStarted.values()) + .as("one execution per test case; a scenario counted twice is how Go's report went wrong") + .containsExactlyInAnyOrderElementsOf(results.pickleOfTestCase.keySet()); + + assertThat(results.finished) + .as("every started scenario also finished") + .containsExactlyInAnyOrderElementsOf(results.testCaseOfStarted.keySet()); + } + + @Test + @DisplayName("the outcome counts are what the fixture describes") + void theOutcomeCountsAreWhatTheFixtureDescribes() { + assertThat(results.outcomeCounts()) + .containsExactlyInAnyOrderEntriesOf(counts("PASSED", 4L, "FAILED", 1L, "SKIPPED", 3L)); + } + + @Test + @DisplayName("a scenario gated on an undeclared capability is skipped, never passed") + void aGatedScenarioIsSkipped() { + List gated = new ArrayList<>(); + for (Map.Entry pickle : results.pickles.entrySet()) { + if (needsSomethingUndeclared(results.tagsOf(pickle.getValue()))) { + gated.add(pickle.getKey()); + } + } + + assertThat(gated) + .as("the premise: the fixture has a gated plain scenario, a gated outline row and an " + + "inexpressible one, which no declaration could have contained") + .hasSize(3); + + for (String pickleId : gated) { + assertThat(results.outcomeOfPickle(pickleId)) + .as("pickle %s carries an undeclared capability tag", pickleId) + .isEqualTo("SKIPPED"); + } + + assertThat(results.outcomeCounts().get("SKIPPED")) + .as("nothing else was skipped, so a skip cannot hide an unrelated one") + .isEqualTo((long) gated.size()); + } + + @Test + @DisplayName("the gate's own reason survives into the stream, and the two kinds of skip stay apart") + void theGateReasonSurvives() { + // The declaration in the envelope is enough to work out *why* most scenarios were skipped, + // but the reason the gate gave is carried too, on the hook result that produced the skip. + List messages = results.messagesOfSkippedSteps(); + assertThat(messages).hasSize(3); + + // And for one of them the declaration is NOT enough, which is why this reason has to travel. + // @large-integers is absent from every Java declaration and absent for a reason that says + // nothing about the provider: the SDK's integer accessor is 32 bits. A reader who inferred + // "the provider declined" from the absence would be reading a decision into something no + // Java provider was ever offered. So the message names the SDK, and says it is not the + // provider declining, in words that could not be confused with the other two. + List unaskable = new ArrayList<>(); + messages.forEach(message -> { + if (message.contains("LARGE_INTEGERS")) { + unaskable.add(message); + } + }); + assertThat(unaskable).hasSize(1); + assertThat(unaskable.get(0)) + .contains("the Java SDK cannot express capability") + .contains("not the provider under test declining") + .doesNotContain("does not declare capability"); + + assertThat(messages) + .filteredOn(message -> !message.contains("LARGE_INTEGERS")) + .hasSize(2) + .allSatisfy(message -> assertThat(message) + .contains("does not declare capability") + .contains("STALE")); + } + + @Test + @DisplayName("a scenario's tags include one set on its Examples block alone") + void tagsIncludeExamplesBlockTags() { + List rows = results.picklesNamed(OUTLINE); + + assertThat(rows).hasSize(3); + assertThat(rows) + .as("the feature tag reaches every row") + .allSatisfy(row -> assertThat(results.tagsOf(row)).contains("@events")); + + // Gherkin allows a tag on an individual Examples block, so two rows of one outline can + // differ in whether the gate stops them. Both rows must appear, and only the tagged one + // may be skipped: a skip that took its siblings with it would be invisible in the totals. + List outcomes = new ArrayList<>(); + List stale = new ArrayList<>(); + for (JsonNode row : rows) { + outcomes.add(results.outcomeOfPickle(row.get("id").asText())); + stale.add(results.tagsOf(row).contains("@stale")); + } + assertThat(stale).containsExactly(false, false, true); + assertThat(outcomes).containsExactly("PASSED", "PASSED", "SKIPPED"); + } + + @Test + @DisplayName("each Scenario Outline row is identified by the Examples row it came from") + void outlineRowsAreDistinguishable() { + List rows = results.picklesNamed(OUTLINE); + + assertThat(rows) + .as("all three rows share one name, which is the premise of this test") + .extracting(row -> row.get("name").asText()) + .containsOnly(OUTLINE); + + // A pickle's astNodeIds are [scenario, table row] for an outline-derived scenario. The + // second is the row's identity, and it resolves in the gherkinDocument to the cells the row + // was compiled from. This is the mechanism the TCK used to reverse-engineer from a pickle's + // reported line number; the stream states it outright. + List rowIds = new ArrayList<>(); + List> cells = new ArrayList<>(); + for (JsonNode row : rows) { + JsonNode astNodeIds = row.get("astNodeIds"); + assertThat(astNodeIds).hasSize(2); + String rowId = astNodeIds.get(1).asText(); + rowIds.add(rowId); + cells.add(results.cellsOfTableRow(rowId)); + } + + assertThat(rowIds).doesNotHaveDuplicates(); + assertThat(cells) + .containsExactly( + Arrays.asList("string-flag", "Boolean"), + Arrays.asList("string-flag", "Integer"), + Arrays.asList("boolean-flag", "String")); + } + + @Test + @DisplayName("the stream carries the source of the feature that executed") + void theStreamCarriesTheExecutedSource() throws IOException { + assertThat(results.sources).containsOnlyKeys(FEATURE_URI); + + // Line endings are normalised on both sides: this repo is checked out with whatever the + // platform does, and the point of the assertion is that the text is the file's, not that + // Cucumber preserves CRLF. + byte[] onDisk = Files.readAllBytes(Paths.get("src/test/resources/report-selftest/report.feature")); + assertThat(results.sources.get(FEATURE_URI).replace("\r\n", "\n")) + .as("what ran, verbatim, rather than a claim about which revision it came from") + .isEqualTo(new String(onDisk, StandardCharsets.UTF_8).replace("\r\n", "\n")); + } + + @Test + @DisplayName("the envelope points at the results and covers them with a digest") + void theEnvelopePointsAtTheResults() throws NoSuchAlgorithmException { + JsonNode reference = envelope.get("results"); + + assertThat(reference.get("format").asText()).isEqualTo("cucumber-messages"); + assertThat(reference.get("location").asText()) + .as("a path relative to the envelope, so a published pair can be moved together") + .isEqualTo(CONFIGURATION + ".ndjson"); + assertThat(reference.get("digest").asText()).isEqualTo(sha256Of(resultsBytes)); + } + + @Test + @DisplayName("the envelope carries everything the schema requires and nothing it forbids") + void theEnvelopeCarriesWhatTheSchemaRequires() { + assertThat(envelope.get("schemaVersion").asText()).isEqualTo("1"); + assertThat(envelope.get("provider").get("name").asText()).isEqualTo("My Provider"); + assertThat(envelope.get("provider").get("language").asText()).isEqualTo("java"); + assertThat(envelope.get("provider").get("configuration").asText()).isEqualTo(CONFIGURATION); + assertThat(envelope.get("sdk").get("name").asText()).isEqualTo("dev.openfeature:sdk"); + assertThat(envelope.get("sdk").get("version").asText()).isNotEmpty(); + assertThat(envelope.get("tck").get("implementation").asText()).isEqualTo("java-sdk-contrib/tools/tck"); + assertThat(envelope.get("tck").get("specRevision").asText()).hasSizeGreaterThanOrEqualTo(7); + assertThat(envelope.get("tck").get("specRevision").asText()) + .as("and it is the revision the packaged assets actually came from; see the test below") + .isEqualTo(CanonicalAssetDigestTest.PINNED_REVISION); + // backend is in the schema's top-level required array and backendType in backend's, so both + // are asserted as present rather than as present-if-set. + assertThat(envelope.has("backend")).isTrue(); + assertThat(envelope.get("backend").get("backendType").asText()).isEqualTo("http"); + + // The schema sets additionalProperties: false throughout, so anything the results payload + // now owns is a validation failure rather than harmless duplication. + List fields = new ArrayList<>(); + envelope.fieldNames().forEachRemaining(fields::add); + assertThat(fields).doesNotContain("scenarios", "capabilities"); + assertThat(envelope.get("tck").has("assetsTree")).isFalse(); + } + + @Test + @DisplayName("the declaration lists the capabilities the provider claims, in vocabulary order") + void theDeclarationListsWhatIsClaimed() { + JsonNode declared = envelope.get("declaration").get("declared"); + + // The declaration is an input to reading the results, not a summary of them: the stream says + // a scenario was skipped, and only this says whether the provider declines what it needed. + assertThat(declared) + .extracting(JsonNode::asText) + .containsExactly(Capability.EVENTS.tag(), Capability.OBJECT.tag()); + } + + @Test + @DisplayName("a reserved capability cannot reach the declaration, even when everything is claimed") + void aReservedCapabilityCannotReachTheDeclaration() { + // The provider that claims the most is the case that used to break the rule: "everything" + // spelt EnumSet.allOf, or "everything except X" spelt EnumSet.complementOf, collected the + // reserved tags along the way and published a claim about capabilities no scenario + // examines. So this asks the maximal declaration for its report. + JsonNode declared = MAPPER.valueToTree(report(Capability.declarable())) + .get("declaration") + .get("declared"); + + List tags = new ArrayList<>(); + declared.forEach(tag -> tags.add(tag.asText())); + + assertThat(tags) + .as("a reserved tag gates nothing, so declaring it is a claim nothing can contradict") + .doesNotContain(Capability.CACHING.tag()); + assertThat(tags) + .as("and every capability some scenario does gate is still there") + .containsExactly( + Capability.LIFECYCLE.tag(), + Capability.REINITIALIZATION.tag(), + Capability.EVENTS.tag(), + Capability.STALE.tag(), + Capability.CONFIGURATION_CHANGE.tag(), + Capability.OBJECT.tag(), + // @variants gates the one outline that asserts a variant. 2.2.4 is a SHOULD + // and types.md types the field optional, so a backend with no variant + // concept withholds it and those rows are skipped with that reason. + Capability.VARIANTS.tag(), + // @disabled-flags gates the four-row outline that asks what a flag disabled + // in the management system resolves to. Gated because the answer follows + // from where the substitution happens rather than from provider quality: a + // provider that evaluates locally can return the caller's default, one whose + // backend decides never sent it. The maximal claim includes it. + Capability.DISABLED_FLAGS.tag(), + Capability.UNAVAILABLE_INIT.tag(), + Capability.NUMERIC_COERCION.tag(), + // @string-typing gates the two outline rows that ask a boolean and an + // integer flag through the String accessor. Gated because every value has a + // string representation, so a backend that stores flag values as strings has + // no mismatch to report and withholds it without being non-conformant. + Capability.STRING_TYPING.tag(), + // @fully-typed-values asks the same of a float and a structure, and is a + // separate claim because a store can record booleans and integers natively + // while keeping those two as text. Both scenarios carry @string-typing as + // well, so the maximal claim has to include both tags to reach them. + Capability.FULLY_TYPED_VALUES.tag(), + // @large-integers is absent, and for a different reason from @caching's. + // Scenarios do carry it -- Go and JavaScript run them -- but the Java SDK's + // integer accessor is 32 bits, so no Java provider can be asked for 2^53 - 1 + // and none may claim it. The maximal claim is the maximal claim a provider + // written against THIS SDK can make, which is what a report has to carry. + // @targeting was reserved until targeting-key-flag's three scenarios + // arrived. It gates something now, so the maximal claim includes it. + Capability.TARGETING.tag(), + // @standard-reasons gates reason.feature in its entirety. It is a claim + // rather than an exemption -- 2.2.5 lets a provider report "some other + // string", so a provider that does not declare it is not thereby deficient + // -- but it is an ordinary declarable capability all the same, and the + // maximal claim includes it. + Capability.STANDARD_REASONS.tag()); + } + + @Test + @DisplayName("the revision a report publishes is the one the packaged assets came from") + void theReportedRevisionIsTheRevisionTheAssetsCameFrom() { + // A report's whole claim to be readable later is that it says which questions were asked. + // The revision it names is a POM property, maintained by hand beside a submodule gitlink + // that moves by a different command -- so until now "keep them in step" was a comment in + // the POM, and a re-pin that updated one and not the other would have published a revision + // that did not produce the scenarios in the same file. + // + // CanonicalAssetDigestTest has already established that the packaged assets are + // PINNED_REVISION's, by digest. This is the last link: what the build filtered into the JAR + // and TckBuildInfo reads back is that same revision. + assertThat(TckBuildInfo.specRevision()) + .as("tck.spec.revision in tools/tck/pom.xml must equal " + + "CanonicalAssetDigestTest.PINNED_REVISION -- a re-pin moves the submodule " + + "gitlink, that constant and this property together") + .isEqualTo(CanonicalAssetDigestTest.PINNED_REVISION); + assertThat(TckBuildInfo.specRevision()) + .as("and the properties resource must actually have been filtered, or both are 'unknown'") + .isNotEqualTo(TckBuildInfo.UNKNOWN); + } + + @Test + @DisplayName("naming a reserved capability fails the run rather than being dropped quietly") + void namingAReservedCapabilityFailsTheRun() { + // Chosen over a warning: the declaration is the one part of the report no result can check, + // and a report is read long after the log it would have been warned in has gone. Nothing is + // lost by refusing, because no scenario carries the tag. + Set overclaimed = EnumSet.of(Capability.OBJECT, Capability.CACHING); + + assertThatThrownBy(() -> metadata(overclaimed)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(Capability.CACHING.tag()) + .hasMessageContaining("Capability.declarable()"); + } + + @Test + @DisplayName("\"everything except X\" means everything declarable except X") + void declarableExceptYieldsOnlyDeclarableCapabilities() { + assertThat(Capability.declarableExcept(Capability.STALE)) + .doesNotContain(Capability.STALE, Capability.CACHING) + .contains(Capability.OBJECT, Capability.NUMERIC_COERCION, Capability.TARGETING); + + // The counterpart it replaces, and why it had to be replaced. + assertThat(EnumSet.complementOf(EnumSet.of(Capability.STALE))) + .as("complementOf is the complement of the enum, not of the declarable vocabulary") + .contains(Capability.CACHING); + } + + @Test + @DisplayName("a withheld capability that is a defect is reported as a deviation") + void aDefectIsReportedAsADeviation() { + JsonNode deviations = envelope.get("knownDeviations"); + + assertThat(deviations).hasSize(1); + assertThat(deviations.get(0).get("capability").asText()).isEqualTo(Capability.CONFIGURATION_CHANGE.tag()); + assertThat(deviations.get(0).get("summary").asText()).isNotEmpty(); + assertThat(deviations.get(0).has("issue")) + .as("the fixture's deviation is untracked, and the field is omitted rather than empty") + .isFalse(); + } + + @Test + @DisplayName("the SDK version is read from the classpath rather than declared") + void theSdkVersionIsRead() { + assertThat(TckBuildInfo.sdkVersion()) + .as("the OpenFeature SDK is on the test classpath, so its version must be discoverable") + .isNotEqualTo(TckBuildInfo.UNKNOWN) + .matches("\\d+\\.\\d+.*"); + } + + @Test + @DisplayName("nothing is collected or written when no report directory is configured") + void nothingIsWrittenByDefault(@TempDir Path dir) throws IOException { + CountingEventPublisher publisher = new CountingEventPublisher(); + new ConformanceReportPlugin(() -> null, () -> Optional.of(metadata(DECLARED))).setEventPublisher(publisher); + + assertThat(publisher.registered) + .as("not even the message formatter is wired up, so an unasked-for run buffers nothing") + .isZero(); + try (Stream written = Files.list(dir)) { + assertThat(written).isEmpty(); + } + } + + @Test + @DisplayName("the report-directory knob is spelled the way the other three languages spell it") + void theReportDirectoryKnobIsSpelledPortably() { + // Asserted as literals rather than through the constants, because the point is the name a + // cross-language CI job sets, not that the code is self-consistent. Go, Python and + // JavaScript read TCK_REPORT_DIR; one job exporting one variable has to drive all four, so + // a rename here is a breaking change to something no compiler checks. + assertThat(ConformanceReportPlugin.REPORT_DIR_ENV).isEqualTo("TCK_REPORT_DIR"); + assertThat(ConformanceReportPlugin.REPORT_DIR_PROPERTY).isEqualTo("tck.report.dir"); + } + + @Test + @DisplayName("both files are named after the configuration, safely") + void theFilesAreNamedAfterTheConfiguration() { + assertThat(ReportNames.configurationOf(MyProviderRpcTckTest.class)).isEqualTo(CONFIGURATION); + assertThat(ReportNames.baseNameOf("flagd-rpc")).isEqualTo("flagd-rpc"); + assertThat(ReportNames.baseNameOf("flagd/rpc")) + .as("a configuration name is chosen to read well, not to be path-safe") + .isEqualTo("flagd-rpc"); + assertThat(ReportNames.baseNameOf("../escape")).isEqualTo("escape"); + } + + /** A suite whose name the default configuration derivation has to cope with. */ + private static final class MyProviderRpcTckTest {} + + /** + * Runs the fixture feature through the real Cucumber engine, with the real plugin registered. + * + *

Driven through the JUnit Platform rather than through Cucumber's CLI because that is how the + * TCK itself runs: {@link ProviderTckTest} is a JUnit Platform Suite, and the plugin's + * position in the event stream is a property of that engine. + */ + private static void runSuite(Path dir, Set declared) { + ReportSelfTestSteps.declare(declared); + TckRunMetadata.record(metadata(declared)); + String previous = System.getProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY); + System.setProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY, dir.toString()); + try { + LauncherFactory.create() + .execute(LauncherDiscoveryRequestBuilder.request() + .selectors(DiscoverySelectors.selectClasspathResource("report-selftest")) + .filters(EngineFilter.includeEngines("cucumber")) + .configurationParameter( + Constants.GLUE_PROPERTY_NAME, + ReportSelfTestSteps.class.getPackage().getName()) + .configurationParameter( + Constants.PLUGIN_PROPERTY_NAME, ConformanceReportPlugin.class.getName()) + .configurationParameter(Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME, "false") + .configurationParameter( + Constants.OBJECT_FACTORY_PROPERTY_NAME, "io.cucumber.picocontainer.PicoFactory") + .build()); + } finally { + if (previous == null) { + System.clearProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY); + } else { + System.setProperty(ConformanceReportPlugin.REPORT_DIR_PROPERTY, previous); + } + TckRunMetadata.record(null); + } + } + + /** Builds the envelope a run with this declaration would emit, without running one. */ + private static ConformanceReport report(Set declared) { + return new ConformanceReportPlugin(() -> null, Optional::empty) + .build(metadata(declared), CONFIGURATION + ".ndjson", "sha256:0", "26.1.0"); + } + + private static TckRunMetadata metadata(Set declared) { + TckRunMetadata metadata = new TckRunMetadata( + CONFIGURATION, + declared, + "a test double", + BackendType.HTTP, + // Withheld-and-skipped, which is the shape a fixture may legitimately depict: this + // provider cannot attempt the behaviour, because it never subscribes to the thing + // that would tell it. A narrowing-coercion deviation used to stand here, and that + // is the shape the guidance discourages -- a provider that attempts a behaviour and + // gets it wrong declares the capability and lets the scenario fail. + Collections.singletonList(KnownDeviation.untracked( + Capability.CONFIGURATION_CHANGE, + "the fixture provider swallows its child's change events and can never " + "report one"))); + metadata.recordProviderName("My Provider"); + return metadata; + } + + private static boolean needsSomethingUndeclared(List tags) { + for (String tag : tags) { + Optional capability = Capability.fromTag(tag); + if (capability.isPresent() && !DECLARED.contains(capability.get())) { + return true; + } + } + return false; + } + + private static Map counts(Object... statusesAndCounts) { + Map expected = new LinkedHashMap<>(); + for (int i = 0; i < statusesAndCounts.length; i += 2) { + expected.put((String) statusesAndCounts[i], (Long) statusesAndCounts[i + 1]); + } + return expected; + } + + private static String sha256Of(byte[] bytes) throws NoSuchAlgorithmException { + byte[] digest = MessageDigest.getInstance("SHA-256").digest(bytes); + StringBuilder hex = new StringBuilder("sha256:"); + for (byte b : digest) { + hex.append(String.format("%02x", b)); + } + return hex.toString(); + } + + /** Counts what a plugin registers, for the case where it should register nothing. */ + private static final class CountingEventPublisher implements EventPublisher { + + private int registered; + + @Override + public void registerHandlerFor(Class eventType, EventHandler handler) { + registered++; + } + + @Override + public void removeHandlerFor(Class eventType, EventHandler handler) { + registered--; + } + } + + /** + * A Cucumber Messages stream, read the way a consumer reads one. + * + *

Deliberately built from the raw ndjson with nothing but Jackson. Using Cucumber's own query + * helpers would make these tests agree with Cucumber by construction; the point is to show that + * the facts the report needs are recoverable from the bytes on disk. + */ + private static final class Results { + + private final Map pickles = new LinkedHashMap<>(); + private final Map pickleOfTestCase = new LinkedHashMap<>(); + private final Map testCaseOfStarted = new LinkedHashMap<>(); + private final Map mostSevereByStarted = new LinkedHashMap<>(); + private final Map tableRows = new LinkedHashMap<>(); + private final Map sources = new TreeMap<>(); + private final List finished = new ArrayList<>(); + private final List skippedStepMessages = new ArrayList<>(); + + static Results parse(String ndjson) throws IOException { + Results results = new Results(); + for (String line : ndjson.split("\n")) { + if (line.trim().isEmpty()) { + continue; + } + results.accept(MAPPER.readTree(line)); + } + return results; + } + + private void accept(JsonNode message) { + if (message.has("source")) { + JsonNode source = message.get("source"); + sources.put(source.get("uri").asText(), source.get("data").asText()); + } + if (message.has("gherkinDocument")) { + collectTableRows(message.get("gherkinDocument")); + } + if (message.has("pickle")) { + JsonNode pickle = message.get("pickle"); + pickles.put(pickle.get("id").asText(), pickle); + } + if (message.has("testCase")) { + JsonNode testCase = message.get("testCase"); + pickleOfTestCase.put( + testCase.get("id").asText(), testCase.get("pickleId").asText()); + } + if (message.has("testCaseStarted")) { + JsonNode started = message.get("testCaseStarted"); + testCaseOfStarted.put( + started.get("id").asText(), started.get("testCaseId").asText()); + } + if (message.has("testCaseFinished")) { + finished.add( + message.get("testCaseFinished").get("testCaseStartedId").asText()); + } + if (message.has("testStepFinished")) { + JsonNode step = message.get("testStepFinished"); + String startedId = step.get("testCaseStartedId").asText(); + JsonNode result = step.get("testStepResult"); + String status = result.get("status").asText(); + mostSevereByStarted.merge(startedId, status, Results::moreSevere); + if ("SKIPPED".equals(status) && result.has("message")) { + skippedStepMessages.add(result.get("message").asText()); + } + } + } + + private void collectTableRows(JsonNode document) { + for (JsonNode child : orEmpty(document.get("feature"), "children")) { + JsonNode scenario = child.get("scenario"); + if (scenario == null) { + continue; + } + for (JsonNode examples : orEmpty(scenario, "examples")) { + for (JsonNode row : orEmpty(examples, "tableBody")) { + tableRows.put(row.get("id").asText(), row); + } + } + } + } + + private static Iterable orEmpty(JsonNode parent, String field) { + JsonNode node = parent == null ? null : parent.get(field); + return node == null ? Collections.emptyList() : node; + } + + private static String moreSevere(String left, String right) { + return SEVERITY.indexOf(left) >= SEVERITY.indexOf(right) ? left : right; + } + + Map outcomeCounts() { + Map counts = new LinkedHashMap<>(); + for (String status : mostSevereByStarted.values()) { + counts.merge(status, 1L, Long::sum); + } + return counts; + } + + String outcomeOfPickle(String pickleId) { + for (Map.Entry started : testCaseOfStarted.entrySet()) { + if (pickleId.equals(pickleOfTestCase.get(started.getValue()))) { + return mostSevereByStarted.get(started.getKey()); + } + } + throw new AssertionError("pickle " + pickleId + " was never executed"); + } + + List tagsOf(JsonNode pickle) { + List tags = new ArrayList<>(); + for (JsonNode tag : orEmpty(pickle, "tags")) { + tags.add(tag.get("name").asText()); + } + return tags; + } + + List picklesNamed(String name) { + List matching = new ArrayList<>(); + for (JsonNode pickle : pickles.values()) { + if (name.equals(pickle.get("name").asText())) { + matching.add(pickle); + } + } + return matching; + } + + List cellsOfTableRow(String rowId) { + JsonNode row = tableRows.get(rowId); + assertThat(row) + .as("ast node %s is a table row in the gherkin document", rowId) + .isNotNull(); + List values = new ArrayList<>(); + for (JsonNode cell : orEmpty(row, "cells")) { + values.add(cell.get("value").asText()); + } + return values; + } + + List messagesOfSkippedSteps() { + return skippedStepMessages; + } + } +} diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java new file mode 100644 index 0000000000..43f26c7d60 --- /dev/null +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java @@ -0,0 +1,74 @@ +package dev.openfeature.contrib.tools.tck.selftest; + +import dev.openfeature.contrib.tools.tck.Capability; +import io.cucumber.java.Before; +import io.cucumber.java.Scenario; +import io.cucumber.java.en.Given; +import java.util.EnumSet; +import java.util.Set; + +/** + * Glue for {@code report-selftest/report.feature}, the fixture the conformance report tests run. + * + *

It stands in for a provider, and for nothing else: there is no Compose stack, no control API + * and no provider here, because none of that is what the report tests are about. What it does share + * with the real suite is the part that matters — the capability gate is + * {@link Capability#requireDeclared}, the same call the real + * {@code ProviderSteps.gateOnCapabilities} makes from the same kind of {@code @Before(order = 0)} + * hook. A fixture that aborted by some other route would prove only that some abort becomes a skip. + */ +public class ReportSelfTestSteps { + + private static volatile Set declared = Capability.declarable(); + + /** + * Sets the capabilities the fixture provider declares, for the run that is about to start. + * + * @param capabilities the declared capabilities + */ + public static void declare(Set capabilities) { + declared = EnumSet.copyOf(capabilities); + } + + /** + * Gates the scenario on the declared capabilities, exactly as the real suite does. + * + * @param scenario the scenario about to run + */ + @Before(order = 0) + public void gateOnCapabilities(Scenario scenario) { + Capability.requireDeclared(scenario.getUri(), scenario.getSourceTagNames(), declared); + } + + /** A step that does nothing, successfully. */ + @Given("a step that passes") + public void aStepThatPasses() { + // A passing scenario needs a step that passes and nothing more. + } + + /** + * A step that does nothing, unsuccessfully. + * + *

An {@link AssertionError} rather than a checked failure, because that is what a failing + * assertion in a step definition throws, and the point is what the results stream makes of it. + */ + @Given("a step that fails") + public void aStepThatFails() { + throw new AssertionError("this fixture scenario is expected to fail"); + } + + /** + * A passing step that takes its arguments from an Examples row. + * + *

The parameters are unused. They exist so the outline's rows differ in their compiled step + * text as well as in their table row, which is how a consumer that reads only the steps can + * still tell the rows apart. + * + * @param key the flag key from the row + * @param requested the requested type from the row + */ + @Given("a step that passes with {string} as {string}") + public void aStepThatPassesWith(String key, String requested) { + // Interpolated into the pickle's step text by Cucumber; nothing to do here. + } +} diff --git a/tools/tck/src/test/resources/report-selftest/report.feature b/tools/tck/src/test/resources/report-selftest/report.feature new file mode 100644 index 0000000000..99d5caaa0d --- /dev/null +++ b/tools/tck/src/test/resources/report-selftest/report.feature @@ -0,0 +1,46 @@ +# Fixture for ConformanceReportPluginTest, and not part of the conformance suite: it lives under a +# separate classpath root so that nothing selecting "gherkin" can pick it up. +# +# It is shaped like the real suite rather than minimal, because the properties the report has to +# preserve are properties of that shape: a capability tag on the feature, one on a scenario, one on +# a single Examples block, and an outline whose rows all share a name. Running real Cucumber over +# this is the only way to check what a capability-gated abort actually becomes in the results. +# +# It also carries both kinds of skip, because they are the two a reader has to be able to tell +# apart: a capability this provider did not declare, and a capability no provider in this language +# can be asked. Both end as SKIPPED, and only the reason carried on the hook result distinguishes +# them. + +@events +Feature: Report self-test + + Scenario: A mandatory scenario + Given a step that passes + + @object + Scenario: A scenario needing a declared capability + Given a step that passes + + @stale + Scenario: A scenario needing an undeclared capability + Given a step that passes + + @large-integers + Scenario: A scenario needing a capability this SDK cannot express + Given a step that passes + + Scenario: A scenario that fails + Given a step that fails + + Scenario Outline: Requesting the wrong type returns the code default + Given a step that passes with "" as "" + + Examples: a string flag requested as something else + | key | requested | + | string-flag | Boolean | + | string-flag | Integer | + + @stale + Examples: gated by a tag on this block alone + | key | requested | + | boolean-flag | String | From 18517d71316aacd4cda21112e9d88f63dcb01df4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 2 Nov 2026 22:02:35 +0100 Subject: [PATCH 2/3] ci: render the conformance reports from the Provider TCK job Set TCK_REPORT_DIR for the TCK run, summarise each suite's envelope and Cucumber Messages stream into the job summary, and upload both as an artifact. A scenario counts by its most severe step, as the report plugin defines it. Signed-off-by: Simon Schrottner --- .github/scripts/tck-report-summary.sh | 37 +++++++++++++++++++++++++++ .github/workflows/ci.yml | 14 ++++++++++ 2 files changed, 51 insertions(+) create mode 100755 .github/scripts/tck-report-summary.sh diff --git a/.github/scripts/tck-report-summary.sh b/.github/scripts/tck-report-summary.sh new file mode 100755 index 0000000000..73f92c8be1 --- /dev/null +++ b/.github/scripts/tck-report-summary.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Renders the conformance reports in $1 as a Markdown table, one row per suite. +# +# A scenario's outcome is the most severe status among its steps, the same rule the TCK's report +# plugin documents, so a capability-gated skip counts as skipped rather than passed. +set -euo pipefail + +dir="${1:?usage: tck-report-summary.sh }" +shopt -s nullglob +envelopes=("$dir"/*.json) + +echo "## Provider conformance" +echo +if [ ${#envelopes[@]} -eq 0 ]; then + echo "No conformance reports were written." + exit 0 +fi + +echo "| Provider | Configuration | Backend | Passed | Failed | Skipped | Declared | Known deviations |" +echo "|---|---|---|--:|--:|--:|---|--:|" +for envelope in "${envelopes[@]}"; do + stream="${envelope%.json}.ndjson" + counts=$(jq -rs ' + def rank: {"FAILED":5,"AMBIGUOUS":4,"UNDEFINED":3,"PENDING":2,"SKIPPED":1,"PASSED":0}[.] // 0; + [ .[] | .testStepFinished // empty ] + | group_by(.testCaseStartedId) + | map([ .[].testStepResult.status ] | max_by(rank)) + | [ (map(select(. == "PASSED")) | length), + (map(select(rank >= 2)) | length), + (map(select(. == "SKIPPED")) | length) ] + | @tsv' "$stream") + read -r passed failed skipped <<<"$counts" + jq -r --arg p "$passed" --arg f "$failed" --arg s "$skipped" ' + "| \(.provider.name) | \(.provider.configuration) | \(.backend.backendType) | \($p) | \($f) | \($s) | " + + ((.declaration.declared // []) | join(" ")) + + " | \((.knownDeviations // []) | length) |"' "$envelope" +done diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c1c02c4103..da07845391 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,20 @@ jobs: # !codequality: a module's own default-active codequality profile (flagd's runs vmlens # *CTest suites) is not switched off by a profile in the root pom; main enforces it anyway run: mvn --batch-mode --activate-profiles 'tck,!codequality' clean verify + env: + TCK_REPORT_DIR: ${{ github.workspace }}/tck-reports + + - name: Summarise the conformance reports + if: ${{ !cancelled() }} + run: bash .github/scripts/tck-report-summary.sh tck-reports >> "$GITHUB_STEP_SUMMARY" + + - name: Upload the conformance reports + if: ${{ !cancelled() }} + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: conformance-reports + path: tck-reports/ + if-no-files-found: ignore main: strategy: From 3b5b59db10b705efcc8cf8502d70c96273c81551 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Mon, 2 Nov 2026 22:13:36 +0100 Subject: [PATCH 3/3] refactor(tck): trim comments to what the code does not say Signed-off-by: Simon Schrottner --- .../tools/tck/CanonicalScenarioGuard.java | 93 +++-------- .../contrib/tools/tck/CanonicalScenarios.java | 46 ++---- .../contrib/tools/tck/ConformanceReport.java | 102 +++--------- .../tools/tck/ConformanceReportPlugin.java | 94 +++-------- .../contrib/tools/tck/ReportNames.java | 12 +- .../contrib/tools/tck/TckBuildInfo.java | 33 ++-- .../contrib/tools/tck/TckRunMetadata.java | 33 ++-- .../tools/tck/CanonicalScenarioGuardTest.java | 39 ++--- .../tck/ConformanceReportPluginTest.java | 150 ++++-------------- .../tck/selftest/ReportSelfTestSteps.java | 25 +-- .../resources/report-selftest/report.feature | 17 +- 11 files changed, 158 insertions(+), 486 deletions(-) diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java index 4d0fc92e74..3a8c079ece 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuard.java @@ -26,58 +26,24 @@ /** * Fails a suite that is set up to run less than the whole canonical scenario set. * - *

Everything else in this module makes the canonical set easy to extend safely. Nothing makes it - * hard to shrink, and shrinking it is the failure that matters: a run that asks fifty-four of the - * fifty-six questions and reports success is indistinguishable, in every artifact it produces, from - * one that asked all fifty-six. The known ways to get there are a feature file dropped into - * {@code gherkin/}, a {@code cucumber.filter.tags} or {@code cucumber.filter.name} expression, and - * selectors or glue overridden in a consuming module's {@code junit-platform.properties}. + *

A run that asks fewer questions than the canonical set and reports success is indistinguishable + * from a full one. Shrinking happens through a feature file dropped into {@code gherkin/}, a {@code + * cucumber.filter.tags} or {@code cucumber.filter.name} expression, or selectors or glue overridden + * in {@code junit-platform.properties}. * - *

Selected by {@link ProviderTckTest} as an ordinary JUnit Jupiter test, so a reduced set - * fails the build the way any other failing test does. A {@link - * org.junit.platform.launcher.TestExecutionListener} cannot do that job: the JUnit Platform catches - * and logs whatever a listener throws, which is exactly the silent pass being guarded against. + *

Run by {@link ProviderTckTest} as an ordinary Jupiter test, because the JUnit Platform swallows + * whatever a {@link org.junit.platform.launcher.TestExecutionListener} throws. It checks the setup + * rather than counting afterwards, from two kinds of evidence available before any scenario runs: + * the discovered test plan (captured by {@link TckSuiteListener}), and the effective filter + * configuration. Cucumber applies the tag filter as a skip at execution, so a filtered scenario is + * still in the plan and the configured expression has to be read directly. * - *

Two kinds of evidence, both available before a single scenario has run: - * - *

    - *
  • the discovered test plan, captured by {@link TckSuiteListener}. Which - * scenarios a suite selected is settled at discovery, so a canonical file that was shadowed, - * replaced or added to is visible there. - *
  • the effective filter configuration, read through this test's own - * {@link ExtensionContext}. Cucumber applies {@code cucumber.filter.tags} as a skip at - * execution rather than as a discovery filter — a filtered scenario is in the plan and never - * runs — so the plan cannot show it and the configured expression has to be read directly. The - * Jupiter engine inside the suite resolves configuration from the same sources the Cucumber - * engine does, whether the value came from a system property, a - * {@code junit-platform.properties} or an annotation on the suite. - *
- * - *

Checking the setup rather than counting afterwards is not a compromise made for convenience. A - * count is only complete once the last scenario has finished, and the only hook that runs there is a - * listener, which cannot fail anything. Both kinds of evidence are settled before the first scenario - * runs, so the check itself takes no measurable time and needs no Compose stack — though where its - * result appears in a run depends on the order the JUnit Platform happens to execute the two engines - * in, which is not specified. - * - *

Extension scenarios are ignored entirely. The check is defined over {@code gherkin/}, so an - * adopter's {@code extensions/} scenarios can neither stand in for a canonical scenario nor look - * like a spurious one. - * - *

Separable from the rest of the extension work by design: it guards a bypass rather than enabling - * anything, and dropping it leaves the extension point unaffected. + *

Extension scenarios are ignored: the check is defined over {@code gherkin/} only. */ @ExtendWith(CanonicalScenarioGuard.CaptureConfiguration.class) public final class CanonicalScenarioGuard { - /** - * System property that downgrades this check to a skip. - * - *

An escape hatch is necessary rather than a weakness. Running one scenario with - * {@code -Dcucumber.filter.tags=@events} is routine while debugging a provider, and a check that - * made that impossible would be switched off permanently instead of temporarily. It produces a - * skip rather than a pass, so the run says out loud that its canonical set was not verified. - */ + /** System property that downgrades this check to a skip, so a deliberately partial run says so. */ public static final String PARTIAL_PROPERTY = "tck.partial"; /** Environment variable equivalent of {@link #PARTIAL_PROPERTY}. */ @@ -89,20 +55,17 @@ public final class CanonicalScenarioGuard { }; /** - * What each TCK suite in a discovered plan selected under {@code gherkin/}. - * - *

Keyed by suite class name and accumulated rather than replaced. A suite discovers its - * children through a launcher of its own, so this is called more than once per build with plans - * of differing scope, and a plan containing no TCK suite says nothing about the ones already - * recorded. + * What each TCK suite selected under {@code gherkin/}, keyed by suite class name. Accumulated, + * because a suite discovers its children through a launcher of its own and so this is called more + * than once per build. */ private static final Map> DISCOVERED = new ConcurrentHashMap<>(); private static final String CANONICAL_PREFIX = ProviderTck.FEATURES + "/"; /** - * Records what the suites in a discovered plan will run. Public only for the suite listener in - * the internal package. + * Records what the suites in a discovered plan will run. Public only for the listener in the + * internal package. * * @param plan the plan about to be executed */ @@ -112,13 +75,13 @@ public static void observe(TestPlan plan) { } } - /** Forgets what has been observed, so a test can drive the guard over a plan of its own. */ + /** Clears the recorded plans, so a test can drive the guard over its own. */ static void forget() { DISCOVERED.clear(); } /** - * Returns what each suite in the observed plans selected under {@code gherkin/}. + * Returns the canonical scenarios each observed suite selected. * * @return canonical scenarios by suite class name */ @@ -217,7 +180,6 @@ static Map filtersIn(ExtensionContext context) { return configured; } - /** Returns whether the run has declared itself partial. */ private static boolean partialRunAllowed() { return isTrue(System.getProperty(PARTIAL_PROPERTY)) || isTrue(System.getenv(PARTIAL_ENV)); } @@ -249,12 +211,8 @@ private static void collectCanonical(TestPlan plan, TestIdentifier identifier, S } /** - * Returns the canonical scenario a test identifier stands for, if it is one. - * - *

A Cucumber scenario discovered from a classpath resource carries a - * {@link ClasspathResourceSource} naming the feature resource and the scenario's line — for a - * Scenario Outline, the line of the {@code Examples} row it was compiled from, which is what makes - * each row count separately. + * Returns the canonical scenario a test identifier stands for, if it is one. For a Scenario + * Outline the position is the {@code Examples} row's line, so each row counts separately. */ private static Optional canonicalRefOf(TestIdentifier identifier) { Optional source = identifier.getSource(); @@ -271,12 +229,9 @@ private static Optional canonicalRefOf(TestIdentifier id } /** - * Hands the guard the configuration of the engine it is running in. - * - *

Jupiter resolves configuration parameters for an extension, not for a test method, so the - * context is captured here rather than injected. Inside a suite it carries the suite's own - * {@code @ConfigurationParameter} values as well as the system properties and - * {@code junit-platform.properties} the Cucumber engine beside it reads. + * Captures the extension context, because Jupiter resolves configuration parameters for an + * extension and not for a test method. Inside a suite it sees the suite's own + * {@code @ConfigurationParameter} values. */ static final class CaptureConfiguration implements BeforeEachCallback { diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java index d417f045fa..4be5149326 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/CanonicalScenarios.java @@ -34,23 +34,17 @@ /** * The canonical scenario set, as this artifact ships it. * - *

Read from this artifact's own code source — the JAR or {@code target/classes} that - * {@link ProviderTck} was loaded from — rather than through the classloader. That is the point of the - * class: a feature file placed in {@code gherkin/} on another classpath root shadows the canonical - * one of the same name, and a check that read the shadowed copy back through - * {@code getResource("gherkin/errors.feature")} would be checking the replacement against itself. - * The code source is the only view of {@code gherkin/} that an adopter's classpath cannot alter. + *

Read from this artifact's own code source rather than through the classloader: a feature file on + * another classpath root shadows the canonical one of the same name, and reading the shadowed copy + * back would check the replacement against itself. * - *

A scenario is identified by its resource name and its line, which is what a Cucumber - * {@code ClasspathResourceSource} in the JUnit test plan carries. For a Scenario Outline that line is - * the {@code Examples} row, so each row counts individually — which is necessary, because eleven rows - * of the type-mismatch matrix share one name. + *

A scenario is identified by its resource name and line, as a Cucumber + * {@code ClasspathResourceSource} in the test plan carries them. For a Scenario Outline the line is + * the {@code Examples} row, so each row counts individually. * - *

What this does not establish is that the canonical files contain what they - * should. A replacement file that happened to place its scenarios on the same lines would satisfy the - * comparison. Content is covered elsewhere and better: the results stream carries the {@code source} - * of every feature that executed, and the report's {@code tck.specRevision} says which revision it - * should match. + *

This does not establish that the canonical files contain what they should; the results stream + * carries the source of every feature that executed, and {@code tck.specRevision} says what it should + * match. */ final class CanonicalScenarios { @@ -79,12 +73,11 @@ static Set shipped() { return cached; } - /** Reads and compiles the canonical feature files out of the code source this class came from. */ + /** Reads and compiles the canonical feature files out of this class's code source. */ @SuppressFBWarnings( value = "PATH_TRAVERSAL_IN", - justification = "The path is this class's own code source, reported by the JVM. Nothing outside the " - + "running artifact can influence it, and reading it from anywhere else would defeat the point " - + "of the class") + justification = "The path is this class's own code source, reported by the JVM; nothing outside the " + + "running artifact can influence it") private static Set read() { CodeSource codeSource = ProviderTck.class.getProtectionDomain().getCodeSource(); URL location = codeSource == null ? null : codeSource.getLocation(); @@ -164,12 +157,8 @@ private static byte[] readAll(InputStream in) throws IOException { } /** - * Compiles one feature file to the scenarios Cucumber would run from it. - * - *

The Gherkin document is walked rather than the pickles it produces, because a pickle carries - * no line number — and the line is what the JUnit test plan identifies a scenario by. The walk is - * the same rule Gherkin's own compiler applies: a Scenario Outline yields one scenario per - * {@code Examples} body row, anything else yields one at its own line. + * Compiles one feature file to the scenarios Cucumber would run from it. Walks the Gherkin + * document rather than its pickles, which carry no line number. */ private static void collect(String resource, byte[] content, Set refs) { GherkinParser parser = GherkinParser.builder() @@ -211,12 +200,7 @@ private static void collectScenario(String resource, Scenario scenario, Set } } - /** - * One scenario, identified the way the JUnit test plan identifies it. - * - *

The name is carried for the failure message only; two scenarios are the same scenario when - * they are at the same line of the same classpath resource. - */ + /** One scenario. The name is for failure messages only; identity is resource and line. */ static final class Ref implements Comparable { private final String resource; diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java index 42b331462b..986eb479b1 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReport.java @@ -5,34 +5,19 @@ /** * The envelope of one conformance run: what was tested, what the provider claims, and where the - * results are. + * results are. Per-scenario outcomes are deliberately not here; they live in the Cucumber Messages + * stream that {@link Results} references. * - *

This document deliberately carries no per-scenario outcomes. The results - * themselves are a standard format — a Cucumber Messages stream, referenced by {@link Results} — - * because per-scenario outcomes, tags, Scenario Outline row identity and the executed feature source - * are all already specified there. Defining them a second time here would create a format to - * maintain and version and two places for the same fact to disagree. - * - *

The field names and nesting are fixed by the report schema in the OpenFeature specification - * repository. This class is a transcription of that schema rather than a shape that would be - * convenient in Java, because the point of the format is that every language's TCK emits the same - * document. - * - *

Fields left {@code null} are omitted from the JSON. The schema sets - * {@code additionalProperties: false} throughout, so an unexpected field is a validation failure - * rather than something a consumer ignores. + *

Field names and nesting transcribe the report schema in the OpenFeature specification + * repository, so every language's TCK emits the same document. Fields left {@code null} are omitted, + * because the schema sets {@code additionalProperties: false}. * * @see open-feature/spec#424 */ @JsonInclude(JsonInclude.Include.NON_NULL) public final class ConformanceReport { - /** - * The major version of the report schema this document conforms to. - * - *

An integer as a string, so a consumer can reject a report it does not understand rather - * than guessing at it. - */ + /** The major version of the report schema this document conforms to, as an integer string. */ public static final String SCHEMA_VERSION = "1"; /** The schema version this document claims. */ @@ -47,13 +32,7 @@ public final class ConformanceReport { /** What asked the questions, and which questions. */ public final Tck tck; - /** - * What the provider was pointed at. - * - *

Always present: {@code backend} is in the schema's top-level {@code required} array, - * including for a provider with no backend — that is what {@code backendType}'s - * {@code in-process} member is for. - */ + /** What the provider was pointed at. Always present, as the schema requires. */ public final Backend backend; /** The capability set this provider claims. */ @@ -87,25 +66,13 @@ public final class ConformanceReport { @JsonInclude(JsonInclude.Include.NON_NULL) public static final class Provider { - /** - * What the provider calls itself, through its own metadata. - * - *

Not the name of the suite. A suite name is chosen to read well in a failure message — - * {@code flagd-rpc} — which makes it the {@link #configuration}, not the identity. - */ + /** What the provider calls itself through its own metadata, not the suite name. */ public final String name; /** The language the provider is written in, always {@code java} here. */ public final String language; - /** - * Which configuration of the provider was tested. - * - *

A provider with more than one materially different mode produces one report per mode, - * and they are not interchangeable: flagd's RPC and in-process resolvers differ in whether - * they emit {@code PROVIDER_STALE}, so a report keyed on the provider name alone would have - * to pick one and misrepresent the other. - */ + /** Which configuration was tested; a provider with materially different modes gets a report per mode. */ public final String configuration; Provider(String name, String language, String configuration) { @@ -142,12 +109,8 @@ public static final class Tck { public final String version; /** - * The open-feature/spec commit the executed artifacts came from. - * - *

The executed Gherkin no longer has to be taken on trust: the results stream carries the - * {@code source} of every feature it ran, so a consumer can diff what executed against what - * this revision contains. The revision still identifies the two artifacts the stream does - * not carry — the canonical flag set and the control API definition. + * The open-feature/spec commit the executed artifacts came from. It identifies what the stream + * does not carry: the canonical flag set and the control API definition. */ public final String specRevision; @@ -166,13 +129,8 @@ public static final class Backend { public final String description; /** - * How the backend was driven: {@code http}, {@code in-process} or {@code custom}. - * - *

{@code in-process} is the narrow allowance made for providers with no backend; a report - * claiming it for a provider that has one should be treated with suspicion. {@code custom} - * is a control the TCK does not ship, which the suite cannot vouch for. - * - *

Always written, and never inferred: it is what the control states through + * How the backend was driven: {@code http}, {@code in-process} or {@code custom}. Always + * written, never inferred: it is what the control states through * {@link BackendControl#backendType()}. */ public final BackendType backendType; @@ -188,14 +146,8 @@ public static final class Backend { public static final class Declaration { /** - * Capabilities the provider declares, including the leading at-sign. - * - *

An input to reading the results rather than a summary of them, which is - * why it cannot be derived from the results payload and has to be stated here. A skipped - * scenario in the stream says the question was not put to this provider; only the declaration - * says whether that is because the provider declines the capability. Given the declaration - * and a scenario's tags — both of which the stream carries — the reason for a skip follows - * without being transported per scenario. + * Capabilities the provider declares, including the leading at-sign. It cannot be derived from + * the results: a skipped scenario does not say whether the provider declined the capability. */ public final List declared; @@ -215,29 +167,13 @@ public static final class Results { public final String format; /** - * The Cucumber Messages release the stream was produced against. - * - *

Messages is versioned and the TCK implementations pin different releases -- this one - * takes whatever cucumber-jvm bundles -- 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. - * - *

Read back out of the stream's own {@code meta.protocolVersion} rather than from a - * constant or the artifact version, so the envelope and the stream cannot disagree about - * which release produced it. + * The Cucumber Messages release the stream was produced against, read back from the stream's + * own {@code meta.protocolVersion}. TCK implementations pin different releases, and validating + * against the wrong schema gives a meaningless result. */ public final String formatVersion; - /** - * Where to fetch the results: a path relative to this document. - * - *

Referenced rather than inlined because a Messages stream carries the feature sources and - * so is far larger than this envelope, and because a consumer deciding whether it cares about - * a report should not have to fetch the whole run to find out. - */ + /** Where to fetch the results: a path relative to this document. Not inlined, as the stream is far larger. */ public final String location; /** Digest over the results payload as {@code sha256:}. */ diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java index 481c6aad52..ffde84b3c0 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ConformanceReportPlugin.java @@ -30,35 +30,15 @@ /** * Writes a machine-readable conformance report for the suite that just ran. * - *

Registered automatically by {@link ProviderTckTest}, so an adopter changes nothing to - * get one. It is opt-in per run: set {@value #REPORT_DIR_ENV} (or the - * {@value #REPORT_DIR_PROPERTY} system property) and each suite writes two files, - * {@code

/.json} and {@code /.ndjson}. Emitting a report is - * a property of the run rather than of the code — CI asks for one, a developer running the suite - * locally does not — and unset means no report, which is not an error. Several suites in one JVM each - * write their own pair of files, so flagd's two resolvers do not collide. + *

Registered automatically by {@link ProviderTckTest}. Opt-in per run: set {@value #REPORT_DIR_ENV} + * (or the {@value #REPORT_DIR_PROPERTY} system property) and each suite writes + * {@code

/.json}, the envelope, and {@code /.ndjson}, a + * Cucumber Messages stream produced by Cucumber's + * own {@link MessageFormatter}. Unset means no report, which is not an error. * - *

The results are not a format this project defines. The {@code .ndjson} file is - * a Cucumber Messages stream, produced by - * Cucumber's own {@link MessageFormatter} — the same class the built-in {@code message:} plugin - * instantiates, so the bytes are what {@code --plugin message:...} would have written. It already - * carries everything a per-scenario report would have had to invent: the outcome of every scenario, - * its tags including any on an individual {@code Examples} block, an exact Scenario Outline row - * identity through pickle AST node ids, and the {@code source} of every feature that executed. - * - *

The {@code .json} file is the envelope, and it exists because a Messages stream cannot say what - * it was a test of. No standard format identifies the provider, the SDK it was driven - * through, the TCK build, or — most importantly — the capability set the provider declared. That - * declaration is an input to reading the results rather than a summary of them: the stream says a - * scenario was skipped, and only the declaration says whether that is because the provider declines - * the capability it needed. - * - *

Why the stream is buffered rather than streamed to the file. The file name is - * derived from the provider configuration, which is not known when Cucumber wires plugins up — the - * suite reports it once its runtime has started, which is after the first messages have already been - * emitted. Buffering keeps one file per suite correctly named, and has the side benefit that - * {@code results.digest} is computed over exactly the bytes that were written. The stream for this - * suite is well under a megabyte. + *

The stream is buffered rather than written as it arrives, because the file name derives from the + * provider configuration, which is only known once the suite's runtime has started. That also makes + * {@code results.digest} exactly the bytes written. * * @see open-feature/spec#424 */ @@ -69,15 +49,11 @@ public final class ConformanceReportPlugin implements ConcurrentEventListener { /** * System property naming the directory reports are written to, taking precedence over the - * environment variable. - * - *

Accepted in addition to {@value #REPORT_DIR_ENV} because {@code -D} is how a Maven or - * Gradle invocation is usually parameterised. The environment variable is the portable spelling - * and is what every other language's TCK reads, so a cross-language CI job can set one thing. + * environment variable, which is the spelling every other language's TCK reads. */ public static final String REPORT_DIR_PROPERTY = "tck.report.dir"; - /** Extension of the envelope, which is what a consumer reads first. */ + /** Extension of the envelope. */ static final String ENVELOPE_EXTENSION = ".json"; /** Extension of the Cucumber Messages stream the envelope points at. */ @@ -104,18 +80,15 @@ public ConformanceReportPlugin() { public void setEventPublisher(EventPublisher publisher) { String dir = reportDir.get(); if (dir == null) { - // Nothing asked for a report, so nothing is collected either. Registering the message - // formatter regardless would buffer a stream for every local test run. + // Without this, the formatter would buffer a stream for every local test run. return; } ByteArrayOutputStream results = new ByteArrayOutputStream(); new MessageFormatter(results).setEventPublisher(publisher); - // Registered *after* the formatter, and deliberately so. Cucumber invokes the handlers for - // one event type in registration order, and the formatter closes its writer when it sees - // the run-finished message; going second is what guarantees the buffer is complete and - // flushed before the digest is taken over it. + // Must be registered after the formatter: handlers run in registration order, and the + // formatter flushes the buffer on the run-finished message. publisher.registerHandlerFor(Envelope.class, envelope -> { if (envelope.getTestRunFinished().isPresent()) { write(dir, results.toByteArray()); @@ -162,16 +135,9 @@ ConformanceReport build(TckRunMetadata run, String location, String digest, Stri } /** - * Lists the declared capabilities as Gherkin tags, in the order the vocabulary declares them. - * - *

Ordered by the enum rather than by the set so that two runs of the same configuration - * produce byte-identical declarations, which is what makes the envelopes diffable. - * - *

{@linkplain Capability#reserved() Reserved} capabilities are skipped. A declaration that - * names one is already rejected where it enters the run, in {@link TckRunMetadata}, so this is - * not the thing that tells the adopter; it is what makes "no reserved tag in the emitted - * declaration" a property of the code that writes the document rather than a consequence of a - * check somewhere upstream of it. + * Lists the declared capabilities as Gherkin tags in enum order, so envelopes of the same + * configuration are byte-identical. {@linkplain Capability#reserved() Reserved} capabilities are + * skipped. */ private static List declaredTags(Set declared) { List tags = new ArrayList<>(declared.size()); @@ -183,24 +149,13 @@ private static List declaredTags(Set declared) { return Collections.unmodifiableList(tags); } - /** - * Assembles the {@code backend} block, which is always emitted. - * - *

The schema requires it at the top level, and {@code backendType} within it, so there is no - * conditional emission here: a control that does not say how it drove the backend is - * {@code custom}. - */ + /** Assembles the {@code backend} block, which the schema requires. */ private static ConformanceReport.Backend backendOf(TckRunMetadata run) { return new ConformanceReport.Backend(run.backendDescription().orElse(null), run.backendType()); } /** - * Reads the Messages release out of the stream's own {@code meta} message. - * - *

Taken from the stream rather than from a constant or the {@code io.cucumber:messages} - * artifact version, because the envelope and the stream disagreeing about which release produced - * them would be worse than either being absent. {@code meta} is the first envelope cucumber - * writes, so only the first line is parsed. + * Reads the Messages release out of the stream's {@code meta} message, which is the first line. * * @param results the buffered results stream * @return the protocol version, or {@code null} when the stream carries none @@ -216,8 +171,7 @@ private static String protocolVersionOf(byte[] results) { JsonNode version = new ObjectMapper().readTree(first).path("meta").path("protocolVersion"); return version.isTextual() ? version.asText() : null; } catch (JsonProcessingException e) { - // A stream whose first line will not parse is a bug worth surfacing, but not here: - // omitting an optional field is better than failing a run that otherwise succeeded. + // Optional field: omit it rather than fail a run that otherwise succeeded. log.warn("tck: could not read the Messages protocol version from the stream", e); return null; } @@ -230,9 +184,7 @@ private static String protocolVersionOf(byte[] results) { private void write(String dir, byte[] results) { Optional observed = metadata.get(); if (!observed.isPresent()) { - // Nothing observed the suite, which means it never got as far as starting the runtime. - // The run has failed for some other reason by now; adding a report with an invented - // provider name on top of that would only mislead. + // The suite never started its runtime, so the run has already failed for another reason. log.error( "{} is set but no TCK suite ran, so there is nothing to report on. " + "This normally means the suite failed before its backend stack started.", @@ -265,9 +217,8 @@ private void write(String dir, byte[] results) { throw new IllegalStateException("tck [" + configuration + "]: could not encode the conformance report", e); } - // A failure to write is raised rather than logged and swallowed. CI that asked for a report - // and silently did not get one is how a publishing pipeline serves a stale result forever. - // The results go first: an envelope naming a stream that is not there is worse than neither. + // Raised, not logged: CI that asked for a report must not silently go without one. Results go + // first, so an envelope never names a stream that is not there. try { Files.createDirectories(directory); Files.write(resultsPath, results); @@ -286,7 +237,6 @@ private static String digestOf(byte[] results) { try { sha256 = MessageDigest.getInstance("SHA-256"); } catch (NoSuchAlgorithmException e) { - // SHA-256 is required of every Java platform, so this cannot happen on a working JVM. throw new IllegalStateException("SHA-256 is not available", e); } byte[] digest = sha256.digest(results); diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java index b2b275ba61..1a87dd00fb 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/ReportNames.java @@ -35,15 +35,9 @@ static String configurationOf(Class suite) { } /** - * Turns a configuration name into the base name both of a run's files share. - * - *

A run writes an envelope and a results stream, and they are matched by name — the envelope's - * {@code results.location} is this base name plus the stream's extension — so one derivation - * produces both. - * - *

Configuration names are chosen to read well in a failure message rather than to be - * path-safe, so anything that is not obviously safe becomes a hyphen. Without this a - * configuration named {@code flagd/rpc} would silently write outside the directory it was given. + * Turns a configuration name into the base name the envelope and the results stream share. + * Anything not obviously path-safe becomes a hyphen, so a configuration named {@code flagd/rpc} + * cannot write outside the report directory. * * @param configuration the configuration name * @return a file name stem, with no extension diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java index 7346db6192..4435ac5117 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckBuildInfo.java @@ -8,17 +8,12 @@ import org.slf4j.LoggerFactory; /** - * What this TCK build is, and which conformance artifacts it carries. + * What this TCK build is, and which conformance artifacts it carries. The spec revision is baked into + * a generated resource at build time; the SDK version is read at runtime, because the TCK depends on + * a version range. * - *

A conformance report has to say which questions were asked, not only what the answers were, and - * neither fact is available from the running code. The spec revision the packaged Gherkin came from - * is a property of the build — the artifacts are in the JAR, the repository they came from is not — - * so it is baked into a generated resource at build time and read back here. The SDK version is the - * opposite case: the TCK depends on a version range, so what a consumer actually ran - * against is only knowable at runtime. - * - *

Everything here degrades to {@code unknown} rather than throwing. A report that is slightly - * less identifiable is worth more than a test run that fails while tidying up after itself. + *

Everything degrades to {@code unknown} rather than throwing, so a missing detail cannot fail a + * test run. */ final class TckBuildInfo { @@ -33,10 +28,10 @@ final class TckBuildInfo { private static final Logger log = LoggerFactory.getLogger(TckBuildInfo.class); - /** Generated by the build from the module POM; see {@code src/main/resources-filtered}. */ + /** Generated by the build from the module POM. */ private static final String BUILD_PROPERTIES = "tck-build.properties"; - /** Written into every JAR by maven-archiver, and the most direct statement of what resolved. */ + /** Written into every JAR by maven-archiver. */ private static final String SDK_POM_PROPERTIES = "META-INF/maven/dev.openfeature/sdk/pom.properties"; private static final Properties BUILD = loadBuildProperties(); @@ -53,13 +48,7 @@ static String specRevision() { return property("spec.revision"); } - /** - * Returns the OpenFeature SDK version that was actually on the classpath. - * - *

Read rather than declared. The TCK depends on an SDK version range so that adopting it can - * never force an upgrade, which means the version is a property of the consumer's build; a - * hardcoded one would be a second place to be wrong. - */ + /** Returns the OpenFeature SDK version that was actually on the classpath. */ static String sdkVersion() { Properties pom = load(SDK_POM_PROPERTIES); if (pom != null) { @@ -69,8 +58,7 @@ static String sdkVersion() { } } - // The Maven descriptor can be stripped, or the SDK can arrive from somewhere that is not a - // JAR at all — an IDE's output directory, say — so fall back to the manifest. + // The descriptor can be stripped, or the SDK may not come from a JAR at all. Package sdkPackage = OpenFeatureAPI.class.getPackage(); String implementation = sdkPackage == null ? null : sdkPackage.getImplementationVersion(); if (implementation != null && !implementation.trim().isEmpty()) { @@ -95,8 +83,7 @@ private static Properties loadBuildProperties() { if (properties == null) { log.warn( "{} is missing from the TCK JAR, so a conformance report cannot identify the build " - + "that produced it. This means the resource filtering configured in the " - + "tck POM did not run.", + + "that produced it; the resource filtering in the tck POM did not run.", BUILD_PROPERTIES); return new Properties(); } diff --git a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java index cbd3d06703..2cc5532aff 100644 --- a/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java +++ b/tools/tck/src/main/java/dev/openfeature/contrib/tools/tck/TckRunMetadata.java @@ -9,21 +9,13 @@ import java.util.concurrent.atomic.AtomicReference; /** - * What a conformance report needs to know about the run, gathered as the run proceeds. + * What a conformance report needs to know about the run, gathered as the run proceeds. The harness + * supplies the declaration and backend when the suite starts, a scenario supplies the provider's own + * name, and the report is written at {@code TestRunFinished}, after {@code @AfterAll} has torn the + * backend down; so the lifecycle records one of these and it outlives the suite. * - *

It exists because the two facts a report needs most are known at different times and are gone - * by the time it is written. The declared capabilities and the backend description come from the - * harness when the suite starts; the provider's own metadata name is only observable once a scenario - * has built a provider. Meanwhile Cucumber emits {@code TestRunFinished} — where the file is written - * — after {@code @AfterAll} has already torn the suite's backend down. - * - *

So the suite lifecycle records one of these when a suite starts and it outlives the suite, and - * the report plugin reads it at the end. Public only for that lifecycle, which lives in the internal - * steps package: its two static entry points are not for adopters. - * - *

It is also the one place every path to a report passes through, which makes it where the - * declaration is checked: a {@linkplain Capability#reserved() reserved} capability is rejected here - * rather than being carried into a document that would then claim it. + *

Public only for the lifecycle in the internal steps package. Every path to a report passes + * through the constructor, which rejects {@linkplain Capability#reserved() reserved} capabilities. */ public final class TckRunMetadata { @@ -87,9 +79,6 @@ public static void begin(ProviderTckHarness harness, BackendControl control) { /** * Records what the provider under test calls itself, against the suite that is running. * - *

A conformance report identifies the provider by its own metadata name rather than by the - * suite's, and only a scenario that has built one can say what that is. - * * @param name the provider's metadata name */ public static void observeProviderName(String name) { @@ -100,8 +89,7 @@ public static void observeProviderName(String name) { } /** - * Replaces the recorded run. Also the seam the report tests use to describe a run without a - * backend. + * Replaces the recorded run; also the seam the report tests use. * * @param metadata what the run is, or {@code null} to forget the last one */ @@ -139,11 +127,8 @@ void recordProviderName(String name) { } /** - * Returns the provider's own metadata name, falling back to the configuration name. - * - *

The fallback covers a run in which no scenario ever built a provider — every scenario - * skipped, or the stack never came up. Reporting the suite name there is more useful than the - * empty string the schema would reject. + * Returns the provider's own metadata name, falling back to the configuration name when no + * scenario ever built a provider. * * @return the name to report as the provider's identity */ diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java index d95bb943b7..6482b85e51 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/CanonicalScenarioGuardTest.java @@ -19,17 +19,10 @@ import org.junit.platform.launcher.listeners.TestExecutionSummary; /** - * A run that quietly asks fewer questions must not be able to report success. - * - *

Two levels here, deliberately. The comparison itself is checked directly, because a failure - * message is the whole product of a guard and it has to name the scenario that went missing. The - * wiring is checked by running the guard through the JUnit Platform under a real filter - * configuration, because "the guard sees what the Cucumber engine beside it sees" is a claim about - * the platform rather than about this code. - * - *

No scenario is executed anywhere in this file. The guard works from the discovered plan and the - * run's configuration, which is what lets it be checked — and, in a real build, fail — without a - * backend. + * A run that quietly asks fewer questions must not be able to report success. The comparison is + * checked directly, since its failure message must name the missing scenario; the wiring is checked by + * running the guard through the JUnit Platform under a real filter configuration. No scenario + * executes, so no backend is needed. */ class CanonicalScenarioGuardTest { @@ -50,10 +43,7 @@ void theCanonicalSetIsReadFromTheArtifact() { .as("every canonical scenario comes from a packaged feature file") .allSatisfy(ref -> assertThat(ref.toString()).startsWith(ProviderTck.FEATURES + "/")); - // The eight rows of the type-mismatch matrix share one name, so a set that counted scenarios - // by name would see one of them. Each Examples row is its own line and its own entry. - // Eleven until specification revision d47a66eb moved the three "requested as a String" rows - // onto @string-typing, where a backend that stores every value as a string can withhold them. + // The rows of the type-mismatch matrix share one name, so each Examples row must be its own entry. assertThat(canonical) .filteredOn(ref -> ref.toString().contains("Requesting the wrong type returns the code default")) .hasSize(8); @@ -77,8 +67,7 @@ void extensionScenariosAreIgnored() { Set observed = CanonicalScenarioGuard.discovered().get(TckSuiteFixture.class.getName()); - // The fixture suite does select an extension feature — ExtensionPointTest checks that it does - // — and none of it reaches the guard. + // The fixture suite selects an extension feature, and none of it reaches the guard. assertThat(observed) .as("the guard is defined over %s/ alone", ProviderTck.FEATURES) .isEqualTo(CanonicalScenarios.shipped()) @@ -95,9 +84,8 @@ void aScenarioFilterFails() { CanonicalScenarioGuard.discovered(), Collections.singletonMap(Constants.FILTER_TAGS_PROPERTY_NAME, "not @events")); - // Cucumber applies a tag filter by skipping scenarios during execution, so the plan still - // contains them and only the configured expression shows what will be left out. The guard - // therefore rejects the filter itself rather than trying to predict what it matches. + // Cucumber applies a tag filter by skipping at execution, so the plan still contains the + // filtered scenarios and the guard rejects the filter itself. assertThat(problems) .isNotNull() .contains(Constants.FILTER_TAGS_PROPERTY_NAME) @@ -138,8 +126,7 @@ void aPartialRunIsSkipped() { try { TestExecutionSummary summary = runGuard(Constants.FILTER_TAGS_PROPERTY_NAME, "@events"); - // Aborted rather than skipped: the test ran and declined to conclude, which is what an - // assumption produces and what Surefire and the JUnit reports show as skipped. + // Aborted rather than skipped: that is what an assumption produces, and reports show it as skipped. assertThat(summary.getTestsAbortedCount()) .as("a skip says the canonical set was not verified; a pass would claim it was") .isEqualTo(1); @@ -171,9 +158,7 @@ void anAddedCanonicalFeatureFails() { @Test @DisplayName("a canonical file replaced by another of the same name fails the run") void aShadowedCanonicalFileFails() { - // What the separate extension directory exists to prevent, expressed as what the guard sees - // if it happens anyway: the replacement compiles to different scenarios, so entries of the - // canonical set go missing. + // The replacement compiles to different scenarios, so entries of the canonical set go missing. Set shadowed = new LinkedHashSet<>(CanonicalScenarios.shipped()); Iterator entries = shadowed.iterator(); CanonicalScenarios.Ref removed = entries.next(); @@ -202,9 +187,7 @@ void anUnobservedRunFails() { @Test @DisplayName("the partial-run escape hatch carries no PROVIDER_ prefix either") void thePartialKnobIsSpelledWithoutThePrefix() { - // Literals on purpose: every TCK knob an adopter or a CI job sets is spelled TCK_*, and one - // of them keeping the old PROVIDER_TCK_* prefix is the half-rename that makes the set - // unguessable. + // Literals on purpose: a knob keeping the old PROVIDER_TCK_ prefix is a half-rename. assertThat(CanonicalScenarioGuard.PARTIAL_ENV).isEqualTo("TCK_PARTIAL"); assertThat(CanonicalScenarioGuard.PARTIAL_PROPERTY).isEqualTo("tck.partial"); } diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java index def029138f..938fcea85c 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/ConformanceReportPluginTest.java @@ -37,27 +37,16 @@ import org.junit.platform.launcher.core.LauncherFactory; /** - * The conformance report exists to make one rule checkable, so these tests check it. - * - *

Appendix F requires that a scenario skipped for an undeclared capability is reported as skipped - * with the reason and never as passed. The Go TCK shipped an emitter that broke the rule silently — - * its runner did not deliver the capability-skip signal to the after-hook, so every skipped scenario - * was recorded twice, once correctly and once as passed. - * - *

Since the results are now a Cucumber Messages stream rather than a format this project defines, - * the rule is a property of what Cucumber emits, and only a real Cucumber run can demonstrate it. - * These tests therefore execute a fixture suite through the JUnit Platform — same engine, same gate, - * same plugin — and read the stream back as a consumer would: outcome per scenario derived from the - * most severe step result, exactly as the {@code cucumber-query} helpers do it. - * - *

What is not asserted here is the serialisation of the stream. That is Cucumber's own - * {@code MessageFormatter}, and re-checking it would only be checking Cucumber. + * Checks the rule the conformance report exists for: a scenario skipped for an undeclared capability + * is reported as skipped with the reason, never as passed. That is a property of what Cucumber emits, + * so a fixture suite runs through the JUnit Platform with the real plugin and the stream is read back + * as a consumer would. Serialisation of the stream is Cucumber's own and is not asserted. */ class ConformanceReportPluginTest { /** - * The fixture's {@code @stale} tag is deliberately absent, so two scenarios must be skipped — and - * a third that no declaration could have covered, because {@code @large-integers} is + * {@code @stale} is deliberately undeclared, so two scenarios are skipped, plus a third that no + * declaration could cover because {@code @large-integers} is * {@linkplain Capability#inexpressible() inexpressible} in Java. */ private static final Set DECLARED = EnumSet.of(Capability.OBJECT, Capability.EVENTS); @@ -69,12 +58,8 @@ class ConformanceReportPluginTest { private static final String OUTLINE = "Requesting the wrong type returns the code default"; /** - * Severity order of {@code TestStepResultStatus}, least to most severe. - * - *

A {@code testCaseFinished} message carries no status: a scenario's outcome is the most - * severe result among its steps, hooks included. That is not incidental — it is the mechanism - * that makes a gated skip truthful, because the aborted {@code @Before} hook contributes a - * {@code SKIPPED} result that outranks every {@code PASSED} step it prevented from running. + * Severity order of {@code TestStepResultStatus}, least to most severe. A scenario's outcome is + * its most severe step result, hooks included, which is what makes a gated skip truthful. */ private static final List SEVERITY = Arrays.asList("UNKNOWN", "PASSED", "SKIPPED", "PENDING", "UNDEFINED", "AMBIGUOUS", "FAILED"); @@ -105,9 +90,7 @@ static void runTheFixtureSuite() throws IOException { @Test @DisplayName("every scenario appears exactly once, whatever happened to it") void everyScenarioAppearsExactlyOnce() { - // Eight pickles: five plain scenarios and three outline rows. A report that quietly omitted - // the ones it did not run would satisfy every other rule here and still mislead, because a - // reader would have no way to know how many questions went unasked. + // Eight pickles: five plain scenarios and three outline rows, none of them omitted. assertThat(results.pickles).hasSize(8); assertThat(results.pickleOfTestCase.values()) @@ -159,17 +142,12 @@ void aGatedScenarioIsSkipped() { @Test @DisplayName("the gate's own reason survives into the stream, and the two kinds of skip stay apart") void theGateReasonSurvives() { - // The declaration in the envelope is enough to work out *why* most scenarios were skipped, - // but the reason the gate gave is carried too, on the hook result that produced the skip. + // The reason the gate gave is carried on the hook result that produced the skip. List messages = results.messagesOfSkippedSteps(); assertThat(messages).hasSize(3); - // And for one of them the declaration is NOT enough, which is why this reason has to travel. - // @large-integers is absent from every Java declaration and absent for a reason that says - // nothing about the provider: the SDK's integer accessor is 32 bits. A reader who inferred - // "the provider declined" from the absence would be reading a decision into something no - // Java provider was ever offered. So the message names the SDK, and says it is not the - // provider declining, in words that could not be confused with the other two. + // @large-integers is absent from every Java declaration because the SDK's integer accessor + // is 32 bits, not because the provider declined, so the message must name the SDK. List unaskable = new ArrayList<>(); messages.forEach(message -> { if (message.contains("LARGE_INTEGERS")) { @@ -200,9 +178,7 @@ void tagsIncludeExamplesBlockTags() { .as("the feature tag reaches every row") .allSatisfy(row -> assertThat(results.tagsOf(row)).contains("@events")); - // Gherkin allows a tag on an individual Examples block, so two rows of one outline can - // differ in whether the gate stops them. Both rows must appear, and only the tagged one - // may be skipped: a skip that took its siblings with it would be invisible in the totals. + // Gherkin allows a tag on a single Examples block, so only the tagged row may be skipped. List outcomes = new ArrayList<>(); List stale = new ArrayList<>(); for (JsonNode row : rows) { @@ -223,10 +199,8 @@ void outlineRowsAreDistinguishable() { .extracting(row -> row.get("name").asText()) .containsOnly(OUTLINE); - // A pickle's astNodeIds are [scenario, table row] for an outline-derived scenario. The - // second is the row's identity, and it resolves in the gherkinDocument to the cells the row - // was compiled from. This is the mechanism the TCK used to reverse-engineer from a pickle's - // reported line number; the stream states it outright. + // A pickle's astNodeIds are [scenario, table row] for an outline row; the second resolves in + // the gherkinDocument to the cells the row was compiled from. List rowIds = new ArrayList<>(); List> cells = new ArrayList<>(); for (JsonNode row : rows) { @@ -250,9 +224,7 @@ void outlineRowsAreDistinguishable() { void theStreamCarriesTheExecutedSource() throws IOException { assertThat(results.sources).containsOnlyKeys(FEATURE_URI); - // Line endings are normalised on both sides: this repo is checked out with whatever the - // platform does, and the point of the assertion is that the text is the file's, not that - // Cucumber preserves CRLF. + // Line endings are normalised: the text must be the file's, whatever the checkout did to CRLF. byte[] onDisk = Files.readAllBytes(Paths.get("src/test/resources/report-selftest/report.feature")); assertThat(results.sources.get(FEATURE_URI).replace("\r\n", "\n")) .as("what ran, verbatim, rather than a claim about which revision it came from") @@ -285,13 +257,11 @@ void theEnvelopeCarriesWhatTheSchemaRequires() { assertThat(envelope.get("tck").get("specRevision").asText()) .as("and it is the revision the packaged assets actually came from; see the test below") .isEqualTo(CanonicalAssetDigestTest.PINNED_REVISION); - // backend is in the schema's top-level required array and backendType in backend's, so both - // are asserted as present rather than as present-if-set. + // Both are required by the schema, so asserted as present rather than present-if-set. assertThat(envelope.has("backend")).isTrue(); assertThat(envelope.get("backend").get("backendType").asText()).isEqualTo("http"); - // The schema sets additionalProperties: false throughout, so anything the results payload - // now owns is a validation failure rather than harmless duplication. + // The schema sets additionalProperties: false, so anything the results payload owns would fail. List fields = new ArrayList<>(); envelope.fieldNames().forEachRemaining(fields::add); assertThat(fields).doesNotContain("scenarios", "capabilities"); @@ -303,8 +273,7 @@ void theEnvelopeCarriesWhatTheSchemaRequires() { void theDeclarationListsWhatIsClaimed() { JsonNode declared = envelope.get("declaration").get("declared"); - // The declaration is an input to reading the results, not a summary of them: the stream says - // a scenario was skipped, and only this says whether the provider declines what it needed. + // Not derivable from the results: a skip does not say whether the provider declined the capability. assertThat(declared) .extracting(JsonNode::asText) .containsExactly(Capability.EVENTS.tag(), Capability.OBJECT.tag()); @@ -313,10 +282,8 @@ void theDeclarationListsWhatIsClaimed() { @Test @DisplayName("a reserved capability cannot reach the declaration, even when everything is claimed") void aReservedCapabilityCannotReachTheDeclaration() { - // The provider that claims the most is the case that used to break the rule: "everything" - // spelt EnumSet.allOf, or "everything except X" spelt EnumSet.complementOf, collected the - // reserved tags along the way and published a claim about capabilities no scenario - // examines. So this asks the maximal declaration for its report. + // EnumSet.allOf and complementOf used to collect the reserved tags and publish a claim no + // scenario examines, so this asks the maximal declaration for its report. JsonNode declared = MAPPER.valueToTree(report(Capability.declarable())) .get("declaration") .get("declared"); @@ -336,56 +303,24 @@ void aReservedCapabilityCannotReachTheDeclaration() { Capability.STALE.tag(), Capability.CONFIGURATION_CHANGE.tag(), Capability.OBJECT.tag(), - // @variants gates the one outline that asserts a variant. 2.2.4 is a SHOULD - // and types.md types the field optional, so a backend with no variant - // concept withholds it and those rows are skipped with that reason. Capability.VARIANTS.tag(), - // @disabled-flags gates the four-row outline that asks what a flag disabled - // in the management system resolves to. Gated because the answer follows - // from where the substitution happens rather than from provider quality: a - // provider that evaluates locally can return the caller's default, one whose - // backend decides never sent it. The maximal claim includes it. Capability.DISABLED_FLAGS.tag(), Capability.UNAVAILABLE_INIT.tag(), Capability.NUMERIC_COERCION.tag(), - // @string-typing gates the two outline rows that ask a boolean and an - // integer flag through the String accessor. Gated because every value has a - // string representation, so a backend that stores flag values as strings has - // no mismatch to report and withholds it without being non-conformant. Capability.STRING_TYPING.tag(), - // @fully-typed-values asks the same of a float and a structure, and is a - // separate claim because a store can record booleans and integers natively - // while keeping those two as text. Both scenarios carry @string-typing as - // well, so the maximal claim has to include both tags to reach them. Capability.FULLY_TYPED_VALUES.tag(), - // @large-integers is absent, and for a different reason from @caching's. - // Scenarios do carry it -- Go and JavaScript run them -- but the Java SDK's - // integer accessor is 32 bits, so no Java provider can be asked for 2^53 - 1 - // and none may claim it. The maximal claim is the maximal claim a provider - // written against THIS SDK can make, which is what a report has to carry. - // @targeting was reserved until targeting-key-flag's three scenarios - // arrived. It gates something now, so the maximal claim includes it. + // @large-integers is absent: the Java SDK's integer accessor is 32 bits, so no Java + // provider may claim it. Capability.TARGETING.tag(), - // @standard-reasons gates reason.feature in its entirety. It is a claim - // rather than an exemption -- 2.2.5 lets a provider report "some other - // string", so a provider that does not declare it is not thereby deficient - // -- but it is an ordinary declarable capability all the same, and the - // maximal claim includes it. Capability.STANDARD_REASONS.tag()); } @Test @DisplayName("the revision a report publishes is the one the packaged assets came from") void theReportedRevisionIsTheRevisionTheAssetsCameFrom() { - // A report's whole claim to be readable later is that it says which questions were asked. - // The revision it names is a POM property, maintained by hand beside a submodule gitlink - // that moves by a different command -- so until now "keep them in step" was a comment in - // the POM, and a re-pin that updated one and not the other would have published a revision - // that did not produce the scenarios in the same file. - // - // CanonicalAssetDigestTest has already established that the packaged assets are - // PINNED_REVISION's, by digest. This is the last link: what the build filtered into the JAR - // and TckBuildInfo reads back is that same revision. + // The revision a report publishes is a POM property kept by hand beside the submodule gitlink. + // This ties what the build filtered into the JAR to PINNED_REVISION, which + // CanonicalAssetDigestTest has already tied to the packaged assets. assertThat(TckBuildInfo.specRevision()) .as("tck.spec.revision in tools/tck/pom.xml must equal " + "CanonicalAssetDigestTest.PINNED_REVISION -- a re-pin moves the submodule " @@ -399,9 +334,7 @@ void theReportedRevisionIsTheRevisionTheAssetsCameFrom() { @Test @DisplayName("naming a reserved capability fails the run rather than being dropped quietly") void namingAReservedCapabilityFailsTheRun() { - // Chosen over a warning: the declaration is the one part of the report no result can check, - // and a report is read long after the log it would have been warned in has gone. Nothing is - // lost by refusing, because no scenario carries the tag. + // Refused rather than warned: the declaration is the one part of the report no result can check. Set overclaimed = EnumSet.of(Capability.OBJECT, Capability.CACHING); assertThatThrownBy(() -> metadata(overclaimed)) @@ -417,7 +350,6 @@ void declarableExceptYieldsOnlyDeclarableCapabilities() { .doesNotContain(Capability.STALE, Capability.CACHING) .contains(Capability.OBJECT, Capability.NUMERIC_COERCION, Capability.TARGETING); - // The counterpart it replaces, and why it had to be replaced. assertThat(EnumSet.complementOf(EnumSet.of(Capability.STALE))) .as("complementOf is the complement of the enum, not of the declarable vocabulary") .contains(Capability.CACHING); @@ -462,10 +394,8 @@ void nothingIsWrittenByDefault(@TempDir Path dir) throws IOException { @Test @DisplayName("the report-directory knob is spelled the way the other three languages spell it") void theReportDirectoryKnobIsSpelledPortably() { - // Asserted as literals rather than through the constants, because the point is the name a - // cross-language CI job sets, not that the code is self-consistent. Go, Python and - // JavaScript read TCK_REPORT_DIR; one job exporting one variable has to drive all four, so - // a rename here is a breaking change to something no compiler checks. + // Literals on purpose: a cross-language CI job exports one variable to drive all four TCKs, + // so a rename is a breaking change no compiler checks. assertThat(ConformanceReportPlugin.REPORT_DIR_ENV).isEqualTo("TCK_REPORT_DIR"); assertThat(ConformanceReportPlugin.REPORT_DIR_PROPERTY).isEqualTo("tck.report.dir"); } @@ -484,13 +414,7 @@ void theFilesAreNamedAfterTheConfiguration() { /** A suite whose name the default configuration derivation has to cope with. */ private static final class MyProviderRpcTckTest {} - /** - * Runs the fixture feature through the real Cucumber engine, with the real plugin registered. - * - *

Driven through the JUnit Platform rather than through Cucumber's CLI because that is how the - * TCK itself runs: {@link ProviderTckTest} is a JUnit Platform Suite, and the plugin's - * position in the event stream is a property of that engine. - */ + /** Runs the fixture feature through the real Cucumber engine with the real plugin registered. */ private static void runSuite(Path dir, Set declared) { ReportSelfTestSteps.declare(declared); TckRunMetadata.record(metadata(declared)); @@ -532,11 +456,8 @@ private static TckRunMetadata metadata(Set declared) { declared, "a test double", BackendType.HTTP, - // Withheld-and-skipped, which is the shape a fixture may legitimately depict: this - // provider cannot attempt the behaviour, because it never subscribes to the thing - // that would tell it. A narrowing-coercion deviation used to stand here, and that - // is the shape the guidance discourages -- a provider that attempts a behaviour and - // gets it wrong declares the capability and lets the scenario fail. + // Withheld-and-skipped: the shape a fixture may depict, as opposed to a deviation, which is + // for a provider that attempts a behaviour and gets it wrong. Collections.singletonList(KnownDeviation.untracked( Capability.CONFIGURATION_CHANGE, "the fixture provider swallows its child's change events and can never " + "report one"))); @@ -588,11 +509,8 @@ public void removeHandlerFor(Class eventType, EventHandler handler) { } /** - * A Cucumber Messages stream, read the way a consumer reads one. - * - *

Deliberately built from the raw ndjson with nothing but Jackson. Using Cucumber's own query - * helpers would make these tests agree with Cucumber by construction; the point is to show that - * the facts the report needs are recoverable from the bytes on disk. + * A Cucumber Messages stream read from the raw ndjson with Jackson alone, so the tests do not + * agree with Cucumber's query helpers by construction. */ private static final class Results { diff --git a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java index 43f26c7d60..7eb8e22e94 100644 --- a/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java +++ b/tools/tck/src/test/java/dev/openfeature/contrib/tools/tck/selftest/ReportSelfTestSteps.java @@ -8,14 +8,9 @@ import java.util.Set; /** - * Glue for {@code report-selftest/report.feature}, the fixture the conformance report tests run. - * - *

It stands in for a provider, and for nothing else: there is no Compose stack, no control API - * and no provider here, because none of that is what the report tests are about. What it does share - * with the real suite is the part that matters — the capability gate is - * {@link Capability#requireDeclared}, the same call the real - * {@code ProviderSteps.gateOnCapabilities} makes from the same kind of {@code @Before(order = 0)} - * hook. A fixture that aborted by some other route would prove only that some abort becomes a skip. + * Glue for {@code report-selftest/report.feature}. It stands in for a provider with no backend, but + * gates on {@link Capability#requireDeclared} exactly as {@code ProviderSteps.gateOnCapabilities} + * does, so an abort here becomes a skip by the same route. */ public class ReportSelfTestSteps { @@ -46,23 +41,15 @@ public void aStepThatPasses() { // A passing scenario needs a step that passes and nothing more. } - /** - * A step that does nothing, unsuccessfully. - * - *

An {@link AssertionError} rather than a checked failure, because that is what a failing - * assertion in a step definition throws, and the point is what the results stream makes of it. - */ + /** A step that fails with an {@link AssertionError}, as a failing assertion in a step definition does. */ @Given("a step that fails") public void aStepThatFails() { throw new AssertionError("this fixture scenario is expected to fail"); } /** - * A passing step that takes its arguments from an Examples row. - * - *

The parameters are unused. They exist so the outline's rows differ in their compiled step - * text as well as in their table row, which is how a consumer that reads only the steps can - * still tell the rows apart. + * A passing step taking its arguments from an Examples row. The parameters are unused; they make + * the outline's rows differ in compiled step text as well as in table row. * * @param key the flag key from the row * @param requested the requested type from the row diff --git a/tools/tck/src/test/resources/report-selftest/report.feature b/tools/tck/src/test/resources/report-selftest/report.feature index 99d5caaa0d..587d129546 100644 --- a/tools/tck/src/test/resources/report-selftest/report.feature +++ b/tools/tck/src/test/resources/report-selftest/report.feature @@ -1,15 +1,8 @@ -# Fixture for ConformanceReportPluginTest, and not part of the conformance suite: it lives under a -# separate classpath root so that nothing selecting "gherkin" can pick it up. -# -# It is shaped like the real suite rather than minimal, because the properties the report has to -# preserve are properties of that shape: a capability tag on the feature, one on a scenario, one on -# a single Examples block, and an outline whose rows all share a name. Running real Cucumber over -# this is the only way to check what a capability-gated abort actually becomes in the results. -# -# It also carries both kinds of skip, because they are the two a reader has to be able to tell -# apart: a capability this provider did not declare, and a capability no provider in this language -# can be asked. Both end as SKIPPED, and only the reason carried on the hook result distinguishes -# them. +# Fixture for ConformanceReportPluginTest, not part of the conformance suite: it lives under a +# separate classpath root so nothing selecting "gherkin" picks it up. Shaped like the real suite +# (capability tags on the feature, a scenario and one Examples block; an outline whose rows share a +# name), and it carries both kinds of skip: an undeclared capability, and one no provider in this +# language can be asked. @events Feature: Report self-test