diff --git a/atom/compass/spec/explain.py b/atom/compass/spec/explain.py index 3a07da8532..8a817055b8 100644 --- a/atom/compass/spec/explain.py +++ b/atom/compass/spec/explain.py @@ -39,10 +39,18 @@ different actions. A term the schema does not know is asked for again by a real path. A term the schema does know, with no field under it that this spec holds, would come back empty however it was spelled, so it is refused as the absent -field it is -- the same refusal `MachineSpec.value` gives for that field, and -for a term naming several fields, for the first of them. A field the spec does -hold is never refused here, even when it holds nothing: a tokenizer table with -no entries explains to no rows, as `MachineSpec.value` answers it with none. +field it is -- the same refusal `MachineSpec.value` gives for that field. + +A term naming several fields is refused when any required field under it is +absent, and the refusal names every one of them. Explaining from the fields +that are there would present part of a basis as the whole of it. An optional +field the document left out is not missing, so a block holding one still +explains from the rest. + +A field the spec does hold is never refused here, even when it holds nothing: +a tokenizer table with no entries explains to no rows, as `MachineSpec.value` +answers it with none. It is still printed, as the empty value it holds, so a +reader can tell a field that contributed nothing from one that was left out. """ from collections.abc import Mapping @@ -52,7 +60,7 @@ from .fields import BY_PATH, SCHEMA, Kind from .machine import MachineSpec from .merge import Merge, entry_label -from .rules import Rule, SpecRefusal, refuse_absent_field +from .rules import Rule, SpecRefusal, refuse_absent_field, refuse_absent_fields from .tokenizers import ENTRY_FIELDS #: Predicted quantities, and the spec fields each one is built out of. @@ -114,11 +122,14 @@ class Basis: term: str digest: str contributions: tuple[Contribution, ...] + #: Fields the spec holds that contribute no rows, printed so as not to + #: read as fields left out. + empty: tuple[Contribution, ...] = () def __str__(self) -> str: return "\n".join( [f"{self.term}, from spec {self.digest}"] - + [f" {contribution}" for contribution in self.contributions] + + [f" {row}" for row in self.contributions + self.empty] ) @@ -200,10 +211,19 @@ def explain( "ask again by the whole dotted path of a field or a block of fields, " f"or by one of the quantities {sorted(QUANTITIES)}", ) - if not any(path in spec.values for path in paths): - refuse_absent_field(paths[0], BY_PATH[paths[0]].required) + absent = [path for path in paths if path not in spec.values] + missing = tuple(path for path in absent if BY_PATH[path].required) + if missing: + refuse_absent_fields(missing) + if len(absent) == len(paths): + refuse_absent_field(paths[0], False) contributions: list[Contribution] = [] + empty: list[Contribution] = [] for path in paths: if path in spec.values: - contributions += _rows(spec, path, tp_width, origin) - return Basis(term, spec.digest(), tuple(contributions)) + rows = _rows(spec, path, tp_width, origin) + contributions += rows + if not rows: + held = spec.values[path] + empty.append(_row(path, held, None, _supplied(origin, path))) + return Basis(term, spec.digest(), tuple(contributions), tuple(empty)) diff --git a/atom/compass/spec/rules.py b/atom/compass/spec/rules.py index 4f88222e41..8fed4a2f94 100644 --- a/atom/compass/spec/rules.py +++ b/atom/compass/spec/rules.py @@ -141,14 +141,7 @@ def refuse_absent_field(path: str, required: bool) -> NoReturn: in the document. """ if required: - raise SpecRefusal( - Rule.TOTALITY, - f"`{path}` is declared by this schema, and this spec carries no value " - "for it", - "a spec read with `MachineSpec.from_mapping` resolves every required " - "field, so this one was assembled from parts; merge the fragment " - "that measures this field before asking for it", - ) + refuse_absent_fields((path,)) raise SpecRefusal( Rule.TOTALITY, f"`{path}` is declared by this schema as optional, and this spec states " @@ -158,6 +151,32 @@ def refuse_absent_field(path: str, required: bool) -> NoReturn: ) +def refuse_absent_fields(paths: tuple[str, ...]) -> NoReturn: + """Decline required fields this spec holds no value for, naming every one. + + The required half of `refuse_absent_field`, which reads one path through + here, so a question over several fields and a question over one are + declined in the same words. Every missing field is named rather than the + first: each is a separate fragment to merge, and a refusal naming one of + them sends the reader back once per field. + """ + one = len(paths) == 1 + named = ", ".join(f"`{path}`" for path in paths) + are, them, measures = ( + ("is", "it", "fragment that measures this field") + if one + else ("are", "them", "fragments that measure these fields") + ) + raise SpecRefusal( + Rule.TOTALITY, + f"{named} {are} declared by this schema, and this spec carries no value " + f"for {them}", + "a spec read with `MachineSpec.from_mapping` resolves every required " + f"field, so this one was assembled from parts; merge the {measures} " + f"before asking for {them}", + ) + + def refuse_unknown_key(path: str) -> NoReturn: """Decline a key the schema does not declare, saying why it is not one.""" owner = DEPLOYMENT_OWNED.get(path.rsplit(".", 1)[-1]) diff --git a/tests/compass/test_spec_verbs.py b/tests/compass/test_spec_verbs.py index 6fa88ddbc0..63609251e3 100644 --- a/tests/compass/test_spec_verbs.py +++ b/tests/compass/test_spec_verbs.py @@ -1057,27 +1057,79 @@ def test_an_empty_tokenizer_table_explains_to_no_rows_rather_than_refusing(): assert spec.value("host.tokenizers") == () basis = explain(spec, "host.tokenizers") assert basis.contributions == () - assert str(basis) == f"host.tokenizers, from spec {spec.digest()}" + assert str(basis).splitlines() == [ + f"host.tokenizers, from spec {spec.digest()}", + " host.tokenizers = ()", + ] -def test_a_block_an_assembled_spec_holds_nothing_under_names_its_first_field(): - # A spec built from parts can lack required fields too, and then the - # absent field is required, so the remedy is in whatever assembled it. +def test_an_empty_tokenizer_table_is_printed_as_empty_under_a_quantity(): + # Three rows from the other fields and none from the table. Without its own + # line the table reads as a field the quantity does not use. + document = copy.deepcopy(merged().document) + document["host"]["tokenizers"] = [] + basis = explain(MachineSpec.from_mapping(document), "admission") + assert len(basis.contributions) == 3 + printed = str(basis).splitlines() + assert len(printed) == 5 + assert printed[-1] == " host.tokenizers = ()" + + +def assembled_without(*paths): + """The resolved spec assembled from parts, with the named fields left out.""" whole = resolved() - spec = MachineSpec( - values={p: v for p, v in whole.values.items() if not p.startswith("host.ipc.")}, + return MachineSpec( + values={p: v for p, v in whole.values.items() if p not in paths}, tokenizers=whole.tokenizers, ) + + +def test_a_quantity_missing_one_field_is_refused_rather_than_explained_from_the_rest(): + # The other six fields still explain to twelve rows, which read as a whole + # basis for a number that cannot be computed. The refusal is the accessor's. + spec = assembled_without("device.memory.capacity_bytes") + with pytest.raises(SpecRefusal) as refused: + explain(spec, "kv_blocks") + with pytest.raises(SpecRefusal) as accessed: + spec.value("device.memory.capacity_bytes") + assert refused.value.rule is Rule.TOTALITY + assert str(refused.value) == str(accessed.value) + + +def test_every_missing_field_under_a_quantity_is_named_and_not_only_the_first(): + spec = assembled_without("host.admission_fixed_s", "host.ipc.shm_broadcast_s") + with pytest.raises(SpecRefusal) as refused: + explain(spec, "admission") + assert refused.value.rule is Rule.TOTALITY + assert refused.value.what.startswith( + "`host.admission_fixed_s`, `host.ipc.shm_broadcast_s` are declared by " + "this schema, and this spec carries no value for them" + ) + assert "fragments that measure these fields" in refused.value.remedy + + +def test_a_block_an_assembled_spec_holds_nothing_under_names_every_field(): + # A spec built from parts can lack required fields too, and then the + # absent field is required, so the remedy is in whatever assembled it. + spec = assembled_without("host.ipc.zmq_roundtrip_s", "host.ipc.shm_broadcast_s") with pytest.raises(SpecRefusal) as refused: explain(spec, "host.ipc") assert refused.value.rule is Rule.TOTALITY - assert "`host.ipc.zmq_roundtrip_s` is declared by this schema" in ( + assert "`host.ipc.zmq_roundtrip_s`, `host.ipc.shm_broadcast_s` are declared" in ( refused.value.what ) - assert "merge the fragment" in refused.value.remedy + assert "merge the fragments" in refused.value.remedy assert "optional" not in refused.value.what +def test_a_block_explains_without_an_optional_field_the_document_left_out(): + spec = resolved() + assert "provenance.notes" not in spec.values + paths = {row.path for row in explain(spec, "provenance").contributions} + assert "provenance.authored_by" in paths + assert "provenance.notes" not in paths + + def test_the_basis_prints_its_spec_and_one_line_per_field(): combination = merged() spec = MachineSpec.from_mapping(combination.document)