tests: add inverter mode contract coverage

[filiplajszczak]

This adds contract-style tests for the core inverter mode methods so backend implementations can share the same expected control semantics.

[Copilot]

Pull request overview

Adds a new test module intended to act as “contract-style” coverage for inverter mode control methods, aiming to standardize expected semantics across inverter backends.

Changes:

• Introduces tests asserting mode-setting behavior for Dummy inverter.
• Introduces tests asserting MQTT publish behavior for MqttInverter mode-setting commands.

[Copilot]

These assertions duplicate coverage that already exists in tests/batcontrol/inverter/test_dummy.py (mode changes) and tests/batcontrol/inverter/test_mqtt_inverter.py (publish semantics). Keeping both sets of tests in sync will be extra maintenance with no additional coverage; consider either converting this file into a true shared/parameterized contract suite used by all inverter implementations, or removing the redundant cases and extending the existing per-driver tests instead.

Suggested change

	from unittest.mock import call
	import pytest
	from batcontrol.inverter.dummy import Dummy
	from batcontrol.inverter.mqtt_inverter import MqttInverter
	@pytest.fixture
	def dummy_inverter():
	return Dummy({'max_grid_charge_rate': 5000})
	@pytest.fixture
	def activated_mqtt_inverter(mocker):
	inverter = MqttInverter(
	{
	'base_topic': 'inverter',
	'capacity': 10000,
	'max_grid_charge_rate': 5000,
	}
	)
	mock_mqtt_api = mocker.MagicMock()
	mock_client = mocker.MagicMock()
	mock_mqtt_api.client = mock_client
	inverter.activate_mqtt(mock_mqtt_api)
	return inverter, mock_client
	def test_dummy_force_charge_sets_force_charge_mode(dummy_inverter):
	inverter = dummy_inverter
	inverter.set_mode_force_charge(3000)
	assert inverter.mode == 'force_charge'
	def test_dummy_avoid_discharge_sets_avoid_discharge_mode(dummy_inverter):
	inverter = dummy_inverter
	inverter.set_mode_avoid_discharge()
	assert inverter.mode == 'avoid_discharge'
	def test_dummy_allow_discharge_sets_allow_discharge_mode(dummy_inverter):
	inverter = dummy_inverter
	inverter.set_mode_allow_discharge()
	assert inverter.mode == 'allow_discharge'
	def test_dummy_limit_battery_charge_sets_limit_battery_charge_mode(dummy_inverter):
	inverter = dummy_inverter
	inverter.set_mode_limit_battery_charge(2000)
	assert inverter.mode == 'limit_battery_charge'
	def test_mqtt_force_charge_publishes_force_charge_mode_and_rate(
	activated_mqtt_inverter,
	):
	inverter, mock_client = activated_mqtt_inverter
	inverter.set_mode_force_charge(3000)
	assert call('inverter/command/mode', 'force_charge', qos=1, retain=False) in (
	mock_client.publish.call_args_list
	)
	assert call('inverter/command/charge_rate', '3000', qos=1, retain=False) in (
	mock_client.publish.call_args_list
	)
	def test_mqtt_avoid_discharge_publishes_avoid_discharge_mode(
	activated_mqtt_inverter,
	):
	inverter, mock_client = activated_mqtt_inverter
	inverter.set_mode_avoid_discharge()
	mock_client.publish.assert_called_once_with(
	'inverter/command/mode',
	'avoid_discharge',
	qos=1,
	retain=False,
	)
	def test_mqtt_allow_discharge_publishes_allow_discharge_mode(
	activated_mqtt_inverter,
	):
	inverter, mock_client = activated_mqtt_inverter
	inverter.set_mode_allow_discharge()
	mock_client.publish.assert_called_once_with(
	'inverter/command/mode',
	'allow_discharge',
	qos=1,
	retain=False,
	)
	def test_mqtt_limit_battery_charge_publishes_limit_mode_and_rate(
	activated_mqtt_inverter,
	):
	inverter, mock_client = activated_mqtt_inverter
	inverter.set_mode_limit_battery_charge(2000)
	assert call(
	'inverter/command/mode',
	'limit_battery_charge',
	qos=1,
	retain=False,
	) in mock_client.publish.call_args_list
	assert call(
	'inverter/command/limit_battery_charge_rate',
	'2000',
	qos=1,
	retain=False,
	) in mock_client.publish.call_args_list
	"""Shared inverter mode contract tests were removed.
	The assertions that used to live in this module duplicated coverage already
	provided by the per-driver test suites:
	- tests/batcontrol/inverter/test_dummy.py
	- tests/batcontrol/inverter/test_mqtt_inverter.py
	Keeping the implementation-specific checks in those files avoids duplicate
	maintenance. If shared behavior needs to be verified in the future, this file
	should be reintroduced as a true parameterized contract suite used by all
	inverter implementations.
	"""

[Copilot]

For the contract behavior, these checks only assert that the expected publish calls are present, but they don't fail if additional/unexpected MQTT publishes occur. To make the test actually enforce the contract, also assert the expected publish call count for this mode (and similarly for limit_battery_charge) so regressions that add extra publishes are caught.

[Copilot]

This test asserts that the expected limit mode/rate publishes occurred, but it doesn't ensure there were no additional publishes. If the intent is a contract test, also assert the expected publish call count for this operation (typically 2) to make the contract strict.

[MaStr]

I am unsure about that PR.
Does it make sense to rename & extend the current dummy inverter test and use that one as contract assurance?

[filiplajszczak]

Yes, that makes sense, and I was aware that this duplicates some existing backend-specific coverage a bit.

My intention was mainly to make the shared mode semantics explicit across backends, not to insist on a separate contract-test pattern if that feels like the wrong framing. A few shapes I could see here are:

1. fold this into the existing test_dummy.py / test_mqtt_inverter.py files and just make the shared intent clearer there
2. rename/extend the current dummy test so it becomes the main contract anchor, as you suggest
3. keep a small separate shared-semantics test file only if you think that makes future backend additions easier

I’m happy to reshape it whichever way fits the project best.

[MaStr]

Puh, I am not sure.

I think I like to avoid duplicate code.

I think I understand your contract based approach, to test, that each implementation of the inverter works the same way. Is that correct?

If so, you can duplicate this across the different inverter types. As a result, we will see if a change will break in an inverter type .. or all (then it is located in the baseclass)

[filiplajszczak]

I dropped the separate contract file and instead made the shared contract intent more explicit in the existing dummy and MQTT inverter tests.