Skip to content

Commit 4d18b7d

Browse files
authored
Merge pull request #741 from cderici/wait_for_idle_arg_type_check
#741 #### Description Currently there's no check for the `apps` argument for the `wait_for_idle()` method. So it goes through any iterable that's passed to it, including strings. As a result `wait_for_idle(apps="test")` produces something like the following: ``` INFO juju.model:model.py:2645 Waiting for model: t (missing) e (missing) s (missing) t (missing) ``` This introduces a check for the argument to be a `List[str]`. Fixes #732 #### QA Steps Unit tests are added, should be a simple QA. ``` tox -e py3 -- tests/unit/test_model.py::TestModelWaitForIdle ```
2 parents 879232c + d3f72a8 commit 4d18b7d

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

juju/model.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2581,6 +2581,12 @@ async def wait_for_idle(self, apps=None, raise_on_error=True, raise_on_blocked=F
25812581
timeout = timedelta(seconds=timeout) if timeout is not None else None
25822582
idle_period = timedelta(seconds=idle_period)
25832583
start_time = datetime.now()
2584+
# Type check against the common error of passing a str for apps
2585+
if apps is not None and (not isinstance(apps, list) or
2586+
any(not isinstance(o, str)
2587+
for o in apps)):
2588+
raise JujuError(f'Expected a List[str] for apps, given {apps}')
2589+
25842590
apps = apps or self.applications
25852591
idle_times = {}
25862592
last_log_time = None

tests/unit/test_model.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
from juju.model import Model
1212
from juju.application import Application
1313
from juju import jasyncio
14-
from juju.errors import JujuConnectionError
14+
from juju.errors import JujuConnectionError, JujuError
1515

1616

1717
def _make_delta(entity, type_, data=None):
@@ -289,6 +289,21 @@ async def test_no_args(self):
289289
# no apps so should return right away
290290
await m.wait_for_idle(wait_for_active=True)
291291

292+
@pytest.mark.asyncio
293+
async def test_apps_no_lst(self):
294+
m = Model()
295+
with self.assertRaises(JujuError):
296+
# apps arg has to be a List[str]
297+
await m.wait_for_idle(apps="should-be-list")
298+
299+
with self.assertRaises(JujuError):
300+
# apps arg has to be a List[str]
301+
await m.wait_for_idle(apps=3)
302+
303+
with self.assertRaises(JujuError):
304+
# apps arg has to be a List[str]
305+
await m.wait_for_idle(apps=[3])
306+
292307
@pytest.mark.asyncio
293308
async def test_timeout(self):
294309
m = Model()

0 commit comments

Comments
 (0)