Re: [PATCH v4 2/7] Acceptance tests: Extract _console_interaction()

[Wainer dos Santos Moschetta <wainersm@redhat.com>]

On 1/28/20 5:34 PM, Wainer dos Santos Moschetta wrote:
>
> On 1/20/20 9:51 PM, Philippe Mathieu-Daudé wrote:
>> Since we are going to re-use the code shared between
>> wait_for_console_pattern() and exec_command_and_wait_for_pattern(),
>> extract the common part into a local function.
>>
>> Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   tests/acceptance/avocado_qemu/__init__.py | 31 +++++++++++++----------
>>   1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index 6618ea67c1..0a50fcf2be 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -55,19 +55,14 @@ def pick_default_qemu_bin(arch=None):
>>           return qemu_bin_from_src_dir_path
>>     -def wait_for_console_pattern(test, success_message, 
>> failure_message=None):
>> -    """
>> -    Waits for messages to appear on the console, while logging the 
>> content
>> -
>> -    :param test: an Avocado test containing a VM that will have its 
>> console
>> -                 read and probed for a success or failure message
>> -    :type test: :class:`avocado_qemu.Test`
>> -    :param success_message: if this message appears, test succeeds
>> -    :param failure_message: if this message appears, test fails
>> -    """
>> +def _console_interaction(test, success_message, failure_message,
>> +                         send_string):
>
>
> Why not just add send_string as a parameter? Like:
>
> def wait_for_console_pattern(test, success_message, 
> failure_message=None, send_msg=None)
>
>
>>       console = test.vm.console_socket.makefile()
>>       console_logger = logging.getLogger('console')
>>       while True:
>> +        if send_string:
>> + test.vm.console_socket.sendall(send_string.encode())
>> +            send_string = None # send only once
>
> If it is going to send the message once, then put it before the loop.

OK, now that I read the next patch in this series (patch 03), I 
understood what you trying to accomplish here. So disregard my comments.

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
>
>>           msg = console.readline().strip()
>>           if not msg:
>>               continue
>> @@ -79,6 +74,17 @@ def wait_for_console_pattern(test, 
>> success_message, failure_message=None):
>>               fail = 'Failure message found in console: %s' % 
>> failure_message
>>               test.fail(fail)
>>   +def wait_for_console_pattern(test, success_message, 
>> failure_message=None):
>> +    """
>> +    Waits for messages to appear on the console, while logging the 
>> content
>> +
>> +    :param test: an Avocado test containing a VM that will have its 
>> console
>> +                 read and probed for a success or failure message
>> +    :type test: :class:`avocado_qemu.Test`
>> +    :param success_message: if this message appears, test succeeds
>> +    :param failure_message: if this message appears, test fails
>> +    """
>> +    _console_interaction(test, success_message, failure_message, None)
>>     def exec_command_and_wait_for_pattern(test, command,
>>                                         success_message, 
>> failure_message=None):
>> @@ -94,10 +100,7 @@ def exec_command_and_wait_for_pattern(test, command,
>>       :param success_message: if this message appears, test succeeds
>>       :param failure_message: if this message appears, test fails
>>       """
>> -    command += '\r'
>> -    test.vm.console_socket.sendall(command.encode())
>> -    wait_for_console_pattern(test, success_message, failure_message)
>> -
>> +    _console_interaction(test, success_message, failure_message, 
>> command + '\r')
>>     class Test(avocado.Test):
>>       def _get_unique_tag_val(self, tag_name):
>
>