Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.**
Expand Down
32 changes: 31 additions & 1 deletion flexmeasures/api/common/schemas/tests/test_sensor_data_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 2 additions & 5 deletions flexmeasures/data/services/sensors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down
Loading