Improve connection settings tests

agners commented 2 months ago

Proposed change

This improves mainly the connection settings fixture. The behavior is more closely to real NetworkManager, mainly in that the actual NetworkManager seems to add some default properties automatically (represented by the new MINIMAL_ settings). Also make the Update() call behave largely like NetworkManager, e.g. requiring a minimal set of settings/properties, and update the complete setting but include the MINIMAL_ setting just like NetworkManager.

Type of change

[ ] Dependency upgrade
[ ] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (which adds functionality to the supervisor)
[ ] Breaking change (fix/feature causing existing functionality to break)
[x] Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:
Link to cli pull request:
Link to client library pull request:

Checklist

[ ] The code change is tested and works locally.
[ ] Local tests pass. Your PR cannot be merged unless tests pass
[ ] There is no commented out code in this PR.
[ ] I have followed the development checklist
[ ] The code has been formatted using Ruff (ruff format supervisor tests)
[ ] Tests have been added to verify that the new code works.

If API endpoints or add-on configuration are added/changed:

[ ] Documentation added/updated for developers.home-assistant.io
[ ] CLI updated (if necessary)
[ ] Client library updated (if necessary)

Summary by CodeRabbit

New Features
- Enhanced network connection settings management with dynamic fixtures for both Ethernet and wireless connections.
- Introduced validation for required properties during settings updates to prevent erroneous configurations.
- Improved handling of IPv4 settings for addresses and DNS, allowing for more robust configuration management.
- Added new test functionality for Ethernet interface updates to ensure comprehensive testing.
Bug Fixes
- Adjusted test cases to reflect updated network settings structure, ensuring accurate validation of connection settings.
- Updated assertions in tests to align with new constant naming conventions, enhancing clarity and maintainability.

coderabbitai[bot] commented 2 months ago

Walkthrough

## Walkthrough The changes involve modifications to various test files related to network settings and device management. Key updates include renaming constants for consistency, introducing new parameterized test cases for multiple device types, and enhancing the handling of network connection settings with dynamic fixtures. Additionally, new mappings and validation logic have been added to improve testing flexibility and accuracy, thereby enhancing the robustness of network-related tests. ## Changes | Files | Change Summary | |-----------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------| | `tests/api/test_network.py`, `tests/dbus/network/test_interface.py`, `tests/dbus/network/test_network_manager.py`, `tests/dbus/network/setting/test_generate.py`, `tests/dbus/network/setting/test_init.py` | Renamed constants related to network interfaces for consistency across tests. Updated fixtures and test functions for better parameterization and coverage of network device types. | | `tests/dbus_service_mocks/network_active_connection.py`, `tests/dbus_service_mocks/network_device.py` | Added new mock entries and updated existing ones to enhance simulation of active connections and device properties. | | `tests/dbus_service_mocks/network_connection_settings.py` | Introduced dynamic handling of network settings with new minimal settings fixtures and validation logic for updating connection properties. | | `tests/dbus/network/test_connection.py` | Renamed constants in test cases related to network connection management to reflect more specific context for Ethernet connections. |

Recent review details

**Configuration used: CodeRabbit UI** **Review profile: CHILL**

Commits

Files that changed from the base of the PR and between 51004b979905038193c5fc5883f64451472e5536 and 8fb9f0d23510dcd0a2e980714b07fa7a49c927df.

Files selected for processing (5)

* tests/api/test_network.py (11 hunks) * tests/dbus/network/setting/test_generate.py (2 hunks) * tests/dbus/network/test_connection.py (4 hunks) * tests/dbus/network/test_interface.py (6 hunks) * tests/dbus/network/test_network_manager.py (4 hunks)

Files skipped from review due to trivial changes (1)

* tests/dbus/network/test_connection.py

Files skipped from review as they are similar to previous changes (3)

* tests/api/test_network.py * tests/dbus/network/test_interface.py * tests/dbus/network/test_network_manager.py

Additional comments not posted (4)

tests/dbus/network/setting/test_generate.py (4)
`12-12`: **Approved import change.** The change from `TEST_INTERFACE` to `TEST_INTERFACE_ETH_NAME` aligns with the PR's objective to focus on Ethernet interfaces, enhancing test specificity and accuracy. --- `17-17`: **Approved test function changes.** The use of `TEST_INTERFACE_ETH_NAME` in `test_get_connection_from_interface` and `test_get_connection_no_path` is appropriate and aligns with the PR's objectives to enhance test accuracy by focusing on Ethernet interfaces. Also applies to: 36-36 --- `36-36`: **Approved test function changes.** The use of `TEST_INTERFACE_ETH_NAME` in `test_get_connection_no_path` ensures that the test is focused on Ethernet interfaces, aligning with the PR's objectives to enhance test accuracy. --- Line range hint `50-74`: **Approved VLAN test function.** The function `test_generate_from_vlan` is well-implemented and crucial for testing VLAN configurations. It is not directly affected by the changes in this PR but is important for the overall robustness of network settings tests.

--- Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` or `@coderabbitai title` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

agners commented 2 months ago

On the note of headache, is there any particular tests you want to add around updating network interfaces for edge cases you've seen? Or is updating this good enough to cover the impact of these new properties and ensure we're handling it properly? No worries if not, this looks great as is.

Yeah good point. I've added one more API test named test_api_network_interface_update_ethernet which changes to "static" then to "auto" and checking what gets written to D-Bus directly, so end-to-end essentially. This tests the current behavior, so we know from what we set out for upcoming changes.

home-assistant / supervisor