Review fix: gate the override race test inside A's transaction
Review round 1 (Codex gpt-5.6-terra, LOW): the racing test completed synchronously and proved nothing. The executor now gates task A's INSERT — inside its open transaction, after its overlap probe — so task B queues on the connection lock, re-runs the probe against A's committed row, and is rejected; exactly one lands. Suite: 990 passed.
This commit is contained in:
+43
-21
@@ -718,13 +718,15 @@ def test_override_remove_then_recreate_overlapping(tmp_path) -> None:
|
||||
|
||||
def test_override_create_racing_conflicts_serialize(tmp_path) -> None:
|
||||
"""Two racing creates for the same child and dates: exactly one
|
||||
lands, the other is rejected with the overlap error (the check and
|
||||
the INSERT share one transaction under the connection lock)."""
|
||||
lands, the other is rejected with the overlap error. DETERMINISTIC
|
||||
gate: task A is paused at its INSERT — inside its open transaction,
|
||||
after its overlap probe — so task B is created while A provably
|
||||
holds the connection lock and must queue; B then re-runs the probe
|
||||
against A's committed row and is rejected."""
|
||||
import asyncio as asyncio_module
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
async def _main():
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from custom_components.nestquest.dao_children import ChildrenDao
|
||||
from custom_components.nestquest.dao_presence import (
|
||||
PresenceOverridesDao,
|
||||
@@ -732,10 +734,24 @@ def test_override_create_racing_conflicts_serialize(tmp_path) -> None:
|
||||
from custom_components.nestquest.db import NestQuestDatabase
|
||||
from custom_components.nestquest.migrations import apply_migrations
|
||||
|
||||
armed = {"active": False, "gated": False}
|
||||
gate_open = asyncio_module.Event()
|
||||
a_started = asyncio_module.Event()
|
||||
|
||||
hass = MagicMock()
|
||||
hass.async_add_executor_job = AsyncMock(
|
||||
side_effect=(lambda fn, *a: fn(*a))
|
||||
)
|
||||
|
||||
async def _gated_executor(fn, *args):
|
||||
if armed["active"]:
|
||||
# Gate the FIRST _execute after arming: task A's INSERT,
|
||||
# which runs inside its open transaction (BEGIN already
|
||||
# executed) after the overlap probe.
|
||||
if getattr(fn, "__name__", "") == "_execute" and not armed["gated"]:
|
||||
armed["gated"] = True
|
||||
a_started.set()
|
||||
await gate_open.wait()
|
||||
return fn(*args)
|
||||
|
||||
hass.async_add_executor_job = _gated_executor
|
||||
database = NestQuestDatabase(hass)
|
||||
await database.open(tmp_path / "override-race.db")
|
||||
try:
|
||||
@@ -743,26 +759,32 @@ def test_override_create_racing_conflicts_serialize(tmp_path) -> None:
|
||||
children = ChildrenDao(database)
|
||||
overrides = PresenceOverridesDao(database)
|
||||
child = await children.create("Ada", NOW)
|
||||
results = await asyncio_module.gather(
|
||||
overrides.create(
|
||||
child.id, "2026-07-20", "2026-07-24", True
|
||||
),
|
||||
overrides.create(
|
||||
child.id, "2026-07-22", "2026-07-26", False
|
||||
),
|
||||
return_exceptions=True,
|
||||
|
||||
armed["active"] = True
|
||||
task_a = asyncio_module.ensure_future(
|
||||
overrides.create(child.id, "2026-07-20", "2026-07-24", True)
|
||||
)
|
||||
outcomes = sorted(
|
||||
"ok" if not isinstance(r, BaseException) else type(r).__name__
|
||||
for r in results
|
||||
await a_started.wait()
|
||||
armed["active"] = False
|
||||
task_b = asyncio_module.ensure_future(
|
||||
overrides.create(child.id, "2026-07-22", "2026-07-26", False)
|
||||
)
|
||||
assert outcomes == ["ValueError", "ok"], (
|
||||
f"exactly one create must land: {outcomes}"
|
||||
await asyncio_module.sleep(0)
|
||||
gate_open.set()
|
||||
record_a, error_b = await asyncio_module.gather(
|
||||
task_a, task_b, return_exceptions=True
|
||||
)
|
||||
assert isinstance(record_a, object) and not isinstance(
|
||||
record_a, BaseException
|
||||
), f"A must land: {record_a!r}"
|
||||
assert isinstance(error_b, ValueError), (
|
||||
f"B must be rejected with the overlap error: {error_b!r}"
|
||||
)
|
||||
assert "overlaps existing" in str(error_b)
|
||||
listed = await overrides.list_by_child_and_range(
|
||||
child.id, "2026-01-01", "2026-12-31"
|
||||
)
|
||||
assert len(listed) == 1
|
||||
assert [row.id for row in listed] == [record_a.id]
|
||||
finally:
|
||||
await database.close()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user