Fix mutable objects in group registry class

[jbouwh]

Proposed change

Fix mutable objects in group registry class. These mutable objects cause tests to become flakey.

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

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation 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
• 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.

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

• Documentation added/updated for www.home-assistant.io

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 link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (group) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of group 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 group 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) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[emontnemery]

I'm not convinced this is a valid fix for the test failures in #111968, I think it just hides the underlying problem.
Instead, I think the problem is that there is only a single global instance of GroupIntegrationRegistry, and cover and lock conflict when both now have an "open" state.

Maybe the solution is to index on_off_mapping and off_on_mapping not just by state but by a tuple (domain, state)?

[jbouwh]

I'm not convinced this is a valid fix for the test failures in #111968, I think it just hides the underlying problem. Instead, I think the problem is that there is only a single global instance of GroupIntegrationRegistry, and cover and lock conflict when both now have an "open" state.

Maybe the solution is to index on_off_mapping and off_on_mapping not just by state but by a tuple (domain, state)?

If the test failes then it is caused by a mismatch between an open state and a unlocked state in CI tests with only cover entities, but locked is not used for covers, also Group registries seem to be per entity type. If that all is not the case, then we should mock out the mutable objects in a test and restore the original.

[emontnemery]

I'm not convinced this is a valid fix for the test failures in #111968, I think it just hides the underlying problem. Instead, I think the problem is that there is only a single global instance of GroupIntegrationRegistry, and cover and lock conflict when both now have an "open" state.
Maybe the solution is to index on_off_mapping and off_on_mapping not just by state but by a tuple (domain, state)?

If the test failes then it is caused by a mismatch between an open state and a unlocked state in CI tests with only cover entities, but locked is not used for covers, also Group registries seem to be per entity type. If that all is not the case, then we should mock out the mutable objects in a test and restore the original.

But that doesn't solve the underlying problem in a real setup with both locks and covers; depending on if lock or cover sets up first, group behavior will be different.

[emontnemery]

To be clear, I think the changes in this PR are OK; they just don't solve the problem introduced by #111968

[emontnemery]

LGTM, thanks @jbouwh 👍

I'd like a second opinion on if the mutable class attributes serve a purpose before we merge this.

[MartinHjelmare]

There's no reason to have mutable class attributes if the class is only instantiated once. We only need mutable class attributes if we have multiple instances of a class and we want all instances of the class to see changes in the mutable attribute. Normally it's very rare that that is the case and that we want that.

[bdraco]

Maybe the solution is to index on_off_mapping and off_on_mapping not just by state but by a tuple (domain, state)?

sounds reasonable to me given we now have domain/namespace conflicts. It should still fallback to the default top level values if the domain/namespace isn’t registered

[jbouwh]

It seems that with mixed domain groups, the group status can only be STATE_ON or STATE_OFF. So it seems fine to isolate the states in registry per domain.

The additional tests should be first applied to dev to avoid breaking the CI: #115849
After that step a rebase is needed.

[jbouwh]

The new seem to run fine, is okay to merge this one?