diff mbox series

[1/2] iotests.py: Filters for VM.run_job()

Message ID 20220318125304.66131-2-hreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series iotests/207: Filter host fingerprint | expand

Commit Message

Hanna Czenczek March 18, 2022, 12:53 p.m. UTC
Allow filters for VM.run_job(), and pass the filters given to
VM.blockdev_create() to it.

(Use this opportunity to annotate VM.run_job()'s parameter types;
unfortunately, for the filter, I could not come up with anything better
than Callable[[Any], Any] that would pass mypy's scrutiny.)

At one point, a plain string is logged, so the filters passed to it must
work fine with plain strings.  The only filters passed to it at this
point are the ones from VM.blockdev_create(), which are
filter_qmp_test_files() (by default) and 207's filter_hash().  Both
cannot handle plain strings yet, but we can make them by amending
filter_qmp() to treat them as plain values with a None key.

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

Comments

John Snow March 18, 2022, 10:36 p.m. UTC | #1
On Fri, Mar 18, 2022 at 8:53 AM Hanna Reitz <hreitz@redhat.com> wrote:
>
> Allow filters for VM.run_job(), and pass the filters given to
> VM.blockdev_create() to it.
>
> (Use this opportunity to annotate VM.run_job()'s parameter types;
> unfortunately, for the filter, I could not come up with anything better
> than Callable[[Any], Any] that would pass mypy's scrutiny.)
>

Yeah, I wrote some of this stuff ... before I started using mypy, and
I'd do it differently if I had to again.

(And might still do so: pulling out things like a generalized Job
Runner is still on my Someday pile, especially now that I have an
async QMP module to play with.)

Long story short: Yeah, sure, cool, I don't want to do any better than
this right now either.

> At one point, a plain string is logged, so the filters passed to it must
> work fine with plain strings.  The only filters passed to it at this
> point are the ones from VM.blockdev_create(), which are
> filter_qmp_test_files() (by default) and 207's filter_hash().  Both
> cannot handle plain strings yet, but we can make them by amending
> filter_qmp() to treat them as plain values with a None key.
>
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>

Looks fine enough to me for now.

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  tests/qemu-iotests/iotests.py | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 508adade9e..ad62d1f641 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -521,8 +521,10 @@ def filter_qmp(qmsg, filter_fn):
>      # Iterate through either lists or dicts;
>      if isinstance(qmsg, list):
>          items = enumerate(qmsg)
> -    else:
> +    elif isinstance(qmsg, dict):
>          items = qmsg.items()
> +    else:
> +        return filter_fn(None, qmsg)
>
>      for k, v in items:
>          if isinstance(v, (dict, list)):
> @@ -858,8 +860,12 @@ def qmp_log(self, cmd, filters=(), indent=None, **kwargs):
>          return result
>
>      # Returns None on success, and an error string on failure
> -    def run_job(self, job, auto_finalize=True, auto_dismiss=False,
> -                pre_finalize=None, cancel=False, wait=60.0):
> +    def run_job(self, job: str, auto_finalize: bool = True,
> +                auto_dismiss: bool = False,
> +                pre_finalize: Optional[Callable[[], None]] = None,
> +                cancel: bool = False, wait: float = 60.0,
> +                filters: Iterable[Callable[[Any], Any]] = (),
> +                ) -> Optional[str]:
>          """
>          run_job moves a job from creation through to dismissal.
>
> @@ -889,7 +895,7 @@ def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>          while True:
>              ev = filter_qmp_event(self.events_wait(events, timeout=wait))
>              if ev['event'] != 'JOB_STATUS_CHANGE':
> -                log(ev)
> +                log(ev, filters=filters)
>                  continue
>              status = ev['data']['status']
>              if status == 'aborting':
> @@ -897,18 +903,18 @@ def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>                  for j in result['return']:
>                      if j['id'] == job:
>                          error = j['error']
> -                        log('Job failed: %s' % (j['error']))
> +                        log('Job failed: %s' % (j['error']), filters=filters)
>              elif status == 'ready':
> -                self.qmp_log('job-complete', id=job)
> +                self.qmp_log('job-complete', id=job, filters=filters)
>              elif status == 'pending' and not auto_finalize:
>                  if pre_finalize:
>                      pre_finalize()
>                  if cancel:
> -                    self.qmp_log('job-cancel', id=job)
> +                    self.qmp_log('job-cancel', id=job, filters=filters)
>                  else:
> -                    self.qmp_log('job-finalize', id=job)
> +                    self.qmp_log('job-finalize', id=job, filters=filters)
>              elif status == 'concluded' and not auto_dismiss:
> -                self.qmp_log('job-dismiss', id=job)
> +                self.qmp_log('job-dismiss', id=job, filters=filters)
>              elif status == 'null':
>                  return error
>
> @@ -921,7 +927,7 @@ def blockdev_create(self, options, job_id='job0', filters=None):
>
>          if 'return' in result:
>              assert result['return'] == {}
> -            job_result = self.run_job(job_id)
> +            job_result = self.run_job(job_id, filters=filters)
>          else:
>              job_result = result['error']
>
> --
> 2.35.1
>
diff mbox series

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 508adade9e..ad62d1f641 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -521,8 +521,10 @@  def filter_qmp(qmsg, filter_fn):
     # Iterate through either lists or dicts;
     if isinstance(qmsg, list):
         items = enumerate(qmsg)
-    else:
+    elif isinstance(qmsg, dict):
         items = qmsg.items()
+    else:
+        return filter_fn(None, qmsg)
 
     for k, v in items:
         if isinstance(v, (dict, list)):
@@ -858,8 +860,12 @@  def qmp_log(self, cmd, filters=(), indent=None, **kwargs):
         return result
 
     # Returns None on success, and an error string on failure
-    def run_job(self, job, auto_finalize=True, auto_dismiss=False,
-                pre_finalize=None, cancel=False, wait=60.0):
+    def run_job(self, job: str, auto_finalize: bool = True,
+                auto_dismiss: bool = False,
+                pre_finalize: Optional[Callable[[], None]] = None,
+                cancel: bool = False, wait: float = 60.0,
+                filters: Iterable[Callable[[Any], Any]] = (),
+                ) -> Optional[str]:
         """
         run_job moves a job from creation through to dismissal.
 
@@ -889,7 +895,7 @@  def run_job(self, job, auto_finalize=True, auto_dismiss=False,
         while True:
             ev = filter_qmp_event(self.events_wait(events, timeout=wait))
             if ev['event'] != 'JOB_STATUS_CHANGE':
-                log(ev)
+                log(ev, filters=filters)
                 continue
             status = ev['data']['status']
             if status == 'aborting':
@@ -897,18 +903,18 @@  def run_job(self, job, auto_finalize=True, auto_dismiss=False,
                 for j in result['return']:
                     if j['id'] == job:
                         error = j['error']
-                        log('Job failed: %s' % (j['error']))
+                        log('Job failed: %s' % (j['error']), filters=filters)
             elif status == 'ready':
-                self.qmp_log('job-complete', id=job)
+                self.qmp_log('job-complete', id=job, filters=filters)
             elif status == 'pending' and not auto_finalize:
                 if pre_finalize:
                     pre_finalize()
                 if cancel:
-                    self.qmp_log('job-cancel', id=job)
+                    self.qmp_log('job-cancel', id=job, filters=filters)
                 else:
-                    self.qmp_log('job-finalize', id=job)
+                    self.qmp_log('job-finalize', id=job, filters=filters)
             elif status == 'concluded' and not auto_dismiss:
-                self.qmp_log('job-dismiss', id=job)
+                self.qmp_log('job-dismiss', id=job, filters=filters)
             elif status == 'null':
                 return error
 
@@ -921,7 +927,7 @@  def blockdev_create(self, options, job_id='job0', filters=None):
 
         if 'return' in result:
             assert result['return'] == {}
-            job_result = self.run_job(job_id)
+            job_result = self.run_job(job_id, filters=filters)
         else:
             job_result = result['error']