#Removing more fields from snapshot
1 messages ยท Page 1 of 1 (latest)
So I think the current snapshot is snapshotting too much stuff that isn't really relevant for the test
# name: test_all_entities[aeotec_home_energy_meter_gen5][sensor.aeotec_energy_monitor_energy-entry]
EntityRegistryEntrySnapshot({
'aliases': list([
None,
]),
'area_id': None,
'capabilities': dict({
'state_class': <SensorStateClass.TOTAL_INCREASING: 'total_increasing'>,
}),
'config_entry_id': <ANY>,
'config_subentry_id': <ANY>,
'device_class': None,
'device_id': <ANY>,
'disabled_by': None,
'domain': 'sensor',
'entity_category': None,
'entity_id': 'sensor.aeotec_energy_monitor_energy',
'has_entity_name': True,
'hidden_by': None,
'icon': None,
'id': <ANY>,
'labels': set({
}),
'name': None,
'object_id_base': 'Energy',
'options': dict({
'sensor': dict({
'suggested_display_precision': 2,
}),
}),
'original_device_class': <SensorDeviceClass.ENERGY: 'energy'>,
'original_icon': None,
'original_name': 'Energy',
'platform': 'smartthings',
'previous_unique_id': None,
'suggested_object_id': None,
'supported_features': 0,
'translation_key': None,
'unique_id': 'f0af21a2-d5a1-437c-b10a-b34a87394b71_main_energyMeter_energy_energy',
'unit_of_measurement': 'kWh',
})
# ---
# name: test_all_entities[aeotec_home_energy_meter_gen5][sensor.aeotec_energy_monitor_energy-state]
StateSnapshot({
'attributes': ReadOnlyDict({
'device_class': 'energy',
'friendly_name': 'Aeotec Energy Monitor Energy',
'state_class': <SensorStateClass.TOTAL_INCREASING: 'total_increasing'>,
'unit_of_measurement': 'kWh',
}),
'context': <ANY>,
'entity_id': 'sensor.aeotec_energy_monitor_energy',
'last_changed': <ANY>,
'last_reported': <ANY>,
'last_updated': <ANY>,
'state': '19978.536',
})
# ---
Stuff like aliases, area_id, config_entry_id, config_subentry_id, device_id, id, labels, previous_unique_id, context, last_changed, last_reported and last_updated never really carry any value
I don't agree
On the contrary, I think we should snapshot more/smarter, to tie entites to devices, config entries and config subentries.
Agreed on that one
I am also open to other suggestions, but just having 1000 lines of 'id': <ANY>
I mean it does make me wonder why
is this triggered by Arturs PR which had to update all snapshots, or what's the context?
now we have a thing where we break CI more often because something that isn't related to what we're generally tested changed
ye
and it's not a direct high prio discussion
but it does make one wonder
ok, then I strongly disagree. for me the point of the snapshots is to capture accidental changes, and not have to opt-in to check everything
we do stuff like arturs PR now maybe 2-3 times per year, and yes, it's a minor inconvenience that snapshots in pending PRs are stale
but it's not worse than bumping a linter which changes the formatting
Yes, but I am wondering if we have snapshots where aliases wasn't an empty list
does it matter? it's included in the entity registry container
There are other things we omit already
meh
but take a look at trying to make sure the expected config entry etc is linked to
that would be actually useful
@classmethod
def _serializable_entity_registry_entry(
cls, data: er.RegistryEntry
) -> SerializableData:
"""Prepare a Home Assistant entity registry entry for serialization."""
serialized = EntityRegistryEntrySnapshot(
attrs.asdict(data)
| {
"config_entry_id": ANY,
"config_subentry_id": ANY,
"device_id": ANY,
"id": ANY,
"options": {k: dict(v) for k, v in data.options.items()},
}
)
serialized.pop("categories")
serialized.pop("compat_aliases")
serialized.pop("compat_name")
serialized.pop("_cache")
serialized["aliases"] = er._serialize_aliases(serialized["aliases"])
return cls._remove_created_and_modified_at(serialized)
should we also not bump linters because it's annoying for CI?
anyway, I've made my point clear. I'll not debate anymore.
Oh I am not saying that we shouldn't do it, it's a minor inconvenience at best
but if all the aliases are [None], what's the point in storing it and creating the inconvenience in the first place
because if someone wants to test aliases, it will just work
I think it's weird categories is not included btw
But aliases isn't a thing to test in the integration level right?
I am wondering if it makes sense to have a separate snapshot serializer?
why not? any conversation integration would want to test that
anyway, I said I would stop debating ๐ฌ
At this point I want to try to understand your stance and when you think this would be useful ๐