diff --git a/AGENTS.md b/AGENTS.md index 540d6fa319..fd25672974 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -629,6 +629,50 @@ When conducting work: - Run the CLI commands or API calls mentioned - Verify the implementation works end-to-end +### Must Validate the Whole Test Module After Any Test Change + +**When fixing or adding a test, always run the entire test module — not just the single test.** + +Fixing one test can silently break adjacent tests in the same module, especially when: +- Fixtures are `scope="module"` and tests share mutable state (SQLAlchemy objects, in-memory dicts) +- A test modifies a shared object (e.g. `asset.sensors_to_show`) and does not clean up after itself +- The cleanup itself introduces a new failure (e.g. resetting to `None` instead of the column default `[]`) + +**Mandatory scope for test validation:** + +| Change type | Minimum scope to run | +|---|---| +| Fix a single failing test | Full test module | +| Add a new test | Full test module | +| Change a shared fixture | All modules that use that fixture | +| Change production code touched by tests | Full test suite | + +**Anti-patterns to avoid:** + +- ❌ Running only `-k test_name` and declaring success +- ❌ Assuming module-scoped fixture state is clean between tests +- ❌ Resetting shared objects to `None` when the column default is `[]` + +**Correct pattern:** + +```bash +# After fixing test_foo, always run the whole module: +python -m pytest path/to/test_module.py -v +# All tests must pass before closing. +``` + +**Lesson from session 2026-04-13:** +- Fixed `test_asset_sensors_metadata_old_sensors_to_show_format` in isolation — passed. +- Ran only `-k test_asset_sensors_metadata_old_sensors_to_show_format` — passed. +- Closed without running the full module. +- Next test `test_asset_sensors_metadata` in the same module was broken because the + regression test left `sensors_to_show = None` on a module-scoped asset fixture. +- Root cause: `validate_sensors_to_show` had a bug — `sensors_to_show = None` with + `suggest_default_sensors=False` fell through to `deserialize(None)` which raised + `ValidationError` instead of returning `[]` as documented. +- Fix required changes in both the test (reset to `[]`, the column default) and + production code (`validate_sensors_to_show` early-return guard split into two cases). + ### Must Make Atomic Commits **Never mix different types of changes in a single commit.** diff --git a/flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py b/flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py index b1590937a8..52dae05cad 100644 --- a/flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py +++ b/flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py @@ -361,11 +361,41 @@ def test_get_status_no_status_specs( assert expected_stale_reason in sensor_status["reason"] +def test_asset_sensors_metadata_old_sensors_to_show_format(db, add_weather_sensors): + """ + Regression test: asset status page crashed with KeyError 'sensors' when sensors_to_show + contained the old format with a singular 'sensor' key, e.g. {"title": "Prices", "sensor": 42}. + """ + asset = add_weather_sensors["asset"] + wind_sensor = add_weather_sensors["wind"] + temperature_sensor = add_weather_sensors["temperature"] + + # Flush to ensure sensors have database IDs before using them. + db.session.flush() + + # Use the old format: one entry with plural "sensors" and one with singular "sensor" + asset.sensors_to_show = [ + {"title": "Group", "sensors": [wind_sensor.id, temperature_sensor.id]}, + {"title": "Solo", "sensor": wind_sensor.id}, + ] + db.session.add(asset) + + # Should not raise a KeyError + status_data = get_asset_sensors_metadata(asset=asset) + + sensor_ids = [s["id"] for s in status_data] + assert wind_sensor.id in sensor_ids + assert temperature_sensor.id in sensor_ids + + # Reset module-scoped fixture state so later tests are not affected. + asset.sensors_to_show = [] + + def test_asset_sensors_metadata( db, mock_get_statuses, add_weather_sensors, add_battery_assets ): """ - Test the function to build status meta data structure, using a weather station asset. + Test the function to build status metadata structure, using a weather station asset. We include the sensor of a different asset (a battery) via the flex context (as production price, does not make too much sense actually). One sensor which the asset already includes is also set in the context as inflexible device, diff --git a/flexmeasures/data/services/sensors.py b/flexmeasures/data/services/sensors.py index 82a32391a0..aa43937f4b 100644 --- a/flexmeasures/data/services/sensors.py +++ b/flexmeasures/data/services/sensors.py @@ -22,6 +22,7 @@ from flexmeasures import Sensor, Account, Asset from flexmeasures.data.models.data_sources import DataSource, DEFAULT_DATASOURCE_TYPES from flexmeasures.data.models.generic_assets import GenericAsset +from flexmeasures.data.schemas.generic_assets import SensorsToShowSchema from flexmeasures.data.schemas.reporting import StatusSchema from flexmeasures.utils.time_utils import server_now @@ -296,11 +297,7 @@ def get_asset_sensors_metadata( validated_asset_sensors = asset.validate_sensors_to_show( suggest_default_sensors=False ) - sensor_groups = [ - sensor["sensors"] for sensor in validated_asset_sensors if sensor is not None - ] - merged_sensor_groups = sum(sensor_groups, []) - sensors_to_show.extend(merged_sensor_groups) + sensors_to_show.extend(SensorsToShowSchema.flatten(validated_asset_sensors)) sensors_list = [ *inflexible_device_sensors,