diff mbox series

[v4,04/10] iotests.py: add event_wait_log and events_wait_log helpers

Message ID 20190807141226.193501-5-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series qcow2-bitmaps: rewrite reopening logic | expand

Commit Message

Vladimir Sementsov-Ogievskiy Aug. 7, 2019, 2:12 p.m. UTC
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

John Snow Sept. 26, 2019, 11:05 p.m. UTC | #1
On 8/7/19 10:12 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/iotests.py | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index ce74177ab1..4ad265f140 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -540,6 +540,16 @@ class VM(qtest.QEMUQtestMachine):
>          log(result, filters, indent=indent)
>          return result
>  
> +    def event_wait_log(self, name, **kwargs):
> +        event = self.event_wait(name, **kwargs)
> +        log(event, filters=[filter_qmp_event])
> +        return event
> +
> +    def events_wait_log(self, events, **kwargs):
> +        event = self.events_wait(events, **kwargs)
> +        log(event, filters=[filter_qmp_event])
> +        return event
> +
>      # Returns None on success, and an error string on failure
>      def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>                  pre_finalize=None, use_log=True, wait=60.0):
> 

I'm not sure these are really needed, since you can just log the event
you get after calling either of these methods anyway. There's nothing
stopping you from:

```
event = event_wait_log(...)
log(filter_qmp_event(event))
```
Vladimir Sementsov-Ogievskiy Sept. 27, 2019, 7:31 a.m. UTC | #2
27.09.2019 2:05, John Snow wrote:
> 
> 
> On 8/7/19 10:12 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/iotests.py | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index ce74177ab1..4ad265f140 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -540,6 +540,16 @@ class VM(qtest.QEMUQtestMachine):
>>           log(result, filters, indent=indent)
>>           return result
>>   
>> +    def event_wait_log(self, name, **kwargs):
>> +        event = self.event_wait(name, **kwargs)
>> +        log(event, filters=[filter_qmp_event])
>> +        return event
>> +
>> +    def events_wait_log(self, events, **kwargs):
>> +        event = self.events_wait(events, **kwargs)
>> +        log(event, filters=[filter_qmp_event])
>> +        return event
>> +
>>       # Returns None on success, and an error string on failure
>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>>                   pre_finalize=None, use_log=True, wait=60.0):
>>
> 
> I'm not sure these are really needed, since you can just log the event
> you get after calling either of these methods anyway. There's nothing
> stopping you from:
> 
> ```
> event = event_wait_log(...)
> log(filter_qmp_event(event))
> ```

two lines vs one

Hm, just simple wrappers like qmp_log(), to make test a bit more readable, why not..
Vladimir Sementsov-Ogievskiy Sept. 27, 2019, 10:38 a.m. UTC | #3
27.09.2019 10:31, Vladimir Sementsov-Ogievskiy wrote:
> 27.09.2019 2:05, John Snow wrote:
>>
>>
>> On 8/7/19 10:12 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/iotests.py | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index ce74177ab1..4ad265f140 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -540,6 +540,16 @@ class VM(qtest.QEMUQtestMachine):
>>>           log(result, filters, indent=indent)
>>>           return result
>>> +    def event_wait_log(self, name, **kwargs):
>>> +        event = self.event_wait(name, **kwargs)
>>> +        log(event, filters=[filter_qmp_event])
>>> +        return event
>>> +
>>> +    def events_wait_log(self, events, **kwargs):
>>> +        event = self.events_wait(events, **kwargs)
>>> +        log(event, filters=[filter_qmp_event])
>>> +        return event
>>> +
>>>       # Returns None on success, and an error string on failure
>>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False,
>>>                   pre_finalize=None, use_log=True, wait=60.0):
>>>
>>
>> I'm not sure these are really needed, since you can just log the event
>> you get after calling either of these methods anyway. There's nothing
>> stopping you from:
>>
>> ```
>> event = event_wait_log(...)
>> log(filter_qmp_event(event))
>> ```
> 
> two lines vs one
> 
> Hm, just simple wrappers like qmp_log(), to make test a bit more readable, why not..
> 

Still keeping in mind idea of global logging turn on/off, it may be bad to increase number of f_log
function versions, it remember me the pain with _locked APIs in dirty-bitmaps.

OK, I'll drop it.
diff mbox series

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce74177ab1..4ad265f140 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -540,6 +540,16 @@  class VM(qtest.QEMUQtestMachine):
         log(result, filters, indent=indent)
         return result
 
+    def event_wait_log(self, name, **kwargs):
+        event = self.event_wait(name, **kwargs)
+        log(event, filters=[filter_qmp_event])
+        return event
+
+    def events_wait_log(self, events, **kwargs):
+        event = self.events_wait(events, **kwargs)
+        log(event, filters=[filter_qmp_event])
+        return event
+
     # Returns None on success, and an error string on failure
     def run_job(self, job, auto_finalize=True, auto_dismiss=False,
                 pre_finalize=None, use_log=True, wait=60.0):