chore: start on app and unit counting mechanics

juju/model.py

@@ -3053,25 +3053,42 @@ async def _check_idle(
         start_time: datetime = datetime.now(),
     ) -> bool:
         now = datetime.now()
-        tmp = await self.get_status()
-        reveal_type(tmp.applications)
+        expected_idle_since = now - timedelta(seconds=idle_period)
+        full_status = await self.get_status()
+        __import__("pdb").set_trace()
+
         for app_name in apps:
-            app = tmp.applications.get(app_name)
-            if not app:
+            if not (app := full_status.applications.get(app_name)):
                 logging.info("Waiting for app %r", app_name)
                 return False
+
             reveal_type(app)
+            reveal_type(app.status)
+            assert app.status
+            if app.status.status == "error" and raise_on_error:
+                raise JujuAppError(f"App {app_name!r} is in error: {app.status.info!r}")
+            if app.status.status == "blocked" and raise_on_blocked:
+                raise JujuAppError(f"App {app_name!r} is blocked: {app.status.info!r}")
+
+            # FIXME the old code treats app status "unset" as special
+            # and uses max(unit statuses) instead (see Application.status)
+            # could be an allwatcher special though
+            # https://matrix.to/#/!wJiiHsLipVywuWOyNi:ubuntu.com/$HU1exJ71wzmyr2Vy8Hp1AYrok2ZfVYRBVex3jXGVZn8?via=ubuntu.com&via=matrix.org
+
             # FIXME what if app has zero units?
             # what if the caller is waiting for units to go down to zero?
             reveal_type(app.units)
             if len(app.units) < _wait_for_units:
-                logging.info("Waitinf for app %r units %s/%s",
+                logging.info("Waiting for app %r units %s/%s",
                              app_name, len(app.units), _wait_for_units)
                 return False
 
             # TODO: refactor to simplify later
             # datetime -> float; check vs outer loop
-            idle_times.setdefault(app_name, now)
+            idle_since = idle_times.setdefault(app_name, now)
+            if expected_idle_since < idle_since:
+                logging.info("App %r has not been idle long enough", app_name)
+                return False
             # TODO continue here...
         return True
 
@@ -3092,7 +3109,7 @@ async def _legacy_check_idle(
         last_log_time: List[Optional[datetime]] = [None],
         start_time: datetime = datetime.now(),
     ):
-        #__import__("pdb").set_trace()
+        __import__("pdb").set_trace()
         _timeout = timedelta(seconds=timeout) if timeout is not None else None
         _idle_period = timedelta(seconds=idle_period)
         log_interval = timedelta(seconds=30)

tests/unit/test_wait_for_idle.py

@@ -5,13 +5,15 @@
 import json
 import pytest
 from datetime import datetime, timedelta
-from typing import Any, Dict, List, Tuple, Union
+from typing import Any, Dict, List, Optional, Tuple, Union
 from typing import reveal_type as reveal_type
 from unittest.mock import Mock
 
 from juju.application import Application
 from juju.client.facade import _convert_response
 from juju.client._definitions import FullStatus
+from juju.errors import JujuAppError
+from juju.machine import Machine
 from juju.model import Model
 from juju.unit import Unit
 
@@ -42,6 +44,28 @@ async def test_idle_app(full_status_response: dict, kwargs: Dict[str, Any]):
     assert idle and legacy
 
 
+async def test_raise_on_error(full_status_response: dict, kwargs: Dict[str, Any]):
+    full_status_response["response"]["applications"]["hexanator"]["status"]["status"] = "error"
+    full_status_response["response"]["applications"]["hexanator"]["status"]["info"] = "big problem"
+    kwargs["apps"] = ["hexanator"]
+    kwargs["raise_on_error"] = True
+    idle, legacy = await model_fake(full_status_response).check_both(**kwargs)
+    assert isinstance(idle, JujuAppError)
+    assert isinstance(legacy, JujuAppError)
+    assert "big problem" in str(idle)
+
+
+async def test_raise_on_blocked(full_status_response: dict, kwargs: Dict[str, Any]):
+    full_status_response["response"]["applications"]["hexanator"]["status"]["status"] = "blocked"
+    full_status_response["response"]["applications"]["hexanator"]["status"]["info"] = "big problem"
+    kwargs["apps"] = ["hexanator"]
+    kwargs["raise_on_blocked"] = True
+    idle, legacy = await model_fake(full_status_response).check_both(**kwargs)
+    assert isinstance(idle, JujuAppError)
+    assert isinstance(legacy, JujuAppError)
+    assert "big problem" in str(idle)
+
+
 async def test_idle_period(full_status_response: dict, kwargs: Dict[str, Any]):
     kwargs["apps"] = ["hexanator"]
     kwargs["idle_period"] = 1
@@ -52,18 +76,34 @@ async def test_idle_period(full_status_response: dict, kwargs: Dict[str, Any]):
 async def test_after_idle_period(full_status_response: dict, kwargs: Dict[str, Any]):
     kwargs["apps"] = ["hexanator"]
     kwargs["idle_period"] = 1
-    kwargs["idle_times"] = {"hexanator": datetime.now() - timedelta(seconds=2)}
+    earlier = datetime.now() - timedelta(seconds=2)
+    kwargs["idle_times"] = {"hexanator": earlier, "hexanator/0": earlier}
     idle, legacy = await model_fake(full_status_response).check_both(**kwargs)
     assert idle and legacy
 
 
-async def test_something_useful(full_status_response: dict, kwargs: Dict[str, Any]):
-    # tweak the state
-    full_status_response["response"]["applications"]["hexanator"]["status"]["status"] = "BROKEN"
-    kwargs["wait_for_exact_units"] = 2
-
+async def test_after_idle_waiting_for_app(full_status_response: dict, kwargs: Dict[str, Any]):
+    kwargs["apps"] = ["hexanator"]
+    kwargs["idle_period"] = 1
+    now = datetime.now()
+    earlier = now - timedelta(seconds=2)
+    kwargs["idle_times"] = {"hexanator": now, "hexanator/0": earlier}
     idle, legacy = await model_fake(full_status_response).check_both(**kwargs)
-    assert idle == legacy
+    assert not idle and not legacy
+
+
+async def test_after_idle_waiting_for_unit(full_status_response: dict, kwargs: Dict[str, Any]):
+    now = datetime.now()
+    earlier = now - timedelta(seconds=2)
+    idle, legacy = await model_fake(full_status_response).check_both(
+        **{
+            **kwargs,
+            "apps": ["hexanator"],
+            "idle_period": 1,
+            "wait_for_at_least_units": 1,
+            "idle_times": {"hexanator": earlier, "hexanator/0": now},
+    })
+    assert not idle and not legacy
 
 
 @pytest.fixture
@@ -132,6 +172,8 @@ class ModelFake(Model):
     _response: Dict
 
     async def check_both(self, **kwargs) -> Tuple[Union[bool, Exception], Union[bool, Exception]]:
+        idle: Union[bool, Exception]
+        legacy: Union[bool, Exception]
         try:
             idle = await self._check_idle(**kwargs)
         except Exception as e:
@@ -140,7 +182,6 @@ async def check_both(self, **kwargs) -> Tuple[Union[bool, Exception], Union[bool
         try:
             legacy = await self._legacy_check_idle(**kwargs)
         except Exception as e:
-            raise
             legacy = e
 
         return idle, legacy
@@ -190,6 +231,7 @@ class UnitFake(Unit):
     _agent_status_message: str = ""
     _workload_status: str = ""
     _workload_status_message: str = ""
+    _machine_id: Optional[str] = None
 
     @property
     def agent_status(self) -> str:
@@ -207,6 +249,11 @@ def workload_status(self) -> str:
     def workload_status_message(self) -> str:
         return self._workload_status_message
 
+    @property
+    def machine(self) -> Optional[Machine]:
+        assert not self._machine_id, "FIXME implement unit's machine interface"
+        return None
+
 
 
 async def test_model_fake(full_status_response):