home-assistant / architecture

Repo to discuss Home Assistant architecture
317 stars 100 forks source link

Splitting tests files in smaller files in components/modules tests #1007

Closed esciara closed 10 months ago

esciara commented 10 months ago

Context

I was told on Discord's dev channel that this was probably the place to put my question/proposal.

One thing that surprised me when I first tried to understand HA code through the testing code, is that some test_*.py files (172 actually) have thousands lines of code (I even saw some with 5000+ lines). This makes it ... so ... hard to go through it because there is no visible organisation or grouping of what part of the component you test at what stage. Let alone the fact that it puts my IDE on its knees... Where I come from, we make it nearly mandatory to group tests in smaller files, grouping them by areas or even sub areas they are testing, which makes it much easier to navigate.

I proposed the PR https://github.com/home-assistant/core/pull/105036 to work on it but the answer was that it was not the standard and that tests are grouped by module, not by functionality. I might have interpreted that as a definite "no" a bit quickly, without offering an alternative (even though I am a newbie and I am conscious I don't have any legitimacy around HA, at least not yet πŸ˜… 😜) .

So just wanted to check here and make another proposal before I bother anyone asking to review a modified PR.

Proposal

Would it be ok/a good idea to split the tests, but this time grouping them by modules as a directory with tests files in it?

Instead of:

home-assistant-core/tests/components/generic_thermostat
β”œβ”€β”€ __init__.py
β”œβ”€β”€ fixtures
β”‚   └── configuration.yaml
└── test_climate.py         <= 1600+ lines = πŸ™€

... We would end up with something along the lines of:

home-assistant-core/tests/components/generic_thermostat
β”œβ”€β”€ __init__.py
β”œβ”€β”€ climate
β”‚   β”œβ”€β”€ conftest.py
β”‚   β”œβ”€β”€ const.py
β”‚   β”œβ”€β”€ common.py
β”‚   β”œβ”€β”€ test_component_reload.py
β”‚   β”œβ”€β”€ test_component_restore_state.py
β”‚   β”œβ”€β”€ test_component_setup.py
β”‚   β”œβ”€β”€ test_cooling_hvac_mode.py
β”‚   β”œβ”€β”€ test_cooling_keep_alive.py
β”‚   β”œβ”€β”€ test_cooling_min_cycle_duration.py
β”‚   β”œβ”€β”€ test_cooling_switch.py
β”‚   β”œβ”€β”€ test_current_temperature.py
β”‚   β”œβ”€β”€ test_heating_hvac_mode.py
β”‚   β”œβ”€β”€ test_heating_keep_alive.py
β”‚   β”œβ”€β”€ test_heating_min_cycle_duration.py
β”‚   β”œβ”€β”€ test_heating_preset_mode.py
β”‚   β”œβ”€β”€ test_heating_switch.py
β”‚   └── test_precision.py
└── fixtures
    └── configuration.yaml

Consequences

esciara commented 10 months ago

Closed for discussion #1008