-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add exception-translations rule to quality_scale pytest validation #131914
Conversation
@jbouwh already found one for
|
@zweckj this gets raised for tedee
|
ffs seems we missed one 😅 |
@epenet wait. I think this is coming from the base LockEntity? |
Ah, looks like it. |
It seems this is in the base component code as well. I found some other exceptions without translation though, I'll open a PR for it. |
@jbouwh did you mean for mqtt or the other ones as well? Otherwise I'll take a look at the lock |
MQTT does not raise in the vacuum platform module. I am looking into a possible explanation for the vacuum translation error. There might be more cases. A specialized MQTT value template exception derives from |
Caused by: core/homeassistant/helpers/service.py Lines 989 to 991 in 6144cc2
There are more untranslated helper exceptions. I think we need to add translations for those too where they are missing. |
Same source for the lock ones. We don't upload anything for those to lokalize yet, do we? |
We lokalize some of them. If they are generic, then they are placed under the |
I couldn't find a single translation in that entire folder. It also looks like exception translations only works for components: core/homeassistant/helpers/translation.py Lines 445 to 447 in 7452239
|
This should address the mqtt case: #131956 |
0a84191
to
ba12605
Compare
ba12605
to
dfb6ced
Compare
Rebased to check for new violations |
4cc5fee
to
5ffd51f
Compare
8178e1a
to
02edf62
Compare
@starkillerOG it seems that reolink does not translate all errors, despite what #132301 implies. |
@epenet this is due to the way I wrote the test files: And the way I do the translations: core/homeassistant/components/reolink/util.py Lines 101 to 170 in 66b4b24
All errors from the upstream reolink-aio library are derived from ReolinkError, but during runtime no ReolinkError is ever expected. Instead more specific errors like InvalidParameterError or ReolinkConnectionError (which are derived from ReolinkError) are expected. The last catch of ReolinkError is just to catch anything that might have sliped through the cracks, but is not expected. core/homeassistant/components/reolink/util.py Lines 169 to 170 in 66b4b24
In the tests I often use ReolinkError because it is easy and general, but I could simply replace them all by ReolinkConnectionError or any other error you prefer. Not sure what the best way forward is with this:
|
We recommend that unexpected errors still use translations, so IMO it makes sense to have the extra one with the placeholder. |
Proposed change
If an integration marks
exception-translations
as done in the quality scale, then we should check them as they are raised in the testsSample (forced) failure:
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: