feat(admin-config-tool): tasks 5.1-5.3 - mapping/wire coverage confirmed, schema bundling fix
5.1 (mapping) and 5.2 (wire framing) are already fully satisfied by task 2.3's test_mapping.py and task 2.4's test_wire.py -- no new tests, would have been pure duplication. 5.3: test_schema_bundling.py cross-checks both bundling mechanisms (capnp schema, JSON validation schemas) resolve consistently under the active install, and found a real gap: a corrupted bundled JSON schema crashed validate_yaml() with a raw json.JSONDecodeError instead of a clean ValidationError. Fixed in xcecommon/validation.py.
This commit is contained in:
parent
ad27147a59
commit
b5040e0877
3 changed files with 152 additions and 4 deletions
|
|
@ -100,20 +100,20 @@
|
|||
- _Depends: 4.1_
|
||||
|
||||
- [ ] 5. Validation: test coverage across build, push, and packaging
|
||||
- [ ] 5.1 (P) Unit test the field-mapping logic in isolation
|
||||
- [x] 5.1 (P) Unit test the field-mapping logic in isolation
|
||||
- Both mapper shapes (list sections and the scalar section) are verified against in-memory input with no file I/O
|
||||
- The one file that feeds two separate output fields is verified explicitly
|
||||
- _Requirements: 1.1, 1.7, 1.8, 1.9, 1.10, 1.11, 1.12, 5.3, 9.1, 9.2, 9.3_
|
||||
- _Boundary: xcecommon.mapping_
|
||||
- _Depends: 2.3_
|
||||
|
||||
- [ ] 5.2 (P) Unit test wire framing
|
||||
- [x] 5.2 (P) Unit test wire framing
|
||||
- Framing round-trips correctly against a fixed, known byte sequence and against a local loopback connection
|
||||
- _Requirements: 2.1_
|
||||
- _Boundary: xcecommon.wire_
|
||||
- _Depends: 2.4_
|
||||
|
||||
- [ ] 5.3 (P) Verify bundled schemas resolve under every install mode
|
||||
- [x] 5.3 (P) Verify bundled schemas resolve under every install mode
|
||||
- Both the wire-protocol schema and the validation schemas resolve to real, parseable files under the currently active install
|
||||
- A deliberately corrupted or absent packaged schema produces the expected installation-hint error rather than a crash
|
||||
- Meaningful confirmation of the pinned install path requires running this under the release build profile
|
||||
|
|
@ -156,3 +156,5 @@
|
|||
- **3.3**: `probe_config()` does NOT go through `xceadmin.discovery.follow_redirects` — `ProbeConfigTagReply` is a direct struct with only `configTag`, no `redirectHost`/`.Port` fields exist on it at all, so there is no redirect signal for `follow_redirects` (which unconditionally decodes replies as `RequestReply`) to read; only `push_config()`'s `Request`/`RequestReply`-wrapped exchange supports redirects. `ProbeConfigTagReply` also carries no accept/reject signal — `DryRunResult.accepted` is a client-side inference (true on any successful round-trip), matching the real C reference client (`config_repl.c`'s `xce_probe_config_tag`, which also always sends an empty `name` and returns only a bare tag). pycapnp union field assignment for Void/Text-typed variants (e.g. `reply.data.nodata = None`, `reply.data.debugmsg = "..."`) must use direct assignment, not `.init(name)` — `.init()` without a size is only valid for struct/list/data-typed variants, confirmed via `KjException: init() without a size is only valid for struct and object fields`.
|
||||
- **4.1**: `xcecommon.wire.connect()` deliberately lets `ConnectionRefusedError`/other bare `OSError`s propagate unwrapped (documented in `wire.py`'s own module docstring) — `xceadmin.cli` is the layer that catches `OSError` and formats it per Requirement 7.1, alongside its `except XceAdminError` dispatch surface; a task 4.1 implementer must not assume `XceAdminError` alone covers every network failure mode. click 8.3.1's `CliRunner` no longer accepts `mix_stderr=`; `result.stdout`/`.stderr` are separated by default, but `CliRunner`'s output capture decodes as text and is **lossy for arbitrary binary** (confirmed: `bytes(range(256))` does not round-trip through the captured `.output` string) — do not assert exact byte content of a `-o -` binary write through `CliRunner`; that's already covered at the `write_output()` unit level (task 3.1's `test_config_build.py`). A CLI test that omits `-o` falls through to the real default (`cluster.bin`, relative to CWD) — always pass an explicit `-o <tmp_path>` in tests that reach the write step, or the test will write a stray artifact into the actual repo working directory (caught via `git status` before commit, not by the test itself).
|
||||
- **4.2**: `deploy-ha.sh` resolves both `xceadmin` (`$SCRIPT_DIR/../xceadmin/.venv/bin/xceadmin`) and the YAML configs source (`$SCRIPT_DIR/../configs`) relative to the script's own location (`$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)`), not the caller's CWD — matches the project's other relative-path scripts and keeps the deploy working regardless of where it's invoked from. `--build-config` is parsed out of `"$@"` before positional-arg assignment so it can appear anywhere in the argument list, not just a fixed position. Verified live against the real `xceadmin` build and `src/configs/`: missing `cluster.bin` auto-builds (8.4), an existing one with no flag is left byte-for-byte untouched — same mtime, same SHA (8.5), and `--build-config` against an existing `cluster.bin` forces a rebuild that reproduces byte-identical content (8.3, confirming 9.1's determinism guarantee holds through this path too).
|
||||
- **5.1, 5.2**: Already fully satisfied by task 2.3's `test_mapping.py` and task 2.4's `test_wire.py` respectively — same requirement lists, same acceptance bullets (in-memory-only mapper coverage + the explicit two-field case for 5.1; a fixed known byte sequence + real loopback round-trips for 5.2). No new tests written; would have been pure duplication.
|
||||
- **5.3**: Found and fixed a real gap while writing `test_schema_bundling.py` — `xcecommon.validation.validate_yaml()`'s `json.loads(schema_path.read_text())` call (loading the *bundled JSON validation schema itself*, not the operator's YAML) had no error handling, so a corrupted bundled JSON schema crashed with a raw `json.JSONDecodeError` instead of a clean `ValidationError` — the same "crash vs. clean error" class of bug task 2.2 already fixed for `controller.capnp`, just on the JSON-schema side, and not caught by task 2.1 since that task's tests only exercised operator-YAML failure modes, never a corrupted *schema* file. Fixed by wrapping it and raising `ValidationError(..., constraint="schema-corrupted")`, matching the existing missing-schema handling's pattern one function above it. **Gotcha for any future task touching `xcecommon`/`xceadmin` source**: editing a source file does NOT propagate to a running `.venv` until reinstalled — always re-run `xce-build.sh xceadmin` after a source edit before trusting a bare `.venv/bin/python -m pytest` result; a stale venv (especially one left in `release`/pinned mode by a prior profile run) will silently test the *old* code and produce a false failure (or false pass) that looks like a real bug.
|
||||
|
|
|
|||
|
|
@ -107,7 +107,16 @@ def validate_yaml(file_path: str, schema_name: str) -> dict:
|
|||
if data is None:
|
||||
data = {}
|
||||
|
||||
schema = json.loads(schema_path.read_text(encoding="utf-8"))
|
||||
try:
|
||||
schema = json.loads(schema_path.read_text(encoding="utf-8"))
|
||||
except json.JSONDecodeError as exc:
|
||||
raise ValidationError(
|
||||
f"{file_path}: bundled schema {schema_path} is corrupted: {exc} "
|
||||
"-- check the xceadmin installation",
|
||||
file=file_path,
|
||||
json_pointer="/",
|
||||
constraint="schema-corrupted",
|
||||
) from exc
|
||||
validator = jsonschema.Draft202012Validator(schema)
|
||||
errors = sorted(validator.iter_errors(data), key=lambda e: list(e.path))
|
||||
if errors:
|
||||
|
|
|
|||
137
src/xceadmin/tests/test_schema_bundling.py
Normal file
137
src/xceadmin/tests/test_schema_bundling.py
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
"""Cross-cutting bundling tests for ``xcecommon.schema`` and
|
||||
``xcecommon.validation`` (design.md, File Structure Plan; Requirements
|
||||
5.5, 7.4).
|
||||
|
||||
``xcecommon.schema``'s own missing/corrupted-``controller.capnp``
|
||||
handling is already exhaustively covered in ``test_schema.py`` (task
|
||||
2.2). This file's job is narrower: prove both bundling mechanisms
|
||||
(capnp schema, JSON validation schemas) resolve consistently under
|
||||
whichever install is currently active, and close the one confirmed gap
|
||||
task 5.3 found -- a corrupted *JSON* validation schema (as opposed to a
|
||||
corrupted ``controller.capnp``) crashed with a raw
|
||||
``json.JSONDecodeError`` instead of a clean ``ValidationError``, since
|
||||
``validate_yaml()``'s ``json.loads()`` call had no error handling. Fixed
|
||||
in ``xcecommon/validation.py`` alongside this test.
|
||||
|
||||
Meaningful confirmation of the pinned (``pip install .``) install path
|
||||
requires running this under the release build profile
|
||||
(``xce-build.sh xceadmin release``) -- under the default editable
|
||||
profile, resolution always succeeds by resolving into the checked-out
|
||||
source tree regardless of whether packaging is actually correct.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from xcecommon.errors import SchemaNotFoundError, ValidationError
|
||||
from xcecommon.schema import default_schema_path, load_schema
|
||||
from xcecommon.validation import default_schema_dir, validate_yaml
|
||||
|
||||
|
||||
def test_both_bundling_mechanisms_resolve_to_real_existing_paths():
|
||||
"""The capnp schema and the JSON validation schemas both resolve to
|
||||
real files under whichever install (editable or pinned) is currently
|
||||
active -- the single check that actually catches a packaging
|
||||
regression like the one bug-1460 found (missing package-data entry
|
||||
silently dropping schema_files/ from a non-editable wheel).
|
||||
"""
|
||||
capnp_path = Path(default_schema_path())
|
||||
assert capnp_path.is_file()
|
||||
|
||||
json_dir = Path(default_schema_dir())
|
||||
assert json_dir.is_dir()
|
||||
schema_files = list(json_dir.glob("*.schema.json"))
|
||||
assert len(schema_files) == 7 # one per config file type
|
||||
|
||||
|
||||
def test_both_mechanisms_produce_genuinely_parseable_content():
|
||||
mod = load_schema()
|
||||
assert hasattr(mod, "ClusterConfig")
|
||||
|
||||
json_dir = Path(default_schema_dir())
|
||||
for schema_file in json_dir.glob("*.schema.json"):
|
||||
with open(schema_file, encoding="utf-8") as fh:
|
||||
parsed = json.load(fh) # raises if genuinely not valid JSON
|
||||
assert isinstance(parsed, dict)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Corrupted / absent bundled schema -> clean error, not a crash
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def corrupted_json_schema():
|
||||
"""Temporarily corrupts the bundled xce.schema.json, restoring it
|
||||
afterward regardless of test outcome.
|
||||
"""
|
||||
target = Path(default_schema_dir()) / "xce.schema.json"
|
||||
backup = target.with_suffix(".json.bak")
|
||||
shutil.copy(str(target), str(backup))
|
||||
try:
|
||||
target.write_text("{not valid json!!!", encoding="utf-8")
|
||||
yield target
|
||||
finally:
|
||||
shutil.move(str(backup), str(target))
|
||||
|
||||
|
||||
def test_corrupted_json_schema_raises_validation_error_not_json_decode_error(
|
||||
corrupted_json_schema, tmp_path
|
||||
):
|
||||
"""The gap this task found: a corrupted bundled *JSON* schema (as
|
||||
opposed to a corrupted controller.capnp) previously crashed with a
|
||||
raw json.JSONDecodeError. validate_yaml() must convert it into a
|
||||
ValidationError instead (7.4's "clear, actionable error" principle,
|
||||
applied here to the JSON validation schemas).
|
||||
"""
|
||||
yaml_file = tmp_path / "xce.yaml"
|
||||
yaml_file.write_text("cluster_name: eda1\n", encoding="utf-8")
|
||||
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
validate_yaml(str(yaml_file), "xce")
|
||||
|
||||
assert exc_info.value.constraint == "schema-corrupted"
|
||||
assert "corrupted" in exc_info.value.message
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def missing_json_schema():
|
||||
target = Path(default_schema_dir()) / "xce.schema.json"
|
||||
backup = target.with_suffix(".json.bak")
|
||||
shutil.move(str(target), str(backup))
|
||||
try:
|
||||
yield target
|
||||
finally:
|
||||
shutil.move(str(backup), str(target))
|
||||
|
||||
|
||||
def test_missing_json_schema_raises_validation_error_not_file_not_found_error(
|
||||
missing_json_schema, tmp_path
|
||||
):
|
||||
yaml_file = tmp_path / "xce.yaml"
|
||||
yaml_file.write_text("cluster_name: eda1\n", encoding="utf-8")
|
||||
|
||||
with pytest.raises(ValidationError) as exc_info:
|
||||
validate_yaml(str(yaml_file), "xce")
|
||||
|
||||
assert exc_info.value.constraint == "schema-not-found"
|
||||
|
||||
|
||||
def test_missing_controller_capnp_raises_schema_not_found_error(monkeypatch, tmp_path):
|
||||
"""Parallel case for the capnp side, using the env-var override
|
||||
(xcecommon.schema.load_schema doesn't cache the corrupted-content
|
||||
precheck failure, so this proves the same "clean error, not a
|
||||
crash" contract using the mechanism already established in
|
||||
test_schema.py -- included here for the cross-cutting file's own
|
||||
completeness, not as a duplicate of that file's exhaustive coverage.
|
||||
"""
|
||||
missing_path = tmp_path / "does-not-exist.capnp"
|
||||
monkeypatch.setenv("XCEADMIN_SCHEMA_PATH", str(missing_path))
|
||||
|
||||
with pytest.raises(SchemaNotFoundError):
|
||||
load_schema()
|
||||
Loading…
Add table
Add a link
Reference in a new issue