diff mbox series

[4/5] iotests: Add @use_log to VM.run_job()

Message ID 20190627223255.3789-5-mreitz@redhat.com
State New
Headers show
Series block: Add BDS.never_freeze | expand

Commit Message

Max Reitz June 27, 2019, 10:32 p.m. UTC
unittest-style tests generally do not use the log file, but VM.run_job()
can still be useful to them.  Add a parameter to it that hides its
output from the log file.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

John Snow July 1, 2019, 10:59 p.m. UTC | #1
On 6/27/19 6:32 PM, Max Reitz wrote:
> unittest-style tests generally do not use the log file, but VM.run_job()
> can still be useful to them.  Add a parameter to it that hides its
> output from the log file.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Wondering out loud:

can log() (and by extension qmp_log, and run_job) be made to use the
python logging module and we can configure the logging environment
instead of bespoke arguments to avoid ever engaging the log?

We could theoretically just pre-disable iotests log output for unittest
style tests, unless you run in debug mode where we allow it.

I don't have a specific proposal for how to accomplish this, I think
there are some nuances to Python logging that I don't quite understand.
Maybe Cleber Rosa can help advise?

I'd like to toy with this idea; it seems like this won't be the last
time we want to turn output on/off.

--js

> ---
>  tests/qemu-iotests/iotests.py | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 3ecef5bc90..ce74177ab1 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -542,7 +542,7 @@ class VM(qtest.QEMUQtestMachine):
>  
>      # Returns None on success, and an error string on failure
>      def run_job(self, job, auto_finalize=True, auto_dismiss=False,
> -                pre_finalize=None, wait=60.0):
> +                pre_finalize=None, use_log=True, wait=60.0):
>          match_device = {'data': {'device': job}}
>          match_id = {'data': {'id': job}}
>          events = [
> @@ -557,7 +557,8 @@ class VM(qtest.QEMUQtestMachine):
>          while True:
>              ev = filter_qmp_event(self.events_wait(events))
>              if ev['event'] != 'JOB_STATUS_CHANGE':
> -                log(ev)
> +                if use_log:
> +                    log(ev)
>                  continue
>              status = ev['data']['status']
>              if status == 'aborting':
> @@ -565,13 +566,20 @@ class VM(qtest.QEMUQtestMachine):
>                  for j in result['return']:
>                      if j['id'] == job:
>                          error = j['error']
> -                        log('Job failed: %s' % (j['error']))
> +                        if use_log:
> +                            log('Job failed: %s' % (j['error']))
>              elif status == 'pending' and not auto_finalize:
>                  if pre_finalize:
>                      pre_finalize()
> -                self.qmp_log('job-finalize', id=job)
> +                if use_log:
> +                    self.qmp_log('job-finalize', id=job)
> +                else:
> +                    self.qmp('job-finalize', id=job)
>              elif status == 'concluded' and not auto_dismiss:
> -                self.qmp_log('job-dismiss', id=job)
> +                if use_log:
> +                    self.qmp_log('job-dismiss', id=job)
> +                else:
> +                    self.qmp('job-dismiss', id=job)
>              elif status == 'null':
>                  return error
>  
>
Max Reitz July 2, 2019, 4:19 p.m. UTC | #2
On 02.07.19 00:59, John Snow wrote:
> 
> 
> On 6/27/19 6:32 PM, Max Reitz wrote:
>> unittest-style tests generally do not use the log file, but VM.run_job()
>> can still be useful to them.  Add a parameter to it that hides its
>> output from the log file.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Wondering out loud:
> 
> can log() (and by extension qmp_log, and run_job) be made to use the
> python logging module and we can configure the logging environment
> instead of bespoke arguments to avoid ever engaging the log?
> 
> We could theoretically just pre-disable iotests log output for unittest
> style tests, unless you run in debug mode where we allow it.
> 
> I don't have a specific proposal for how to accomplish this, I think
> there are some nuances to Python logging that I don't quite understand.
> Maybe Cleber Rosa can help advise?
> 
> I'd like to toy with this idea; it seems like this won't be the last
> time we want to turn output on/off.

Sounds good.  But considering this is just test infrastructure, I’ll
leave that for when someone(TM) gets around to doing it.  (Hopefully
when the next function is about to get a @use_log parameter.)

Max
John Snow July 2, 2019, 8:21 p.m. UTC | #3
On 7/2/19 12:19 PM, Max Reitz wrote:
> On 02.07.19 00:59, John Snow wrote:
>>
>>
>> On 6/27/19 6:32 PM, Max Reitz wrote:
>>> unittest-style tests generally do not use the log file, but VM.run_job()
>>> can still be useful to them.  Add a parameter to it that hides its
>>> output from the log file.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>
>> Wondering out loud:
>>
>> can log() (and by extension qmp_log, and run_job) be made to use the
>> python logging module and we can configure the logging environment
>> instead of bespoke arguments to avoid ever engaging the log?
>>
>> We could theoretically just pre-disable iotests log output for unittest
>> style tests, unless you run in debug mode where we allow it.
>>
>> I don't have a specific proposal for how to accomplish this, I think
>> there are some nuances to Python logging that I don't quite understand.
>> Maybe Cleber Rosa can help advise?
>>
>> I'd like to toy with this idea; it seems like this won't be the last
>> time we want to turn output on/off.
> 
> Sounds good.  But considering this is just test infrastructure, I’ll
> leave that for when someone(TM) gets around to doing it.  (Hopefully
> when the next function is about to get a @use_log parameter.)
> 
> Max
> 

Yes, understood.

I just noticed that our pal run_job is getting a little long in the
arguments list, which is a good hint that we're doing something wrong.

I'll try to look into this as part of my next bitmap set.

--js
diff mbox series

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3ecef5bc90..ce74177ab1 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -542,7 +542,7 @@  class VM(qtest.QEMUQtestMachine):
 
     # Returns None on success, and an error string on failure
     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
-                pre_finalize=None, wait=60.0):
+                pre_finalize=None, use_log=True, wait=60.0):
         match_device = {'data': {'device': job}}
         match_id = {'data': {'id': job}}
         events = [
@@ -557,7 +557,8 @@  class VM(qtest.QEMUQtestMachine):
         while True:
             ev = filter_qmp_event(self.events_wait(events))
             if ev['event'] != 'JOB_STATUS_CHANGE':
-                log(ev)
+                if use_log:
+                    log(ev)
                 continue
             status = ev['data']['status']
             if status == 'aborting':
@@ -565,13 +566,20 @@  class VM(qtest.QEMUQtestMachine):
                 for j in result['return']:
                     if j['id'] == job:
                         error = j['error']
-                        log('Job failed: %s' % (j['error']))
+                        if use_log:
+                            log('Job failed: %s' % (j['error']))
             elif status == 'pending' and not auto_finalize:
                 if pre_finalize:
                     pre_finalize()
-                self.qmp_log('job-finalize', id=job)
+                if use_log:
+                    self.qmp_log('job-finalize', id=job)
+                else:
+                    self.qmp('job-finalize', id=job)
             elif status == 'concluded' and not auto_dismiss:
-                self.qmp_log('job-dismiss', id=job)
+                if use_log:
+                    self.qmp_log('job-dismiss', id=job)
+                else:
+                    self.qmp('job-dismiss', id=job)
             elif status == 'null':
                 return error