Skip to content

Commit 33b5ab2

Browse files
jopemachineclaude
andcommitted
test(BA-5983): seed DB baseline via fixture; parametrize on input/result
Restructure the DB-backed merge test so the baseline ``ModelDefinitionDraft`` lives in a fixture (the DB-side value) and the parametrize tables only carry the actual request input + expected outcome. Two scenario groups, each pinned to its own baseline fixture: - ``TestMergeWithFullBaseline`` — baseline ships every required field; parametrize cases probe how different requests (empty, partial override) combine with that baseline. - ``TestMergeRaisesWithIncompleteBaseline`` — baseline ships an incomplete definition so the resolve-time check fires. The request is always all-empty here; the parametrize table only varies the baseline shape + expected error pattern. Shared helpers (``_seed_variant``, ``_merge_via_reader``) move to module scope so both classes consume the same DB and reader setup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8f74efc commit 33b5ab2

1 file changed

Lines changed: 129 additions & 118 deletions

File tree

tests/unit/manager/repositories/deployment/test_revision_merge_db.py

Lines changed: 129 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,22 @@
11
"""DB-backed verification of the BA-5983 revision merge contract.
22
3-
Inserts real ``RuntimeVariantRow`` records (so the variant's
3+
A real ``RuntimeVariantRow`` is seeded via a fixture (so the variant's
44
``default_model_definition`` round-trips through ``PydanticColumn``
5-
serialization) and runs the production ``RevisionDraftReader`` +
6-
``RevisionDraft.merge`` pipeline against a request draft built from
7-
``ModelDefinitionInput.to_draft()``. The resolved output is then
8-
inspected to confirm:
5+
serialization). The production ``RevisionDraftReader`` +
6+
``RevisionDraft.merge`` pipeline is then run against request drafts
7+
built from various ``ModelDefinitionInput`` shapes; the parametrized
8+
table only carries the request input and the expected outcome.
99
10-
- An empty request inherits every required field from the variant
11-
baseline; the resolved ``ModelDefinition`` carries the baseline
12-
values verbatim.
13-
- A request that supplies a subset of fields overrides only those
14-
fields; baseline-supplied fields survive.
15-
- When no source supplies a required field, ``to_resolved()`` raises
16-
``ValueError`` with the field-specific message.
10+
Two scenario groups, each pinned to its own DB baseline fixture:
1711
18-
This exercises the full read path (DB → ``PydanticColumn`` →
19-
``RuntimeVariantData`` → ``RevisionDraft``) plus the merge and
20-
resolve phases that the ``add_model_revision`` action ultimately runs.
12+
- ``TestMergeWithFullBaseline`` — variant ships every required field;
13+
the parametrized inputs probe how different requests combine with
14+
it (inherit-all, partial override).
15+
- ``TestMergeRaisesWithIncompleteBaseline`` — variant ships an
16+
incomplete definition where ``to_resolved()`` is expected to raise
17+
because no source supplies a required nested field. Each parametrize
18+
entry pairs an incomplete baseline shape with the expected error
19+
pattern; the request is always all-empty.
2120
"""
2221

2322
from __future__ import annotations
@@ -60,92 +59,105 @@ class ResolvedExpectation:
6059
health_check_path: str | None = None
6160

6261

63-
class TestRevisionMergeWithRealVariantBaseline:
64-
@pytest.fixture
65-
async def db_with_variant_table(
66-
self,
67-
database_connection: ExtendedAsyncSAEngine,
68-
) -> AsyncGenerator[ExtendedAsyncSAEngine, None]:
69-
async with with_tables(database_connection, [RuntimeVariantRow]):
70-
yield database_connection
62+
@pytest.fixture
63+
async def db_with_variant_table(
64+
database_connection: ExtendedAsyncSAEngine,
65+
) -> AsyncGenerator[ExtendedAsyncSAEngine, None]:
66+
async with with_tables(database_connection, [RuntimeVariantRow]):
67+
yield database_connection
7168

72-
@pytest.fixture
73-
def reader(
74-
self,
75-
db_with_variant_table: ExtendedAsyncSAEngine,
76-
) -> RevisionDraftReader:
77-
# ``load_deployment_revision_read_bundle`` only touches the
78-
# runtime_variants table when ``preset_id`` is ``None``; the
79-
# other repository dependencies are not exercised and can be
80-
# stubbed out.
81-
repo = DeploymentRepository(
82-
db=db_with_variant_table,
83-
storage_manager=MagicMock(),
84-
valkey_stat=MagicMock(),
85-
valkey_live=MagicMock(),
86-
valkey_schedule=MagicMock(),
87-
)
88-
return RevisionDraftReader(deployment_repository=repo)
8969

90-
@pytest.fixture
91-
def mounts(self) -> MountMetadata:
92-
return MountMetadata(
93-
model_vfolder_id=VFolderUUID(uuid.uuid4()),
94-
model_definition_path=None,
95-
model_mount_destination="/models",
96-
extra_mounts=[],
97-
)
70+
@pytest.fixture
71+
def reader(
72+
db_with_variant_table: ExtendedAsyncSAEngine,
73+
) -> RevisionDraftReader:
74+
# ``load_deployment_revision_read_bundle`` only touches the
75+
# runtime_variants table when ``preset_id`` is ``None``; the other
76+
# repository dependencies are not exercised here and can be stubbed.
77+
repo = DeploymentRepository(
78+
db=db_with_variant_table,
79+
storage_manager=MagicMock(),
80+
valkey_stat=MagicMock(),
81+
valkey_live=MagicMock(),
82+
valkey_schedule=MagicMock(),
83+
)
84+
return RevisionDraftReader(deployment_repository=repo)
9885

99-
@staticmethod
100-
async def _seed_variant_baseline(
101-
db: ExtendedAsyncSAEngine,
102-
baseline: ModelDefinitionDraft,
103-
) -> RuntimeVariantID:
104-
variant_id = RuntimeVariantID(uuid.uuid4())
105-
async with db.begin_session() as sess:
106-
sess.add(
107-
RuntimeVariantRow(
108-
id=variant_id,
109-
name=f"test-variant-{variant_id.hex[:8]}",
110-
description="BA-5983 merge-test variant baseline",
111-
reads_vfolder_config_files=False,
112-
default_model_definition=baseline,
113-
)
86+
87+
@pytest.fixture
88+
def mounts() -> MountMetadata:
89+
return MountMetadata(
90+
model_vfolder_id=VFolderUUID(uuid.uuid4()),
91+
model_definition_path=None,
92+
model_mount_destination="/models",
93+
extra_mounts=[],
94+
)
95+
96+
97+
async def _seed_variant(
98+
db: ExtendedAsyncSAEngine,
99+
baseline: ModelDefinitionDraft,
100+
) -> RuntimeVariantID:
101+
variant_id = RuntimeVariantID(uuid.uuid4())
102+
async with db.begin_session() as sess:
103+
sess.add(
104+
RuntimeVariantRow(
105+
id=variant_id,
106+
name=f"test-variant-{variant_id.hex[:8]}",
107+
description="BA-5983 merge-test variant baseline",
108+
reads_vfolder_config_files=False,
109+
default_model_definition=baseline,
114110
)
115-
await sess.commit()
116-
return variant_id
111+
)
112+
await sess.commit()
113+
return variant_id
117114

118-
@staticmethod
119-
async def _merge_via_reader(
120-
reader: RevisionDraftReader,
121-
variant_id: RuntimeVariantID,
122-
request: RevisionDraft,
123-
mounts: MountMetadata,
124-
) -> RevisionDraft:
125-
drafts = await reader.read_for_deployment_revision(
126-
runtime_variant_id=variant_id,
127-
request_draft=request,
128-
mounts=mounts,
129-
preset_id=None,
115+
116+
async def _merge_via_reader(
117+
reader: RevisionDraftReader,
118+
variant_id: RuntimeVariantID,
119+
request: RevisionDraft,
120+
mounts: MountMetadata,
121+
) -> RevisionDraft:
122+
drafts = await reader.read_for_deployment_revision(
123+
runtime_variant_id=variant_id,
124+
request_draft=request,
125+
mounts=mounts,
126+
preset_id=None,
127+
)
128+
return functools.reduce(RevisionDraft.merge, drafts, RevisionDraft())
129+
130+
131+
class TestMergeWithFullBaseline:
132+
"""Baseline supplies every required field. The parametrize table
133+
pairs each ``ModelDefinitionInput`` shape with the resolved values
134+
we expect after merging it against this baseline."""
135+
136+
@pytest.fixture
137+
async def variant_id(
138+
self,
139+
db_with_variant_table: ExtendedAsyncSAEngine,
140+
) -> RuntimeVariantID:
141+
return await _seed_variant(
142+
db_with_variant_table,
143+
ModelDefinitionDraft(
144+
models=[
145+
ModelConfigDraft(
146+
name="baseline-llama",
147+
model_path="/models/baseline",
148+
service=ModelServiceConfigDraft(
149+
port=9000,
150+
health_check=ModelHealthCheckDraft(path="/healthz"),
151+
),
152+
),
153+
],
154+
),
130155
)
131-
return functools.reduce(RevisionDraft.merge, drafts, RevisionDraft())
132156

133157
@pytest.mark.parametrize(
134-
("baseline", "request_input", "expected"),
158+
("request_input", "expected"),
135159
[
136160
pytest.param(
137-
ModelDefinitionDraft(
138-
models=[
139-
ModelConfigDraft(
140-
name="baseline-llama",
141-
model_path="/models/baseline",
142-
service=ModelServiceConfigDraft(
143-
port=9000,
144-
health_check=ModelHealthCheckDraft(path="/healthz"),
145-
),
146-
),
147-
],
148-
),
149161
ModelDefinitionInput(),
150162
ResolvedExpectation(
151163
name="baseline-llama",
@@ -156,30 +168,28 @@ async def _merge_via_reader(
156168
id="empty_request_inherits_full_baseline",
157169
),
158170
pytest.param(
159-
ModelDefinitionDraft(
160-
models=[
161-
ModelConfigDraft(name="baseline-name", model_path="/baseline/path"),
162-
],
163-
),
164171
ModelDefinitionInput(models=[ModelConfigInput(name="user-name")]),
165-
ResolvedExpectation(name="user-name", model_path="/baseline/path"),
166-
id="request_overrides_name_baseline_keeps_model_path",
172+
ResolvedExpectation(
173+
name="user-name",
174+
model_path="/models/baseline",
175+
service_port=9000,
176+
health_check_path="/healthz",
177+
),
178+
id="request_overrides_name_only",
167179
),
168180
],
169181
)
170182
async def test_merge_resolves_to_expected_values(
171183
self,
172-
db_with_variant_table: ExtendedAsyncSAEngine,
173184
reader: RevisionDraftReader,
174185
mounts: MountMetadata,
175-
baseline: ModelDefinitionDraft,
186+
variant_id: RuntimeVariantID,
176187
request_input: ModelDefinitionInput,
177188
expected: ResolvedExpectation,
178189
) -> None:
179-
variant_id = await self._seed_variant_baseline(db_with_variant_table, baseline)
180190
request = RevisionDraft(model_definition=request_input.to_draft())
181191

182-
merged = await self._merge_via_reader(reader, variant_id, request, mounts)
192+
merged = await _merge_via_reader(reader, variant_id, request, mounts)
183193

184194
assert merged.model_definition is not None
185195
resolved = merged.model_definition.to_resolved()
@@ -194,19 +204,25 @@ async def test_merge_resolves_to_expected_values(
194204
assert model.service.health_check is not None
195205
assert model.service.health_check.path == expected.health_check_path
196206

207+
208+
class TestMergeRaisesWithIncompleteBaseline:
209+
"""Each parametrize entry seeds its own incomplete baseline (via the
210+
``baseline_factory``) and expects ``to_resolved()`` to raise because
211+
no source supplies a required field. The request is always
212+
all-empty so the failure mode comes entirely from the baseline."""
213+
197214
@pytest.mark.parametrize(
198-
("baseline", "error_pattern"),
215+
("incomplete_baseline", "error_pattern"),
199216
[
200217
pytest.param(
201-
# baseline supplies model_path only; reader's mount-destination
202-
# default would also fill model_path → only ``name`` remains unfilled.
218+
# Reader's mount-destination default also fills model_path,
219+
# so the only required ``ModelConfig`` field that ends up
220+
# unfilled is ``name``.
203221
ModelDefinitionDraft(models=[ModelConfigDraft(model_path="/p")]),
204222
r"ModelConfig\.name is required",
205-
id="name_unfilled_across_baseline_and_request",
223+
id="name_unfilled",
206224
),
207225
pytest.param(
208-
# baseline supplies name + model_path + an empty service →
209-
# service.port has no default and no override.
210226
ModelDefinitionDraft(
211227
models=[
212228
ModelConfigDraft(
@@ -217,11 +233,9 @@ async def test_merge_resolves_to_expected_values(
217233
],
218234
),
219235
r"ModelServiceConfig\.port is required",
220-
id="service_port_unfilled_across_baseline_and_request",
236+
id="service_port_unfilled",
221237
),
222238
pytest.param(
223-
# baseline supplies service.port but an empty health_check →
224-
# health_check.path has no default.
225239
ModelDefinitionDraft(
226240
models=[
227241
ModelConfigDraft(
@@ -235,7 +249,7 @@ async def test_merge_resolves_to_expected_values(
235249
],
236250
),
237251
r"ModelHealthCheck\.path is required",
238-
id="health_check_path_unfilled_across_baseline_and_request",
252+
id="health_check_path_unfilled",
239253
),
240254
],
241255
)
@@ -244,16 +258,13 @@ async def test_required_field_unfilled_after_merge_raises(
244258
db_with_variant_table: ExtendedAsyncSAEngine,
245259
reader: RevisionDraftReader,
246260
mounts: MountMetadata,
247-
baseline: ModelDefinitionDraft,
261+
incomplete_baseline: ModelDefinitionDraft,
248262
error_pattern: str,
249263
) -> None:
250-
# Request is always an all-empty ``ModelDefinitionInput`` for these
251-
# scenarios — the merge result depends entirely on whether the
252-
# baseline (or reader-supplied defaults) cover every required field.
253-
variant_id = await self._seed_variant_baseline(db_with_variant_table, baseline)
264+
variant_id = await _seed_variant(db_with_variant_table, incomplete_baseline)
254265
request = RevisionDraft(model_definition=ModelDefinitionInput().to_draft())
255266

256-
merged = await self._merge_via_reader(reader, variant_id, request, mounts)
267+
merged = await _merge_via_reader(reader, variant_id, request, mounts)
257268

258269
assert merged.model_definition is not None
259270
with pytest.raises(ValueError, match=error_pattern):

0 commit comments

Comments
 (0)