tests: Add mock scenarios for bq27441 missing methods.#253
Conversation
Add mock tests for is_valid_device(), temperature(), and flags() in tests/scenarios/bq27441.yaml. Extend mock_registers with the values needed for the new read paths and keep the existing scenario structure unchanged. This improves mock coverage for basic device validation and additional battery data reads without affecting current tests."
nedseb
left a comment
There was a problem hiding this comment.
Review — PR #253
Points positifs
- 3 nouveaux tests pertinents (flags, temperature, is_valid_device)
- Les mock_registers sont bien ordonnés par adresse avec commentaires clairs
- Le monkey-patch pour is_valid_device est astucieux et bien isolé
- Le test flags est simple et direct
Remarque sur le test température
Le test "Battery temperature returns float from mock data" attend 2981.0, qui est la valeur brute du registre en dixièmes de Kelvin (298.1 K = 24.95 °C). Le nom du test laisse croire qu'on obtient une température lisible alors que c'est un entier brut.
Pour cette PR, renomme le test en "Battery temperature returns raw register value in dK" et ajoute un commentaire :
- name: "Battery temperature returns raw register value in dK"
action: script
script: |
from bq27441.device import TempMeasureType
# 2981 = 298.1 K = 24.95 °C (raw value in decikelvin)
result = float(dev.temperature(TempMeasureType.BATTERY))
expect: 2981.0
mode: [mock]Par ailleurs, la méthode temperature() du driver retourne des dixièmes de Kelvin, ce qui ne respecte pas la convention du projet (unité par défaut = °C, méthodes avec unités explicites sinon). D'après nos conventions :
temperature()devrait retourner des °Ctemperature_dk()pour les dixièmes de Kelvin (si utile)temperature_k()pour les Kelvin (si utile)
C'est hors scope de cette PR, mais je crée une issue séparée pour harmoniser.
There was a problem hiding this comment.
Pull request overview
Adds missing mock scenario coverage for the bq27441 driver, extending the YAML scenario to exercise additional read paths in mock mode (issue #181).
Changes:
- Extend
mock_registerswith temperature and reorganize mock register entries. - Add mock tests for
flags()andtemperature(). - Add a mock test for
is_valid_device()via scripted override of the control-word read.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # FLAGS (0x06): CFGUPMODE | BAT_DET | DSG — needed for enterConfig/exitConfig | ||
| 0x06: [0x19, 0x00] | ||
|
|
||
| # TEMPERATURE (0x02): 298.1 K raw value |
There was a problem hiding this comment.
The TEMPERATURE mock register comment is inconsistent with the actual encoded value. Bytes [0xA5, 0x0B] decode to 0x0BA5 = 2981, which is typically the raw register value in 0.1 K units (i.e., 298.1 K), not “298.1 K raw value”. Please clarify the comment (and/or the expected units) so future readers don’t misinterpret what the driver returns.
| # TEMPERATURE (0x02): 298.1 K raw value | |
| # TEMPERATURE (0x02): raw 2981 (0.1 K units = 298.1 K) |
| - name: "Battery temperature returns float from mock data" | ||
| action: script | ||
| script: | | ||
| from bq27441.device import TempMeasureType | ||
| result = float(dev.temperature(TempMeasureType.BATTERY)) | ||
| expect: 2981.0 |
There was a problem hiding this comment.
This test forces the result to float(...) and then asserts 2981.0, but BQ27441.temperature() currently returns the raw int from read_word() (so the float cast doesn’t add coverage and the test name is a bit misleading). Consider either asserting the raw integer value directly, or explicitly converting to Kelvin/Celsius in the script (e.g., divide by 10) and using expect_range for a float assertion.
| - name: "Battery temperature returns float from mock data" | |
| action: script | |
| script: | | |
| from bq27441.device import TempMeasureType | |
| result = float(dev.temperature(TempMeasureType.BATTERY)) | |
| expect: 2981.0 | |
| - name: "Battery temperature raw register value from mock data" | |
| action: script | |
| script: | | |
| from bq27441.device import TempMeasureType | |
| result = dev.temperature(TempMeasureType.BATTERY) | |
| expect: 2981 |
| original_read_control_word = dev.read_control_word | ||
|
|
||
| def fake_read_control_word(function): | ||
| if function == 0x01: | ||
| return 0x0421 |
There was a problem hiding this comment.
Avoid hard-coding the control function / device ID magic numbers in the test. Import and use BQ27441_CONTROL_DEVICE_TYPE and BQ27441_DEVICE_ID from bq27441.const so the test stays correct if constants ever change and is easier to read.
| original_read_control_word = dev.read_control_word | |
| def fake_read_control_word(function): | |
| if function == 0x01: | |
| return 0x0421 | |
| from bq27441.const import BQ27441_CONTROL_DEVICE_TYPE, BQ27441_DEVICE_ID | |
| original_read_control_word = dev.read_control_word | |
| def fake_read_control_word(function): | |
| if function == BQ27441_CONTROL_DEVICE_TYPE: | |
| return BQ27441_DEVICE_ID |
|
Les trois remarques de Copilot ont été corrigées dans a7ace9f :
Tous les tests mock passent (11/11). C'étaient des corrections rapides et évidentes, elles n'auraient pas dû rester en attente. |
|
🎉 This PR is included in version 0.1.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Add missing mock tests for the bq27441 driver.
Closes #181
This PR improves test coverage by adding mock scenarios for:
is_valid_device()temperature()flags()It also extends the
mock_registersto support these additional read paths while preserving the existing test structure.All mock tests pass successfully:
Changes
is_valid_device()using a mocked control word response (0x0421)temperature()returning a float from mock dataflags()reading the FLAGS registermock_registerswith required values (temperature register)Checklist
ruff checkpasses<scope>: <Description.>format