Fail tests with uncaught exceptions#33312
Conversation
| if isinstance(ex, ServiceNotFound): | ||
| continue |
There was a problem hiding this comment.
I wonder if we can reduce this by setting up persistent notification, which seems to be causing most errors. But that is for future us, this is already a great improvement.
There was a problem hiding this comment.
the problem is that it happens for components that don't use persistent notifications directly (like mine, Dynalite...). I plan to take a look at this as part of the exception reduction, but i have a bit less than 200 exceptions today in the file. If i remove this line it becomes 1600.
There was a problem hiding this comment.
We could consider to set persistent_notification up as part of the hass fixture.
There was a problem hiding this comment.
Right now we ignore all service not found errors, I bet that we're missing some real ones too. But let's keep this for the future.
There was a problem hiding this comment.
i believe that setting it for all the tests will increase the test time significantly. We can add mock service calls for the persistent notifications in the hass fixture and will likely catch the same errors since persistent notifications are always supposed to be set up, right?
balloob
left a comment
There was a problem hiding this comment.
Awesome! I think our next step will be to create issues for the integrations that have uncaught exceptions and notify code owners.
|
Done ;) |
Breaking change
Not a breaking change
Proposed change
Today, if a test has an exception that gets uncaught in the event loop, the test still succeeds. However, it is a better programming practice to have exceptions caught, as it is not good to completely ignore it.
Most tests pass today and I added a file with the ignored tests.
This will immediately force all new tests to either not have uncaught exceptions or be added to the ignored tests (which we should not permit too easily).
Over time, I will create PRs to fix the tests. In a few random ones I picked, it is usually a very minor fix. The file can definitely be reduced significantly and potentially eliminated, which will make the code better and more stable
Type of change
Example entry for
configuration.yaml:# Example configuration.yamlAdditional information
Checklist
black --fast 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..coveragerc.The integration reached or maintains the following Integration Quality Scale: