Isolate common test functions in Modbus#33447
Isolate common test functions in Modbus#33447Adminiuga merged 1 commit intohome-assistant:devfrom janiversen:modbustest
Conversation
|
@vzahradnik would you mind taking a look ? also it would be nice if you included a climate test in your PR. |
|
@janiversen I'm not much familiar with Python testing yet, and as such I can't review your code at the moment. My plan regarding Modbus is to finally start upstreaming the rest of my changes, especially new entities, and then I will try to write some tests for them. I will need some time to study how it's done. |
|
@vzahradnik one way of learning unit-testing is to read code, that you how should work :-) When you start writing upstreaming, I am happy to both review your code and help you with writing tests. |
|
@janiversen thanks! I will gladly use your offer. |
springstan
changed the title
|
Try rebasing against latest dev. The tests seem to be failing in TTS, which is unrelated to this PR. |
Since all entity test functions are going to use the modbus class, isolate the common parts in conftest.py, and thereby make it simpler to write additional test cases. cleaned up test_modbus_sensor.py while splitting the code.
|
@Adminiuga done, let us see if this solves the problem |
|
@Adminiuga all checks are green. |
| async def simulate_read_registers(unit, address, count): | ||
| """Simulate modbus register read.""" | ||
| del unit, address, count # not used in simulation, but in real connection | ||
| global read_result |
There was a problem hiding this comment.
Why do we need to use global? Can we instead move this function inside run_test?
There was a problem hiding this comment.
No, moving the function inside run_test, will work as long as it is called from within run_test, but it is called from core, so it will not have a reference to run_test local variables, at least as I understand it.
There was a problem hiding this comment.
simulate_read_registers will remember its scope, so it should work to move it inside.
| return hub | ||
|
|
||
|
|
||
| common_register_config = {CONF_NAME: "test-config", CONF_REGISTER: 1234} |
There was a problem hiding this comment.
Constants should be all caps, ie COMMON_REGISTER_CONFIG.
There was a problem hiding this comment.
That is correct, an even better solution is to remove the variable since it is unused. This is done in a PR I am preparing.
Since all entity test functions are going to use the modbus class, isolate the common parts in conftest.py, and thereby make it simpler to write additional test cases. cleaned up test_modbus_sensor.py while splitting the code.
7 participants
Proposed change
Since all entity test functions are going to use the modbus class, isolate the common parts in
conftest.py, and thereby make it simpler to write additional test cases.
cleaned up test_modbus_sensor.py while splitting the code.
Type of change
Example entry for
configuration.yaml:# Example configuration.yamlAdditional information
pytest .../test_modbus_sensor.py
to run the test.
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: