Skip to content

Fix shelly tests for Python 3.14.3#166683

Closed
justanotherariel wants to merge 4 commits into
home-assistant:devfrom
justanotherariel:3.14.3-fix-shelly
Closed

Fix shelly tests for Python 3.14.3#166683
justanotherariel wants to merge 4 commits into
home-assistant:devfrom
justanotherariel:3.14.3-fix-shelly

Conversation

@justanotherariel
Copy link
Copy Markdown
Member

@justanotherariel justanotherariel commented Mar 27, 2026

Breaking change

Proposed change

Python 3.14.3 changes how tasks are scheduled which suddenly makes many tests flaky. This PR (in addition to #166851 and #166850) addresses the flakyness of the shelly tests which is caused by a number of different things - all of them being bugs within the tests even before the python update, but were masked because of how tasks were scheduled. I will try to outline each failure case and the fix:

  1. test_climate.py: Here we add two monkeypatch.delattr calls to the two tests that were missing it in the climate tests. Without them, the integration picks up the wrong 'block' as its temperature sensor (a valve block) and when it tries to read the temperature from it, it returns a mock instead of a int/float.
  2. test_device_trigger.py: We patch unload to make sure that the integration doesn't error on teardown, as unload tries to access entry.runtime_data otherwise when the device is connected (coordinator.py: 845 in is_rpc_ble_scanner_supported). I'm not sure if this is a bug and we should also/instead guard against in the shutdown method or if this can never happen in the real world.
  3. Rest of the files: The affected tests were the 'sleeping' ones which test if a shelly device is unavailable (sleeping) the entity wouldn't be created until it is online. It seems like this wasn't actually tested before however, and these tests passed because of how tasks were scheduled. We now make sure the device throws an DeviceConnectionError which properly tests the functionality and makes them deterministic. There are more sleeping tests which don't require the same fix because it goes through different paths within the integration.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • I understand the code I am submitting and can explain how it works.
  • 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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies a diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

Copilot AI review requested due to automatic review settings March 27, 2026 11:52
@home-assistant home-assistant Bot added cla-signed code-quality has-tests integration: shelly small-pr PRs with less than 30 lines. Top 100 Integration is ranked within the top 100 by usage Top 200 Integration is ranked within the top 200 by usage Top 50 Integration is ranked within the top 50 by usage labels Mar 27, 2026
@home-assistant
Copy link
Copy Markdown
Contributor

Hey there @bieniu, @thecode, @chemelli74, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (shelly) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of shelly can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign shelly Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) on the pull request.

Copy link
Copy Markdown
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

Stabilizes Shelly integration tests under Python 3.14.3 by making “sleeping/offline” device scenarios deterministic and preventing teardown failures in config flow/device-trigger tests.

Changes:

  • Make “sleeping device” tests deterministic by forcing initialize() to raise DeviceConnectionError until the device is brought online.
  • Prevent unintended competing Bluetooth config flows in config flow tests by injecting BLE discovery while patching flow.async_init.
  • Patch Shelly unload during mocked setup to avoid teardown errors when entry.runtime_data is intentionally absent.

Reviewed changes

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

Show a summary per file
File Description
tests/components/shelly/conftest.py Patch async_unload_entry alongside async_setup_entry in mock_setup_entry to avoid teardown errors in flow tests.
tests/components/shelly/test_config_flow.py Add _async_inject_ble_discovery helper and update BLE discovery injections to avoid duplicate/competing flows; make sleeping-device paths deterministic.
tests/components/shelly/test_device_trigger.py Patch Shelly unload in a runtime_data-negative scenario to avoid teardown failures.
tests/components/shelly/test_binary_sensor.py Make sleeping RPC binary sensor tests deterministic via DeviceConnectionError on initialize().
tests/components/shelly/test_button.py Make sleeping RPC smoke mute button test deterministic by controlling initialize() behavior.
tests/components/shelly/test_update.py Make sleeping RPC update entity test deterministic by controlling initialize() behavior.
tests/components/shelly/test_coordinator.py Make late setup of sleeping RPC device deterministic by controlling initialize() behavior.
tests/components/shelly/test_climate.py Ensure correct target temperature block selection in specific climate error-path tests via missing-attribute setup.

Comment thread tests/components/shelly/conftest.py
) -> None:
"""Test RPC online sleeping binary sensor."""
entity_id = f"{BINARY_SENSOR_DOMAIN}.test_name_cloud"
mock_rpc_device.initialize.side_effect = DeviceConnectionError
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is incorrect and changes the role of this test

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you think this test failing is thus a bug in the integration itself and requires a seperate PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems the core issue is a race condition in

The tests are using await init_integration with sleep_period=xyz, and changing them to use sleep_period=None fixes most of the issues.

However, this most likely highlights a deeper issue in the integration that should be investigated by the code owners.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will take a look, the problem is that I don't have an environment to reproduce it, if we change the tests so they pass but they don't really validate what they need to than we better disable them with a note that they need to be fixed.

I suggest to finish #169293 first and get back to this one

@abmantis
Copy link
Copy Markdown
Member

Is it possible to split this PR into multiple smaller PRs, each addressing a specific issue?

@justanotherariel
Copy link
Copy Markdown
Member Author

Yes, I actually just wrote with @thecode and we decided to split this into 3 PRs: one PR for async_unload_entry, one PR for bluetooth, and the final PR with the rest of the changes.

@justanotherariel justanotherariel marked this pull request as draft March 27, 2026 15:44
Copilot AI review requested due to automatic review settings March 30, 2026 08:11
Copy link
Copy Markdown
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

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread tests/components/shelly/test_coordinator.py Outdated
Comment thread tests/components/shelly/test_button.py Outdated
Copilot AI review requested due to automatic review settings March 30, 2026 08:52
Copy link
Copy Markdown
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

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@justanotherariel justanotherariel marked this pull request as ready for review March 30, 2026 08:55
@justanotherariel
Copy link
Copy Markdown
Member Author

As discussed, split the PR into 3:

Adjusted the PR description accordingly

@epenet
Copy link
Copy Markdown
Contributor

epenet commented Apr 28, 2026

@justanotherariel I think this has been resolved with #169301 / #169304 / #169305

@justanotherariel
Copy link
Copy Markdown
Member Author

Yes indeed, thank you @epenet for taking care of it :)

@github-actions github-actions Bot locked and limited conversation to collaborators Apr 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed code-quality has-tests integration: shelly Quality Scale: platinum small-pr PRs with less than 30 lines. Top 50 Integration is ranked within the top 50 by usage Top 100 Integration is ranked within the top 100 by usage Top 200 Integration is ranked within the top 200 by usage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants