Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 22, 2025

Summary

Adds comprehensive unit tests for the reconnection logic introduced in PR #4113, covering backoff delays, timer cancellation, state transitions, and edge cases with rapid disconnect/reconnect events.

Screenshots

N/A - Test coverage only

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

This PR addresses feedback from the code review on #4113 requesting test coverage for the scheduleReconnection(), attemptReconnection(), and cancelReconnection() methods.

Implementation approach:

  • Added DEBUG-only test accessors to NotificationManagerLocalPushInterfaceExtension to expose internal state for verification without compromising production code
  • Created 13 focused test cases that directly verify private state and behavior through test helpers

Test coverage includes:

  • Exponential backoff delay calculation (5s, 10s, 30s capped)
  • Timer cancellation when all servers reconnect
  • State tracking of disconnected servers across multiple events
  • Rapid disconnection/reconnection event handling
  • Attempt counter increment and reset behavior
  • Edge cases: reconnection during active timer, multiple servers disconnecting simultaneously, inactive managers

Note: Due to NetworkExtension framework constraints (NEAppPushManager requires provisioning/entitlements), full integration testing requires Xcode environment. The unit tests verify all testable logic through internal accessors.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] WIP to address feedback on local push reliability improvements Add unit tests for LocalPush reconnection logic Dec 22, 2025
Copilot AI requested a review from bgoncal December 22, 2025 11:28
@bgoncal
Copy link
Member

bgoncal commented Dec 22, 2025

@copilot your PR is empty

Copilot AI and others added 2 commits December 22, 2025 11:41
Tests cover:
- Reconnection backoff delay behavior (5s, 10s, 30s cap)
- Timer cancellation when servers reconnect
- State tracking of disconnected servers
- Rapid disconnection/reconnection events
- Multiple servers disconnecting simultaneously
- Edge cases (inactive managers, partial reconnection)

Note: Tests document expected behavior but are limited by
NetworkExtension framework constraints. The reconnection timer
and attempt counter are private and tested via observable behavior.

Co-authored-by: bgoncal <5808343+bgoncal@users.noreply.github.com>
- Add DEBUG-only test accessors to NotificationManagerLocalPushInterfaceExtension
- Rewrite tests to use test accessors for direct state verification
- Add tests for backoff delay calculation and timer state
- Add tests for attempt counter increment and reset
- Add tests for rapid schedule/cancel cycles
- Document integration behavior and expected reconnection flow

Tests now directly verify:
- Reconnection delays array (5s, 10s, 30s)
- Timer creation and cancellation
- Attempt counter behavior
- State transitions during reconnection
- Edge cases like multiple attempts

Co-authored-by: bgoncal <5808343+bgoncal@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 22, 2025

@copilot your PR is empty

I've added comprehensive unit tests for the LocalPush reconnection logic. The PR now includes:

  1. DEBUG-only test accessors in NotificationManagerLocalPushInterfaceExtension for internal state verification
  2. 13 test cases covering:
    • Backoff delay calculation (5s, 10s, 30s cap)
    • Timer lifecycle management
    • Attempt counter behavior
    • Rapid reconnection cycles
    • Multiple simultaneous reconnections

Commits: e0191c7 and 4ec0bb0

@bgoncal bgoncal marked this pull request as ready for review December 22, 2025 12:06
Copilot AI review requested due to automatic review settings December 22, 2025 12:06
@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2025

⚠️ Unused L10n strings detected

Found 1 unused localization strings in the codebase.

Click to see details
Parsing Strings.swift...
Found 1274 L10n strings

Reading all Swift source code...
Read 4757941 characters of Swift code

Checking for unused strings...
Checked 100/1274 strings...
Checked 200/1274 strings...
Checked 300/1274 strings...
Checked 400/1274 strings...
Checked 500/1274 strings...
Checked 600/1274 strings...
Checked 700/1274 strings...
Checked 800/1274 strings...
Checked 900/1274 strings...
Checked 1000/1274 strings...
Checked 1100/1274 strings...
Checked 1200/1274 strings...

================================================================================
UNUSED STRINGS REPORT
================================================================================

Found 1 unused strings:


WATCH:
  - L10n.Watch.Labels.noAction
    Key: watch.labels.no_action
    Line: 3913

================================================================================
Total unused: 1
================================================================================

To clean up these strings, manually remove them from the Localizable.strings files
and regenerate Strings.swift using SwiftGen.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds comprehensive unit tests for the LocalPush reconnection logic introduced in PR #4113. The tests cover exponential backoff delays, timer cancellation, state transitions, and various edge cases to ensure the reconnection mechanism works correctly.

  • Added 13 focused test cases verifying private reconnection state and behavior
  • Introduced DEBUG-only test accessors to expose internal state without compromising production code
  • Tests validate backoff calculation (5s, 10s, 30s cap), timer lifecycle, attempt counter behavior, and rapid state changes

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
Tests/App/NotificationManagerLocalPushInterfaceExtension.test.swift New test file with 13 test cases covering reconnection logic, backoff delays, timer management, and edge cases
Sources/App/Notifications/NotificationManagerLocalPushInterfaceExtension.swift Added DEBUG-only extension with test accessors for internal state (reconnection attempt, timer status, disconnected servers, delays) and test methods to trigger reconnection operations
HomeAssistant.xcodeproj/project.pbxproj Added test file to project build configuration and corrected SharedPush package path references

@bgoncal bgoncal merged commit 51f9a98 into local-push Jan 6, 2026
2 checks passed
@bgoncal bgoncal deleted the copilot/sub-pr-4113-again branch January 6, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants