diff --git a/app/src/test-fixtures.js b/app/src/test-fixtures.js index 3b7df007..54d79542 100644 --- a/app/src/test-fixtures.js +++ b/app/src/test-fixtures.js @@ -28,10 +28,26 @@ import {defaultTheme} from './themes/names'; /* API shapes */ /* --------------------------------------------------------------------------- */ +/* + * GET /api/settings and GET /api/status. + * + * Every field each endpoint returns, and nothing it does not. Held to the API's own OpenAPI + * spec by wrolpi/test/test_api_fixtures.py, which is where drift now surfaces: seven settings + * fields were missing from this file, and the status fixture had two -- `cpu_percent` and + * `memory_percent` -- that no endpoint has ever returned and no component has ever read. They + * came from a hand-written cypress body and had been copied onwards since. + * + * A missing field is invisible in the frontend suite: the component reads `undefined` from a + * body the real API would have filled in, and every assertion about everything else still + * passes. + */ + /** GET /api/settings */ export const settingsFixture = (overrides = {}) => ({ archive_destination: 'archive/%(domain_tag)s/%(domain)s', check_for_upgrades: true, + download_daily_limit_global: null, + download_daily_limit_per_domain: null, download_manager_disabled: false, download_manager_stopped: false, download_on_startup: true, @@ -44,16 +60,22 @@ export const settingsFixture = (overrides = {}) => ({ hotspot_password: 'wrolpi hotspot', hotspot_ssid: 'WROLPi', ignore_outdated_zims: false, + // Relative to the media directory, as the API returns them. + ignored_directories: [], log_level: 'info', + map_default_location: null, map_destination: 'map', // `/media/wrolpi` on a Pi, but the API is the authority on this and a test that // hardcodes it elsewhere is asserting its own guess. media_directory: '/media/wrolpi', nav_color: 'violet', playlists_destination: 'playlists', + require_cookies_unlocked: false, + require_media_mounted: false, save_ffprobe_json: true, tags_directory: true, throttle_on_startup: false, + timezone: null, version: '1.0.0', videos_destination: 'videos/%(channel_tag)s/%(channel_name)s', wrol_mode: false, @@ -61,12 +83,25 @@ export const settingsFixture = (overrides = {}) => ({ ...overrides, }); -/** GET /api/status */ +/** + * GET /api/status + * + * The stats are empty and the counters idle: a healthy machine with nothing happening, which is + * the state most specs want as a starting point. A spec testing a warning threshold or an + * available upgrade overrides the one field it is about. + */ export const statusFixture = (overrides = {}) => ({ - version: '1.0.0', - flags: {}, - cpu_percent: 10, - memory_percent: 30, + cpu_stats: { + cores: 4, + cur_frequency: 1500, + max_frequency: 1800, + min_frequency: 600, + percent: 10, + temperature: 45, + high_temperature: 75, + critical_temperature: 85, + }, + dockerized: false, downloads: { pending: 0, recurring: 0, @@ -74,6 +109,24 @@ export const statusFixture = (overrides = {}) => ({ stopped: false, outside_download_window: false, }, + drives_stats: [], + flags: {}, + is_rpi: false, + is_rpi4: false, + is_rpi5: false, + load_stats: {minute_1: 0.1, minute_5: 0.1, minute_15: 0.1}, + local_time: '2026-07-30T12:00:00-06:00', + memory_stats: {total: 4000000000, used: 1000000000, free: 3000000000, cached: 500000000}, + processes_stats: [], + sanic_workers: {}, + version: '1.0.0', + wrol_mode: false, + // Upgrade info, seeded by the API and refreshed by its upgrade check. + commits_behind: 0, + current_commit: null, + git_branch: null, + latest_commit: null, + update_available: false, ...overrides, }); diff --git a/wrolpi/schema.py b/wrolpi/schema.py index 5bfef337..61afee8b 100644 --- a/wrolpi/schema.py +++ b/wrolpi/schema.py @@ -27,17 +27,24 @@ class SettingsResponse: hotspot_on_startup: bool hotspot_password: str hotspot_ssid: str - hotspot_status: str - ignore_outdated_zims: str - ignored_directories: str + # `hotspot_status` and `throttle_status` were declared here long after the handler stopped + # returning them -- both moved to the Controller (/controller/api/hotspot/status and + # /controller/api/throttle/status). This is what /docs publishes, so a field it names and + # the endpoint never sends is a promise to a caller that cannot be kept; + # test/test_api_fixtures.py now holds the two together. + check_for_upgrades: bool + ignore_outdated_zims: bool + ignored_directories: List[str] log_level: str + map_default_location: Optional[dict] nav_color: str media_directory: str require_cookies_unlocked: bool require_media_mounted: bool + save_ffprobe_json: bool tags_directory: bool throttle_on_startup: bool - throttle_status: str + timezone: Optional[str] version: str wrol_mode: bool archive_destination: str = 'archive/%(domain_tag)s/%(domain)s' @@ -188,26 +195,39 @@ class DiskBandwidthStatusStat: @dataclass class StatusResponse: + """The response of GET /api/status. + + Half of this is assembled from `shared_ctx.status`, which the status worker fills in. Its + keys are not open-ended: `initialize_configs_contexts` seeds exactly the stats and upgrade + keys below and the worker only ever updates those, which is what makes this declarable at + all. Add a key to that seed and it belongs here too -- test/test_api_fixtures.py compares + the two. + + `hotspot_ssid`, `hotspot_status`, `throttle_status`, `last_status`, `drive_status`, + `disk_bandwidth_stats` and `nic_bandwidth_stats` were declared here after the handler had + stopped returning them; hotspot and throttle moved to the Controller. + """ cpu_stats: CPUStatusResponse - disk_bandwidth_stats: dict[str, DiskBandwidthStatusStat] dockerized: bool downloads: int - drive_status: List[DriveStatusStat] + drives_stats: List[DriveStatusStat] flags: FlagsStatusResponse - hotspot_ssid: str - hotspot_status: str is_rpi4: bool is_rpi5: bool is_rpi: bool - last_status: str load_stats: LoadStatusStat + local_time: str memory_stats: MemoryStatusStat - nic_bandwidth_stats: dict[str, NicBandwidthStatusStat] processes_stats: List[ProcessStatusStat] sanic_workers: dict - throttle_status: str version: str wrol_mode: str + # Upgrade info, seeded in contexts.py and refreshed by the upgrade check. + commits_behind: int + current_commit: Optional[str] + git_branch: Optional[str] + latest_commit: Optional[str] + update_available: bool @dataclass diff --git a/wrolpi/test/test_api_fixtures.py b/wrolpi/test/test_api_fixtures.py new file mode 100644 index 00000000..892af397 --- /dev/null +++ b/wrolpi/test/test_api_fixtures.py @@ -0,0 +1,195 @@ +"""Hold the frontend's test fixtures to the API's OpenAPI spec. + +The React specs stub API responses with the fixtures in `app/src/test-fixtures.js`. Nothing +connected those to the API, so they were a snapshot of what the endpoints returned whenever +each was written, and they drifted silently: a field added to an endpoint simply never appeared +in them, and every frontend test carried on passing against a body the API had stopped +returning. A fixture that has drifted is worse than none, because the test still passes. + +These tests close the loop in two steps, because the spec was not trustworthy either -- it +declared `hotspot_status` and `throttle_status`, which moved to the Controller and which the +handler had long stopped returning, and omitted four fields it did return: + + handler <-> OpenAPI spec <-> test-fixtures.js + +The first pair keeps the published spec honest about the endpoint (it is what /docs shows a +user, so it is worth being right on its own account). The second keeps the fixtures honest +about the spec. Neither can drift without failing here. +""" +import pathlib +import re + +import pytest + +TEST_FIXTURES_JS = pathlib.Path(__file__).parents[2] / 'app/src/test-fixtures.js' + + +def spec_properties(spec: dict, path: str, method: str = 'get') -> set: + """The property names the OpenAPI spec promises for an endpoint's response. + + sanic-ext inlines the response schema under the path rather than naming it in + components/schemas, so read it from where it actually is. + """ + try: + responses = spec['paths'][path][method]['responses'] + except KeyError as e: + raise AssertionError(f'{method.upper()} {path} is missing from the OpenAPI spec') from e + + # 'default' is what sanic-ext uses when the definition names no status code. + response = responses.get('default') or responses.get('200') + assert response, f'{method.upper()} {path} declares no response schema' + content = response['content'] + schema = (content.get('*/*') or content.get('application/json'))['schema'] + properties = schema.get('properties') + assert properties, f'{method.upper()} {path} declares a response with no properties' + return set(properties) + + +def fixture_keys(name: str) -> set: + """The top-level keys of a fixture factory in test-fixtures.js. + + Read from source rather than executed: node is not necessarily available where pytest runs, + and this only needs the shape. Anchoring on four-space indentation takes the top level + only, leaving nested objects (statusFixture's `downloads`) out of it. + """ + source = TEST_FIXTURES_JS.read_text() + match = re.search( + rf'export const {name} = \(overrides = {{}}\) => \(\{{(.*?)\n}}\);', + source, + re.S, + ) + assert match, f'{name} is not defined in {TEST_FIXTURES_JS.name} in the expected form' + body = match.group(1) + # Drop nested objects so their inner keys are not mistaken for top-level ones. + body = re.sub(r'\{[^{}]*}', '{}', body) + # Digits included: `is_rpi4` and `is_rpi5` are real keys, and a reader that skips them + # reports the fixture as missing fields it actually has. + keys = set(re.findall(r'^ ([a-z0-9_]+):', body, re.M)) + assert keys, f'{name} appears to have no keys' + return keys + + +async def get_spec(async_client) -> dict: + request, response = await async_client.get('/docs/openapi.json') + assert response.status == 200, 'the OpenAPI spec is not being served' + return response.json + + +@pytest.mark.asyncio +async def test_settings_spec_matches_the_handler(async_client, test_directory): + """What /api/settings returns is what the spec says it returns.""" + spec = await get_spec(async_client) + declared = spec_properties(spec, '/api/settings') + + request, response = await async_client.get('/api/settings') + assert response.status == 200 + returned = set(response.json) + + assert declared == returned, ( + 'the OpenAPI spec and /api/settings disagree.\n' + f' spec declares, handler omits: {sorted(declared - returned)}\n' + f' handler returns, spec omits: {sorted(returned - declared)}' + ) + + +@pytest.mark.asyncio +async def test_settings_fixture_matches_the_spec(async_client): + """app/src/test-fixtures.js settingsFixture() carries every field the API returns. + + A missing field is invisible in the frontend suite: the component reads `undefined` from a + body the real API would have filled in, and the assertion about everything else still + passes. + """ + spec = await get_spec(async_client) + declared = spec_properties(spec, '/api/settings') + fixture = fixture_keys('settingsFixture') + + assert declared == fixture, ( + 'settingsFixture has drifted from /api/settings.\n' + f' API returns, fixture omits: {sorted(declared - fixture)}\n' + f' fixture invents: {sorted(fixture - declared)}' + ) + + +@pytest.mark.asyncio +async def test_status_spec_matches_the_handler(async_client, test_directory): + spec = await get_spec(async_client) + declared = spec_properties(spec, '/api/status') + + request, response = await async_client.get('/api/status') + assert response.status == 200 + returned = set(response.json) + + assert declared == returned, ( + 'the OpenAPI spec and /api/status disagree.\n' + f' spec declares, handler omits: {sorted(declared - returned)}\n' + f' handler returns, spec omits: {sorted(returned - declared)}' + ) + + +@pytest.mark.asyncio +async def test_status_fixture_matches_the_spec(async_client): + spec = await get_spec(async_client) + declared = spec_properties(spec, '/api/status') + fixture = fixture_keys('statusFixture') + + assert declared == fixture, ( + 'statusFixture has drifted from /api/status.\n' + f' API returns, fixture omits: {sorted(declared - fixture)}\n' + f' fixture invents: {sorted(fixture - declared)}' + ) + + +@pytest.mark.asyncio +async def test_which_fixtures_the_spec_can_vouch_for(async_client): + """Name the API-shaped fixtures this file does not cover, and why. + + `domainFixture` and `tagFixture` stand in for endpoint responses too, but the endpoints + behind them carry an `@openapi.definition` with a description and no `response=`, so the + spec promises nothing about their bodies and there is nothing to compare against. They are + therefore still hand-maintained guesses. + + This is a ratchet: declare a response on one of those endpoints, add its fixture to the + checks above, and delete its line here. Without it the gap reads as coverage. + """ + spec = await get_spec(async_client) + + checked = {'settingsFixture': '/api/settings', 'statusFixture': '/api/status'} + for name, path in checked.items(): + assert spec_properties(spec, path), f'{name} claims to be checked against {path}' + + unchecked = { + 'domainFixture': 'the domains endpoint declares no response schema', + 'tagFixture': 'GET /api/tag declares no response schema', + # This one is a provider value rather than an API body, so it does not belong above -- + # but it is unguarded at the other end too. test-fixtures.test.js compares each + # context fixture against its `createContext` default, and this context's default is + # `null` on purpose: `useFileWorkerStatus` relies on that to detect a missing provider. + # So there is no declared shape at either end. Naming it beats it looking covered. + 'fileWorkerStatusFixture': 'FileWorkerStatusContext defaults to null, by design', + } + + source = TEST_FIXTURES_JS.read_text() + api_fixtures = set(re.findall(r'^export const ([a-zA-Z]+Fixture) =', source, re.M)) + # domainsFixture is a list built from domainFixture, and the *Context* fixtures are held to + # their React contexts by the frontend's own test-fixtures.test.js. + api_fixtures -= {'domainsFixture'} + api_fixtures = {n for n in api_fixtures if 'Context' not in n} + + assert api_fixtures == set(checked) | set(unchecked), ( + 'an API-shaped fixture is neither checked against the spec nor listed as uncheckable:\n' + f' {sorted(api_fixtures - set(checked) - set(unchecked))}' + ) + + +def test_the_fixture_reader_actually_reads(): + """Guard the regex above: it silently returning nothing would make every check vacuous.""" + keys = fixture_keys('settingsFixture') + + assert 'media_directory' in keys + assert len(keys) > 10 + # Nested objects must not leak their keys into the top level. + assert 'pending' not in fixture_keys('statusFixture') + # Keys containing digits must be read; missing them reports fields the fixture has as + # absent, which is exactly what this reader did at first. + assert {'is_rpi4', 'is_rpi5'}.issubset(fixture_keys('statusFixture'))