Add a Django test harness and initial REST API tests - #388
jamesmulcahy wants to merge 16 commits into
Conversation
Adds web/tests/ with a shared NSPMTestCase harness and 97 tests covering rooms, entity create/edit (lights, switches, buttons, thermostats, scenes) and entity/page layout, exercised through the same REST endpoints and payloads the React UI uses. The harness: - runs each test against an isolated in-memory database - replaces the MQTTManager process-control hooks with a recorder, so tests can assert the manager was told to reload and can never signal a real nspm_mqttmanager process on the host - fakes Home Assistant (at the HTTP layer) and OpenHAB, and fails any other network request - provides assertLoadableByManager(), which checks saved rows have the shape MQTTManager parses when it reads the database Known bugs and validation gaps are pinned as @expectedfailure tests with a KNOWN BUG / KNOWN GAP comment, so the suite passes today and flags when a fix lands (remove the decorator then). settings.py now reads the data directory from NSPM_DATA_DIR (default /data, unchanged in the container) so the app can start outside Docker. Run with docker/web/run_tests.sh; CI runs it on PRs touching docker/web. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Adds 113 tests (210 total) covering the rest of the web app: - test_pages: every full page and htmx partial renders, including on a fresh install, and the temperature-sensor pickers route to the configured source - test_settings: settings form (secret handling, trailing slashes, defaults), weather & time, theme, the first-run wizard, the settings REST endpoints and web.settings_helper - test_panels: accept/deny/delete/unblock, the per-panel settings form and the legacy api/ panel endpoints - test_relay_groups: form views, htmx endpoints and the REST listing - test_files: firmware/LittleFS/TFT upload, checksum and download, including which TFT each panel type gets and a contract test for the exclusive Range end the panel firmware depends on - test_misc: messages, HA/OpenHAB entity lookups, hostname lookup and read-only listings New known issues are pinned with @expectedfailure as before. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…itive
banned_setting_keys lists secrets in upper case ("HOME_ASSISTANT_TOKEN"),
but settings are stored in lower case ("home_assistant_token") and the
comparison was case-sensitive, so /rest/mqttmanager/settings returned the
MQTT password and Home Assistant/OpenHAB tokens it was meant to withhold.
Nothing in the repo calls this endpoint (MQTTManager reads settings from
the database directly), so refusing these keys breaks no caller.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
These views changed config MQTTManager reads from the database but never called send_mqttmanager_reload_command(), so panels kept the old config until something else triggered a reload: - DELETE /rest/scenes/<id> - PUT /rest/entities_pages/<id> (page resize) - htmx/nspanels/<id>/delete - htmx relay group save and delete Co-Authored-By: Claude Opus 5.5 <[email protected]>
- POST /rest/rooms now creates the room's entities and scenes pages, as save_new_room does, instead of leaving them to the next restart. The per-room part of create_entity_pages_for_all_rooms() is split out as create_entity_pages_for_room() so both paths share it. - The name must be 1-30 characters (Room.friendly_name's max_length, which SQLite does not enforce). - DELETE /rest/rooms/<id> moves the room's panels to another room, as delete_room does, and tells MQTTManager to reload. Creating a room now reloads too. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Each entry was saved as it was read, so an unknown id part-way through the list returned an error but left the entries before it already moved, a half-applied layout. Run the updates in one transaction so a failed request changes nothing. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The /rest/entities/* endpoints stored room_view_position and entities_page_id as given, so the UI (or any other caller) could: - put two entities in the same slot; - put one in a slot past the end of its page, where the panel never shows it; - put an entity on another room's page. get_placement_error() checks all three and the endpoints return 400 with a message. Only a new or changed placement is checked, so an entity already saved in a bad slot can still be edited in place. PUT /rest/rooms/<id>/entities/order now also rejects slots past the end of a page. It does not check for occupied slots, because a swap moves two entities through each other's slot in one request. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The entity endpoints stored controller and scene_type as given. MQTTManager logs an error for an entity whose controller its class does not handle and skips a scene with an unknown scene_type, so these saved fine but never worked on a panel. Now rejected with 400: - a controller the entity type does not support (e.g. an OpenHAB button); - an unknown scene_type; - a Home Assistant or OpenHAB scene with an empty backend_name, which could never be activated. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…rink - Creating a page (room or global) with any other size is rejected with 400; the panel GUI only has those three layouts. - PUT /rest/entities_pages/<id> checks the new size the same way, and refuses to shrink a page while an entity or scene sits in a slot past the new end. Those entities stayed attached to the page but were never shown on the panel. The UI already only offers 4/8/12. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The method check in entities_thermostats() and entities_scenes() was commented out, so a GET, POST or DELETE was handled as a save. Restore it, and answer unsupported methods on all five /rest/entities/* save endpoints with 405 rather than 403. Co-Authored-By: Claude Opus 5.5 <[email protected]>
- GET /rest/settings returned 500 until the Home Assistant token, the OpenHAB token and the MQTT password had all been saved at least once, because it del'd each key without checking it existed (e.g. OpenHAB never configured). Missing secrets now report *_set: false. - GET /rest/scenes?scene_id=N checked for a light_id parameter, then filtered on scene_id, so the filter was ignored and every scene came back. - The checksum_firmware, checksum_data_file and checksum_tft_file endpoints returned 200 with the body "None" when no image had been uploaded. They now return 404 with an empty body. The panel firmware does not check the status code, so for a panel this only changes the string it compares against its stored checksum. Co-Authored-By: Claude Opus 5.5 <[email protected]>
- api/get_nspanel_config always returned 500: it read button1/2_detached_mode_light, which were renamed to button1/2_detached_mode_entity. Use the new names, and fix the else branch writing -1 to "button1_detached_mode_light" instead of the "button1_detached_light" key the other branch sets. Kept rather than removed because panels on older firmware may still call it. - Remove htmx/partial/nspanel_index_view/<id>, which always raised ImproperlyConfigured (nspanel_status_header has no template for that URL name), and htmx/partial/edit_entities_page/<id>, whose template was removed in the move to React, along with its save/<page_type> route and the edit button in the old htmx entities-page component that linked to it. Nothing in the current UI reaches any of them. Co-Authored-By: Claude Opus 5.5 <[email protected]>
- The htmx first-run wizard and the index panels partial are gone; the React InitialSetup now saves everything with one POST /rest/settings. Test that endpoint (saves, only touches submitted keys, rejects bad payloads, reloads once) and that the index shows InitialSetup until a manager address is configured. - /rest/mqttmanager/settings (POST) was removed; keep the banned-key tests for the per-key GET endpoint. - GET /rest/rooms now returns friendly_name, display_order and the temperature sensor fields; PUT /rest/rooms updates a room. Pin the display_order/displayOrder mismatch in room_put as a known bug. - Thermostats now require use_current_temperature. Co-Authored-By: Claude Opus 5.5 <[email protected]>
040a33b to
f303838
Compare
… install NSPManager#391 replaced the htmx first-run wizard with the React InitialSetup and removed its htmx_initial_setup_* URLs, but base.html still included modals/initial_setup/welcome.html whenever manager_address is empty. That template reverses the removed URLs, so on a fresh install every page, including the index that shows the new wizard, raised NoReverseMatch and returned 500. Remove the include and the now-unreferenced modals/initial_setup templates, and the index.html script tag for initial_setup.js, which NSPManager#391 deleted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
Rebased onto current @tpanajott two things the tests caught in #391:
|
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Write JUnit XML from the test run in CI (via unittest-xml-reporting, enabled by NSPM_TEST_JUNIT_DIR) and publish it with action-junit-report. annotate_only is used because creating a check run needs checks: write, which pull requests from forks don't get. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
This is a huge amount of works, thank you so much for all the help! I think I will hold off on merging this as it will take me a good while reviewing it and and I do not want to do such big changes now when we are but a few days from a new beta. I will however cherrypick commit 328186a. Good catch on that one! This is one of those reasons on why i hate python for larger projects.
Regarding this. I do not now of a good way to enforce correct arguments and types when communicating between the web interface and the back end. The only think I can think of is start using Protobuf for the REST API but that then causes other issues, or simply rely on the test suite to catch thinks but then the tests that run needs to be identical to how React is setup to communicate with the REST API. It seems that if we could somehow integrate Django REST Framework and DRF-spectacular then it might be possible to generate type definitions for the REST endpoints to use in React. |
Summary
Adds automated tests for the Django app. The tests simulate the REST calls the React UI makes and check the resulting database changes, including whether MQTTManager would have been told to reload.
Harness (
web/tests/harness.py, base classNSPMTestCase):send_mqttmanager_reload_command()and the restart/start hooks are replaced with a recorder, so tests canassertManagerReloaded()/assertManagerNotReloaded(). Tests can never signal or spawn a realnspm_mqttmanageron the machine running them./api/states) and OpenHAB at its item-list functions. Any other network request fails the test.assertLoadableByManager(obj)checks a savedEntity/Scenerow has the shape MQTTManager parses when it reads the database, e.g.entity_data["controller"]is a string it recognises.Tests (216, 66% line coverage of
web/;views.py93%,rest.py69%.rest.pydropped from 82% because Feat first page react #391 added media player, panel accept/deny/delete and connection-test endpoints that aren't covered yet):/rest/rooms), creating and editing every entity type through/rest/entities/*, and moving/deleting entities and managing entities pages;POST /rest/settingsand the other settings REST endpoints;Rangeend stays exclusive (the panel firmware'sUpdateManager.cpprelies on this non-RFC convention, so it must not be "fixed" on one side only);Invalid input is checked to be rejected with nothing written and no reload.
CI:
.github/workflows/django-tests.ymlruns the suite on PRs that touchdocker/web/.Bug fixes for problems the tests found, in their own commits after the test commits, so each can be reviewed or dropped on its own:
/rest/mqttmanager/settings/<key>returned secrets.banned_setting_keysis upper case (HOME_ASSISTANT_TOKEN) but settings are stored lower case and the check was case-sensitive, so it returned the MQTT password and HA/OpenHAB tokens. The check is now case-insensitive. Nothing in the repo calls this endpoint.DELETE /rest/scenes/<id>,PUT /rest/entities_pages/<id>(page resize),DELETE /rest/rooms/<id>,htmx/nspanels/<id>/delete, or saving/deleting a relay group via htmx. Panels kept the old config until something else triggered a reload./rest/roomscreate and delete now match the UI's room views. Creating a room adds its entities and scenes pages and enforces the 30-character name limit. Deleting one moves its panels to another room.PUT /rest/rooms/<id>/entities/orderis atomic. A bad entry part-way through used to leave a half-applied layout.controller, an unknownscene_type, or an HA/OpenHAB scene with an emptybackend_name./rest/entities/thermostatsand/rest/entities/sceneshandled any HTTP method as a save. Unsupported methods on all/rest/entities/*endpoints now return 405.GET /rest/settingsreturned 500 until all three secrets had been saved at least once.GET /rest/scenes?scene_id=Nignored the filter and returned every scene.Nonewhen no image had been uploaded. They now return 404.api/get_nspanel_configalways returned 500 because it used the renamedbutton1_detached_mode_light. Also removed two htmx partials that always returned 500 and that nothing in the UI reaches.base.htmlstill included the old htmx setup wizard, whose URLs Feat first page react #391 removed. Fixed in 328186a, which only touches templates, so it can be cherry-picked ontodevelon its own.settings.py: the data directory can now be overridden withNSPM_DATA_DIR(default/data, so nothing changes in the container), so the app can start outside Docker.Running the tests
The easy way, from the repo root (needs uv; it builds and caches a Python 3.12 environment):
Or run
docker/web/run_tests.shin any Python 3.12 environment withdocker/web/requirements.txtinstalled. Anything afterrun_tests.shis passed tomanage.py test:Logging is silenced by default, because many tests send bad input on purpose and the server tracebacks would bury the results. To see them while debugging, prefix the command with
NSPM_TEST_LOG_LEVEL=ERROR.Known issues
Pinned as
@expectedFailurewith aKNOWN BUGcomment. When it gets fixed the test starts passing, which fails the run as a reminder to remove the decorator.PUT /rest/roomsignoresdisplay_order.room_put(added in Feat first page react #391) checks for"displayOrder"but readsdata["display_order"]. Sendingdisplay_order(the keyGET /rest/roomsreturns) is silently ignored, and sendingdisplayOrderreturns 500.Test plan
run_tests.shpasses ondevel(44abbd97, after Feat first page react #391): 216 tests, 1 expected failureuv runcommand above works from a clean checkoutruff check/ruff format --checkclean on the test files🤖 Generated with Claude Code