randomparity / randomparity/kdive
Integration seed writes a build_profile the model rejects, bypassing BuildProfile.parse
- Dominant language
- Python
- Stars
- 0
- Forks
- 0
- Avg merge
- 1h 26m
- Merged PRs (30d)
- 311
Description
## Problem
`tests/integration/_seed.py` seeds a `build_profile` document that `BuildProfile` rejects. It
carries `kernel_source_ref` and `config` — two fields the model forbids outright — and it reaches
the `build_profile` jsonb column directly through the repository, never through
`BuildProfile.parse`, so nothing ever refuses it.
Three integration seed helpers use it. Every run they create therefore holds a `build_profile` the
production write path could not have produced. Tests built on those runs are asserting against a
state the system cannot reach.
Nothing is failing today. The consequences are that a regression in the real validation path is
invisible to these tests, and the first test that does parse a seeded profile fails for a reason
that has nothing to do with the change under test.
## Evidence
`tests/integration/_seed.py:63-67`:
```python
BUILD_PROFILE: dict[str, Any] = {
"schema_version": 1,
"kernel_source_ref": "git+https://git.kernel.org#v6.9",
"config": {"kind": "catalog", "provider": "system", "name": "kdump"},
}
```
Validated against the model directly:
```
>>> BuildProfile.model_validate(BUILD_PROFILE)
ValidationError: 2 validation errors for BuildProfile
kernel_source_ref
Extra inputs are not permitted [type=extra_forbidden, ...]
config
Extra inputs are not permitted [type=extra_forbidden, ...]
```
- `src/kdive/profiles/build.py:33-46` — `BuildProfile` sets `model_config = ConfigDict(extra="forbid", frozen=True)` and declares exactly two fields, `schema_version: Literal[1]` and `arch: str`. Its docstring states the intent the fixture predates: "a thin, versioned document with **no source-tree fields**". `kernel_source_ref` and `config` are source-tree fields.
- `src/kdive/profiles/build.py` — `BuildProfile.parse` is documented as "the boundary that maps a structural `ValidationError` onto `configuration_error`". The seed path does not cross that boundary.
- Consumers, all writing the dict straight into the repository:
- `tests/integration/_seed.py:247` — `seed_running_run`
- `tests/integration/_seed.py:290` — `seed_unbound_running_run`
- `tests/integration/_seed.py:337` — `seed_crashed_system_with_run`
For contrast, a sibling fixture already holds the correct shape:
- `tests/mcp/lifecycle/runs_support.py:38` — `BUILD_PROFILE: dict[str, Any] = {"schema_version": 1}`
- `tests/integration/test_finalization_measurement.py:185` — `{"schema_version": 1, "arch": arch}`
So the repository has both shapes in its test tree, and only the out-of-band one is wrong.
## Expected
Every `build_profile` a test seeds is a document `BuildProfile.parse` accepts. A fixture that
bypasses the validator does not thereby get to carry a shape the validator forbids.
## Proposed approach
1. Reduce `_seed.py`'s `BUILD_PROFILE` to the fields the model declares — `schema_version`, plus
`arch` where a test depends on it. Drop `kernel_source_ref` and `config`.
2. Run the integration suite. Any test that was depending on the forbidden fields is a second
finding, and should be reported rather than quietly accommodated: it would mean production code
reads a field the profile model does not define.
3. Add an assertion that keeps the fixture honest — parse the seed constant through
`BuildProfile.parse` once, in a non-gated test, so a future edit that reintroduces a forbidden
field fails in ordinary CI rather than at the next person to read the column. Verify it bites by
adding a forbidden key, observing red, and reverting.
Step 3 is the part that matters. Without it this is a one-time cleanup of a fixture that drifted
once and can drift again, because nothing in the seed path consults the model.
## Out of scope
- Changing `BuildProfile` itself, or whether `extra="forbid"` is the right setting. It is
deliberate.
- The remaining inline build-profile restatements (`test_finalization_measurement.py:185` and the
`_provision_profile` / `_remote_provision_profile` pair). Those are valid documents that are
merely duplicated; #2511 consolidated the build-profile family and deliberately left these.
## Provenance
Reported by the implementer of #2511 while consolidating six build-profile restatements into one
shared factory, as adjacent and outside its approved surface. The claim was that the seed was
"divergent"; validating it against the model showed it is rejected outright, which is the reason
this is filed rather than held. Reproduced before filing. Filed at the repository operator's
explicit request.
Contributor guide
Research direction
Start with tests/integration/_seed.py:63-67 and the three seed helpers at lines 247, 290, and 337, then read src/kdive/profiles/build.py to confirm the accepted BuildProfile fields and parse boundary. Compare the valid fixtures in tests/mcp/lifecycle/runs_support.py and tests/integration/test_finalization_measurement.py, run the integration suite, and add the proposed non-gated assertion so invalid seed fields fail CI.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- Half a day
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100