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.

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

Timer: Remove periodic() and timeout() #264

Merged
merged 7 commits into from
Jan 16, 2024
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Remove Timer.periodic() and Timer.timeout() from tests
Signed-off-by: Leandro Lucarella <[email protected]>
llucax committed Jan 16, 2024
commit d17e44e2a3df763d8a01fcb1c6321d178fc38caf
8 changes: 4 additions & 4 deletions tests/test_file_watcher_integration.py
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@

from frequenz.channels import select, selected_from
from frequenz.channels.file_watcher import Event, EventType, FileWatcher
from frequenz.channels.timer import Timer
from frequenz.channels.timer import SkipMissedAndDrift, Timer


@pytest.mark.integration
@@ -27,7 +27,7 @@ async def test_file_watcher(tmp_path: pathlib.Path) -> None:
expected_number_of_writes = 3

file_watcher = FileWatcher(paths=[str(tmp_path)])
timer = Timer.timeout(timedelta(seconds=0.1))
timer = Timer(timedelta(seconds=0.1), SkipMissedAndDrift())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think SkipMissedAndDrift should be an instance created in the channel package, so that we don't have to specify the () to create an instance of the thing?

This was a bit confusing for me the first time I did this, because I was expecting to pass just the policy, not an object of the policy that I had to create.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The policies can have options, and SkipMissedAndDrift() have one (delay_tolerance). I think if we provide fixed instances with fixed options we'll eventually need another PR to remove them because they are still hiding the complexity :)


async for selected in select(file_watcher, timer):
if selected_from(selected, timer):
@@ -55,8 +55,8 @@ async def test_file_watcher_deletes(tmp_path: pathlib.Path) -> None:
"""
filename = tmp_path / "test-file"
file_watcher = FileWatcher(paths=[str(tmp_path)], event_types={EventType.DELETE})
write_timer = Timer.timeout(timedelta(seconds=0.1))
deletion_timer = Timer.timeout(timedelta(seconds=0.25))
write_timer = Timer(timedelta(seconds=0.1), SkipMissedAndDrift())
deletion_timer = Timer(timedelta(seconds=0.25), SkipMissedAndDrift())

number_of_write = 0
number_of_deletes = 0
52 changes: 7 additions & 45 deletions tests/test_timer.py
Original file line number Diff line number Diff line change
@@ -266,53 +266,15 @@ async def test_timer_construction_custom_args() -> None:
assert timer.is_running is True


async def test_timer_construction_timeout_custom_args() -> None:
"""Test the construction of a timeout timer with custom arguments."""
timer = Timer.timeout(
timedelta(seconds=5.0),
auto_start=True,
loop=None,
)
assert timer.interval == timedelta(seconds=5.0)
assert isinstance(timer.missed_tick_policy, SkipMissedAndDrift)
assert timer.missed_tick_policy.delay_tolerance == timedelta(0)
assert timer.loop is asyncio.get_running_loop()
assert timer.is_running is True


async def test_timer_construction_periodic_defaults() -> None:
"""Test the construction of a periodic timer."""
timer = Timer.periodic(timedelta(seconds=5.0))
assert timer.interval == timedelta(seconds=5.0)
assert isinstance(timer.missed_tick_policy, TriggerAllMissed)
assert timer.loop is asyncio.get_running_loop()
assert timer.is_running is True


async def test_timer_construction_periodic_custom_args() -> None:
"""Test the construction of a timeout timer with custom arguments."""
timer = Timer.periodic(
timedelta(seconds=5.0),
skip_missed_ticks=True,
auto_start=True,
start_delay=timedelta(seconds=1.0),
loop=None,
)
assert timer.interval == timedelta(seconds=5.0)
assert isinstance(timer.missed_tick_policy, SkipMissedAndResync)
assert timer.loop is asyncio.get_running_loop()
assert timer.is_running is True


async def test_timer_construction_wrong_args() -> None:
"""Test the construction of a timeout timer with wrong arguments."""
"""Test the construction of a timer with wrong arguments."""
with pytest.raises(
ValueError,
match="^The `interval` must be positive and at least 1 microsecond, not -1 day, 23:59:55$",
):
_ = Timer.periodic(
_ = Timer(
timedelta(seconds=-5.0),
skip_missed_ticks=True,
SkipMissedAndResync(),
auto_start=True,
loop=None,
)
@@ -321,9 +283,9 @@ async def test_timer_construction_wrong_args() -> None:
ValueError,
match="^`start_delay` can't be negative, got -1 day, 23:59:59$",
):
_ = Timer.periodic(
_ = Timer(
timedelta(seconds=5.0),
skip_missed_ticks=True,
SkipMissedAndResync(),
auto_start=True,
start_delay=timedelta(seconds=-1.0),
loop=None,
@@ -333,9 +295,9 @@ async def test_timer_construction_wrong_args() -> None:
ValueError,
match="^`auto_start` must be `True` if a `start_delay` is specified$",
):
_ = Timer.periodic(
_ = Timer(
timedelta(seconds=5.0),
skip_missed_ticks=True,
SkipMissedAndResync(),
auto_start=False,
start_delay=timedelta(seconds=1.0),
loop=None,
4 changes: 2 additions & 2 deletions tests/test_timer_integration.py
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@
import async_solipsism
import pytest

from frequenz.channels.timer import Timer
from frequenz.channels.timer import SkipMissedAndDrift, Timer


@pytest.mark.integration
@@ -24,7 +24,7 @@ async def timer_wait(timer: Timer) -> None:

async with asyncio.timeout(2.0):
async with asyncio.TaskGroup() as task_group:
timer = Timer.timeout(timedelta(seconds=1.0))
timer = Timer(timedelta(seconds=1.0), SkipMissedAndDrift())
start_time = event_loop.time()
task_group.create_task(timer_wait(timer))
await asyncio.sleep(0.5)