fixup! Rewrite internal arc

Author: Marenz

src/frequenz/dispatch/actor.py

@@ -266,10 +266,11 @@ def _schedule_start(self, dispatch: Dispatch) -> None:
             return
 
         # Schedule the next run
-        next_run = dispatch.next_run
-        assert next_run is not None
-        heappush(self._scheduled_events, (next_run, dispatch))
-        _logger.debug("Scheduled dispatch %s to start at %s", dispatch, next_run)
+        if next_run := dispatch.next_run:
+            heappush(self._scheduled_events, (next_run, dispatch))
+            _logger.debug("Scheduled dispatch %s to start at %s", dispatch.id, next_run)
+        else:
+            _logger.debug("Dispatch %s has no next run", dispatch.id)
 
     def _schedule_stop(self, dispatch: Dispatch) -> None:
         """Schedule a dispatch to stop.

tests/test_frequenz_dispatch.py

@@ -15,7 +15,8 @@
 from frequenz.client.dispatch.test.client import FakeClient, to_create_params
 from frequenz.client.dispatch.test.generator import DispatchGenerator
 from frequenz.client.dispatch.types import Dispatch as BaseDispatch
-from frequenz.client.dispatch.types import Frequency
+from frequenz.client.dispatch.types import DispatchEvent as BaseDispatchEvent
+from frequenz.client.dispatch.types import Event, Frequency, RecurrenceRule
 from pytest import fixture
 
 from frequenz.dispatch import (
@@ -153,8 +154,9 @@ async def _test_new_dispatch_created(
         case Deleted(dispatch) | Updated(dispatch):
             assert False, "Expected a created event"
         case Created(dispatch):
-            sample = update_dispatch(sample, dispatch)
-            assert dispatch == Dispatch(sample)
+            sample = Dispatch(update_dispatch(sample, dispatch))
+            sample._set_running_status_notified()  # pylint: disable=protected-access
+            assert dispatch == sample
 
     return sample
 
@@ -424,3 +426,44 @@ async def test_dispatch_inf_duration_updated_to_finite_and_continues(
     # Expect notification to stop the dispatch because the duration has now passed
     stopped_dispatch = await actor_env.running_state_change.receive()
     assert stopped_dispatch.running(sample.type) == RunningState.STOPPED
+
+
+async def test_dispatch_new_but_finished(
+    actor_env: ActorTestEnv,
+    generator: DispatchGenerator,
+    fake_time: time_machine.Coordinates,
+) -> None:
+    """Test that a dispatch that is already finished is not started."""
+    # Generate a dispatch that is already finished
+    finished_dispatch = generator.generate_dispatch()
+    finished_dispatch = replace(
+        finished_dispatch,
+        active=True,
+        duration=timedelta(seconds=5),
+        start_time=_now() - timedelta(seconds=50),
+        recurrence=None,
+        type="I_SHOULD_NEVER_RUN",
+    )
+    # Create an old dispatch
+    actor_env.client.set_dispatches(actor_env.microgrid_id, [finished_dispatch])
+    await actor_env.actor.stop()
+    actor_env.actor.start()
+
+    # Create another dispatch the normal way
+    new_dispatch = generator.generate_dispatch()
+    new_dispatch = replace(
+        new_dispatch,
+        active=True,
+        duration=timedelta(seconds=10),
+        start_time=_now() + timedelta(seconds=5),
+        recurrence=RecurrenceRule(),
+        type="NEW_BETTER_DISPATCH",
+    )
+    # Consume one lifecycle_updates event
+    await actor_env.updated_dispatches.receive()
+    new_dispatch = await _test_new_dispatch_created(actor_env, new_dispatch)
+
+    # Advance time to when the new dispatch should still not start
+    fake_time.shift(timedelta(seconds=100))
+
+    assert await actor_env.running_state_change.receive() == new_dispatch