WIP: Hue lan integration tests - #3177
Open
varzac wants to merge 22 commits into
Open
Conversation
dkjson.decode returns (value, next_position, error_message), but process_rest_response propagated all of pcall's captured return values after decoding, not just the decoded value its own doc comment promises. That means the parse position (e.g. 74 for a 73-byte body) gets returned in the position every caller treats as `err`, so every successful REST call with a JSON body logs a spurious "Error performing <action>: <parse position>". Found via the first integration test to exercise a real, successful JSON-decoded REST response through this path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds hue_test_helpers.lua, a shared fixture for building a "known, already-paired" bridge + light so added/init lifecycle resolves synchronously to a steady state instead of falling into discovery/ pairing (covered separately). SSE is deliberately left uninitialized -- these fixtures are REST-path only. test_hue_light_commands.lua covers switch on/off, setLevel, and setColorTemperature, asserting the exact outbound REST PUT each produces via a real connect -> TLS handshake -> HTTP request round trip against the new mock LAN socket. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds hue_test_helpers.mark_bridge_initialized, letting a test mark a bridge's Fields._INIT directly instead of running do_bridge_network_init for real -- that function also creates the bridge's SSE EventSource, which connects to the same host:port as REST calls; since the mock LAN socket models one connection per address, letting it run would interleave the SSE connection's bytes into these REST-focused tests' assertions. test_hue_light_refresh.lua covers the refresh capability command's full non-cached path: two REST calls to resolve zigbee connectivity (required before any light attribute events can emit at all), then the light state REST call, asserting the resulting switch/switchLevel capability events for both an on/bright and an off/dim response. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers HueDiscovery.discover's core flow: mDNS scan -> GET /api/config to resolve the bridge's MAC (used to derive its device_network_id) -> POST /api to request an API key. Asserts two outcomes: the Link Button not being pressed (no device created), and a successful key exchange (bridge device created via try_create_device with the expected metadata). Discovery is triggered via the "discovery" channel's start/stop messages, the same path the real hub uses when a user initiates a scan (see st.handlers.discovery_message_handlers) -- this runs Discovery.discover inside a real cosock-managed thread, which is required since it makes blocking (mocked) REST calls internally that only resolve correctly in that context. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A child device add is dispatched through LifecycleHandlers.device_added, which checks disco's device_state_disco_cache *before* ever calling into the device-type-specific added handler (e.g. LightLifecycleHandlers.added) -- so a light added without cached resource state is routed to StrayDeviceHelper rather than reaching light.lua's own separate "fetch over REST if not cached" branch, which is unreachable through this path. Asserts that a child device added this way doesn't crash, makes no REST calls, and still gets its unconditional levelRange event on init. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
table.pack(pcall(json.decode, msg.data)) followed by table.remove(...,1) to strip the pcall success flag left `events, err = table.unpack(...)` capturing dkjson's second return value (the position it stopped scanning at, a non-nil number even on success) into `err` instead of its real third return value. Every SSE message was therefore logged as a JSON parse error and dropped without ever reaching the update/add/delete handling below -- there was no prior test coverage of this path to catch it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds opts.enable_sse to build_paired_bridge_and_light: a fixture variant with a qualifying swversion and driver.joined_bridges pre-set so do_bridge_network_init runs for real (existing fixtures deliberately route around it), plus a get_sse_connection() accessor for the reserved SSE connection built on scripting-engine's new labeled-connection support. test_hue_bridge_sse.lua covers: the EventSource connect/handshake marking the bridge online, an "update" SSE event emitting a light's attributes, an "add" event creating a new device, a "delete" event deleting one, and a dropped connection marking everything offline. Two things race to connect to the bridge address right after test_init returns (the EventSource itself and the light's own unconditionally-injected refresh), so identify_sse_and_rest_connections() disambiguates after the fact rather than assuming an order. Not covered: reconnecting successfully after a drop. That needs a real fix to cosock/timers.lua (a due-timer off-by-one) combined with a fix for the previously-dormant retry loops it would otherwise wake up elsewhere -- tracked separately, not attempted here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both loops retried a failed rooms/zones or connectivity-status fetch forever with no backoff, hammering the bridge at unthrottled speed on any fast-failing response (empty/errored body) rather than a real timeout, which is naturally rate-limited by the REST client's own settimeout. Cap each at 5 attempts with exponential backoff, logging and giving up gracefully rather than spinning indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
grouped_utils.queue_group_scan's own 45-second debounce timer now genuinely fires during any test (see the cosock timer fix), sending real, unmocked rooms/zones REST requests that interleave with whatever a test is asserting on the shared bridge connection. Add a scanning_enabled flag, defaulted off in every fixture built by hue_test_helpers -- a test that specifically wants to exercise group scanning can set it back on itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The driver template unconditionally spawns StrayDeviceHelper, a background thread with its own perpetually re-arming 30-second timeout. Now that cosock's timers actually fire, that keeps the mock scheduler legitimately advancing mock time forever chasing it -- which can let some other, shorter-lived timeout a test needs to answer first (e.g. an injected refresh's 45-second REST reply-channel timeout) elapse for real before the test's own coroutine ever gets a turn to respond. Opt into integration_test.set_test_coroutine_priority to fix that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
With cosock's timers fixed and this test suite's coroutine-priority opt-in in place, the real default 1-second EventSource reconnect delay now resolves correctly, so the previously-descoped os.execute/retry:0 workaround is no longer needed. Restore the reconnect-completes-and-comes-back-online assertions: reserve the next "sse" connection ahead of the reconnect, answer its handshake, and confirm the bridge and light are marked online again once the reconnect's own connectivity poll reports the light connected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Invitation URL: |
|
philips-hue_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against 896179a |
Test Results 73 files 551 suites 0s ⏱️ For more details on these errors, see this check. Results for commit 896179a. ♻️ This comment has been updated with latest results. |
varzac
commented
Aug 13, 2026
| ) | ||
| return | ||
| end | ||
| cosock.socket.sleep(backoff()) |
Contributor
Author
There was a problem hiding this comment.
not sure I agree with this change. I'll have to investigate further
Contributor
There was a problem hiding this comment.
The max attempts could potentially be problematic because we only queue scans on connect or device adds so we might end up with no group information. I think the backoff is fine and might actually prevent a hot loop if the api calls don't end up yielding for some reason.
NoahCornell
reviewed
Aug 13, 2026
| ) | ||
| ) | ||
| else | ||
| cosock.socket.sleep(backoff()) |
Contributor
There was a problem hiding this comment.
Same feeling here, the backoff is probably good but not getting the connectivity of the devices could leave some offline until the we reconnect sse or until the device connectivity changes and results in new sse event. This happens in a spawned cosock task so probably no harm in continuing to keep trying.
NoahCornell
reviewed
Aug 13, 2026
| -- without needing to mock rooms/zones REST responses or race the scan's own 45-second debounce | ||
| -- timing against whatever else a test is asserting on the same connection. Defaults to true; | ||
| -- production code never touches this. | ||
| grouped_utils.scanning_enabled = true |
Contributor
There was a problem hiding this comment.
I think its fine but I think ideally the mock bridge server would be able to be configured to default respond to these requests with an empty valid response.
Update integration tests to use the renamed function from the lan_test_utils module, which now correctly indicates after-the-fact assertion semantics rather than pre-action expectation setup.
Remove the 5-attempt cap on group scan retries. In production, transient bridge issues (network blips, temporary API unavailability) could cause permanent loss of group/scene support. Retry indefinitely with exponential backoff (capped at 30s) until successful. Test environments are unaffected: group scanning is already disabled by default via grouped_utils.scanning_enabled = false in test fixtures.
Remove the 5-attempt cap on connectivity poll retries for consistency with group scanning. Retry indefinitely with exponential backoff until successful, avoiding permanent loss of connectivity monitoring due to transient failures.
Add integration tests for color control capabilities: - setColor: Verifies HSV to XY conversion and color.xy REST format - setHue: Tests partial color update using existing saturation - setSaturation: Tests partial color update using existing hue All tests verify proper REST request structure and light auto-on behavior. Test coverage: 7/7 passing (4 original + 3 new)
Add comment noting that command tests use white-and-color-ambiance profile which supports all capabilities. Profile-specific restrictions (white-only, white-ambiance) are implicitly validated through the profile YAML definitions and are better tested in refresh scenarios where attribute presence matters. Test coverage: 7/7 passing
Add build_paired_bridge_and_child() helper function to support testing non-light device types (buttons, sensors) with SSE enabled. This is a more flexible version of build_paired_bridge_and_light() that: - Accepts any device type (button, motion, contact, etc.) - Allows custom init event expectations via callback - Properly handles SSE setup for sensor/button devices - Bypasses light-specific lifecycle assumptions This infrastructure enables future expansion of test coverage to buttons and sensors once SSE test patterns are fully understood. Test coverage: 24/24 passing (17 original + 7 new Tier 1)
Add comprehensive integration tests for button devices with SSE support: Test files: - test_hue_button_lifecycle.lua: Basic button lifecycle test (1 test) - test_hue_button_sse.lua: SSE event handling tests (4 tests) Tests verify: - Button lifecycle handlers (added/init) work correctly - supportedButtonValues capability emitted during init - SSE connection establishes for button devices - Button refresh REST call sequence (5 calls: device info, zigbee connectivity, device info again, button details, device power) - short_release SSE events → pushed capability events - long_press SSE events → held capability events - Battery level updates via SSE - Relaxed event ordering for simultaneous battery+button events Key learnings from incremental development: - Hue resource IDs must use UUID format (not simple strings) - Button init injects refresh via _REFRESH_AFTER_INIT field - Button refresh makes 5 REST calls (vs 3 for lights) - Connection racing: SSE EventSource and REST both connect to same host:port; labeled connection helps identify which is which - Button events route via hue_id_to_device mapping set up during added/init lifecycle handlers Test coverage: 22/22 passing (17 original + 5 new button tests)
Add comprehensive SSE integration tests for multi-button remotes and motion sensors, completing the sensor/button SSE test coverage. **Multi-button remote tests (test_hue_multibutton_sse.lua - 4 tests):** - 4-button remote lifecycle with all button components - Event routing to correct components (main, button2, button3, button4) - BUTTON_INDEX_MAP verification for multi-button devices - 8 REST calls during refresh (device info, zigbee connectivity, device info again, 4x button details, device power) **Motion sensor tests (test_hue_motion_sensor_sse.lua - 6 tests):** - Motion sensor lifecycle with all capabilities - Motion active/inactive SSE events - Temperature update events (Celsius) - Illuminance update events (lux conversion from Hue log scale) - Battery level updates - Combined updates with multiple attributes (relaxed ordering) - 7 REST calls during refresh (device info, zigbee connectivity, device info again, motion, temperature, light_level, device power) Key patterns established: - Device-specific refresh sequences vary by sensor type - Multi-service devices register multiple RIDs in hue_id_to_device - Relaxed event ordering for multi-attribute updates - Illuminance formula: lux = round(10^((light_level - 1) / 10000)) Test coverage: 32/32 passing (17 original + 15 new sensor/button tests)
Extract common SSE test infrastructure to hue_test_helpers.lua to reduce duplication and provide clear patterns for future SSE tests. Changes: - Add identify_sse_and_rest_connections() helper function Handles connection racing between SSE EventSource and REST API that both connect to same host:port. Inspects sent bytes to determine which connection is which. - Add comprehensive SSE test pattern documentation Documents the standard 5-step pattern for SSE device tests: 1. Fixture setup with enable_sse=true 2. Connection helper that drains device refresh 3. Device-specific refresh sequences (documented for all types) 4. Test body pattern with SSE event queuing 5. Event format and multi-attribute handling - Update all SSE test files to use new helper Removes duplicated identify_sse_and_rest_connections() from: - test_hue_button_sse.lua - test_hue_multibutton_sse.lua - test_hue_motion_sensor_sse.lua Benefits: - Single source of truth for SSE connection identification logic - Clear documentation for future test authors - Reduced code duplication (60+ lines removed across tests) - Maintains device-specific refresh logic in each test file for clarity Test coverage: 32/32 passing (no regressions)
Add comprehensive SSE integration tests for contact sensors, completing Tier 2 sensor test coverage. **Contact sensor tests (test_hue_contact_sensor_sse.lua - 6 tests):** - Contact sensor lifecycle with all capabilities - Contact open/closed SSE events - Tamper detected/clear SSE events - Battery level updates - Combined updates with multiple attributes (relaxed ordering) - 6 REST calls during refresh (device info, zigbee connectivity, device info again, contact, tamper, device power) Contact sensor capabilities: - contactSensor (open/closed based on contact_report.state) - "contact" → closed - "no_contact" → open - tamperAlert (clear/detected based on tamper_reports array) - Checks if any tamper_report has state "tampered" - battery (from power_state.battery_level) Follows established SSE test pattern with device-specific refresh sequence and relaxed event ordering for multi-attribute updates. Test coverage: 38/38 passing (17 original + 21 new) Tier 2.3 COMPLETE ✅
Add comprehensive error handling tests covering HTTP errors, malformed responses, and edge cases in both command and refresh flows. **Error handling tests (test_hue_error_handling.lua - 8 tests):** Command error scenarios: - 404 (resource not found) - logs error, no state change - 500 (internal server error) - logs error, no state change - Hue API errors in response body - logs error gracefully - Connection timeout - handles without crashing - Malformed JSON response - parses error gracefully - Empty response body - handles without crashing Refresh error scenarios: - Missing zigbee_connectivity in services - logs error, returns early - 404 for deleted device - logs error gracefully Key behaviors validated: - Errors are logged appropriately - Driver doesn't crash on unexpected responses - No capability events emitted when commands fail - Graceful degradation when services are missing Test pattern: - Uses standard light fixture without SSE - Injects capability commands via __queue_receive - Mocks error responses from bridge - Verifies requests sent and graceful handling Test coverage: 46/46 passing (17 original + 29 new) Tier 2.4 COMPLETE ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests