diff --git a/docs/architecture.md b/docs/architecture.md index cf7184b..58a1aed 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -151,6 +151,14 @@ in the vendored catalogue, never from a hardcoded column in the check itself. Th one code base validate against several specification versions at once: relaxing a `MUST` to a `SHOULD` upstream turns a failure into a warning here with no code change. +One severity does not come from the catalogue, because it is not about the document at all. A +check that raises something it does not expect reports `fault`: a defect in this package, not a +finding. It fails the run like `fail` does, but says the check produced no verdict, so the +document is neither condemned nor cleared. A fault keeps the id of the check that raised it +rather than reporting under one of its own, so "which part of the validator broke" is answered by +the same identifier that names what it was trying to establish, and the run still reports every +other target - one broken check costs one check. + A rule absent from the selected version's catalogue, or marked deprecated there, is skipped with a message saying so, rather than checked anyway - older meta-schema versions ship no catalogue at all and skip the whole `rule.*` family. A false positive costs far more than a missed finding, diff --git a/src/oold/validation/cli.py b/src/oold/validation/cli.py index 8f886ef..369f23e 100644 --- a/src/oold/validation/cli.py +++ b/src/oold/validation/cli.py @@ -21,7 +21,7 @@ from .meta_store import MetaSchemaError, describe_store, fetch_remote, load_index, resolve_selection from .meta_vendor import vendor_version from .pipeline import Options, run_compliance, validate_directory, validate_instance, validate_schema -from .report import FAIL, OK, SKIP, WARN, Report +from .report import FAIL, FAULT, OK, SKIP, WARN, Report EXIT_OK = 0 EXIT_FAILED = 1 @@ -86,6 +86,9 @@ def _classify(target: Path) -> str: FAIL: {"fg": "red", "bold": True}, WARN: {"fg": "yellow"}, SKIP: {"fg": "cyan"}, + # Brighter than FAIL on purpose: a fault is our bug, and the reader should not spend time + # looking for it in their own document. + FAULT: {"fg": "magenta", "bold": True}, } _meta_option = click.option( @@ -136,9 +139,12 @@ def _print_human(report: Report, verbose: bool) -> None: versions = ", ".join(report.meta_versions) or "none" click.echo(f"{status} {report.source}") click.echo(f" meta-schema: {versions}") + # Faults appear only when there are some. A permanent "0 fault(s)" would train the reader to + # skip the field, which is the opposite of what it is for. + faults = f", {counts[FAULT]} VALIDATOR FAULT(S)" if counts[FAULT] else "" click.echo( f" {counts[OK]} ok, {counts[FAIL]} failed, {counts[WARN]} warning(s), " - f"{counts[SKIP]} skipped, across {len(report.targets())} target(s)" + f"{counts[SKIP]} skipped{faults}, across {len(report.targets())} target(s)" ) shown = report.checks if verbose else [c for c in report.checks if c.status != OK] @@ -146,7 +152,7 @@ def _print_human(report: Report, verbose: bool) -> None: click.echo() for check in shown: style = _STATUS_STYLE.get(check.status, {}) - label = click.style(check.status.upper().ljust(4), **style) + label = click.style(check.status.upper().ljust(5), **style) rule = click.style(f" {check.rule}", fg="blue") if check.rule else "" version = f" [{check.meta_version}]" if check.meta_version else "" message = f": {check.message}" if check.message else "" diff --git a/src/oold/validation/mcp_server.py b/src/oold/validation/mcp_server.py index dbd2ec8..d29e7b7 100644 --- a/src/oold/validation/mcp_server.py +++ b/src/oold/validation/mcp_server.py @@ -60,7 +60,11 @@ class CheckResult(BaseModel): id: str = Field(description="The check id, e.g. lint.container or rule.id-fragment.") target: str = Field(description="What was checked: a schema file, an instance, or a directory entry.") - status: str = Field(description="ok, fail, warn, or skip.") + status: str = Field( + description="ok, fail, warn, skip, or fault. A fault is a defect in the validator: the " + "check raised something it does not expect, so it produced no verdict and the document " + "is neither condemned nor cleared. Like fail it makes the run not pass." + ) message: str = Field(default="", description="Why the check produced this status, when it is not ok.") detail: dict[str, Any] | None = Field( default=None, description="Extra structured detail behind the message. Only present with verbosity='full'." diff --git a/src/oold/validation/pipeline.py b/src/oold/validation/pipeline.py index 68d632f..02309ed 100644 --- a/src/oold/validation/pipeline.py +++ b/src/oold/validation/pipeline.py @@ -13,6 +13,9 @@ from __future__ import annotations import json +import traceback +from collections.abc import Iterator +from contextlib import contextmanager from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -32,7 +35,7 @@ from .meta_store import MetaBundle, MetaSchemaError, Rule, resolve_selection from .pattern_lint import lint from .predicates import check_predicates -from .report import FAIL, OK, SKIP, WARN, Report +from .report import FAIL, FAULT, OK, SKIP, WARN, Report from .resolve import Resolver, SchemaResolutionError, bound_schema from .roundtrip import roundtrip from .schema_checks import check_usable_as_validator, validate_against_meta @@ -171,6 +174,32 @@ def _read(path: Path) -> Any: # ---------------------------------------------------------------------------- schema checks +@contextmanager +def _guard(run: _Run, check_id: str, target: str) -> Iterator[None]: + """Record an unexpected exception as a fault of ``check_id`` rather than a finding. + + A check raising something it does not expect is a defect in this package, and the two ways + of handling it without this are both wrong: reporting FAIL names the user's document for a + fault of ours, and letting it propagate discards the verdicts already computed for every + other target in the run. + + The guard is deliberately broad where the code it wraps is not. Narrowing a call site (#127) + is what turns a swallowed exception into a propagating one, and this is where the propagating + one is allowed to land. The traceback goes in ``detail`` because a fault is a bug report + against this package and the message alone will not locate it. + """ + try: + yield + except Exception as exc: + run.add( + check_id, + target, + FAULT, + f"{type(exc).__name__}: {exc}", + {"traceback": traceback.format_exc()}, + ) + + def _check_schema(run: _Run, name: str) -> None: """Every check that applies to one schema file.""" try: @@ -179,29 +208,35 @@ def _check_schema(run: _Run, name: str) -> None: return # -- meta-schema well-formedness (per version) --------------------------------- - for bundle in run.bundles: - result = validate_against_meta(raw, bundle) - problems = list(result.errors) + check_usable_as_validator(raw) - if problems: - extra = f" (+{result.truncated} more)" if result.truncated else "" - run.add( - "schema.meta", - name, - FAIL, - problems[0] + extra, - {"errors": problems}, - bundle.version, - ) - else: - run.add("schema.meta", name, OK, meta_version=bundle.version) + with _guard(run, "schema.meta", name): + for bundle in run.bundles: + result = validate_against_meta(raw, bundle) + problems = list(result.errors) + check_usable_as_validator(raw) + if problems: + extra = f" (+{result.truncated} more)" if result.truncated else "" + run.add( + "schema.meta", + name, + FAIL, + problems[0] + extra, + {"errors": problems}, + bundle.version, + ) + else: + run.add("schema.meta", name, OK, meta_version=bundle.version) # -- $ref composition (version independent) ------------------------------------ + # Not guarded as a section: everything downstream needs `deref`, so a fault here has to stop + # this schema rather than let the later checks raise on a name that was never bound. try: resolved = run.resolver.load(run.directory / name) deref = run.resolver.dereference(resolved) except SchemaResolutionError as exc: run.add("schema.refs", name, FAIL, str(exc)) return + except Exception as exc: + run.add("schema.refs", name, FAULT, f"{type(exc).__name__}: {exc}", {"traceback": traceback.format_exc()}) + return if deref.unresolved: run.add("schema.refs", name, FAIL, deref.unresolved[0], {"unresolved": deref.unresolved}) @@ -210,77 +245,91 @@ def _check_schema(run: _Run, name: str) -> None: # -- pattern lint -------------------------------------------------------------- first = None - for bundle in run.bundles: - result = lint(raw, bundle) - first = first or result - gate = run.catalog_gate("lint.pattern", bundle) - if gate is not None: - run.add("lint.pattern", name, gate.status, gate.message, meta_version=bundle.version) - elif result.schema_errors: - run.add( - "lint.pattern", - name, - FAIL, - result.schema_errors[0], - {"errors": result.schema_errors}, - bundle.version, - ) - else: - run.add("lint.pattern", name, OK, meta_version=bundle.version) + with _guard(run, "lint.pattern", name): + for bundle in run.bundles: + result = lint(raw, bundle) + first = first or result + gate = run.catalog_gate("lint.pattern", bundle) + if gate is not None: + run.add("lint.pattern", name, gate.status, gate.message, meta_version=bundle.version) + elif result.schema_errors: + run.add( + "lint.pattern", + name, + FAIL, + result.schema_errors[0], + {"errors": result.schema_errors}, + bundle.version, + ) + else: + run.add("lint.pattern", name, OK, meta_version=bundle.version) if first is not None: # These two correlate `properties` with `@context`, so no meta-schema version can # express them and they are reported once rather than per version, gated against the # first selected bundle - the same one `first` was computed from. - gate = run.catalog_gate("lint.container", run.bundles[0]) - if gate is not None: - run.add("lint.container", name, gate.status, gate.message) - elif first.missing_container: - joined = ", ".join(first.missing_container) - plural = "ies" if len(first.missing_container) > 1 else "y" - run.add( - "lint.container", - name, - FAIL, - f"strict array propert{plural} without @container @set/@list: {joined}", - {"properties": first.missing_container}, - ) - else: - run.add("lint.container", name, OK) - - gate = run.catalog_gate("lint.iri-format", run.bundles[0]) - if gate is not None: - run.add("lint.iri-format", name, gate.status, gate.message) - elif first.missing_iri_format: - joined = ", ".join(first.missing_iri_format) - plural = "ies" if len(first.missing_iri_format) > 1 else "y" - run.add( - "lint.iri-format", - name, - WARN, - f"IRI reference propert{plural} without an iri-reference/uri* format: {joined}", - {"properties": first.missing_iri_format}, - ) + with _guard(run, "lint.container", name): + gate = run.catalog_gate("lint.container", run.bundles[0]) + if gate is not None: + run.add("lint.container", name, gate.status, gate.message) + elif first.missing_container: + joined = ", ".join(first.missing_container) + plural = "ies" if len(first.missing_container) > 1 else "y" + run.add( + "lint.container", + name, + FAIL, + f"strict array propert{plural} without @container @set/@list: {joined}", + {"properties": first.missing_container}, + ) + else: + run.add("lint.container", name, OK) + + with _guard(run, "lint.iri-format", name): + gate = run.catalog_gate("lint.iri-format", run.bundles[0]) + if gate is not None: + run.add("lint.iri-format", name, gate.status, gate.message) + elif first.missing_iri_format: + joined = ", ".join(first.missing_iri_format) + plural = "ies" if len(first.missing_iri_format) > 1 else "y" + run.add( + "lint.iri-format", + name, + WARN, + f"IRI reference propert{plural} without an iri-reference/uri* format: {joined}", + {"properties": first.missing_iri_format}, + ) _check_schema_jsonld(run, name, raw) def _check_schema_jsonld(run: _Run, name: str, raw: dict[str, Any]) -> None: """Generation, round-trip, remote-context and attribution for one schema.""" + from jsonschema import Draft202012Validator from pyld import jsonld - schema = run.bounded(name) - # -- satisfiability ------------------------------------------------------------ - generated = generate(schema) - if not generated.ok: - run.add("generate.satisfiable", name, FAIL, generated.error or "generation failed") + # An early return on fault rather than a guarded section: every check below needs `schema`, + # `generated` and `validator`, so carrying on would raise on names that were never bound and + # report one defect once per section that tripped over it. + try: + schema = run.bounded(name) + generated = generate(schema) + if not generated.ok: + run.add("generate.satisfiable", name, FAIL, generated.error or "generation failed") + return + validator = Draft202012Validator(schema, format_checker=OOLD_FORMAT_CHECKER) + errors = sorted(validator.iter_errors(generated.instance), key=lambda e: list(e.absolute_path)) + except Exception as exc: + run.add( + "generate.satisfiable", + name, + FAULT, + f"{type(exc).__name__}: {exc}", + {"traceback": traceback.format_exc()}, + ) return - from jsonschema import Draft202012Validator - - validator = Draft202012Validator(schema, format_checker=OOLD_FORMAT_CHECKER) - errors = sorted(validator.iter_errors(generated.instance), key=lambda e: list(e.absolute_path)) if errors: run.add( "generate.satisfiable", @@ -297,78 +346,85 @@ def _check_schema_jsonld(run: _Run, name: str, raw: dict[str, Any]) -> None: promoted = promoted_terms(raw) # -- generated-instance round-trip --------------------------------------------- - if cyclic: - run.add("roundtrip.generated", name, SKIP, CYCLIC_NOTE) - else: - result = roundtrip(schema, generated.instance, context_url, run.loader, promoted=promoted) - # A property with no @context term does not reach RDF, so it cannot come back. That is - # permitted (OOLD-SCH-2d05) and is context.coverage's finding, not a round-trip defect. - # Reporting it here as well would fail the schema for something the specification allows, - # under a check that cites no rule. What is left is a genuine loss: a property that was - # mapped and still did not survive. - unmapped = _unmapped_properties(run, name, raw, schema, generated.instance) - lost = [key for key in result.lost if key.split("[")[0].split(".")[0] not in unmapped] - if result.error: - run.add("roundtrip.generated", name, FAIL, result.error) - elif lost: - joined = ", ".join(lost) - plural = "ies" if len(lost) > 1 else "y" - run.add( - "roundtrip.generated", - name, - FAIL, - f"propert{plural} lost through RDF despite being mapped: {joined}", - {"lost": lost, "unmapped": sorted(unmapped)}, - ) + with _guard(run, "roundtrip.generated", name): + if cyclic: + run.add("roundtrip.generated", name, SKIP, CYCLIC_NOTE) else: - re_errors = sorted(validator.iter_errors(result.restored), key=lambda e: list(e.absolute_path)) - if re_errors: + result = roundtrip(schema, generated.instance, context_url, run.loader, promoted=promoted) + # A property with no @context term does not reach RDF, so it cannot come back. That is + # permitted (OOLD-SCH-2d05) and is context.coverage's finding, not a round-trip defect. + # Reporting it here as well would fail the schema for something the specification allows, + # under a check that cites no rule. What is left is a genuine loss: a property that was + # mapped and still did not survive. + unmapped = _unmapped_properties(run, name, raw, schema, generated.instance) + lost = [key for key in result.lost if key.split("[")[0].split(".")[0] not in unmapped] + if result.error: + run.add("roundtrip.generated", name, FAIL, result.error) + elif lost: + joined = ", ".join(lost) + plural = "ies" if len(lost) > 1 else "y" run.add( "roundtrip.generated", name, FAIL, - "reconstruction fails its schema (shape not preserved by @context?): " + re_errors[0].message, - {"restored": result.restored}, + f"propert{plural} lost through RDF despite being mapped: {joined}", + {"lost": lost, "unmapped": sorted(unmapped)}, ) else: - run.add( - "roundtrip.generated", - name, - OK, - "", - {"triples": result.triples, "method": result.method}, - ) + re_errors = sorted(validator.iter_errors(result.restored), key=lambda e: list(e.absolute_path)) + if re_errors: + run.add( + "roundtrip.generated", + name, + FAIL, + "reconstruction fails its schema (shape not preserved by @context?): " + re_errors[0].message, + {"restored": result.restored}, + ) + else: + run.add( + "roundtrip.generated", + name, + OK, + "", + {"triples": result.triples, "method": result.method}, + ) # -- schema usable as a remote context ----------------------------------------- - if cyclic: - run.add("context.remote", name, SKIP, CYCLIC_NOTE) - else: - try: - jsonld.expand( - {"@context": context_url, "@id": "https://example.org/dummy"}, - run.loader.options(base=run.loader.base_url), - ) - run.add("context.remote", name, OK) - # Kept broad deliberately. This calls jsonld.expand on the same shared pyld.jsonld - # module predicates.py calls it through, so an unexpected exception here has the same - # nowhere-to-go problem as the one documented there: no attribution path of its own, and - # no per-file guard in validate_directory to land in if it propagates. Narrowing this - # waits on #145 for the same reason predicates.py's catch does. - except Exception as exc: - from .loader import describe_jsonld_error + with _guard(run, "context.remote", name): + if cyclic: + run.add("context.remote", name, SKIP, CYCLIC_NOTE) + else: + try: + jsonld.expand( + {"@context": context_url, "@id": "https://example.org/dummy"}, + run.loader.options(base=run.loader.base_url), + ) + run.add("context.remote", name, OK) + # Still broad, so nothing reaches the guard above yet. Narrowing it to JsonLdError is + # #127's remaining work; what changed is that there is now somewhere for the + # unexpected case to land, which is what that narrowing was waiting for. + except Exception as exc: + from .loader import describe_jsonld_error - run.add("context.remote", name, FAIL, describe_jsonld_error(exc)) + run.add("context.remote", name, FAIL, describe_jsonld_error(exc)) - _check_predicates(run, name, raw, schema, generated.instance) + # Attributed to `context.predicates` although the call also produces `context.coverage`: the + # guard has to name one id before knowing where the fault came from, and coverage is derived + # from the predicate attribution rather than computed independently. + with _guard(run, "context.predicates", name): + _check_predicates(run, name, raw, schema, generated.instance) # -- per-branch variant coverage ----------------------------------------------- if cyclic: return - variants, total = collect_variants(schema, limit=run.options.max_variants) - if total > len(variants): - run.report.notes.append(f"{name}: {total} oneOf/anyOf branches, checking the first {len(variants)}") - for variant in variants: - _check_variant(run, name, schema, variant, validator, context_url, promoted) + with _guard(run, "variants", name): + variants, total = collect_variants(schema, limit=run.options.max_variants) + if total > len(variants): + run.report.notes.append(f"{name}: {total} oneOf/anyOf branches, checking the first {len(variants)}") + for variant in variants: + # Per variant, so one broken branch does not cost the verdicts of the others. + with _guard(run, "variants", f"{name} {variant.label}"): + _check_variant(run, name, schema, variant, validator, context_url, promoted) def _check_variant( @@ -616,25 +672,26 @@ def _check_instance_file(run: _Run, name: str, instance: Any = None) -> None: run.add("roundtrip.instance", name, SKIP, f"its schema {CYCLIC_NOTE}") return - rt = roundtrip_instance(instance, schema, run.loader, run.loader.url_for(name), run.loader.url_for(schema_ref)) - if rt.error: - run.add("roundtrip.instance", name, FAIL, rt.error) - elif not rt.lossless: - run.add( - "roundtrip.instance", - name, - FAIL, - "instance != roundtrip (incomplete @context?)", - {"in": rt.original_canonical, "out": rt.restored_canonical}, - ) - else: - run.add( - "roundtrip.instance", - name, - OK, - f"{rt.triples} triples, lossless ({rt.method})", - {"triples": rt.triples, "method": rt.method}, - ) + with _guard(run, "roundtrip.instance", name): + rt = roundtrip_instance(instance, schema, run.loader, run.loader.url_for(name), run.loader.url_for(schema_ref)) + if rt.error: + run.add("roundtrip.instance", name, FAIL, rt.error) + elif not rt.lossless: + run.add( + "roundtrip.instance", + name, + FAIL, + "instance != roundtrip (incomplete @context?)", + {"in": rt.original_canonical, "out": rt.restored_canonical}, + ) + else: + run.add( + "roundtrip.instance", + name, + OK, + f"{rt.triples} triples, lossless ({rt.method})", + {"triples": rt.triples, "method": rt.method}, + ) # ---------------------------------------------------------------------------- entry points @@ -672,10 +729,16 @@ def validate_directory(path: str | Path, options: Options | None = None) -> Repo return run.report _collect(run, schema_names) + # The last-resort guard. The per-check ones above attribute a fault precisely; this one + # exists so that a fault raised between them - or by the guard machinery itself - still + # costs one file rather than the whole directory, which is what collect-everything-then- + # report means. for name in schema_names: - _check_schema(run, name) + with _guard(run, "schema.meta", name): + _check_schema(run, name) for name in instance_names: - _check_instance_file(run, name) + with _guard(run, "instance.schema", name): + _check_instance_file(run, name) return run.report diff --git a/src/oold/validation/report.py b/src/oold/validation/report.py index 407e120..db3c2d1 100644 --- a/src/oold/validation/report.py +++ b/src/oold/validation/report.py @@ -4,8 +4,15 @@ sync. A run is a flat list of :class:`Check` records; grouping (by target, by check id, by meta-schema version) is done at render time rather than baked into the structure. -Only ``fail`` is fatal to the verdict. ``warn`` marks a SHOULD-level finding, ``skip`` marks a -check that could not run for a documented reason (a cyclic scoped ``@context``, for instance). +``fail`` and ``fault`` are both fatal to the verdict, and they say different things. ``fail`` is a +finding about the document under test. ``fault`` is a defect in this package: the check raised +something it does not expect, so it produced no verdict at all and the document is neither +condemned nor cleared. ``warn`` marks a SHOULD-level finding, ``skip`` marks a check that could +not run for a documented reason (a cyclic scoped ``@context``, for instance). + +A fault keeps the id of the check that raised it rather than reporting under one of its own, so +"which part of the validator broke" is answered by the same identifier that names what it was +trying to establish. """ from __future__ import annotations @@ -14,13 +21,14 @@ from dataclasses import dataclass, field from typing import Any, Literal -Status = Literal["ok", "fail", "warn", "skip"] +Status = Literal["ok", "fail", "warn", "skip", "fault"] Verbosity = Literal["summary", "full"] OK: Status = "ok" FAIL: Status = "fail" WARN: Status = "warn" SKIP: Status = "skip" +FAULT: Status = "fault" @dataclass @@ -52,6 +60,13 @@ class Check: def failed(self) -> bool: return self.status == FAIL + @property + def is_fault(self) -> bool: + """The check broke rather than the document. Kept separate from :attr:`failed` so that + "how many findings" and "how many of our own defects" are never the same number. + """ + return self.status == FAULT + def to_dict(self, verbosity: Verbosity = "summary") -> dict[str, Any]: payload: dict[str, Any] = { "id": self.id, @@ -68,16 +83,6 @@ def to_dict(self, verbosity: Verbosity = "summary") -> dict[str, Any]: payload["detail"] = self.detail return payload - def line(self) -> str: - """A single-line rendering: status, rule, check id, target, version and message, each in - a fixed-width column. - """ - label = self.status.upper().ljust(4) - rule = f" {self.rule}" if self.rule else "" - version = f" [{self.meta_version}]" if self.meta_version else "" - message = f": {self.message}" if self.message else "" - return f"{label}{rule} {self.id:<24} {self.target}{version}{message}" - @dataclass class Report: @@ -120,12 +125,14 @@ def extend(self, checks: list[Check]) -> None: @property def passed(self) -> bool: - return self.fatal_error is None and not any(c.failed for c in self.checks) + # A fault is not a finding, but it is not a pass either: the check produced no verdict, + # so the run cannot claim the document is clean. + return self.fatal_error is None and not any(c.failed or c.is_fault for c in self.checks) @property def counts(self) -> dict[str, int]: tally = Counter(c.status for c in self.checks) - return {status: tally.get(status, 0) for status in (OK, FAIL, WARN, SKIP)} + return {status: tally.get(status, 0) for status in (OK, FAIL, WARN, SKIP, FAULT)} def by_status(self, status: Status) -> list[Check]: return [c for c in self.checks if c.status == status] @@ -133,6 +140,9 @@ def by_status(self, status: Status) -> list[Check]: def failures(self) -> list[Check]: return self.by_status(FAIL) + def faults(self) -> list[Check]: + return self.by_status(FAULT) + def warnings(self) -> list[Check]: return self.by_status(WARN) @@ -172,7 +182,12 @@ def to_dict(self, verbosity: Verbosity = "summary") -> dict[str, Any]: def failure_reasons(report: Report) -> list[str]: - """Human-readable reasons the run did not pass, most important first.""" + """Human-readable reasons the run did not pass, most important first. + + Faults lead. A finding tells the reader to change their document; a fault tells them part of + the answer is missing, which they need to know before acting on any of the rest. + """ if report.fatal_error: return [report.fatal_error] - return [f"{c.id} {c.target}: {c.message}" for c in report.failures()] + faults = [f"{c.id} {c.target}: validator fault, {c.message}" for c in report.faults()] + return faults + [f"{c.id} {c.target}: {c.message}" for c in report.failures()] diff --git a/tests/test_validation/test_cli.py b/tests/test_validation/test_cli.py index 78f6e2e..977236c 100644 --- a/tests/test_validation/test_cli.py +++ b/tests/test_validation/test_cli.py @@ -35,6 +35,33 @@ def test_a_broken_schema_exits_nonzero(run, broken_dir): assert "context.predicates" in result.output +def test_a_validator_fault_is_named_as_ours_and_exits_nonzero(run, data_dir, monkeypatch): + """A fault must not send the reader looking for a defect in their own document. + + So the count is reported separately from failures, and only when there are some: a permanent + "0 fault(s)" would train people to skip the field, which is the opposite of what it is for. + """ + from oold.validation import pipeline + + monkeypatch.setattr( + pipeline, "roundtrip", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("frame derivation broke")) + ) + result = run("validate", str(data_dir), "--offline") + + assert result.exit_code == 1 + assert "VALIDATOR FAULT(S)" in result.output + assert "FAULT roundtrip.generated" in result.output + assert "RuntimeError: frame derivation broke" in result.output + # Nothing was concluded about the documents, so nothing is reported as failing. + assert "0 failed" in result.output + + +def test_a_clean_run_does_not_mention_faults(run, data_dir): + result = run("validate", str(data_dir), "--offline") + assert result.exit_code == 0, result.output + assert "FAULT" not in result.output + + def test_failures_are_shown_by_default_and_passes_are_not(run, data_dir): result = run("validate", str(data_dir), "--offline") assert "hidden" in result.output diff --git a/tests/test_validation/test_pipeline.py b/tests/test_validation/test_pipeline.py index 9f45e38..07c644d 100644 --- a/tests/test_validation/test_pipeline.py +++ b/tests/test_validation/test_pipeline.py @@ -308,6 +308,93 @@ def test_a_processor_failure_is_not_downgraded_to_a_coverage_warning(broken_dir, assert "context.coverage" not in failures +def test_an_unexpected_exception_is_a_fault_of_its_check_not_a_finding(data_dir, monkeypatch): + """A defect in this package must not read as a defect in the user's schema. + + The fault carries the id of the check that raised it, so the report answers "which part of + the validator broke" with the same identifier that names what it was trying to establish. + """ + from oold.validation import pipeline + + monkeypatch.setattr( + pipeline, "roundtrip", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("frame derivation broke")) + ) + report = validate_directory(data_dir, OFFLINE) + + faults = report.faults() + assert faults, "a raising check produced no fault" + # Both checks that call `roundtrip` fault, each under its own id rather than a shared one, + # and nothing else is implicated. + assert {c.id for c in faults} == {"roundtrip.generated", "variants"} + assert all("RuntimeError: frame derivation broke" in c.message for c in faults) + assert all("frame derivation broke" in c.detail["traceback"] for c in faults) + # A fault is not a finding: nothing was concluded about the document. + assert not {c.id for c in report.failures()} & {"roundtrip.generated", "variants"} + + +def test_ref_resolution_faults_rather_than_blaming_the_schema(data_dir, monkeypatch): + """`schema.refs` guards by hand rather than with `_guard`, because it must return. + + Everything below it needs the dereferenced schema, so carrying on would report one defect + once per section that tripped over the missing name. + """ + from oold.validation import pipeline + + monkeypatch.setattr( + pipeline.Resolver, "dereference", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("resolver broke")) + ) + report = validate_directory(data_dir, OFFLINE) + faults = {c.id: c for c in report.faults()} + + assert "schema.refs" in faults, f"no schema.refs fault: {sorted(faults)}" + assert "RuntimeError: resolver broke" in faults["schema.refs"].message + assert not report.passed + # The early return is the point: one fault per schema, and no finding from the same section. + assert "schema.refs" not in {c.id for c in report.failures()} + assert "roundtrip.generated" not in faults, "the return did not stop the dependent sections" + + +def test_generation_faults_rather_than_blaming_the_schema(data_dir, monkeypatch): + """`generate.satisfiable` returns for the same reason: it produces the instance others reuse.""" + from oold.validation import pipeline + + monkeypatch.setattr(pipeline, "generate", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("generator broke"))) + report = validate_directory(data_dir, OFFLINE) + faults = {c.id: c for c in report.faults()} + + assert "generate.satisfiable" in faults, f"no generate.satisfiable fault: {sorted(faults)}" + assert "RuntimeError: generator broke" in faults["generate.satisfiable"].message + assert not report.passed + assert "generate.satisfiable" not in {c.id for c in report.failures()} + # Checks that do not depend on the generated instance still ran and still reported. + assert "schema.meta" in {c.id for c in report.checks} + + +def test_a_fault_fails_the_run_without_discarding_the_other_verdicts(data_dir, monkeypatch): + """Collect-everything-then-report is the model, so one broken check must cost one check. + + Before this, the two available outcomes were a FAIL naming the user's schema or a propagating + exception that threw away every verdict already computed in the directory. + """ + from oold.validation import pipeline + + monkeypatch.setattr( + pipeline, "roundtrip", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("frame derivation broke")) + ) + report = validate_directory(data_dir, OFFLINE) + + assert not report.passed, "a fault must not read as a pass" + assert report.counts["fault"] == len(report.faults()) + # Every other check still ran, over every target, including on the faulting schemas. + assert report.counts["ok"] > 0 + assert {c.id for c in report.checks} > {"roundtrip.generated"} + assert len(report.targets()) > 1 + # And it leads the reasons, because it means part of the answer is missing. + from oold.validation.report import failure_reasons + + assert "validator fault" in failure_reasons(report)[0] + + def test_strict_promotes_an_unmapped_term_to_a_failure(broken_dir): report = validate_schema( broken_dir / "missing_context_term.schema.json",