Load dependencies from manifests. Fallback to current DEPENDENCIES.

[rohankapoorcom]

Description:

Load dependencies from manifests (if present). If not, fallback to the current approach of reading DEPENDENCIES from the module attribute.

Related issue (if applicable): relates to #22700

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[balloob]

Difficult to see MQTT tests because Circle doesn't make it past the static check.

[rohankapoorcom]

Difficult to see MQTT tests because Circle doesn't make it past the static check.

Sorry about that. Didn't realize mypy was unhappy. We should make it past that part now.

[awarecan]

1. You can check here to see all details of failed tests: https://circleci.com/build-insights/gh/home-assistant/home-assistant/load-dependencies-manifest

2. Root [one of] cause is that the dependencies field in manifest.json are missing the DEPENDENCIES defined in the "platform" file.

For example, DEPENDENCIES in automation/__init__.py only has groups. However, automation/webhook.py depends on webhook component which have not been extracted to manifest.json

[awarecan]

Opened an arch issue to find the solution home-assistant/architecture#187

[awarecan]

There is another manifest issue. When we do the great migration, we create an empty __init__.py, in such case, the DEPENDENCIES still in the platform file, and not be extracted to manifest.json. I updated several cases broken the tests. I changed manifest.json those has only one platform.

[awarecan]

Now, I expected most test been fixed. automation.webhook will still fail due the manifest issue.

The majority issues I fixed in the tests are timing issue. Component and platform setup are separated tasks. async_setup_component finished doesn't mean platform is ready, we have to async_block_till_done() to make sure platform setup finished.

Those tests got passed before, but start failing after manifest change. My theory is their platform setup pretty quick in before, but became slow now as we have to do disk I/O to read manifest.json file.

[rohankapoorcom]

Those tests got passed before, but start failing after manifest change. My theory is their platform setup pretty quick in before, but became slow now as we have to do disk I/O to read manifest.json file.

So we basically got lucky before? Had any of those platforms introduced anything that did I/O, we would have started seeing tests fail then too, right?

[awarecan]

We don't have so much disk I/O before, and most network I/O has been mocked in test. So there is another performance concern. We have 5000+ tests, that means at least 5000 more file I/O introduced in our tests. Let's see the result

[rohankapoorcom]

What's the alternative? I don't think it's reasonable to mock out the manifest loads.

[awarecan]

Still 50+ failures and they are new. It seems that previous test run didn't executed all tests at all.

[balloob]

Yeah, we should load them.

[rohankapoorcom]

I will continue on with this tonight. Thanks for moving it along @awarecan!

[rohankapoorcom]

I managed to resolve another 10 failures. There's still one more that I haven't figured out: https://circleci.com/gh/home-assistant/home-assistant/5524#tests/containers/0

Annoyingly I'm unable to fail that one test on my local machine.

Help is always welcome :)

[rohankapoorcom]

This is getting more interesting, I've been able to reproduce this test failure, but only when running the entire test suite. If I just run the module or the specific test it passes, making it particularly hard to debug. As far as I can tell, it seems like a test in a different class is causing this one to break.

I'm curious now, if it's possible to get the requirements loaded from the manifest first (without all of this test case difficulties). That way we could unblock #22717 and possibly merge that one first and then figure this out.

Okay, confirmed my theory, this single test is still broken when just requirements are read from the manifest, not only for dependencies. We'll need to get this test fixed before either can move forward.

[balloob]

The test fixes from this PR have been cherry-picked into dev. Please rebase.

[rohankapoorcom]

Rebased and pushed up.

[codecov]

Codecov Report

Merging #22716 into dev will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev   #22716      +/-   ##
==========================================
- Coverage   93.85%   93.83%   -0.02%     
==========================================
  Files         449      449              
  Lines       36752    36756       +4     
==========================================
- Hits        34492    34490       -2     
- Misses       2260     2266       +6

Impacted Files	Coverage Δ
setup.py	91.5% <0%> (-4.47%)	⬇️
homeassistant/components/emulated_roku/binding.py	92.64% <0%> (+1.47%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cac00f5...86851cf. Read the comment docs.

[balloob]

I have merged this PR into #22717

[balloob]

This was merged into #22717