alexei.dolgolyov
6cd5e057da
Final-review blocker: the setup scaffold created the LED output target in the store but never registered it with the ProcessorManager, so the wizard's "Start" step 404'd on a fresh setup (target not found) — the lights never started despite a success screen. Now the scaffold calls target.register_with_manager(manager) right after create (mirroring the canonical POST /output-targets route, same ValueError guard), so start_processing finds the target. Rollback unregisters via manager.remove_target before deleting the store entity, so a post-registration failure leaves no half-registered target. Also from the final review: - solve corner_indices elements now bounded ge=0 (clear 422 instead of silent modulo-wrap). - setup-wizard.ts: reuse tutorials' suppressGettingStartedTour()/TOUR_KEY instead of a duplicated 'tour_completed' literal; drop a duplicate manual-form submit listener. Tests: + adversarial pass over the whole feature (solver/session/scaffold edge cases) and a scaffold->register->startable regression test. Full suite 2149 passed / 2 skipped; tsc clean; build passes; ruff clean.
507 lines
19 KiB
Python
507 lines
19 KiB
Python
"""Adversarial tests for the setup scaffold and onboarding preference endpoints.
|
|
|
|
Phase 2 acceptance criteria (NOT what the code happens to do):
|
|
- Rollback when the FINAL step (output target create) fails leaves ZERO orphans
|
|
AND emits ZERO "created" events.
|
|
- Reused capture template is NOT deleted on rollback.
|
|
- display_index > 63 → 422.
|
|
- Missing device_id → 422 (Pydantic validation before handler runs).
|
|
- Unknown device_id → 404.
|
|
- PUT onboarding false clears completed_at to null.
|
|
- Corrupt stored onboarding value falls back to default (onboarded=false).
|
|
|
|
These fill the gaps in the existing 22 happy-path tests.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Shared fixtures (mirrors test_setup_routes.py exactly)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.fixture
|
|
def tmp_db(tmp_path):
|
|
from ledgrab.storage.database import Database
|
|
|
|
db = Database(tmp_path / "adv_setup.db")
|
|
yield db
|
|
db.close()
|
|
|
|
|
|
@pytest.fixture
|
|
def device_store(tmp_db):
|
|
from ledgrab.storage import DeviceStore
|
|
|
|
return DeviceStore(tmp_db)
|
|
|
|
|
|
@pytest.fixture
|
|
def template_store(tmp_db):
|
|
from ledgrab.storage.template_store import TemplateStore
|
|
|
|
return TemplateStore(tmp_db)
|
|
|
|
|
|
@pytest.fixture
|
|
def picture_source_store(tmp_db):
|
|
from ledgrab.storage.picture_source_store import PictureSourceStore
|
|
|
|
return PictureSourceStore(tmp_db)
|
|
|
|
|
|
@pytest.fixture
|
|
def css_store(tmp_db):
|
|
from ledgrab.storage.color_strip_store import ColorStripStore
|
|
|
|
return ColorStripStore(tmp_db)
|
|
|
|
|
|
@pytest.fixture
|
|
def output_target_store(tmp_db):
|
|
from ledgrab.storage.output_target_store import OutputTargetStore
|
|
|
|
return OutputTargetStore(tmp_db)
|
|
|
|
|
|
@pytest.fixture
|
|
def sample_device(device_store):
|
|
return device_store.create_device(
|
|
name="Test LED Strip",
|
|
url="http://192.168.1.10",
|
|
led_count=60,
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def event_log():
|
|
log = []
|
|
return log
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_manager():
|
|
"""A MagicMock ProcessorManager that silently accepts all calls."""
|
|
mgr = MagicMock()
|
|
mgr.remove_target = MagicMock()
|
|
return mgr
|
|
|
|
|
|
@pytest.fixture
|
|
def setup_client(
|
|
tmp_db,
|
|
device_store,
|
|
template_store,
|
|
picture_source_store,
|
|
css_store,
|
|
output_target_store,
|
|
event_log,
|
|
mock_manager,
|
|
):
|
|
from ledgrab.api.routes.setup import router
|
|
from ledgrab.api.auth import verify_api_key
|
|
from ledgrab.api import dependencies as deps
|
|
|
|
app = FastAPI()
|
|
app.include_router(router)
|
|
app.dependency_overrides[verify_api_key] = lambda: "test"
|
|
app.dependency_overrides[deps.get_device_store] = lambda: device_store
|
|
app.dependency_overrides[deps.get_template_store] = lambda: template_store
|
|
app.dependency_overrides[deps.get_picture_source_store] = lambda: picture_source_store
|
|
app.dependency_overrides[deps.get_color_strip_store] = lambda: css_store
|
|
app.dependency_overrides[deps.get_output_target_store] = lambda: output_target_store
|
|
app.dependency_overrides[deps.get_processor_manager] = lambda: mock_manager
|
|
|
|
def _fire(entity_type, action, entity_id):
|
|
event_log.append((entity_type, action, entity_id))
|
|
|
|
with patch("ledgrab.api.routes.setup.fire_entity_event", side_effect=_fire):
|
|
yield TestClient(app, raise_server_exceptions=False)
|
|
|
|
|
|
def _scaffold(client, **overrides):
|
|
body = {"display_index": 0, **overrides}
|
|
return client.post("/api/v1/setup/scaffold", json=body)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Rollback when the FINAL step (output target) fails
|
|
# Criteria: "zero orphans AND zero 'created' events"
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestFinalStepRollback:
|
|
def test_final_step_failure_leaves_zero_orphans(
|
|
self,
|
|
setup_client,
|
|
sample_device,
|
|
picture_source_store,
|
|
css_store,
|
|
output_target_store,
|
|
):
|
|
"""output_target_store.create_wled_target failing leaves NO orphaned entities."""
|
|
original_create = output_target_store.create_wled_target
|
|
|
|
def _fail(*args, **kwargs):
|
|
raise ValueError("Injected final step failure")
|
|
|
|
output_target_store.create_wled_target = _fail
|
|
try:
|
|
resp = _scaffold(setup_client, device_id=sample_device.id)
|
|
assert resp.status_code in (
|
|
400,
|
|
500,
|
|
), f"Expected 4xx/5xx on final-step failure, got {resp.status_code}: {resp.text}"
|
|
|
|
# Zero picture sources remaining
|
|
remaining_ps = picture_source_store.get_all_streams()
|
|
assert len(remaining_ps) == 0, f"Picture sources not rolled back: {remaining_ps}"
|
|
|
|
# Zero color-strip sources remaining
|
|
remaining_css = css_store.get_all_sources()
|
|
assert len(remaining_css) == 0, f"Color-strip sources not rolled back: {remaining_css}"
|
|
|
|
# Zero output targets (trivially true since creation was never reached,
|
|
# but included for completeness)
|
|
remaining_ot = output_target_store.get_all_targets()
|
|
assert len(remaining_ot) == 0, f"Output targets not rolled back: {remaining_ot}"
|
|
finally:
|
|
output_target_store.create_wled_target = original_create
|
|
|
|
def test_final_step_failure_emits_zero_created_events(
|
|
self,
|
|
setup_client,
|
|
sample_device,
|
|
output_target_store,
|
|
event_log,
|
|
):
|
|
"""No 'created' events must be emitted when the final step fails (deferred-event contract)."""
|
|
original_create = output_target_store.create_wled_target
|
|
|
|
def _fail(*args, **kwargs):
|
|
raise ValueError("Final step injected failure")
|
|
|
|
output_target_store.create_wled_target = _fail
|
|
try:
|
|
resp = _scaffold(setup_client, device_id=sample_device.id)
|
|
assert resp.status_code in (400, 500)
|
|
|
|
created_events = [(et, act) for et, act, _ in event_log if act == "created"]
|
|
assert (
|
|
created_events == []
|
|
), f"'created' events leaked on final-step rollback: {created_events}"
|
|
finally:
|
|
output_target_store.create_wled_target = original_create
|
|
|
|
def test_final_step_failure_reused_template_survives(
|
|
self,
|
|
setup_client,
|
|
sample_device,
|
|
template_store,
|
|
output_target_store,
|
|
):
|
|
"""A reused capture template must NOT be deleted when the final step fails."""
|
|
templates_before = {t.id for t in template_store.get_all_templates()}
|
|
original_create = output_target_store.create_wled_target
|
|
|
|
def _fail(*args, **kwargs):
|
|
raise ValueError("Final step injected failure")
|
|
|
|
output_target_store.create_wled_target = _fail
|
|
try:
|
|
_scaffold(setup_client, device_id=sample_device.id)
|
|
finally:
|
|
output_target_store.create_wled_target = original_create
|
|
|
|
templates_after = {t.id for t in template_store.get_all_templates()}
|
|
assert templates_before == templates_after, (
|
|
f"Template set changed after rollback: "
|
|
f"before={templates_before} after={templates_after}"
|
|
)
|
|
|
|
def test_final_step_failure_device_not_deleted(
|
|
self,
|
|
setup_client,
|
|
sample_device,
|
|
device_store,
|
|
output_target_store,
|
|
):
|
|
"""The pre-existing device must never be touched by rollback of any step."""
|
|
original_create = output_target_store.create_wled_target
|
|
|
|
def _fail(*args, **kwargs):
|
|
raise ValueError("Final step injected failure")
|
|
|
|
output_target_store.create_wled_target = _fail
|
|
try:
|
|
_scaffold(setup_client, device_id=sample_device.id)
|
|
finally:
|
|
output_target_store.create_wled_target = original_create
|
|
|
|
device = device_store.get(sample_device.id)
|
|
assert device is not None, "Pre-existing device was deleted during rollback"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Validation: display_index bounds
|
|
# Criteria: display_index > 63 → 422; display_index 63 → 201; negative → 422
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestDisplayIndexBounds:
|
|
def test_display_index_64_returns_422(self, setup_client, sample_device):
|
|
"""display_index=64 (one above max) must be rejected with 422."""
|
|
resp = _scaffold(setup_client, device_id=sample_device.id, display_index=64)
|
|
assert (
|
|
resp.status_code == 422
|
|
), f"Expected 422 for display_index=64, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_display_index_63_returns_201(self, setup_client, sample_device):
|
|
"""display_index=63 is the maximum valid value — must be accepted."""
|
|
resp = _scaffold(setup_client, device_id=sample_device.id, display_index=63)
|
|
assert (
|
|
resp.status_code == 201
|
|
), f"Expected 201 for display_index=63, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_display_index_0_returns_201(self, setup_client, sample_device):
|
|
"""display_index=0 is the minimum valid value."""
|
|
resp = _scaffold(setup_client, device_id=sample_device.id, display_index=0)
|
|
assert (
|
|
resp.status_code == 201
|
|
), f"Expected 201 for display_index=0, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_display_index_negative_1_returns_422(self, setup_client, sample_device):
|
|
"""display_index=-1 must be rejected with 422."""
|
|
resp = _scaffold(setup_client, device_id=sample_device.id, display_index=-1)
|
|
assert (
|
|
resp.status_code == 422
|
|
), f"Expected 422 for display_index=-1, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_display_index_very_large_returns_422(self, setup_client, sample_device):
|
|
"""display_index=10000 must be rejected with 422."""
|
|
resp = _scaffold(setup_client, device_id=sample_device.id, display_index=10000)
|
|
assert (
|
|
resp.status_code == 422
|
|
), f"Expected 422 for display_index=10000, got {resp.status_code}: {resp.text}"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Validation: missing / unknown device_id
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestDeviceValidation:
|
|
def test_missing_device_id_returns_422(self, setup_client):
|
|
"""Omitting device_id entirely must yield 422 (Pydantic required field)."""
|
|
resp = setup_client.post("/api/v1/setup/scaffold", json={"display_index": 0})
|
|
assert (
|
|
resp.status_code == 422
|
|
), f"Expected 422 for missing device_id, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_empty_string_device_id_handled(self, setup_client):
|
|
"""device_id='' — should yield 404 (empty string not in device store) or 422."""
|
|
resp = _scaffold(setup_client, device_id="")
|
|
assert resp.status_code in (
|
|
404,
|
|
422,
|
|
), f"Expected 404 or 422 for empty device_id, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_unknown_device_id_returns_404(self, setup_client):
|
|
"""device_id that does not exist in the store must yield 404."""
|
|
resp = _scaffold(setup_client, device_id="device_definitely_does_not_exist")
|
|
assert (
|
|
resp.status_code == 404
|
|
), f"Expected 404 for unknown device_id, got {resp.status_code}: {resp.text}"
|
|
|
|
def test_none_device_id_returns_422(self, setup_client):
|
|
"""device_id=null must yield 422 (not None-convertible to str)."""
|
|
resp = setup_client.post(
|
|
"/api/v1/setup/scaffold", json={"device_id": None, "display_index": 0}
|
|
)
|
|
assert (
|
|
resp.status_code == 422
|
|
), f"Expected 422 for null device_id, got {resp.status_code}: {resp.text}"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Rollback idempotency: calling scaffold twice with the final step failing
|
|
# must still leave exactly zero orphans (no accumulation across calls).
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestRollbackIdempotency:
|
|
def test_two_failed_scaffolds_leave_zero_orphans(
|
|
self,
|
|
setup_client,
|
|
sample_device,
|
|
picture_source_store,
|
|
css_store,
|
|
output_target_store,
|
|
):
|
|
"""Two sequential scaffold failures must not accumulate orphans."""
|
|
original_create = output_target_store.create_wled_target
|
|
|
|
def _fail(*args, **kwargs):
|
|
raise ValueError("Always fails")
|
|
|
|
output_target_store.create_wled_target = _fail
|
|
try:
|
|
_scaffold(setup_client, device_id=sample_device.id)
|
|
_scaffold(setup_client, device_id=sample_device.id)
|
|
finally:
|
|
output_target_store.create_wled_target = original_create
|
|
|
|
assert (
|
|
len(picture_source_store.get_all_streams()) == 0
|
|
), "Picture sources accumulated across two failed scaffolds"
|
|
assert (
|
|
len(css_store.get_all_sources()) == 0
|
|
), "Color-strip sources accumulated across two failed scaffolds"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Onboarding: adversarial cases
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.fixture
|
|
def pref_client(tmp_db):
|
|
from ledgrab.api.routes.preferences import router
|
|
from ledgrab.api.auth import verify_api_key
|
|
from ledgrab.api import dependencies as deps
|
|
|
|
app = FastAPI()
|
|
app.include_router(router)
|
|
app.dependency_overrides[verify_api_key] = lambda: "test"
|
|
app.dependency_overrides[deps.get_database] = lambda: tmp_db
|
|
|
|
return TestClient(app)
|
|
|
|
|
|
class TestOnboardingAdversarial:
|
|
def test_put_false_clears_completed_at(self, pref_client):
|
|
"""PUT onboarded=false must clear completed_at to null, per criteria."""
|
|
# First mark as onboarded
|
|
r1 = pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": True})
|
|
assert r1.status_code == 200
|
|
assert r1.json()["completed_at"] is not None
|
|
|
|
# Now set to false — completed_at must be cleared
|
|
r2 = pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": False})
|
|
assert r2.status_code == 200
|
|
data = r2.json()
|
|
assert data["onboarded"] is False
|
|
assert (
|
|
data["completed_at"] is None
|
|
), f"completed_at should be null after PUT onboarded=false, got {data['completed_at']!r}"
|
|
|
|
def test_put_false_then_get_returns_null_completed_at(self, pref_client):
|
|
"""After PUT false, GET must also return null completed_at (persisted correctly)."""
|
|
pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": True})
|
|
pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": False})
|
|
resp = pref_client.get("/api/v1/preferences/onboarding")
|
|
assert resp.status_code == 200
|
|
assert (
|
|
resp.json()["completed_at"] is None
|
|
), "GET after PUT false should return null completed_at"
|
|
|
|
def test_corrupt_stored_value_falls_back_to_default(self, tmp_db, pref_client):
|
|
"""If the stored onboarding value is corrupt, GET must fall back to default.
|
|
|
|
Criteria: "corrupt stored value falls back to default".
|
|
We inject garbage into the db directly, then hit GET.
|
|
"""
|
|
# Inject a value that is syntactically valid JSON (dict) but fails
|
|
# Pydantic validation because the types are wrong.
|
|
tmp_db.set_setting("onboarded", {"onboarded": "not_a_bool", "completed_at": 12345})
|
|
|
|
resp = pref_client.get("/api/v1/preferences/onboarding")
|
|
assert (
|
|
resp.status_code == 200
|
|
), f"Expected 200 for corrupt onboarding value, got {resp.status_code}"
|
|
data = resp.json()
|
|
# Must fall back to default
|
|
assert (
|
|
data["onboarded"] is False
|
|
), f"Expected onboarded=false as default after corrupt value, got {data['onboarded']!r}"
|
|
assert (
|
|
data["completed_at"] is None
|
|
), f"Expected completed_at=null as default, got {data['completed_at']!r}"
|
|
|
|
def test_corrupt_stored_value_as_wrong_type_falls_back(self, tmp_db, pref_client):
|
|
"""Stored value is a string (not a dict) — must fall back to default."""
|
|
tmp_db.set_setting("onboarded", "this_is_not_valid")
|
|
|
|
resp = pref_client.get("/api/v1/preferences/onboarding")
|
|
assert resp.status_code == 200
|
|
data = resp.json()
|
|
assert data["onboarded"] is False
|
|
assert data["completed_at"] is None
|
|
|
|
def test_corrupt_stored_null_falls_back_to_default(self, tmp_db, pref_client):
|
|
"""Stored value is null/None — must return default (not crash)."""
|
|
tmp_db.set_setting("onboarded", None)
|
|
|
|
resp = pref_client.get("/api/v1/preferences/onboarding")
|
|
assert resp.status_code == 200
|
|
assert resp.json()["onboarded"] is False
|
|
|
|
def test_put_true_without_completed_at_stamps_timestamp(self, pref_client):
|
|
"""PUT onboarded=true without completed_at must auto-stamp a timestamp."""
|
|
resp = pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": True})
|
|
assert resp.status_code == 200
|
|
data = resp.json()
|
|
assert (
|
|
data["completed_at"] is not None
|
|
), "Server must auto-stamp completed_at when onboarded=true is sent without a timestamp"
|
|
# Should be a non-empty ISO timestamp string
|
|
assert len(data["completed_at"]) > 10
|
|
|
|
def test_put_true_with_completed_at_preserves_it(self, pref_client):
|
|
"""PUT onboarded=true with explicit completed_at must preserve that value."""
|
|
ts = "2025-03-15T10:00:00+00:00"
|
|
resp = pref_client.put(
|
|
"/api/v1/preferences/onboarding",
|
|
json={"onboarded": True, "completed_at": ts},
|
|
)
|
|
assert resp.status_code == 200
|
|
assert resp.json()["completed_at"] == ts
|
|
|
|
def test_put_false_with_completed_at_clears_it(self, pref_client):
|
|
"""PUT onboarded=false even with a completed_at payload must clear it.
|
|
|
|
The criteria say: 'Setting onboarded=false clears completed_at to null.'
|
|
"""
|
|
ts = "2025-01-01T00:00:00+00:00"
|
|
resp = pref_client.put(
|
|
"/api/v1/preferences/onboarding",
|
|
json={"onboarded": False, "completed_at": ts},
|
|
)
|
|
assert resp.status_code == 200
|
|
data = resp.json()
|
|
assert data["onboarded"] is False
|
|
assert (
|
|
data["completed_at"] is None
|
|
), f"completed_at should be cleared to null when onboarded=false, got {data['completed_at']!r}"
|
|
|
|
def test_multiple_true_puts_only_stamp_once(self, pref_client):
|
|
"""Two successive PUT true calls — second call must preserve the original timestamp."""
|
|
r1 = pref_client.put("/api/v1/preferences/onboarding", json={"onboarded": True})
|
|
ts1 = r1.json()["completed_at"]
|
|
assert ts1 is not None
|
|
|
|
# Explicitly provide the original timestamp in second call
|
|
r2 = pref_client.put(
|
|
"/api/v1/preferences/onboarding",
|
|
json={"onboarded": True, "completed_at": ts1},
|
|
)
|
|
assert (
|
|
r2.json()["completed_at"] == ts1
|
|
), "Explicit completed_at should be preserved on second PUT"
|