From e19391d5f1a8bc299dcc997675a0324efaa785f9 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 10:45:59 -0400 Subject: [PATCH 1/7] spec: scaffold for DSPX-4794 xtest entry point registries --- spec/DSPX-4794.md | 50 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 spec/DSPX-4794.md diff --git a/spec/DSPX-4794.md b/spec/DSPX-4794.md new file mode 100644 index 00000000..817216c0 --- /dev/null +++ b/spec/DSPX-4794.md @@ -0,0 +1,50 @@ +--- +ticket: DSPX-4794 +title: upstream(opentdf/tests): entry-point registries for SDKs, containers and features +status: draft +authors: [dmihalcik@virtru.com] +branches: [opentdf/tests:DSPX-4794-xtest-entry-point-registries] +prs: [] +created: 2026-09-15 +updated: 2026-09-15 +--- + +# upstream(opentdf/tests): entry-point registries for SDKs, containers and features + +## Summary +Blocked by the "make xtest a buildable distribution" ticket (B1-B3). +Step B4. Replace xtest's hard-coded Literal type unions with registries populated from entry points, so an out-of-tree plugin can add an SDK, a container format or a feature without patching xtest. +WhyThis is what turns a downstream consumer from a patch into a plugin. Without it, anyone adding a container format has to either fork or monkeypatch xtest's module globals at import time — and get the ordering right relative to conftest.pytest_addoption. +Frame the PR as "xtest is consumed by four repos and should be extensible by all of them", not as a hook for any one consumer. The design is genuinely generic. +Supporting evidence that hard-coded container enums are the wrong shape: NanoTDF was deliberately deleted upstream (150e3135, #366, -2004 lines), and otdf-sdk-mgr/tests/test_schema.py::test_removed_nano_container_is_rejected now actively guards against a second container kind. Hard-coding the enum is what made removal a 2000-line change. +Scopecontainer_type / sdk_type / feature_type (tdfs.py:112 / :103 / :117) become registries populated from entry points. Keep today's Literals as the built-in defaults so upstream type-checking is unaffected. +FORCED_SUPPORTS parsing (tdfs.py:240) moves from import time into pytest_configure, so a plugin can register features before XT_FORCE_SUPPORTS is validated against the known set. Today the parse happens at module import, against the narrow Literal. +conftest.sdk_specs_opt (conftest.py:345) stops defaulting to get_args(sdk_type) and defaults to installed SDKs via all_versions_of over the dist dirs. This is more correct today — a bare pytest currently tries to construct SDK(...) objects for SDKs that were never installed and dies with FileNotFoundError surfaced as a UsageError — and it does not break when the registry widens. +simple_container() (tdfs.py:658) becomes a container-adapter lookup, and the post-encrypt ZIP-native assertions move behind it. Today test_tdfs.py:70 calls tdfs.manifest(ct_file) (which does zipfile.ZipFile(...).open("0.manifest.json")) unconditionally right after encrypt; ~12 tamper tests go through tdfs.update_manifest; zipinspect.py and test_zip64*.py are pure ZIP. +Precedent for the adapter idea already in-tree: ztdf-ecwrap is a container variant that is not a distinct wire format — simple_container() collapses it back to "ztdf" and it carries its difference in a flag. +RiskThis is the highest-review-risk item in the upstream series. Sequence it last, after the mechanical packaging change and the behaviour-identical adapter landing have built confidence. +Acceptance criteria[ ] A throwaway out-of-tree plugin registering a fake SDK and a fake container is discovered, collected and parametrized without touching the xtest source tree. +[ ] XT_FORCE_SUPPORTS accepts a feature contributed by that plugin. +[ ] A bare pytest with no --sdks runs against installed SDKs only, and does not raise UsageError. +[ ] Existing type-checking (pyright) passes with the built-in defaults unchanged. +[ ] ZIP-native assertions in test_tdfs.py run through the container adapter, not directly. +[ ] Full run green, skip counts unchanged. + +## Problem / Motivation +_Why does this work need to happen? What is the user/business pain?_ + +## Proposed Solution +_What will you build, at a functional level? Sketch the approach._ + +## Inputs / Outputs / Contracts +_Function signatures, data shapes, API contracts, CLI flags._ + +## Edge Cases & Constraints +_Boundary conditions, error states, performance limits, security considerations._ + +## Out of Scope +_What this work item explicitly does not cover._ + +## Acceptance Criteria +- [ ] _Clear, testable condition_ +- [ ] _…_ From 800f5fef554edcc930923373896f6cfa7a50ae1d Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 10:58:13 -0400 Subject: [PATCH 2/7] docs(spec): DSPX-4794 entry-point registries for SDKs, containers and features --- spec/DSPX-4794.md | 507 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 483 insertions(+), 24 deletions(-) diff --git a/spec/DSPX-4794.md b/spec/DSPX-4794.md index 817216c0..ef57edc1 100644 --- a/spec/DSPX-4794.md +++ b/spec/DSPX-4794.md @@ -12,39 +12,498 @@ updated: 2026-09-15 # upstream(opentdf/tests): entry-point registries for SDKs, containers and features ## Summary -Blocked by the "make xtest a buildable distribution" ticket (B1-B3). -Step B4. Replace xtest's hard-coded Literal type unions with registries populated from entry points, so an out-of-tree plugin can add an SDK, a container format or a feature without patching xtest. -WhyThis is what turns a downstream consumer from a patch into a plugin. Without it, anyone adding a container format has to either fork or monkeypatch xtest's module globals at import time — and get the ordering right relative to conftest.pytest_addoption. -Frame the PR as "xtest is consumed by four repos and should be extensible by all of them", not as a hook for any one consumer. The design is genuinely generic. -Supporting evidence that hard-coded container enums are the wrong shape: NanoTDF was deliberately deleted upstream (150e3135, #366, -2004 lines), and otdf-sdk-mgr/tests/test_schema.py::test_removed_nano_container_is_rejected now actively guards against a second container kind. Hard-coding the enum is what made removal a 2000-line change. -Scopecontainer_type / sdk_type / feature_type (tdfs.py:112 / :103 / :117) become registries populated from entry points. Keep today's Literals as the built-in defaults so upstream type-checking is unaffected. -FORCED_SUPPORTS parsing (tdfs.py:240) moves from import time into pytest_configure, so a plugin can register features before XT_FORCE_SUPPORTS is validated against the known set. Today the parse happens at module import, against the narrow Literal. -conftest.sdk_specs_opt (conftest.py:345) stops defaulting to get_args(sdk_type) and defaults to installed SDKs via all_versions_of over the dist dirs. This is more correct today — a bare pytest currently tries to construct SDK(...) objects for SDKs that were never installed and dies with FileNotFoundError surfaced as a UsageError — and it does not break when the registry widens. -simple_container() (tdfs.py:658) becomes a container-adapter lookup, and the post-encrypt ZIP-native assertions move behind it. Today test_tdfs.py:70 calls tdfs.manifest(ct_file) (which does zipfile.ZipFile(...).open("0.manifest.json")) unconditionally right after encrypt; ~12 tamper tests go through tdfs.update_manifest; zipinspect.py and test_zip64*.py are pure ZIP. -Precedent for the adapter idea already in-tree: ztdf-ecwrap is a container variant that is not a distinct wire format — simple_container() collapses it back to "ztdf" and it carries its difference in a flag. -RiskThis is the highest-review-risk item in the upstream series. Sequence it last, after the mechanical packaging change and the behaviour-identical adapter landing have built confidence. -Acceptance criteria[ ] A throwaway out-of-tree plugin registering a fake SDK and a fake container is discovered, collected and parametrized without touching the xtest source tree. -[ ] XT_FORCE_SUPPORTS accepts a feature contributed by that plugin. -[ ] A bare pytest with no --sdks runs against installed SDKs only, and does not raise UsageError. -[ ] Existing type-checking (pyright) passes with the built-in defaults unchanged. -[ ] ZIP-native assertions in test_tdfs.py run through the container adapter, not directly. -[ ] Full run green, skip counts unchanged. + +Three `Literal` unions in `xtest/tdfs.py` decide what this suite is allowed to +test: `sdk_type` (`:103`), `container_type` (`:112`) and `feature_type` +(`:117`). Every one of them is a closed set written into the source file, so +the only way to add an SDK, a container format or a capability gate is to edit +`tdfs.py`. xtest is pinned at `@main` by four repos — `opentdf/platform`, +`web-sdk`, `java-sdk` and `otdfctl` — and none of them can extend it. + +This replaces the three closed unions with registries seeded from today's +`Literal`s and widened by Python entry points, under three named groups: +`otdf.adapters`, `otdf.containers`, `otdf.installers`. Today's values stay +exactly where they are, so in-tree type-checking is unchanged and a `pyright` +run over `xtest/` still rejects `container="ztfd"`. + +Three call sites move as a consequence, and two of them are worth doing on +their own merits regardless of extensibility: + +- `FORCED_SUPPORTS` (`tdfs.py:240`) is parsed **at module import**, against + `get_args(feature_type)`. Nothing can register a feature before that runs. + It moves into `pytest_configure`. +- `conftest.sdk_specs_opt` (`conftest.py:345`) defaults to + `get_args(tdfs.sdk_type)`, which is the enum rather than the installed set. +- `simple_container()` (`tdfs.py:658`) is a two-line `if` that every encrypt + and decrypt path funnels through, and immediately after it `test_tdfs.py:70` + reads the container as a ZIP. A container that is not a ZIP has nowhere to + say so. + +Sequenced last in the upstream series, after the packaging change +(`src/xtest/` + `[build-system]`, which this is stacked on) and after the +behaviour-identical adapter landing. It is the only piece carrying real +review risk. ## Problem / Motivation -_Why does this work need to happen? What is the user/business pain?_ + +**Upstream already paid for this once.** NanoTDF was deleted from xtest in +`150e3135` — *"fix: remove NanoTDF tests and support (#366)"*, 2026-01-06 — +11 files changed, **+22 / −2003**. A container format that was never more than +a second value in an enum cost two thousand lines to remove, because the enum +value was load-bearing across the whole suite rather than confined to one +object. `otdf-sdk-mgr/tests/test_schema.py:159` +(`test_removed_nano_container_is_rejected`) now actively asserts that +`containers: [nano]` fails validation, against +`ContainerKind = Literal["ztdf", "ztdf-ecwrap"]` +(`otdf-sdk-mgr/src/otdf_sdk_mgr/schema.py:39`). There are now **two** +hand-maintained copies of the container enum in this repo, in two packages, +and a regression test whose entire job is to keep them in sync. + +That is the tell. The problem is not that nano was the wrong format; it is that +the suite has no seam at which a container format can be attached or detached. + +**The enum is not the only closed thing; it is the visible one.** Widening +`container_type` alone would not work, because the suite reaches past the +abstraction at the points that matter: + +| Where | What it assumes | +|---|---| +| `tdfs.manifest()` (`:479`) | `zipfile.ZipFile(...).open("0.manifest.json")` | +| `tdfs.update_manifest()` (`:486`) | extract ZIP → edit `0.manifest.json` → rezip | +| `tdfs.update_payload()` (`:523`) | extract ZIP → edit `0.payload` → rezip | +| `tdfs.validate_manifest_schema()` (`:632`) | same, plus `SCHEMA_FILE` | +| `tdfs.elides_segment_sizes()` (`:996`) | ztdf `integrityInformation` | +| `test_tdfs.py:70` | calls `tdfs.manifest(ct_file)` unconditionally, one line after encrypt, inside a test parametrized over `container` | +| `test_tdfs.py`, `test_root_signature.py`, `test_audit_logs_integration.py` | 23 `update_manifest` / `update_payload` call sites | + +So a new container value would be collected, parametrized, encrypted — and +then asserted to be a ZIP. It would not fail with "unsupported container"; it +would fail with `BadZipFile` from a helper three modules away. + +**`FORCED_SUPPORTS` has an ordering problem that no plugin can work around.** +`tdfs.py:240` runs `_parse_forced_supports(os.environ.get("XT_FORCE_SUPPORTS", +""))` at import, and that function validates against +`set(get_args(feature_type))` (`:219`) and raises on an unknown name. The +strictness is correct and deliberate — the docstring at `:210` says so: a typo +that silently left a skip in place would be indistinguishable from a clean run. +But it means the set of legal feature names is frozen the moment `tdfs` is +imported, which is before `pytest_addoption`, before `pytest_configure`, and +before any plugin has run. A plugin contributing a feature has exactly one +option today: mutate `tdfs.feature_type` from a `sitecustomize` hook that +sorts ahead of conftest import. That is the "monkeypatch the module globals and +get the ordering right" failure mode this ticket exists to remove. + +**`sdk_specs_opt` defaults to the enum, not to reality.** `conftest.py:351` +returns `list(typing.get_args(tdfs.sdk_type))`. I verified what that actually +does today, because the obvious claim about it is wrong: + +- With **nothing installed**, `parse_sdk_spec("go")` falls through to + `all_versions_of("go")` (`tdfs.py:947`), which returns `[]` for a missing + `sdk/go/dist`. All three names resolve to `[]`, `metafunc.parametrize` gets + an empty list, and pytest's default `empty_parameter_set_mark` converts the + lot into skips. Measured on a clean checkout: + `20 skipped … got empty parameter set for (encrypt_sdk)`, **exit 0**. +- The `FileNotFoundError` → `pytest.UsageError` path (`conftest.py:362`) does + exist, but it fires on a *partial* install, not an absent one: `mkdir -p + sdk/go/dist/main` with no `cli.sh` gives + `UsageError: SDK executable not found at path: sdk/go/dist/main/cli.sh`. + +So the default is already routed through `all_versions_of` by +`parse_sdk_spec`, and making that explicit is behaviour-preserving, not a +bug fix. **The bug is the other one**: a bare `pytest` on an unprovisioned +checkout reports twenty skips and exits zero. That is precisely the +vacuous-green failure mode this repo already guards against elsewhere — +`conftest.sizes_opt_type` (`:129-140`) carries a whole comment block about an +empty parameter set being *"a skip, exit 0. A whole matrix disappears and the +run stays green."* The same hole is open one function further down. ## Proposed Solution -_What will you build, at a functional level? Sketch the approach._ + +A single new module, `xtest/registry.py`, and three call-site changes. The +`Literal`s stay in `tdfs.py`, unedited. + +### The registry + +``` + built-in defaults entry-point group +sdks Literal["go","java","js"] otdf.adapters +containers Literal["ztdf","ztdf-ecwrap"] otdf.containers +features Literal[... 26 names ...] (declared by the two above) +``` + +A `Registry[T]` is a name→object map seeded with the built-ins and widened once, +at `pytest_configure`, by `importlib.metadata.entry_points(group=...)`. +Features are deliberately **not** their own group: a feature name is contributed +by the adapter or container that declares it, so a feature cannot exist with +nothing behind it, and `XT_FORCE_SUPPORTS` cannot be handed a name no plugin +will ever answer to. + +`otdf.installers` is the third group and the one this ticket only reserves: +it is how an out-of-tree SDK becomes materialisable under `dist//`, +and its consumer is `otdf-sdk-mgr`, not xtest. Named here so the three groups +are designed together; wiring it is a separate change. + +### Keeping static typing useful once the set is open + +This is the hard review question, so it gets a direct answer rather than a +widened annotation and a shrug. + +The three `Literal`s are **not** replaced by `str`, and they are **not** widened +to `Literal[...] | str` — pyright collapses that union to `str` and silently +stops diagnosing anything, which is the worst of both. Instead the type surface +splits in two, and four separate mechanisms cover what the `Literal` used to +cover alone: + +1. **The `Literal`s stay, and stay authoritative for in-tree code.** Every + in-tree call site that writes a container or feature name down as a string + literal keeps its `Literal` annotation: `PlatformFeatureSet.features` + (`:260`), `skip_if_unsupported` (`:355`, `:889`, `:977`), + `fixtures/keys.py:43,85`, `fixtures/bench.py:237`. In-tree, `pyright` still + rejects `"ztfd"` and still rejects deleting a built-in, because deleting one + makes every literal mentioning it an error at once. That property is the + reason for not touching them. + +2. **Parameters carrying a *parametrized* value widen exactly one step**, to + `ContainerName = str` / `SdkName = str`. That is the honest type: their + value comes from `metafunc.parametrize` over a runtime set, so it was never + statically knowable. These are `SDK.encrypt/decrypt` and their + `*_command` builders (`:722`, `:770`, `:803`, `:843`), + `fixtures/encryption.py:38`, `simple_container()`, and the `container:` + fixture parameter in `test_tdfs.py`, `test_abac.py` and + `test_policytypes.py`. Nothing is lost here that pyright was catching: + `container` at those sites is already an opaque fixture value. + +3. **The typo check moves to collection time and gets better.** + `registry.get(name)` raises `UnknownName` listing every registered name, and + `--containers` / `--sdks` / `XT_FORCE_SUPPORTS` validate against the + registry in `pytest_configure`. A misspelling fails the session before a + single test runs, with a message naming the alternatives — which is strictly + more than pyright gave a CI run, since CI does not type-check the value of + `--containers`. The repo already prefers this shape: `--sizes` is validated + at runtime against `sizes.SIZES` (`conftest.py:110-141`) precisely because + the size vocabulary is a dict, not a `Literal`. + +4. **The two sets are pinned against each other by an offline test.** + `registry.py` deliberately does **not** import `tdfs` — that is what lets + `tdfs` import `registry` later without a cycle — so the built-in tuples are + written out twice. `test_registry_units.py` asserts + `registry.BUILTIN_CONTAINERS == get_args(tdfs.container_type)` and the same + for SDKs and features, so the copies cannot drift, and any widening of the + built-in set shows up as an edit to a test that spells out today's surface. + This runs on `check.yml`'s offline step, on every PR. + +`is_sdk_type` (`:106`) is already a `TypeIs` narrowing helper; the same pattern +generalises to `is_container_name`, so a caller that needs to get back into +`Literal` territory can. + +### `simple_container()` becomes the container seam + +`simple_container()` (`:658`) is already the chokepoint — both +`encrypt_command` (`:739`) and `decrypt_command` (`:816`) call it to map a +container *variant* onto the wire format the shim understands, and +`ztdf-ecwrap` → `ztdf` is the in-tree precedent for a variant that is not a +distinct format and carries its difference in a flag (`XT_WITH_ECWRAP`, +`:762`). It becomes `containers.get(name).wire_format`, and the ZIP-shaped +helpers move behind `ContainerAdapter`: + +- `test_tdfs.py:70`'s `tdfs.manifest(ct_file)` becomes + `adapter.inspect(ct_file)` returning a format-agnostic `Inspection`. The + ztdf adapter synthesises it from the manifest it already parses. +- The 23 `update_manifest` / `update_payload` call sites become + `adapter.tamper(path, mutation)` over a named mutation vocabulary. A + container that cannot express a mutation raises `UnsupportedMutation` and + the test skips with that reason — not `BadZipFile` from three modules away. +- `zipinspect.py` and `test_zip64*.py` stay ZIP-native and stay gated on + ztdf. They are tests *of the ZIP encoding*; there is nothing to abstract. + +Strangler, as with the adapter work: `tdfs.manifest()` and friends keep +working, and the ztdf adapter's implementation *is* a call to them. + +### Landing order + +| PR | Change | Risk | +|---|---|---| +| 1 | `FORCED_SUPPORTS` parsing → `pytest_configure`, with a test | behaviour-identical for every existing invocation | +| 2 | `sdk_specs_opt` → installed SDKs; guard the empty default | one behaviour change, deliberate (below) | +| 3 | `registry.py` + built-ins + the pin test; `XT_FORCE_SUPPORTS` validates against it | pure addition plus one wired call site | +| 4 | `ContainerAdapter`, ztdf adapter, `simple_container()` lookup | the real one | +| 5 | `test_tdfs.py` ZIP assertions move behind `inspect`/`tamper` | mechanical once 4 lands | +| 6 | `--containers` / `--sdks` validate against the registry; `otdf-sdk-mgr`'s `ContainerKind` follows | deletes the duplicate enum | + +PRs 1–3 land on the flat layout and do not depend on the packaging ticket. +PRs 4–6 want `src/xtest/` first. ## Inputs / Outputs / Contracts -_Function signatures, data shapes, API contracts, CLI flags._ + +### Entry-point groups + +```toml +[project.entry-points."otdf.adapters"] +acme = "acme_xtest.adapters:AcmeAdapter" + +[project.entry-points."otdf.containers"] +acme = "acme_xtest.containers:AcmeContainer" + +[project.entry-points."otdf.installers"] +acme = "acme_xtest.installer:install" +``` + +The entry-point *name* is the registered name. A plugin that collides with a +built-in is a hard error, not a silent override: xtest's own `ztdf` behaviour +must not be replaceable by an installed package. + +### `xtest/registry.py` + +```python +GROUP_ADAPTERS = "otdf.adapters" +GROUP_CONTAINERS = "otdf.containers" +GROUP_INSTALLERS = "otdf.installers" + +BUILTIN_SDKS: tuple[str, ...] # ("go", "java", "js") +BUILTIN_CONTAINERS: tuple[str, ...] # ("ztdf", "ztdf-ecwrap") +BUILTIN_FEATURES: tuple[str, ...] # today's feature_type, verbatim + +class UnknownName(LookupError): ... +class DuplicateName(RuntimeError): ... + +class Registry[T]: + group: str + def load(self) -> None # idempotent; entry-point discovery + def register(self, name: str, obj: T) -> None + def names(self) -> tuple[str, ...] # built-ins first, then plugins + def get(self, name: str) -> T # UnknownName lists the alternatives + def __contains__(self, name: str) -> bool + +SDKS: Registry[SdkAdapterFactory] +CONTAINERS: Registry[ContainerAdapter] +INSTALLERS: Registry[Installer] + +def feature_names() -> frozenset[str] # built-ins ∪ every plugin's .features +def sdk_names() -> tuple[str, ...] +def container_names() -> tuple[str, ...] +def load_all() -> None # called once from pytest_configure +``` + +A built-in registers with `obj = None` until PR 4 supplies the adapters, so the +name set is usable before the objects exist. That is what makes PR 3 a pure +addition. + +### `ContainerAdapter` + +```python +class ContainerAdapter(Protocol): + name: str + wire_format: str # what the CLI is handed; "ztdf" for ztdf-ecwrap + features: frozenset[str] # feature names this container contributes + def inspect(self, path: Path) -> Inspection: ... + def tamper(self, path: Path, mutation: Mutation) -> Path: ... + def requires_attributes(self) -> bool: ... +``` + +`requires_attributes()` exists because a format whose encrypt path demands at +least one attribute has no no-attribute roundtrip cell, and today that case is +unrepresentable — the no-attribute roundtrip is xtest's *default*. A container +that answers `True` has those cells skipped with a reason rather than failing +on an unhelpful CLI usage error. + +### `Inspection` — the format-agnostic intersection + +```python +@dataclass(frozen=True) +class KeyAccessRecord: + kas_url: str + kid: str | None + split_id: str | None + wrap_algorithm: str | None + has_ephemeral_key: bool + +@dataclass(frozen=True) +class Inspection: + container: str # the registered name + total_size: int + ciphertext_size: int + mime_type: str | None + key_access_mode: str # "wrapped" | "ec-wrapped" | ... + key_access: tuple[KeyAccessRecord, ...] + policy_present: bool + policy_attribute_count: int + encrypted: bool + spec_version: str | None + raw: object = None # the native structure, for format-gated tests +``` + +Every field is answerable by any container format that binds a policy to a +wrapped key and hands it to a KAS; none of them presumes a ZIP, a JSON +manifest, or a named entry. `test_tdfs.py:70-77`'s current assertions — +`payload.isEncrypted`, exactly one KAO, `kao.type`, `kao.ephemeralPublicKey is +not None` for ecwrap — map onto `encrypted`, `len(key_access)`, +`key_access_mode` and `has_ephemeral_key` with nothing lost. + +`raw` is the escape hatch, and it is typed `object` on purpose: a test that +wants the ztdf `Manifest` must reach for `tdfs.manifest()` and thereby declare +itself ztdf-only. A convenient typed `raw` would quietly re-close the seam. + +### `Mutation` — the tamper vocabulary + +```python +class Mutation(StrEnum): + UNBIND_POLICY = "unbind_policy" + ALTER_POLICY_BINDING = "alter_policy_binding" + ALTER_ROOT_SIGNATURE = "alter_root_signature" + FORGE_GMAC_ROOT = "forge_gmac_root" + ALTER_SEGMENT_HASH = "alter_segment_hash" + ALTER_SEGMENT_SIZE = "alter_segment_size" + ALTER_PAYLOAD_TAIL = "alter_payload_tail" + ALTER_ASSERTION = "alter_assertion" + MALICIOUS_KAO = "malicious_kao" + DUPLICATE_KAO = "duplicate_kao" + +class UnsupportedMutation(NotImplementedError): ... +``` + +Ten names, derived from the 23 existing call sites, not invented. Each is a +*property of the attack*, not of the encoding — "replace the policy with one +the binding does not cover" is meaningful for any container that binds a +policy. The ztdf adapter implements each as the `update_manifest` callable +that exists today. + +### CLI and environment + +No new flags. Behaviour changes to existing ones: + +``` +--containers validated against the registry, not against a Literal +--sdks / --sdks-* default resolves to installed SDKs across registered names +XT_FORCE_SUPPORTS=<...> validated in pytest_configure, after plugins load +``` + +`tdfs.FORCED_SUPPORTS` (the module global) is replaced by +`tdfs.forced_supports()`. It lazily parses `XT_FORCE_SUPPORTS` on first call if +`pytest_configure` never ran, so importing `tdfs` outside pytest keeps working. ## Edge Cases & Constraints -_Boundary conditions, error states, performance limits, security considerations._ + +**A plugin that fails to import must be loud.** `entry_points()` returns +metadata; `ep.load()` executes code. A `try/except ImportError: continue` +around the load would turn a broken plugin into a run that quietly tests less +than it was asked to — with no non-zero exit anywhere, because the missing +container simply produces fewer cells. So a load failure raises, and the +message names the entry point and the group. + +**Entry-point discovery must happen exactly once, and before +`pytest_generate_tests`.** `pytest_configure` is the only hook that satisfies +both. `load_all()` is idempotent and records the loaded set, because under +`pytest-xdist` each worker configures independently and a second scan would +double-register. + +**A plugin may not shadow a built-in.** `register()` raises `DuplicateName`. +Silent override would make `--containers ztdf` mean different things depending +on what is installed in the virtualenv, which is unauditable from a CI log. + +**Empty parameter sets stay the enemy.** Making `sdk_specs_opt` default to +installed SDKs does not by itself fix the twenty-silent-skips problem measured +above; the guard does. It fires **only on the default path** — if no +`--sdks`/`--sdks-encrypt`/`--sdks-decrypt` was given and nothing is installed, +that is an unprovisioned harness and a `UsageError` naming +`otdf-sdk-mgr install` is the useful answer. An explicit `--sdks go@v9.9.9` that +resolves to nothing is left alone: it may be a deliberate narrowing in a +script, and turning that into an error is a bigger change than this ticket +should make. Flagged in the PR as the one debatable line; it is one `if` and is +cheap to drop if upstream disagrees. + +**The duplicated built-in tuples.** `registry.py` not importing `tdfs` is a +deliberate acyclicity constraint, and the cost is two copies of each name list. +Mitigated by the pin test, which is the only reason the duplication is +acceptable — without it this would be a second `ContainerKind`, i.e. exactly +the defect this ticket is removing. If the pin test is ever deleted, the +duplication must be collapsed in the same PR. + +**`otdf-sdk-mgr` has the same enum and must not be forgotten.** +`schema.py:39` and the regression test at `tests/test_schema.py:159` are a +second, independent closed set. They are out of scope for the first PRs and +land in PR 6; until then a plugin's container name works with `pytest +--containers` but not with an `otdf-sdk-mgr` scenario file. Stating the gap +rather than leaving it to be discovered. + +**`focus_type` is derived** (`tdfs.py:110`: `Literal[sdk_type, "all"]`), so it +widens with `sdk_type` for free and needs no separate registry. The `--focus` +validator (`conftest.py:199`) and the `"all"` expansion (`:384`) both need to +read the registry rather than `get_args`, or a plugin SDK is never in focus and +every one of its cells skips with `"Not in focus"` — a silent, fully-green +way to test nothing. + +**The `supports` seam is per-SDK, not per-container.** `SDK.supports(feature)` +(`:881`) has no container parameter, so a build that supports a feature for one +container and not another cannot say so. That is a real limitation of the +current signature; it belongs to the adapter ticket, and this ticket does not +widen it. Noted so the container registry is not blamed for it later. + +**Python version.** `requires-python = ">=3.14"` +(`xtest/pyproject.toml:6`), so `importlib.metadata.entry_points(group=...)`, +`StrEnum` and PEP 695 generics are all available unconditionally. No +`importlib_metadata` backport, no `sys.version_info` branch. + +**License metadata.** `xtest/pyproject.toml:7` says `BSD-3-Clause`; the repo +`LICENSE` is BSD-3-Clause-**Clear**. Entry points make xtest a thing other +distributions declare a dependency on, so the metadata starts mattering. The +fix belongs to the packaging ticket that adds `[build-system]`; recorded here +because this is the ticket that makes it consequential. ## Out of Scope -_What this work item explicitly does not cover._ + +- **`src/xtest/` and `[build-system]`.** The packaging ticket (B1–B3). This is + stacked on it for PRs 4–6; PRs 1–3 do not need it, because + `importlib.metadata.entry_points()` scans the environment and does not care + whether *xtest* is itself installed. +- **`otdf-adapter` and the `SdkAdapter` protocol.** A sibling ticket owns the + typed SDK protocol and the `cli.sh` replacement. This ticket defines the + `otdf.adapters` *group* and the registry that reads it; the object on the + other end of the entry point is that ticket's shape. +- **Deleting `sdk/*/cli.sh`.** Same sibling. +- **Wiring `otdf.installers` into `otdf-sdk-mgr`.** The group name is reserved + and the protocol sketched; the install path is a separate change. +- **A second in-tree container format.** Nothing is added to + `container_type`. The point of the registry is that new formats arrive + out-of-tree; adding one in-tree would re-create the `150e3135` problem. +- **`zipinspect.py` and `test_zip64*.py`.** ZIP-encoding tests, correctly + ztdf-only. +- **Per-container `supports()`.** Adapter ticket. +- **Any change to the `feature_type` membership.** Names move into a registry; + none are added or removed. ## Acceptance Criteria -- [ ] _Clear, testable condition_ -- [ ] _…_ + +Verified offline — no platform, no SDK builds: + +- [ ] A throwaway out-of-tree plugin registering a fake container and a fake + SDK is discovered and collected without touching the xtest source tree. + Built by a fixture that writes a `.dist-info` with an `entry_points.txt` + into `tmp_path` and puts it on `sys.path`, so the test proves the real + `importlib.metadata` path rather than a monkeypatched registry. +- [ ] `XT_FORCE_SUPPORTS` accepts a feature name contributed by that plugin, + and still raises `ValueError` naming the unknown name for a typo. Both + directions, because a validator that accepts everything is the failure + mode the strictness exists to prevent. +- [ ] Importing `tdfs` with `XT_FORCE_SUPPORTS` set to an unknown name no + longer raises at import. Checked in a subprocess, since the parse it + replaces happened at module scope and cannot be re-observed by reimport + in the same interpreter. +- [ ] `registry.BUILTIN_SDKS`, `BUILTIN_CONTAINERS` and `BUILTIN_FEATURES` + each equal `get_args()` of the corresponding `Literal`, so today's + extension-point surface is written down and any widening is visible in a + future diff. +- [ ] A plugin whose entry point fails to import fails the session, and the + error names the entry point and the group. +- [ ] A plugin that reuses a built-in name raises `DuplicateName`. +- [ ] A bare `pytest` on an unprovisioned checkout no longer reports + `20 skipped … got empty parameter set` and exit 0. +- [ ] `ruff check`, `ruff format --check` and `pyright` pass with the built-in + `Literal`s unedited. +- [ ] The new offline tests run in `check.yml`'s existing offline step. + +Requiring a platform: + +- [ ] ZIP-native assertions in `test_tdfs.py` run through the container + adapter, not through `zipfile` directly, and a full run is green with + the skip counts unchanged. +- [ ] `--containers ztdf ztdf-ecwrap` produces the same cells as today. From 61280b968e8728e2928df5c45ff04b397fc82b1b Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 11:00:22 -0400 Subject: [PATCH 3/7] refactor(xtest): resolve XT_FORCE_SUPPORTS in pytest_configure, not at import tdfs.FORCED_SUPPORTS was evaluated at module scope, and its parse rejects any name not in get_args(feature_type). Wherever that parse runs is the moment the set of legal feature names freezes -- at import, that is before pytest_addoption, before pytest_configure, and before any plugin has run. The strictness is right (a typo must not silently leave a skip in place) and so is extensibility; they only conflicted because of when the parse happened. Move it into conftest.pytest_configure, behind tdfs.configure_forced_supports() / tdfs.forced_supports(), with a lazy environment fallback so callers that import tdfs outside a pytest session keep working. A bad name now surfaces as a pytest.UsageError rather than a ValueError escaping through the plugin manager as an INTERNALERROR traceback. --- xtest/conftest.py | 13 ++++++ xtest/tdfs.py | 86 ++++++++++++++++++++++++++++++---------- xtest/test_tdfs_units.py | 84 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 22 deletions(-) diff --git a/xtest/conftest.py b/xtest/conftest.py index c2b8d5c7..9dddaa92 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -426,6 +426,19 @@ def _parametrize_bench_cells(metafunc: pytest.Metafunc): def pytest_configure(config: pytest.Config): + # Resolve XT_FORCE_SUPPORTS here rather than at tdfs import. The parse + # rejects unknown names, so wherever it runs is the moment the set of legal + # feature names freezes; at import that is before any plugin could have + # contributed one. See tdfs.configure_forced_supports. + # + # UsageError, not the bare ValueError: a typo in XT_FORCE_SUPPORTS is a + # mistake in the invocation, and pytest reports a UsageError as such + # instead of as an INTERNALERROR traceback through the plugin manager. + try: + tdfs.configure_forced_supports() + except ValueError as e: + raise pytest.UsageError(str(e)) from e + if not config.getoption("--bench", default=False): return # Parallel workers contend for the CPU the benchmark is measuring, which diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 8f3bf8e0..159eda53 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -226,26 +226,67 @@ def _parse_forced_supports(raw: str) -> frozenset[str]: return frozenset(names) -#: Features to treat as supported no matter what the SDK reports. +#: Resolved ``XT_FORCE_SUPPORTS``, or None until something asks for it. #: -#: The ``supports`` case statements live in this repo (``sdk/*/cli.sh``) and -#: answer from a *released* version number, so they say "no" for precisely the -#: unreleased builds a fix needs to be evaluated against. Setting -#: ``XT_FORCE_SUPPORTS=chunky`` alongside ``otdf-sdk-mgr install tip --ref ...`` -#: makes those cells run for real and report pass or fail. -#: -#: Applies to every SDK in the run. To force a feature for one side only, narrow -#: the run with ``--sdks-encrypt`` / ``--sdks-decrypt`` rather than adding -#: per-SDK syntax here. -FORCED_SUPPORTS = _parse_forced_supports(os.environ.get("XT_FORCE_SUPPORTS", "")) - -if FORCED_SUPPORTS: - logger.warning( - "XT_FORCE_SUPPORTS is set: treating %s as supported by every SDK. " - "Results for those features reflect the build under test, not the " - "shim's version gate.", - ", ".join(sorted(FORCED_SUPPORTS)), - ) +#: Deliberately not populated at import. See :func:`configure_forced_supports`. +_forced_supports: frozenset[str] | None = None + + +def configure_forced_supports(raw: str | None = None) -> frozenset[str]: + """Resolve ``XT_FORCE_SUPPORTS`` into the set :func:`forced_supports` returns. + + Features to treat as supported no matter what the SDK reports. + + The ``supports`` case statements live in this repo (``sdk/*/cli.sh``) and + answer from a *released* version number, so they say "no" for precisely the + unreleased builds a fix needs to be evaluated against. Setting + ``XT_FORCE_SUPPORTS=chunky`` alongside ``otdf-sdk-mgr install tip --ref ...`` + makes those cells run for real and report pass or fail. + + Applies to every SDK in the run. To force a feature for one side only, + narrow the run with ``--sdks-encrypt`` / ``--sdks-decrypt`` rather than + adding per-SDK syntax here. + + Called from ``conftest.pytest_configure`` rather than evaluated at module + import. The parse validates every name against the known feature set and + raises on an unknown one -- correctly, see :func:`_parse_forced_supports` -- + which means the set of legal names is frozen at whatever moment the parse + runs. At import that is before ``pytest_addoption``, before + ``pytest_configure`` and before any plugin has had a chance to contribute a + feature, so the strictness and the extensibility were in direct conflict. + Running it from ``pytest_configure`` puts the validation after plugin + discovery and keeps both. + + Idempotent, so a second call (an xdist worker configuring itself, a test + exercising the parse) simply re-resolves. + """ + global _forced_supports + if raw is None: + raw = os.environ.get("XT_FORCE_SUPPORTS", "") + forced = _parse_forced_supports(raw) + _forced_supports = forced + if forced: + logger.warning( + "XT_FORCE_SUPPORTS is set: treating %s as supported by every SDK. " + "Results for those features reflect the build under test, not the " + "shim's version gate.", + ", ".join(sorted(forced)), + ) + return forced + + +def forced_supports() -> frozenset[str]: + """The features this session forces on, resolving from the environment once. + + The lazy fallback matters: ``tdfs`` is importable outside a pytest session + -- ``otdf-sdk-mgr`` and ad-hoc scripts both do it -- and those callers never + run ``pytest_configure``. Without it, moving the parse would silently turn + ``XT_FORCE_SUPPORTS`` into a no-op for them, which is the exact class of + quiet failure the variable exists to escape. + """ + if _forced_supports is None: + return configure_forced_supports() + return _forced_supports container_version = Literal["4.2.2", "4.3.0"] @@ -879,7 +920,7 @@ def decrypt( ) def supports(self, feature: feature_type) -> bool: - if feature in FORCED_SUPPORTS: + if feature in forced_supports(): return True if feature in self._supports: return self._supports[feature] @@ -916,7 +957,7 @@ def _uncached_supports(self, feature: feature_type) -> bool: # happen by itself when the SDK merges a patch. # # To evaluate a fix before it releases, set - # XT_FORCE_SUPPORTS=chunky -- see FORCED_SUPPORTS above. + # XT_FORCE_SUPPORTS=chunky -- see configure_forced_supports above. return True case ("better-messages-2024", ("js" | "java")): return True @@ -1025,7 +1066,8 @@ def skip_chunky_skew(ct_file: Path, decrypt_sdk: SDK): To evaluate an unreleased fix, set ``XT_FORCE_SUPPORTS=chunky`` (or pass ``force-supports: chunky`` to the workflow dispatch) so this returns early - and the cell reports a real pass or fail. See :data:`FORCED_SUPPORTS`. + and the cell reports a real pass or fail. See + :func:`configure_forced_supports`. """ if decrypt_sdk.supports("chunky"): return diff --git a/xtest/test_tdfs_units.py b/xtest/test_tdfs_units.py index e6d86ed7..ea05b013 100644 --- a/xtest/test_tdfs_units.py +++ b/xtest/test_tdfs_units.py @@ -13,6 +13,9 @@ import base64 import json +import os +import subprocess +import sys import zipfile from pathlib import Path from types import SimpleNamespace @@ -41,6 +44,87 @@ def test_unknown_name_raises(self): tdfs._parse_forced_supports("hexles") +# --- tdfs.configure_forced_supports / forced_supports ------------------------- + + +@pytest.fixture(autouse=True) +def _restore_forced_supports(): + """Put the module global back after any test that resolves it. + + ``forced_supports()`` caches, and a test that leaves ``chunky`` forced on + would make ``SDK.supports`` lie for every test collected after it in the + same process. + """ + saved = tdfs._forced_supports + yield + tdfs._forced_supports = saved + + +class TestForcedSupportsIsNotResolvedAtImport: + """``XT_FORCE_SUPPORTS`` must be parsed from ``pytest_configure``, not import. + + The parse rejects unknown names, so wherever it runs is the moment the set + of legal feature names freezes. At import that is before any plugin could + contribute one, which is the ordering problem DSPX-4794 removes. + """ + + def test_importing_tdfs_with_an_unknown_name_does_not_raise( + self, monkeypatch: pytest.MonkeyPatch + ): + """Checked in a subprocess, on purpose. + + The behaviour under test is what happens during module execution, and + ``tdfs`` is already in ``sys.modules`` here. Reimporting would not + re-run module scope, and ``importlib.reload`` would re-run it in an + interpreter whose state the rest of this file depends on. A fresh + interpreter is the only honest observation. + """ + env = dict(os.environ, XT_FORCE_SUPPORTS="not-a-real-feature") + r = subprocess.run( + [sys.executable, "-c", "import tdfs"], + cwd=Path(__file__).parent, + env=env, + capture_output=True, + text=True, + ) + assert r.returncode == 0, r.stderr + assert "not-a-real-feature" not in r.stderr + + # ... and the same interpreter still rejects it once something asks. + monkeypatch.setenv("XT_FORCE_SUPPORTS", "not-a-real-feature") + with pytest.raises(ValueError, match="unknown feature"): + tdfs.configure_forced_supports() + + def test_configure_reads_the_environment(self, monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("XT_FORCE_SUPPORTS", "chunky,hexless") + assert tdfs.configure_forced_supports() == frozenset({"chunky", "hexless"}) + assert tdfs.forced_supports() == frozenset({"chunky", "hexless"}) + + def test_explicit_argument_beats_the_environment( + self, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.setenv("XT_FORCE_SUPPORTS", "chunky") + assert tdfs.configure_forced_supports("dpop") == frozenset({"dpop"}) + + def test_lazy_fallback_for_callers_outside_a_pytest_session( + self, monkeypatch: pytest.MonkeyPatch + ): + """``tdfs`` is imported by scripts that never run ``pytest_configure``. + + Without the fallback, moving the parse would turn ``XT_FORCE_SUPPORTS`` + into a silent no-op for them. + """ + monkeypatch.setattr(tdfs, "_forced_supports", None) + monkeypatch.setenv("XT_FORCE_SUPPORTS", "ecwrap") + assert tdfs.forced_supports() == frozenset({"ecwrap"}) + + def test_supports_consults_the_resolved_set(self, monkeypatch: pytest.MonkeyPatch): + """The forced set has to reach ``SDK.supports`` without a subprocess.""" + monkeypatch.setattr(tdfs, "_forced_supports", frozenset({"chunky"})) + sdk = cast(tdfs.SDK, SimpleNamespace(_supports={}, sdk="go", version="main")) + assert tdfs.SDK.supports(sdk, "chunky") is True + + # --- tdfs.zip64_reader_is_broken ---------------------------------------------- From aec96cbd02e1851f90aff46171e545f08d307f88 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 11:03:26 -0400 Subject: [PATCH 4/7] fix(xtest): default the SDK matrix to installed builds, and fail on an empty one conftest's SDK selection defaulted to get_args(tdfs.sdk_type) -- the set of names the suite knows about rather than the set of builds present on disk. parse_sdk_spec routes a bare name through all_versions_of anyway, so the two agreed; ask the dist tree directly, which is the question that was being asked and which keeps agreeing when the name set stops being a Literal. The behaviour change is the empty case. metafunc.parametrize over [] does not collect zero items: empty_parameter_set_mark turns it into one skip per test, so a checkout with nothing installed reported "20 skipped ... got empty parameter set" and exited 0. Measured on a clean tree before this change. That is the same vacuous-green hole sizes_opt_type already guards against a few functions up, so the default path now raises a UsageError naming `otdf-sdk-mgr install`. An explicit --sdks that narrows to nothing is left alone -- that is the caller's own doing. all_versions_of now sorts, so parameter ids and fixtures/bench.py's heads[0] tie-break no longer depend on readdir order. --- .github/workflows/check.yml | 1 + xtest/conftest.py | 76 ++++++++++++------ xtest/tdfs.py | 21 ++++- xtest/test_conftest_units.py | 146 +++++++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 25 deletions(-) create mode 100644 xtest/test_conftest_units.py diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 31b7d516..71117ea5 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -49,6 +49,7 @@ jobs: test_bench_stats.py test_bench_measure.py test_bench_runner.py test_bench_arms.py test_sdk_commands.py test_tdfs_units.py test_encryption_units.py test_sizes_units.py test_zip64_units.py + test_conftest_units.py working-directory: xtest - name: Lint and test otdf-local run: | diff --git a/xtest/conftest.py b/xtest/conftest.py index 9dddaa92..85ca0962 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -316,6 +316,56 @@ def _add_benchmark_options(parser: pytest.Parser): ) +def resolve_sdks( + config: pytest.Config, option_names: list[str], role: str +) -> list[tdfs.SDK]: + """SDK builds for one side of the matrix. + + The first option in ``option_names`` that was given wins; otherwise the + default is every build actually installed under ``sdk/*/dist/``. + + That default used to be ``get_args(tdfs.sdk_type)`` -- the set of names the + suite knows about rather than the set of builds present. + :func:`tdfs.parse_sdk_spec` routes a bare name through + :func:`tdfs.all_versions_of` anyway, so the two agreed; they stop agreeing + the moment anything other than the ``Literal`` can contribute a name, and + "what is installed" was always the question being asked. + + The empty case is an error rather than an empty parametrization, and only + on the default path. ``metafunc.parametrize`` over ``[]`` does not collect + zero items: pytest's ``empty_parameter_set_mark`` turns it into one *skip* + per test, so a checkout with nothing installed reports "20 skipped ... got + empty parameter set" and exits 0. A whole matrix disappears and the run + stays green -- the same failure mode :func:`sizes_opt_type` guards against + a few functions up. + + An explicit ``--sdks`` that resolves to nothing is left alone: that is the + caller narrowing the run on purpose, possibly from a script, and is not + this function's to second-guess. + """ + for name in option_names: + v = config.getoption(name) + if v: + try: + return [ + sdk for spec in str(v).split() for sdk in tdfs.parse_sdk_spec(spec) + ] + except (FileNotFoundError, ValueError) as e: + raise pytest.UsageError(str(e)) from e + try: + installed = tdfs.installed_sdks() + except FileNotFoundError as e: + raise pytest.UsageError(str(e)) from e + if not installed: + raise pytest.UsageError( + f"no SDK builds are installed under sdk/*/dist/, so the {role} side " + "of the matrix is empty; every cell would report as a skip and the " + "run would exit 0. Install some (otdf-sdk-mgr install stable) or " + f"name them explicitly with {' / '.join(option_names)}." + ) + return installed + + def pytest_generate_tests(metafunc: pytest.Metafunc): """Dynamically parametrize test functions based on CLI options. @@ -342,36 +392,14 @@ def list_opt(name: str, t: typing.Any) -> list[str]: raise ValueError(f"Invalid value for {name}: {i}, must be one of {ttt}") return a - def sdk_specs_opt(names: list[str]) -> list[str]: - """Return SDK specifier tokens from the first matching option, or all sdk types.""" - for name in names: - v = metafunc.config.getoption(name) - if v: - return v.split() - return list(typing.get_args(tdfs.sdk_type)) - subject_sdks: set[tdfs.SDK] = set() if "encrypt_sdk" in metafunc.fixturenames: - try: - e_sdks = [ - sdk - for spec in sdk_specs_opt(["--sdks-encrypt", "--sdks"]) - for sdk in tdfs.parse_sdk_spec(spec) - ] - except (FileNotFoundError, ValueError) as e: - raise pytest.UsageError(str(e)) from e + e_sdks = resolve_sdks(metafunc.config, ["--sdks-encrypt", "--sdks"], "encrypt") metafunc.parametrize("encrypt_sdk", e_sdks, ids=[str(x) for x in e_sdks]) subject_sdks |= set(e_sdks) if "decrypt_sdk" in metafunc.fixturenames: - try: - d_sdks = [ - sdk - for spec in sdk_specs_opt(["--sdks-decrypt", "--sdks"]) - for sdk in tdfs.parse_sdk_spec(spec) - ] - except (FileNotFoundError, ValueError) as e: - raise pytest.UsageError(str(e)) from e + d_sdks = resolve_sdks(metafunc.config, ["--sdks-decrypt", "--sdks"], "decrypt") metafunc.parametrize("decrypt_sdk", d_sdks, ids=[str(x) for x in d_sdks]) subject_sdks |= set(d_sdks) diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 159eda53..2e0e5e65 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -986,16 +986,35 @@ def _uncached_supports(self, feature: feature_type) -> bool: def all_versions_of(sdk: sdk_type) -> list[SDK]: + """Every installed build of one SDK, in a stable order. + + Sorted by version name because ``os.listdir`` is not ordered: the result + becomes pytest parameter ids, and ``fixtures/bench.py`` breaks a tie + between branch builds with ``heads[0]``. Neither should depend on the + order a filesystem happened to hand back. + """ sdk_path = os.path.join("sdk", sdk, "dist") if not os.path.isdir(sdk_path): return [] return [ SDK(sdk, version) - for version in os.listdir(sdk_path) + for version in sorted(os.listdir(sdk_path)) if os.path.isdir(os.path.join(sdk_path, version)) ] +def installed_sdks() -> list[SDK]: + """Every SDK build present under ``sdk//dist//``. + + The default subject set for a run, and the answer to "what is actually on + this machine" rather than "what names does the suite know about". Those + two questions had the same answer while :data:`sdk_type` was the only + source of SDK names; they stop having the same answer as soon as anything + can contribute one. + """ + return [sdk for name in get_args(sdk_type) for sdk in all_versions_of(name)] + + def parse_sdk_spec(spec: str) -> list[SDK]: """Parse an SDK specifier into SDK objects. diff --git a/xtest/test_conftest_units.py b/xtest/test_conftest_units.py new file mode 100644 index 00000000..302146f3 --- /dev/null +++ b/xtest/test_conftest_units.py @@ -0,0 +1,146 @@ +"""Offline tests for conftest's SDK selection (DSPX-4794). + +``resolve_sdks`` decides what the encrypt and decrypt axes of the whole matrix +fan out over, and its failure mode is silence: parametrizing over an empty list +does not collect zero items, it collects one *skip* per test and exits 0. So +the interesting assertions here are the ones about emptiness, not the ones +about the happy path. + +No platform and no real SDK -- the selector only needs a ``cli.sh`` to exist, +so these run against a stub tree in ``tmp_path``. +""" + +from pathlib import Path +from types import SimpleNamespace +from typing import Any, cast + +import pytest + +import tdfs +from conftest import resolve_sdks + + +def _config(**options: Any) -> pytest.Config: + """A stand-in exposing only the ``getoption`` resolve_sdks reads.""" + return cast( + pytest.Config, + SimpleNamespace(getoption=lambda name: options.get(name)), + ) + + +@pytest.fixture +def dist(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Install stub ``cli.sh`` files into a throwaway ``sdk/`` tree.""" + + def install(*specs: str) -> None: + for spec in specs: + sdk, version = spec.split("@", 1) + cli = tmp_path / "sdk" / sdk / "dist" / version / "cli.sh" + cli.parent.mkdir(parents=True, exist_ok=True) + cli.write_text("#!/bin/sh\nexit 0\n") + + monkeypatch.chdir(tmp_path) + return install + + +class TestDefaultsToInstalled: + def test_empty_default_is_an_error_not_an_empty_matrix(self): + """The bug this change exists to close. + + Before: 20 skipped, "got empty parameter set for (encrypt_sdk)", + exit 0 -- a green run that tested nothing. + """ + with pytest.raises(pytest.UsageError, match="otdf-sdk-mgr install"): + resolve_sdks(_config(), ["--sdks-encrypt", "--sdks"], "encrypt") + + def test_error_names_the_side_and_the_options(self): + with pytest.raises(pytest.UsageError) as e: + resolve_sdks(_config(), ["--sdks-decrypt", "--sdks"], "decrypt") + assert "decrypt side" in str(e.value) + assert "--sdks-decrypt / --sdks" in str(e.value) + + def test_default_is_every_installed_build(self, dist: Any): + dist("go@main", "go@v0.18.0", "js@main") + got = resolve_sdks(_config(), ["--sdks"], "encrypt") + assert [str(s) for s in got] == ["go@main", "go@v0.18.0", "js@main"] + + def test_default_ignores_sdks_that_are_known_but_not_installed(self, dist: Any): + """java is in ``sdk_type`` and absent from disk; it must not be selected.""" + dist("go@main") + assert [str(s) for s in resolve_sdks(_config(), ["--sdks"], "encrypt")] == [ + "go@main" + ] + + def test_order_is_stable(self, dist: Any): + """Parameter ids and bench arm tie-breaks must not depend on readdir order.""" + dist("go@v0.9.0", "go@main", "go@v0.18.0") + assert [s.version for s in tdfs.all_versions_of("go")] == [ + "main", + "v0.18.0", + "v0.9.0", + ] + + +class TestExplicitOptions: + def test_first_matching_option_wins(self, dist: Any): + dist("go@main", "js@main") + got = resolve_sdks( + _config(**{"--sdks-encrypt": "js@main", "--sdks": "go@main"}), + ["--sdks-encrypt", "--sdks"], + "encrypt", + ) + assert [str(s) for s in got] == ["js@main"] + + def test_falls_through_to_the_shared_option(self, dist: Any): + dist("go@main", "js@main") + got = resolve_sdks( + _config(**{"--sdks": "go@main"}), ["--sdks-encrypt", "--sdks"], "encrypt" + ) + assert [str(s) for s in got] == ["go@main"] + + def test_star_expands_to_every_version_of_that_sdk(self, dist: Any): + dist("go@main", "go@v0.18.0", "js@main") + got = resolve_sdks(_config(**{"--sdks": "go@*"}), ["--sdks"], "encrypt") + assert [str(s) for s in got] == ["go@main", "go@v0.18.0"] + + def test_an_explicit_narrowing_to_nothing_is_left_alone(self, dist: Any): + """Deliberately *not* an error. + + ``--sdks go`` with no go installed may be a scripted narrowing of a + larger matrix. The guard is on the default path only, where an empty + result means the harness was never provisioned. + """ + dist("js@main") + assert resolve_sdks(_config(**{"--sdks": "go"}), ["--sdks"], "encrypt") == [] + + def test_unknown_sdk_name_is_a_usage_error(self, dist: Any): + dist("go@main") + with pytest.raises(pytest.UsageError, match="Unknown SDK type"): + resolve_sdks(_config(**{"--sdks": "rust"}), ["--sdks"], "encrypt") + + def test_missing_build_is_a_usage_error_not_a_traceback(self, dist: Any): + dist("go@main") + with pytest.raises(pytest.UsageError, match="SDK executable not found"): + resolve_sdks(_config(**{"--sdks": "go@v0.1.0"}), ["--sdks"], "encrypt") + + +class TestInstalledSdks: + def test_reports_nothing_when_the_dist_tree_is_absent( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(tmp_path) + assert tdfs.installed_sdks() == [] + + def test_a_half_installed_build_still_raises( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + """A dist directory with no ``cli.sh`` is a broken install, not an absence. + + Worth pinning separately: this is the one path that already produced a + ``FileNotFoundError``, and it must keep doing so rather than being + swallowed into "nothing installed". + """ + (tmp_path / "sdk" / "go" / "dist" / "main").mkdir(parents=True) + monkeypatch.chdir(tmp_path) + with pytest.raises(FileNotFoundError): + tdfs.installed_sdks() From a4dc5f7e71e728ab7da77fc8b9cb1a6aabccee9d Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 11:10:50 -0400 Subject: [PATCH 5/7] feat(xtest): entry-point registries for SDKs, containers and features xtest is consumed by four repos -- opentdf/platform, web-sdk, java-sdk and otdfctl -- and today every one of them has to patch the xtest source tree to add an SDK build or a container format. The three closed `Literal` unions in tdfs.py are the reason: sdk_type, container_type and feature_type are the single source of truth for the parametrized matrix, so a new name is a diff against this repo rather than something a consumer can supply. This adds xtest/registry.py: a small typed registry over three named entry-point groups (otdf.adapters, otdf.containers, otdf.installers), seeded with today's Literal values as built-in defaults. It is discovered once from pytest_configure and is not yet wired into the matrix -- nothing that runs today changes behaviour. Static typing survives the open set, which is the part worth reviewing: the Literals stay authoritative for in-tree code, parametrized values widen to str only where they cross the registry boundary, typo detection moves from import time to collection time with a better message, and the built-in tuples are pinned against get_args() by an offline test so the two copies cannot drift. Literal[...] | str was rejected: pyright collapses it to str and the checking is lost everywhere, not just at the boundary. The acceptance gate is test_registry_units.py::TestOutOfTreePlugin, which builds a real .dist-info with entry_points.txt on sys.path and shows an out-of-tree plugin contributing a container and an SDK without touching the xtest source tree. registry.py deliberately does not import tdfs, so the built-in tuples are duplicated rather than derived; the pin test is what keeps them honest. --- .github/workflows/check.yml | 2 +- xtest/conftest.py | 7 + xtest/pyproject.toml | 1 + xtest/registry.py | 394 +++++++++++++++++++++++++++++++++++ xtest/tdfs.py | 8 +- xtest/test_registry_units.py | 361 ++++++++++++++++++++++++++++++++ 6 files changed, 771 insertions(+), 2 deletions(-) create mode 100644 xtest/registry.py create mode 100644 xtest/test_registry_units.py diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 71117ea5..ebbef161 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -49,7 +49,7 @@ jobs: test_bench_stats.py test_bench_measure.py test_bench_runner.py test_bench_arms.py test_sdk_commands.py test_tdfs_units.py test_encryption_units.py test_sizes_units.py test_zip64_units.py - test_conftest_units.py + test_conftest_units.py test_registry_units.py working-directory: xtest - name: Lint and test otdf-local run: | diff --git a/xtest/conftest.py b/xtest/conftest.py index 85ca0962..d1dc0863 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -25,6 +25,7 @@ import pytest +import registry import sizes import tdfs from otdfctl import OpentdfCommandLineTool @@ -454,6 +455,12 @@ def _parametrize_bench_cells(metafunc: pytest.Metafunc): def pytest_configure(config: pytest.Config): + # Entry-point discovery first: everything below validates names against + # the registries, and pytest_configure is the only hook late enough for + # plugins to be importable and early enough to precede + # pytest_generate_tests, where the names become parameters. + registry.load_all() + # Resolve XT_FORCE_SUPPORTS here rather than at tdfs import. The parse # rejects unknown names, so wherever it runs is the moment the set of legal # feature names freezes; at import that is before any plugin could have diff --git a/xtest/pyproject.toml b/xtest/pyproject.toml index d2f13502..c868ae8a 100644 --- a/xtest/pyproject.toml +++ b/xtest/pyproject.toml @@ -88,6 +88,7 @@ known-first-party = [ "assertions", "fixtures", "perf", + "registry", "sizes", "zipinspect", ] diff --git a/xtest/registry.py b/xtest/registry.py new file mode 100644 index 00000000..c040b927 --- /dev/null +++ b/xtest/registry.py @@ -0,0 +1,394 @@ +"""Entry-point registries for SDKs, container formats and feature gates. + +xtest is pinned at ``@main`` by four repos -- ``opentdf/platform``, +``web-sdk``, ``java-sdk`` and ``otdfctl`` -- and none of them can extend it. +What the suite is allowed to test is decided by three closed ``Literal``\\ s in +``tdfs.py``: ``sdk_type``, ``container_type`` and ``feature_type``. Adding an +SDK, a container format or a capability gate means editing this repo. + +Upstream has already paid for that once. NanoTDF was removed in ``150e3135`` +("fix: remove NanoTDF tests and support (#366)") -- 11 files, +22/-2003 -- and +``otdf-sdk-mgr/tests/test_schema.py::test_removed_nano_container_is_rejected`` +now exists to keep a *second* hand-maintained copy of the container enum +(``otdf_sdk_mgr.schema.ContainerKind``) in step with the first. A format that +was never more than an enum value cost two thousand lines to remove, because +the value was load-bearing everywhere instead of confined to one object. + +This module is the seam. Today's ``Literal`` values stay exactly where they +are, as the built-in defaults; anything installed alongside xtest can add to +them by declaring an entry point. + +Keeping static typing useful once the set is open +------------------------------------------------- + +The ``Literal``\\ s are not widened to ``str`` and not widened to +``Literal[...] | str`` (pyright collapses that union to ``str`` and silently +stops diagnosing). Four mechanisms replace the one: + +1. The ``Literal``\\ s stay authoritative for in-tree code, so a literal typo + is still a type error and deleting a built-in still breaks every mention of + it at once. +2. Parameters carrying a *parametrized* value -- whose contents come from + ``metafunc.parametrize`` over a runtime set -- widen to plain ``str``, + which is the type they always really had. +3. The typo check for names arriving from outside moves to collection time: + :meth:`Registry.get` raises :class:`UnknownName` listing every registered + name, before a single test runs. +4. The built-in tuples below are pinned against ``get_args()`` of the + corresponding ``Literal`` by ``test_registry_units.py``. + +That last one is load-bearing. This module deliberately does **not** import +``tdfs`` -- that is what lets ``tdfs`` import *it* without a cycle -- so the +built-in names are written out twice. Without the pin test, that duplication +is a second ``ContainerKind``, i.e. exactly the defect being removed here. If +the pin test is ever deleted, collapse the duplication in the same change. +""" + +from __future__ import annotations + +import logging +from collections.abc import Iterator +from dataclasses import dataclass, field +from enum import StrEnum +from importlib.metadata import EntryPoint, entry_points +from pathlib import Path +from typing import Any, Protocol, runtime_checkable + +logger = logging.getLogger("xtest") + +#: Entry-point group for SDK adapters. An entry registers an SDK name. +GROUP_ADAPTERS = "otdf.adapters" + +#: Entry-point group for container-format adapters. An entry registers a +#: container name. +GROUP_CONTAINERS = "otdf.containers" + +#: Entry-point group for SDK installers -- how an out-of-tree SDK becomes +#: materialisable under ``sdk//dist//``. Reserved here so the +#: three groups are designed together; its consumer is ``otdf-sdk-mgr``, not +#: xtest, and wiring it is a separate change. +GROUP_INSTALLERS = "otdf.installers" + +#: Mirrors ``tdfs.sdk_type``. Pinned by ``test_registry_units.py``. +BUILTIN_SDKS: tuple[str, ...] = ("go", "java", "js") + +#: Mirrors ``tdfs.container_type``. Pinned by ``test_registry_units.py``. +BUILTIN_CONTAINERS: tuple[str, ...] = ("ztdf", "ztdf-ecwrap") + +#: Mirrors ``tdfs.feature_type``. Pinned by ``test_registry_units.py``. +#: +#: Order follows the ``Literal`` so a diff against it reads cleanly. The +#: rationale for each name lives on the ``Literal``; this is a name list, not +#: a second place to document them. +BUILTIN_FEATURES: tuple[str, ...] = ( + "assertions", + "assertion_verification", + "attribute_traversal", + "audit_logging", + "autoconfigure", + "better-messages-2024", + "bulk_rewrap", + "chunky", + "connectrpc", + "dpop", + "dpop_nonce_challenge", + "ecwrap", + "gmac_root_rejected", + "hexless", + "hexaflexible", + "kasallowlist", + "key_management", + "mechanism-rsa-4096", + "mechanism-ec-curves-384-521", + "mechanism-xwing", + "mechanism-secpmlkem", + "mechanism-mlkem", + "multikao", + "ns_grants", + "obligations", + "zip64-at-2gib", +) + + +class UnknownName(LookupError): + """A name that no built-in and no plugin registered.""" + + +class DuplicateName(RuntimeError): + """Two registrations claimed the same name.""" + + +class PluginLoadError(RuntimeError): + """An entry point was declared but could not be imported.""" + + +class UnsupportedMutation(NotImplementedError): + """This container format cannot express the requested tamper.""" + + +# --- what a plugin provides --------------------------------------------------- + + +@runtime_checkable +class Extension(Protocol): + """Common surface of anything reachable through one of the groups above. + + ``features`` is how :func:`feature_names` widens: a feature name is + contributed by the adapter or container that answers to it, rather than + coming from a fourth entry-point group of its own. That way a feature + cannot exist with nothing behind it, and ``XT_FORCE_SUPPORTS`` cannot be + handed a name that nothing will ever report on. + """ + + name: str + features: frozenset[str] + + +@dataclass(frozen=True) +class KeyAccessRecord: + """One key-access object, in terms every container format can answer.""" + + kas_url: str + kid: str | None = None + split_id: str | None = None + wrap_algorithm: str | None = None + has_ephemeral_key: bool = False + + +@dataclass(frozen=True) +class Inspection: + """What a test may ask about a container without knowing its encoding. + + Every field is answerable by any format that binds a policy to a wrapped + key and hands it to a KAS. None presumes a ZIP, a JSON manifest, or a + named entry -- which is what ``test_tdfs.py`` presumes today when it calls + ``tdfs.manifest()`` one line after encrypt, inside a test parametrized over + ``container``. + + ``raw`` is typed ``object`` on purpose. A conveniently typed escape hatch + would quietly re-close the seam; as it stands, a test that wants the ztdf + ``Manifest`` has to reach for ``tdfs.manifest()`` and thereby declare + itself ztdf-only. + """ + + container: str + total_size: int + ciphertext_size: int + encrypted: bool + key_access_mode: str + key_access: tuple[KeyAccessRecord, ...] = () + mime_type: str | None = None + policy_present: bool = False + policy_attribute_count: int = 0 + spec_version: str | None = None + raw: object = None + + +class Mutation(StrEnum): + """The tamper vocabulary, as properties of the attack rather than the encoding. + + Derived from the 23 ``update_manifest`` / ``update_payload`` call sites in + ``test_tdfs.py``, ``test_root_signature.py`` and + ``test_audit_logs_integration.py``, not invented. "Replace the policy with + one the binding does not cover" is meaningful for any container that binds + a policy; "edit ``0.manifest.json`` inside the ZIP" is not. + """ + + UNBIND_POLICY = "unbind_policy" + ALTER_POLICY_BINDING = "alter_policy_binding" + ALTER_ROOT_SIGNATURE = "alter_root_signature" + FORGE_GMAC_ROOT = "forge_gmac_root" + ALTER_SEGMENT_HASH = "alter_segment_hash" + ALTER_SEGMENT_SIZE = "alter_segment_size" + ALTER_PAYLOAD_TAIL = "alter_payload_tail" + ALTER_ASSERTION = "alter_assertion" + MALICIOUS_KAO = "malicious_kao" + DUPLICATE_KAO = "duplicate_kao" + + +@runtime_checkable +class ContainerAdapter(Extension, Protocol): + """Everything the suite needs to know about a container format. + + ``wire_format`` is what the CLI is handed. ``ztdf-ecwrap`` maps to + ``ztdf``: it is a variant rather than a distinct format, and carries its + difference in a flag. That collapse is ``tdfs.simple_container()`` today, + and it is the in-tree precedent for this whole protocol. + + ``requires_attributes()`` exists because a format whose encrypt path + demands at least one attribute has no no-attribute roundtrip cell -- and + the no-attribute roundtrip is xtest's *default*. A format that answers + ``True`` gets those cells skipped with a reason instead of failing on an + unhelpful CLI usage error. + """ + + wire_format: str + + def inspect(self, path: Path) -> Inspection: ... + + def tamper(self, path: Path, mutation: Mutation) -> Path: + """Produce a tampered copy, or raise :class:`UnsupportedMutation`.""" + ... + + def requires_attributes(self) -> bool: ... + + +@runtime_checkable +class SdkProvider(Extension, Protocol): + """An SDK name the matrix may fan out over. + + The object on the other end of an ``otdf.adapters`` entry point is the + typed SDK adapter defined by the adapter ticket. This registry only needs + the name and the contributed features; it deliberately does not pin the + rest of that protocol here. + """ + + def versions(self) -> tuple[str, ...]: + """Installed versions of this SDK, newest-agnostic, in a stable order.""" + ... + + +@runtime_checkable +class Installer(Protocol): + """Materialises one version of an SDK under a dist directory.""" + + name: str + + def __call__(self, version: str, dest: Path) -> None: ... + + +# --- the registry ------------------------------------------------------------- + + +@dataclass +class Registry[T]: + """A name -> object map seeded with built-ins and widened by entry points. + + A built-in may be registered with ``None`` until something supplies the + object: the *name* set is what the parametrizers and validators need, and + it is useful before any adapter exists. That is what lets this module land + as a pure addition. + """ + + group: str + builtins: tuple[str, ...] + _entries: dict[str, T | None] = field(default_factory=dict, init=False) + _loaded: bool = field(default=False, init=False) + + def __post_init__(self) -> None: + self.reset() + + def reset(self) -> None: + """Drop plugin registrations and forget that discovery ran. + + For tests. Entry-point discovery is otherwise once per process. + """ + self._entries = dict.fromkeys(self.builtins) + self._loaded = False + + def register(self, name: str, obj: T | None) -> None: + if name in self._entries: + raise DuplicateName( + f"{self.group}: {name!r} is already registered. A plugin may " + "not shadow a built-in or another plugin -- otherwise " + f"--containers/--sdks would mean different things depending on " + "what happens to be installed, which is unauditable from a CI log." + ) + self._entries[name] = obj + + def load(self) -> None: + """Discover and register everything declared under :attr:`group`. + + Idempotent: each xdist worker configures itself, and a second scan + would raise :class:`DuplicateName` on every plugin. + + A load failure raises. ``entry_points()`` only reads metadata, so a + broken plugin is not discovered until ``ep.load()`` executes its + module; swallowing that would produce a run that quietly tests less + than it was asked to, with no non-zero exit anywhere, because the + missing container simply yields fewer cells. + """ + if self._loaded: + return + self._loaded = True + for ep in entry_points(group=self.group): + self.register(ep.name, _load_entry_point(ep)) + logger.info("registered %s %r from %s", self.group, ep.name, ep.value) + + def names(self) -> tuple[str, ...]: + """Registered names: built-ins in declaration order, then plugins.""" + return tuple(self._entries) + + def get(self, name: str) -> T: + try: + obj = self._entries[name] + except KeyError: + raise UnknownName( + f"unknown {self.group} name {name!r}; " + f"registered: {', '.join(sorted(self._entries))}" + ) from None + if obj is None: + raise UnknownName( + f"{self.group} name {name!r} is registered but has no " + "implementation yet" + ) + return obj + + def objects(self) -> tuple[T, ...]: + """Every registered implementation, skipping names with none yet.""" + return tuple(o for o in self._entries.values() if o is not None) + + def __contains__(self, name: str) -> bool: + return name in self._entries + + def __iter__(self) -> Iterator[str]: + return iter(self._entries) + + +def _load_entry_point(ep: EntryPoint) -> Any: + try: + return ep.load() + except Exception as e: + raise PluginLoadError( + f"entry point {ep.name!r} in group {ep.group!r} " + f"({ep.value}) could not be loaded: {e}" + ) from e + + +SDKS: Registry[SdkProvider] = Registry(GROUP_ADAPTERS, BUILTIN_SDKS) +CONTAINERS: Registry[ContainerAdapter] = Registry(GROUP_CONTAINERS, BUILTIN_CONTAINERS) +INSTALLERS: Registry[Installer] = Registry(GROUP_INSTALLERS, ()) + + +def load_all() -> None: + """Run entry-point discovery for every group. Called from ``pytest_configure``. + + ``pytest_configure`` is the only hook late enough for plugins to have been + imported and early enough to precede ``pytest_generate_tests``, which is + where the names turn into parameters. + """ + for r in (SDKS, CONTAINERS, INSTALLERS): + r.load() + + +def reset_all() -> None: + """Undo :func:`load_all`. For tests.""" + for r in (SDKS, CONTAINERS, INSTALLERS): + r.reset() + + +def sdk_names() -> tuple[str, ...]: + return SDKS.names() + + +def container_names() -> tuple[str, ...]: + return CONTAINERS.names() + + +def feature_names() -> frozenset[str]: + """Built-in feature names plus every one contributed by a loaded plugin.""" + names = set(BUILTIN_FEATURES) + for registry in (SDKS, CONTAINERS): + for obj in registry.objects(): + names |= set(getattr(obj, "features", ())) + return frozenset(names) diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 2e0e5e65..33621dcc 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -17,6 +17,7 @@ from pydantic import BaseModel import assertions as tdfassertions +import registry logger = logging.getLogger("xtest") logging.basicConfig() @@ -216,7 +217,12 @@ def _parse_forced_supports(raw: str) -> frozenset[str]: exact failure mode the override is meant to escape. """ names = {n.strip() for n in raw.split(",") if n.strip()} - known = set(get_args(feature_type)) + # The registry, not ``get_args(feature_type)``: a feature contributed by a + # plugin is as real as a built-in one, and the whole reason this parse + # moved out of module scope was so it could see them. ``feature_names()`` + # is seeded from the ``Literal`` above, so with nothing installed the two + # are the same set. + known = set(registry.feature_names()) unknown = names - known if unknown: raise ValueError( diff --git a/xtest/test_registry_units.py b/xtest/test_registry_units.py new file mode 100644 index 00000000..765a6fb7 --- /dev/null +++ b/xtest/test_registry_units.py @@ -0,0 +1,361 @@ +"""Offline tests for the entry-point registries (DSPX-4794). + +Two jobs. + +**Pin today's extension-point surface.** ``registry.py`` deliberately does not +import ``tdfs`` -- that acyclicity is what lets ``tdfs`` import it -- so the +built-in name lists exist twice. The pin tests are the only thing keeping the +copies honest, and they double as the record of what the closed sets contained +before any of this: a future widening has to edit a test that spells out +today's values. + +**Prove the seam actually works.** The acceptance gate for this ticket is a +throwaway out-of-tree plugin being discovered without touching the xtest source +tree. ``_install_plugin`` therefore builds a real ``.dist-info`` on ``sys.path`` +and lets ``importlib.metadata`` find it. Monkeypatching ``registry.CONTAINERS`` +would pass while proving nothing about the mechanism consumers would actually +use. + +No platform, no SDK, no subprocess. +""" + +import importlib +import sys +import textwrap +from collections.abc import Iterator +from pathlib import Path +from typing import Any, get_args + +import pytest + +import registry +import tdfs + + +@pytest.fixture(autouse=True) +def _clean_registries() -> Iterator[None]: + """Every test starts from the built-ins and only the built-ins.""" + registry.reset_all() + yield + registry.reset_all() + + +# --- the pin ------------------------------------------------------------------- + + +class TestBuiltinsMatchTheLiterals: + """The duplication in ``registry.py`` cannot be allowed to drift. + + Each assertion here is also the written-down form of one closed set, so + widening it is visible in a diff rather than implicit in a Literal edit. + """ + + def test_sdks(self): + assert registry.BUILTIN_SDKS == get_args(tdfs.sdk_type) + assert registry.BUILTIN_SDKS == ("go", "java", "js") + + def test_containers(self): + assert registry.BUILTIN_CONTAINERS == get_args(tdfs.container_type) + assert registry.BUILTIN_CONTAINERS == ("ztdf", "ztdf-ecwrap") + + def test_features(self): + assert registry.BUILTIN_FEATURES == get_args(tdfs.feature_type) + + def test_focus_is_derived_and_needs_no_registry_of_its_own(self): + """``focus_type`` is ``Literal[sdk_type, "all"]``, so it widens for free.""" + assert set(get_args(tdfs.focus_type)) == set(registry.BUILTIN_SDKS) | {"all"} + + def test_group_names(self): + assert registry.GROUP_ADAPTERS == "otdf.adapters" + assert registry.GROUP_CONTAINERS == "otdf.containers" + assert registry.GROUP_INSTALLERS == "otdf.installers" + + +# --- registry mechanics -------------------------------------------------------- + + +class _FakeContainer: + """An in-process stand-in, for the mechanics that do not need a real dist.""" + + name = "acme" + wire_format = "acme" + features = frozenset({"acme-sealed"}) + + def inspect(self, path: Path) -> registry.Inspection: + raise NotImplementedError + + def tamper(self, path: Path, mutation: registry.Mutation) -> Path: + raise registry.UnsupportedMutation(mutation) + + def requires_attributes(self) -> bool: + return True + + +class TestRegistry: + def test_builtins_are_registered_before_any_discovery(self): + assert registry.container_names() == ("ztdf", "ztdf-ecwrap") + assert registry.sdk_names() == ("go", "java", "js") + + def test_a_builtin_without_an_implementation_is_still_a_known_name(self): + """What makes this module landable ahead of the adapters. + + The parametrizers and validators need the *names*; the objects arrive + with the container-adapter change. + """ + assert "ztdf" in registry.CONTAINERS + with pytest.raises(registry.UnknownName, match="no implementation yet"): + registry.CONTAINERS.get("ztdf") + + def test_unknown_name_lists_the_alternatives(self): + with pytest.raises(registry.UnknownName) as e: + registry.CONTAINERS.get("ztfd") + assert "ztdf" in str(e.value) + assert "ztdf-ecwrap" in str(e.value) + + def test_a_plugin_may_not_shadow_a_builtin(self): + """Silent override would make --containers ztdf mean whatever is installed.""" + with pytest.raises(registry.DuplicateName): + registry.CONTAINERS.register("ztdf", _FakeContainer()) + + def test_two_plugins_may_not_share_a_name(self): + registry.CONTAINERS.register("acme", _FakeContainer()) + with pytest.raises(registry.DuplicateName): + registry.CONTAINERS.register("acme", _FakeContainer()) + + def test_load_is_idempotent(self): + """Each xdist worker configures itself; a second scan must not re-register.""" + registry.load_all() + before = registry.container_names() + registry.load_all() + assert registry.container_names() == before + + def test_feature_names_start_as_the_builtins(self): + assert registry.feature_names() == frozenset(registry.BUILTIN_FEATURES) + + def test_feature_names_pick_up_a_plugins_declaration(self): + registry.CONTAINERS.register("acme", _FakeContainer()) + assert "acme-sealed" in registry.feature_names() + assert frozenset(registry.BUILTIN_FEATURES) <= registry.feature_names() + + +# --- the acceptance gate: a real out-of-tree distribution ----------------------- + + +_PLUGIN_SOURCE = textwrap.dedent( + ''' + """A throwaway out-of-tree plugin. Knows nothing about xtest's internals.""" + + from dataclasses import dataclass + + + @dataclass + class Container: + name: str = "acme" + wire_format: str = "acme" + features: frozenset = frozenset({"acme-sealed"}) + + def inspect(self, path): + raise NotImplementedError + + def tamper(self, path, mutation): + raise NotImplementedError + + def requires_attributes(self): + return True + + + @dataclass + class Sdk: + name: str = "acme" + features: frozenset = frozenset({"acme-dialect"}) + + def versions(self): + return ("v1.0.0",) + + + CONTAINER = Container() + SDK = Sdk() + BROKEN = None + ''' +) + + +def _install_plugin( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, entry_points_txt: str +) -> None: + """Put a real installed distribution on ``sys.path``. + + A ``.dist-info`` directory with ``METADATA`` and ``entry_points.txt`` is + all ``importlib.metadata`` needs, so this exercises the same discovery + path a ``uv pip install`` would produce -- without building a wheel or + touching the environment the test session itself runs in. + """ + site = tmp_path / "site" + (site / "acme_xtest").mkdir(parents=True) + (site / "acme_xtest" / "__init__.py").write_text(_PLUGIN_SOURCE) + dist = site / "acme_xtest-0.1.0.dist-info" + dist.mkdir() + dist.joinpath("METADATA").write_text( + "Metadata-Version: 2.1\nName: acme-xtest\nVersion: 0.1.0\n" + ) + dist.joinpath("entry_points.txt").write_text(entry_points_txt) + + monkeypatch.syspath_prepend(str(site)) + importlib.invalidate_caches() + monkeypatch.delitem(sys.modules, "acme_xtest", raising=False) + + +@pytest.fixture +def plugin(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _install_plugin( + tmp_path, + monkeypatch, + "[otdf.containers]\n" + "acme = acme_xtest:CONTAINER\n" + "\n" + "[otdf.adapters]\n" + "acme = acme_xtest:SDK\n", + ) + + +@pytest.mark.usefixtures("plugin") +class TestOutOfTreePlugin: + """DSPX-4794's acceptance gate.""" + + def test_a_container_is_discovered_without_touching_xtest(self): + registry.load_all() + assert "acme" in registry.container_names() + assert registry.CONTAINERS.get("acme").wire_format == "acme" + + def test_an_sdk_is_discovered_without_touching_xtest(self): + registry.load_all() + assert "acme" in registry.sdk_names() + assert registry.SDKS.get("acme").versions() == ("v1.0.0",) + + def test_builtins_survive_the_widening(self): + registry.load_all() + assert set(registry.BUILTIN_CONTAINERS) < set(registry.container_names()) + assert set(registry.BUILTIN_SDKS) < set(registry.sdk_names()) + + def test_the_plugins_features_join_the_known_set(self): + registry.load_all() + assert {"acme-sealed", "acme-dialect"} <= registry.feature_names() + + def test_force_supports_accepts_a_feature_the_plugin_contributed(self): + """The ordering fix and the registry, end to end. + + Before DSPX-4794 this name was rejected, and there was no moment at + which it could have been accepted: the parse ran at ``tdfs`` import, + before any plugin existed. + """ + registry.load_all() + assert tdfs.configure_forced_supports("acme-sealed") == frozenset( + {"acme-sealed"} + ) + + def test_force_supports_still_rejects_a_typo(self): + """A validator that accepts everything is the failure mode it exists to stop.""" + registry.load_all() + with pytest.raises(ValueError, match="unknown feature"): + tdfs.configure_forced_supports("acme-seeled") + + def test_a_container_adapter_declares_itself_attribute_requiring(self): + """A format with no no-attribute roundtrip has to be able to say so. + + xtest's default cell encrypts with no attributes; today that is + unrepresentable as anything but a confusing CLI usage failure. + """ + registry.load_all() + assert registry.CONTAINERS.get("acme").requires_attributes() is True + + +class TestPluginFailuresAreLoud: + def test_an_entry_point_that_cannot_be_imported_fails_the_session( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + """Never a ``continue``. + + ``entry_points()`` only reads metadata, so a broken plugin surfaces at + ``ep.load()``. Swallowing it yields a run that quietly tests less than + it was asked to, with nothing non-zero anywhere: the missing container + simply produces fewer cells. + """ + _install_plugin( + tmp_path, + monkeypatch, + "[otdf.containers]\nacme = acme_xtest_typo:CONTAINER\n", + ) + with pytest.raises(registry.PluginLoadError) as e: + registry.load_all() + assert "otdf.containers" in str(e.value) + assert "acme" in str(e.value) + + def test_a_plugin_claiming_a_builtin_name_fails_the_session( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + _install_plugin( + tmp_path, + monkeypatch, + "[otdf.containers]\nztdf = acme_xtest:CONTAINER\n", + ) + with pytest.raises(registry.DuplicateName): + registry.load_all() + + +# --- the shapes the container seam will be built on ---------------------------- + + +class TestContractShapes: + def test_inspection_covers_every_assertion_test_tdfs_makes_after_encrypt(self): + """``test_tdfs.py:70-77`` reads a ZIP one line after encrypt. + + Those assertions -- payload encrypted, exactly one KAO, the KAO's + wrap type, and an ephemeral public key for ecwrap -- have to survive + the move behind ``ContainerAdapter.inspect``, or the seam is not a + drop-in. + """ + i = registry.Inspection( + container="ztdf", + total_size=1024, + ciphertext_size=128, + encrypted=True, + key_access_mode="ec-wrapped", + key_access=( + registry.KeyAccessRecord( + kas_url="http://localhost:8080/kas", has_ephemeral_key=True + ), + ), + ) + assert i.encrypted + assert len(i.key_access) == 1 + assert i.key_access_mode == "ec-wrapped" + assert i.key_access[0].has_ephemeral_key + + def test_raw_is_untyped_on_purpose(self): + """A typed escape hatch would quietly re-close the seam.""" + assert registry.Inspection.__annotations__["raw"] in ("object", object) + + def test_the_mutation_vocabulary_covers_todays_tamper_tests(self): + """Ten names, derived from the 23 in-tree update_manifest/update_payload sites.""" + assert {m.value for m in registry.Mutation} == { + "unbind_policy", + "alter_policy_binding", + "alter_root_signature", + "forge_gmac_root", + "alter_segment_hash", + "alter_segment_size", + "alter_payload_tail", + "alter_assertion", + "malicious_kao", + "duplicate_kao", + } + + def test_an_inexpressible_mutation_is_reportable_not_a_crash(self): + """A container that cannot express a tamper must say so. + + Today a non-ZIP container reaching ``update_manifest`` would raise + ``BadZipFile`` from three modules away. + """ + c: Any = _FakeContainer() + with pytest.raises(registry.UnsupportedMutation): + c.tamper(Path("x"), registry.Mutation.FORGE_GMAC_ROOT) From 8a66f3c1fcd5d1b51c712917483e03a1eefb2037 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 13:23:40 -0400 Subject: [PATCH 6/7] fix(xtest): validate registry plugin shapes --- xtest/registry.py | 42 +++++++++++++++++++++----- xtest/test_registry_units.py | 58 ++++++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 11 deletions(-) diff --git a/xtest/registry.py b/xtest/registry.py index c040b927..67976e01 100644 --- a/xtest/registry.py +++ b/xtest/registry.py @@ -52,7 +52,7 @@ from enum import StrEnum from importlib.metadata import EntryPoint, entry_points from pathlib import Path -from typing import Any, Protocol, runtime_checkable +from typing import Any, Protocol, cast, runtime_checkable logger = logging.getLogger("xtest") @@ -272,6 +272,7 @@ class Registry[T]: group: str builtins: tuple[str, ...] + entry_type: type[T] _entries: dict[str, T | None] = field(default_factory=dict, init=False) _loaded: bool = field(default=False, init=False) @@ -287,6 +288,10 @@ def reset(self) -> None: self._loaded = False def register(self, name: str, obj: T | None) -> None: + self._ensure_available(name) + self._entries[name] = obj + + def _ensure_available(self, name: str) -> None: if name in self._entries: raise DuplicateName( f"{self.group}: {name!r} is already registered. A plugin may " @@ -294,7 +299,6 @@ def register(self, name: str, obj: T | None) -> None: f"--containers/--sdks would mean different things depending on " "what happens to be installed, which is unauditable from a CI log." ) - self._entries[name] = obj def load(self) -> None: """Discover and register everything declared under :attr:`group`. @@ -310,10 +314,30 @@ def load(self) -> None: """ if self._loaded: return + original_entries = self._entries.copy() self._loaded = True - for ep in entry_points(group=self.group): - self.register(ep.name, _load_entry_point(ep)) - logger.info("registered %s %r from %s", self.group, ep.name, ep.value) + try: + for ep in entry_points(group=self.group): + self._ensure_available(ep.name) + obj = _load_entry_point(ep) + if not isinstance(obj, self.entry_type): + raise PluginLoadError( + f"entry point {ep.name!r} in group {ep.group!r} " + f"({ep.value}) loaded an object that does not implement " + f"{self.entry_type.__name__}" + ) + obj_name = cast(Extension | Installer, obj).name + if obj_name != ep.name: + raise PluginLoadError( + f"entry point {ep.name!r} in group {ep.group!r} " + f"({ep.value}) loaded an object whose name is {obj_name!r}" + ) + self.register(ep.name, obj) + logger.info("registered %s %r from %s", self.group, ep.name, ep.value) + except Exception: + self._entries = original_entries + self._loaded = False + raise def names(self) -> tuple[str, ...]: """Registered names: built-ins in declaration order, then plugins.""" @@ -355,9 +379,11 @@ def _load_entry_point(ep: EntryPoint) -> Any: ) from e -SDKS: Registry[SdkProvider] = Registry(GROUP_ADAPTERS, BUILTIN_SDKS) -CONTAINERS: Registry[ContainerAdapter] = Registry(GROUP_CONTAINERS, BUILTIN_CONTAINERS) -INSTALLERS: Registry[Installer] = Registry(GROUP_INSTALLERS, ()) +SDKS: Registry[SdkProvider] = Registry(GROUP_ADAPTERS, BUILTIN_SDKS, SdkProvider) +CONTAINERS: Registry[ContainerAdapter] = Registry( + GROUP_CONTAINERS, BUILTIN_CONTAINERS, ContainerAdapter +) +INSTALLERS: Registry[Installer] = Registry(GROUP_INSTALLERS, (), Installer) def load_all() -> None: diff --git a/xtest/test_registry_units.py b/xtest/test_registry_units.py index 765a6fb7..153da0c3 100644 --- a/xtest/test_registry_units.py +++ b/xtest/test_registry_units.py @@ -34,10 +34,25 @@ @pytest.fixture(autouse=True) def _clean_registries() -> Iterator[None]: - """Every test starts from the built-ins and only the built-ins.""" - registry.reset_all() - yield + """Isolate registry and forced-feature mutations from the rest of the run.""" + saved_sdks = (registry.SDKS._entries.copy(), registry.SDKS._loaded) + saved_containers = ( + registry.CONTAINERS._entries.copy(), + registry.CONTAINERS._loaded, + ) + saved_installers = ( + registry.INSTALLERS._entries.copy(), + registry.INSTALLERS._loaded, + ) + saved_forced_supports = tdfs._forced_supports registry.reset_all() + try: + yield + finally: + registry.SDKS._entries, registry.SDKS._loaded = saved_sdks + registry.CONTAINERS._entries, registry.CONTAINERS._loaded = saved_containers + registry.INSTALLERS._entries, registry.INSTALLERS._loaded = saved_installers + tdfs._forced_supports = saved_forced_supports # --- the pin ------------------------------------------------------------------- @@ -175,6 +190,7 @@ def versions(self): CONTAINER = Container() SDK = Sdk() + MISNAMED_CONTAINER = Container(name="not-acme") BROKEN = None ''' ) @@ -301,6 +317,42 @@ def test_a_plugin_claiming_a_builtin_name_fails_the_session( with pytest.raises(registry.DuplicateName): registry.load_all() + @pytest.mark.parametrize( + ("group", "target", "contract"), + [ + ("otdf.containers", "BROKEN", "ContainerAdapter"), + ("otdf.containers", "SDK", "ContainerAdapter"), + ("otdf.adapters", "CONTAINER", "SdkProvider"), + ("otdf.installers", "SDK", "Installer"), + ], + ) + def test_a_plugin_object_must_match_its_groups_contract( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + group: str, + target: str, + contract: str, + ): + _install_plugin( + tmp_path, + monkeypatch, + f"[{group}]\nacme = acme_xtest:{target}\n", + ) + with pytest.raises(registry.PluginLoadError, match=contract): + registry.load_all() + + def test_a_plugin_object_name_must_match_the_entry_point( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + _install_plugin( + tmp_path, + monkeypatch, + "[otdf.containers]\nacme = acme_xtest:MISNAMED_CONTAINER\n", + ) + with pytest.raises(registry.PluginLoadError, match="name is 'not-acme'"): + registry.load_all() + # --- the shapes the container seam will be built on ---------------------------- From d1922f6a618dfe32f87c040157978b4d8117dd41 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 15 Sep 2026 17:06:58 -0400 Subject: [PATCH 7/7] docs(xtest): trim change-narration from comments added in this PR Comments that describe what the code used to do, or that argue for the change against its predecessor, belong on the PR rather than in the tree: once merged they document a state no reader can see, and they rot the moment the next change lands. Dropped the "that default used to be get_args(tdfs.sdk_type)" paragraph from resolve_sdks, the module-scope-vs-pytest_configure walkthrough in configure_forced_supports, and the "Before: 20 skipped ... exit 0" and "Before DSPX-4794" openers in the unit tests. The invariants those paragraphs were justifying are restated as invariants. Kept the durable rationale: why an empty default parametrization is a UsageError, why all_versions_of sorts, why the pin test is load-bearing, and why forced_supports needs a lazy fallback. --- xtest/conftest.py | 14 +++----------- xtest/registry.py | 16 ++++++---------- xtest/tdfs.py | 35 +++++++++++++---------------------- xtest/test_conftest_units.py | 6 +++--- xtest/test_registry_units.py | 7 ++----- xtest/test_tdfs_units.py | 6 +++--- 6 files changed, 30 insertions(+), 54 deletions(-) diff --git a/xtest/conftest.py b/xtest/conftest.py index d1dc0863..6ec8c1fc 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -325,13 +325,6 @@ def resolve_sdks( The first option in ``option_names`` that was given wins; otherwise the default is every build actually installed under ``sdk/*/dist/``. - That default used to be ``get_args(tdfs.sdk_type)`` -- the set of names the - suite knows about rather than the set of builds present. - :func:`tdfs.parse_sdk_spec` routes a bare name through - :func:`tdfs.all_versions_of` anyway, so the two agreed; they stop agreeing - the moment anything other than the ``Literal`` can contribute a name, and - "what is installed" was always the question being asked. - The empty case is an error rather than an empty parametrization, and only on the default path. ``metafunc.parametrize`` over ``[]`` does not collect zero items: pytest's ``empty_parameter_set_mark`` turns it into one *skip* @@ -461,10 +454,9 @@ def pytest_configure(config: pytest.Config): # pytest_generate_tests, where the names become parameters. registry.load_all() - # Resolve XT_FORCE_SUPPORTS here rather than at tdfs import. The parse - # rejects unknown names, so wherever it runs is the moment the set of legal - # feature names freezes; at import that is before any plugin could have - # contributed one. See tdfs.configure_forced_supports. + # Then XT_FORCE_SUPPORTS, whose parse validates names against the feature + # registry and so has to run after discovery. See + # tdfs.configure_forced_supports. # # UsageError, not the bare ValueError: a typo in XT_FORCE_SUPPORTS is a # mistake in the invocation, and pytest reports a UsageError as such diff --git a/xtest/registry.py b/xtest/registry.py index 67976e01..2d642b4c 100644 --- a/xtest/registry.py +++ b/xtest/registry.py @@ -6,13 +6,9 @@ ``tdfs.py``: ``sdk_type``, ``container_type`` and ``feature_type``. Adding an SDK, a container format or a capability gate means editing this repo. -Upstream has already paid for that once. NanoTDF was removed in ``150e3135`` -("fix: remove NanoTDF tests and support (#366)") -- 11 files, +22/-2003 -- and -``otdf-sdk-mgr/tests/test_schema.py::test_removed_nano_container_is_rejected`` -now exists to keep a *second* hand-maintained copy of the container enum -(``otdf_sdk_mgr.schema.ContainerKind``) in step with the first. A format that -was never more than an enum value cost two thousand lines to remove, because -the value was load-bearing everywhere instead of confined to one object. +Spreading a format's name through the suite is expensive: removing NanoTDF +(``150e3135``) touched 11 files for +22/-2003, and the container enum still +has a second hand-maintained copy in ``otdf_sdk_mgr.schema.ContainerKind``. This module is the seam. Today's ``Literal`` values stay exactly where they are, as the built-in defaults; anything installed alongside xtest can add to @@ -23,7 +19,7 @@ The ``Literal``\\ s are not widened to ``str`` and not widened to ``Literal[...] | str`` (pyright collapses that union to ``str`` and silently -stops diagnosing). Four mechanisms replace the one: +stops diagnosing). Four mechanisms carry the checking instead: 1. The ``Literal``\\ s stay authoritative for in-tree code, so a literal typo is still a type error and deleting a built-in still breaks every mention of @@ -40,8 +36,8 @@ That last one is load-bearing. This module deliberately does **not** import ``tdfs`` -- that is what lets ``tdfs`` import *it* without a cycle -- so the built-in names are written out twice. Without the pin test, that duplication -is a second ``ContainerKind``, i.e. exactly the defect being removed here. If -the pin test is ever deleted, collapse the duplication in the same change. +is a second ``ContainerKind``. If the pin test is ever deleted, collapse the +duplication in the same change. """ from __future__ import annotations diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 33621dcc..17c14aa2 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -217,11 +217,9 @@ def _parse_forced_supports(raw: str) -> frozenset[str]: exact failure mode the override is meant to escape. """ names = {n.strip() for n in raw.split(",") if n.strip()} - # The registry, not ``get_args(feature_type)``: a feature contributed by a - # plugin is as real as a built-in one, and the whole reason this parse - # moved out of module scope was so it could see them. ``feature_names()`` - # is seeded from the ``Literal`` above, so with nothing installed the two - # are the same set. + # The registry rather than ``get_args(feature_type)``: a plugin-contributed + # feature is as forceable as a built-in one. ``feature_names()`` is seeded + # from the ``Literal`` above, so with nothing installed the two agree. known = set(registry.feature_names()) unknown = names - known if unknown: @@ -253,15 +251,11 @@ def configure_forced_supports(raw: str | None = None) -> frozenset[str]: narrow the run with ``--sdks-encrypt`` / ``--sdks-decrypt`` rather than adding per-SDK syntax here. - Called from ``conftest.pytest_configure`` rather than evaluated at module - import. The parse validates every name against the known feature set and - raises on an unknown one -- correctly, see :func:`_parse_forced_supports` -- - which means the set of legal names is frozen at whatever moment the parse - runs. At import that is before ``pytest_addoption``, before - ``pytest_configure`` and before any plugin has had a chance to contribute a - feature, so the strictness and the extensibility were in direct conflict. - Running it from ``pytest_configure`` puts the validation after plugin - discovery and keeps both. + Call this from ``conftest.pytest_configure``, not at module import: the + parse rejects unknown names (correctly, see :func:`_parse_forced_supports`) + so whenever it runs is the moment the set of legal feature names freezes, + and only ``pytest_configure`` is late enough for a plugin to have + contributed one. Idempotent, so a second call (an xdist worker configuring itself, a test exercising the parse) simply re-resolves. @@ -286,9 +280,9 @@ def forced_supports() -> frozenset[str]: The lazy fallback matters: ``tdfs`` is importable outside a pytest session -- ``otdf-sdk-mgr`` and ad-hoc scripts both do it -- and those callers never - run ``pytest_configure``. Without it, moving the parse would silently turn - ``XT_FORCE_SUPPORTS`` into a no-op for them, which is the exact class of - quiet failure the variable exists to escape. + run ``pytest_configure``. Without it, ``XT_FORCE_SUPPORTS`` would be a + silent no-op for them, which is the exact class of quiet failure the + variable exists to escape. """ if _forced_supports is None: return configure_forced_supports() @@ -1012,11 +1006,8 @@ def all_versions_of(sdk: sdk_type) -> list[SDK]: def installed_sdks() -> list[SDK]: """Every SDK build present under ``sdk//dist//``. - The default subject set for a run, and the answer to "what is actually on - this machine" rather than "what names does the suite know about". Those - two questions had the same answer while :data:`sdk_type` was the only - source of SDK names; they stop having the same answer as soon as anything - can contribute one. + The default subject set for a run: "what is actually on this machine", + not "what names does the suite know about". """ return [sdk for name in get_args(sdk_type) for sdk in all_versions_of(name)] diff --git a/xtest/test_conftest_units.py b/xtest/test_conftest_units.py index 302146f3..a7a16f6b 100644 --- a/xtest/test_conftest_units.py +++ b/xtest/test_conftest_units.py @@ -45,10 +45,10 @@ def install(*specs: str) -> None: class TestDefaultsToInstalled: def test_empty_default_is_an_error_not_an_empty_matrix(self): - """The bug this change exists to close. + """Nothing installed must fail the run, not skip it. - Before: 20 skipped, "got empty parameter set for (encrypt_sdk)", - exit 0 -- a green run that tested nothing. + An empty parametrization collects as "got empty parameter set for + (encrypt_sdk)" and exits 0 -- a green run that tested nothing. """ with pytest.raises(pytest.UsageError, match="otdf-sdk-mgr install"): resolve_sdks(_config(), ["--sdks-encrypt", "--sdks"], "encrypt") diff --git a/xtest/test_registry_units.py b/xtest/test_registry_units.py index 153da0c3..d7bd94e5 100644 --- a/xtest/test_registry_units.py +++ b/xtest/test_registry_units.py @@ -258,11 +258,8 @@ def test_the_plugins_features_join_the_known_set(self): assert {"acme-sealed", "acme-dialect"} <= registry.feature_names() def test_force_supports_accepts_a_feature_the_plugin_contributed(self): - """The ordering fix and the registry, end to end. - - Before DSPX-4794 this name was rejected, and there was no moment at - which it could have been accepted: the parse ran at ``tdfs`` import, - before any plugin existed. + """The ordering and the registry, end to end: discovery has to happen + before the parse for a plugin's feature name to be forceable at all. """ registry.load_all() assert tdfs.configure_forced_supports("acme-sealed") == frozenset( diff --git a/xtest/test_tdfs_units.py b/xtest/test_tdfs_units.py index ea05b013..fb481bbe 100644 --- a/xtest/test_tdfs_units.py +++ b/xtest/test_tdfs_units.py @@ -65,7 +65,7 @@ class TestForcedSupportsIsNotResolvedAtImport: The parse rejects unknown names, so wherever it runs is the moment the set of legal feature names freezes. At import that is before any plugin could - contribute one, which is the ordering problem DSPX-4794 removes. + have contributed one. """ def test_importing_tdfs_with_an_unknown_name_does_not_raise( @@ -111,8 +111,8 @@ def test_lazy_fallback_for_callers_outside_a_pytest_session( ): """``tdfs`` is imported by scripts that never run ``pytest_configure``. - Without the fallback, moving the parse would turn ``XT_FORCE_SUPPORTS`` - into a silent no-op for them. + Without the fallback, ``XT_FORCE_SUPPORTS`` would be a silent no-op + for them. """ monkeypatch.setattr(tdfs, "_forced_supports", None) monkeypatch.setenv("XT_FORCE_SUPPORTS", "ecwrap")