Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix statistic_during_period wrongly prioritizing ST statistics over LT #115291

Merged
merged 10 commits into from
Jun 10, 2024
Merged

Fix statistic_during_period wrongly prioritizing ST statistics over LT #115291

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e8edee6
Fix statistic_during_period wrongly prioritizing ST statistics over LT
karwosts Apr 9, 2024
62a9844
comment
karwosts Apr 9, 2024
f5f0246
Merge branch 'home-assistant:dev' into rolling_interval_fix
karwosts Jun 8, 2024
8c67f3f
start of a test
karwosts Jun 8, 2024
0b15fcd
more testcases
karwosts Jun 9, 2024
8b673e8
fix sts insertion range
karwosts Jun 10, 2024
5797f95
update from review
karwosts Jun 10, 2024
563392a
remove unneeded comments
karwosts Jun 10, 2024
e8aa370
update logic
karwosts Jun 10, 2024
604a76b
min/mean/max testing
karwosts Jun 10, 2024
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
11 changes: 4 additions & 7 deletions homeassistant/components/recorder/statistics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,16 +1485,13 @@ def statistic_during_period(

# Calculate the tail period
tail_start_time: datetime | None = None
tail_end_time: datetime | None = None
tail_end_time: datetime | None = end_time
karwosts marked this conversation as resolved.
Show resolved Hide resolved
if end_time is None:
tail_start_time = now.replace(minute=0, second=0, microsecond=0)
elif tail_only:
tail_start_time = start_time
elif end_time.minute:
tail_start_time = (
start_time
if tail_only
else end_time.replace(minute=0, second=0, microsecond=0)
)
tail_end_time = end_time
tail_start_time = end_time.replace(minute=0, second=0, microsecond=0)

# Calculate the main period
main_start_time: datetime | None = None
Expand Down
13 changes: 4 additions & 9 deletions tests/components/recorder/test_websocket_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def next_id():
}


@pytest.mark.freeze_time(datetime.datetime(2022, 10, 21, 7, 25, tzinfo=datetime.UTC))
@pytest.mark.freeze_time(datetime.datetime(2022, 10, 21, 6, 31, tzinfo=datetime.UTC))
async def test_statistic_during_period_partial_overlap(
recorder_mock: Recorder,
hass: HomeAssistant,
Expand Down Expand Up @@ -879,10 +879,7 @@ async def test_statistic_during_period_partial_overlap(
)
response = await client.receive_json()
assert response["success"]
# FIXME: Only getting "360" as a result, so seems like STS might be being ignored for the end range?
# assert response["result"] == {
# "change": 390,
# }
assert response["result"] == {"change": 390}

async def assert_change_during_fixed(client, start_time, end_time, expected):
json = {
Expand Down Expand Up @@ -944,8 +941,7 @@ async def assert_change_during_fixed(client, start_time, end_time, expected):
start_time = start.replace(hour=4, minute=55)
end_time = start.replace(hour=5, minute=0)
expect = 5
# FIXME: this returns None, I might have expected 5 given the behavior of other 5 minute intervals
# await assert_change_during_fixed(client, start_time, end_time, expect)
await assert_change_during_fixed(client, start_time, end_time, expect)

# Five minutes of growth in STS-only, with a minute offset. Despite that this does not cover the full period, result is still 5
start_time = start.replace(hour=6, minute=16)
Expand Down Expand Up @@ -999,8 +995,7 @@ async def assert_change_during_fixed(client, start_time, end_time, expected):
# will be actually 2:00 - end
expect = 4 * 60 + 30
# Pre-115291 says "115" (1 hour and 55 minutes)? which I do not understand
# FIXME: Postfix says 240, which doesn't seem right either, I would have expected 270 or 275? for 2:00 to the end
# await assert_change_during_fixed(client, start_time, end_time, expect)
await assert_change_during_fixed(client, start_time, end_time, expect)

# 5 hours of growth, end_time_only (0:00-5:00)
start_time = None
Expand Down
Loading