[v7,14/15] scripts: add python_qmp_updater.py

A script, to update the pattern

    result = self.vm.qmp(...)
    self.assert_qmp(result, 'return', {})

(and some similar ones) into

    self.vm.cmd(...)

Used in the next commit
    "python: use vm.cmd() instead of vm.qmp() where appropriate"

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 scripts/python_qmp_updater.py | 136 ++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100755 scripts/python_qmp_updater.py

On Fri, Oct 06, 2023 at 06:41:24PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> A script, to update the pattern
> 
>     result = self.vm.qmp(...)
>     self.assert_qmp(result, 'return', {})
> 
> (and some similar ones) into
> 
>     self.vm.cmd(...)
> 
> Used in the next commit
>     "python: use vm.cmd() instead of vm.qmp() where appropriate"
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  scripts/python_qmp_updater.py | 136 ++++++++++++++++++++++++++++++++++
>  1 file changed, 136 insertions(+)
>  create mode 100755 scripts/python_qmp_updater.py
> 
> diff --git a/scripts/python_qmp_updater.py b/scripts/python_qmp_updater.py
> new file mode 100755
> index 0000000000..494a169812
> --- /dev/null
> +++ b/scripts/python_qmp_updater.py
> @@ -0,0 +1,136 @@
> +#!/usr/bin/env python3
> +#
> +# Intended usage:
> +#
> +# git grep -l '\.qmp(' | xargs ./scripts/python_qmp_updater.py
> +#
> +
> +import re
> +import sys
> +from typing import Optional
> +
> +start_reg = re.compile(r'^(?P<padding> *)(?P<res>\w+) = (?P<vm>.*).qmp\(',
> +                       flags=re.MULTILINE)
> +
> +success_reg_templ = re.sub('\n *', '', r"""
> +    (\n*{padding}(?P<comment>\#.*$))?
> +    \n*{padding}
> +    (
> +        self.assert_qmp\({res},\ 'return',\ {{}}\)
> +    |
> +        assert\ {res}\['return'\]\ ==\ {{}}
> +    |
> +        assert\ {res}\ ==\ {{'return':\ {{}}}}
> +    |
> +        self.assertEqual\({res}\['return'\],\ {{}}\)
> +    )""")

We may find other patterns, but this is a nice way to capture the most
common ones and a simple place to update if we find another one.

I did a quick grep for 'assert.*return' and noticed things like:

tests/qemu-iotests/056:        self.assert_qmp(res, 'return', {})
tests/qemu-iotests/056:        self.assert_qmp(res, 'return', [])

This script only simplifies the {} form, not the []; but that makes
sense: when we are testing a command known to return an array rather
than nothing, we still want to check if the array is empty, and not
just that the command didn't crash.  We are only simplifying the
commands that check for nothing in particular returned, on the grounds
that not crashing was probably good enough, and explicitly checking
that nothing extra was returned is not worth the effort.

> +
> +some_check_templ = re.sub('\n *', '', r"""
> +    (\n*{padding}(?P<comment>\#.*$))?
> +    \s*self.assert_qmp\({res},""")
> +
> +
> +def tmatch(template: str, text: str,
> +           padding: str, res: str) -> Optional[re.Match[str]]:
> +    return re.match(template.format(padding=padding, res=res), text,
> +                    flags=re.MULTILINE)
> +
> +
> +def find_closing_brace(text: str, start: int) -> int:
> +    """
> +    Having '(' at text[start] search for pairing ')' and return its index.
> +    """
> +    assert text[start] == '('
> +
> +    height = 1
> +
> +    for i in range(start + 1, len(text)):
> +        if text[i] == '(':
> +            height += 1
> +        elif text[i] == ')':
> +            height -= 1
> +        if height == 0:
> +            return i

I might have referred to this as 'nest' or 'depth', as I tend to think
of nesting depth rather than nesting height; but it's not a
show-stopper to use your naming.

> +
> +    raise ValueError
> +
> +
> +def update(text: str) -> str:
> +    result = ''
> +
> +    while True:
> +        m = start_reg.search(text)
> +        if m is None:
> +            result += text
> +            break
> +
> +        result += text[:m.start()]
> +
> +        args_ind = m.end()
> +        args_end = find_closing_brace(text, args_ind - 1)
> +
> +        all_args = text[args_ind:args_end].split(',', 1)
> +
> +        name = all_args[0]
> +        args = None if len(all_args) == 1 else all_args[1]
> +
> +        unchanged_call = text[m.start():args_end+1]
> +        text = text[args_end+1:]
> +
> +        padding, res, vm = m.group('padding', 'res', 'vm')
> +
> +        m = tmatch(success_reg_templ, text, padding, res)
> +
> +        if m is None:
> +            result += unchanged_call
> +
> +            if ('query-' not in name and
> +                    'x-debug-block-dirty-bitmap-sha256' not in name and
> +                    not tmatch(some_check_templ, text, padding, res)):
> +                print(unchanged_call + text[:200] + '...\n\n')
> +
> +            continue

Feels a bit hacky - but if it does the job, I'm not too worried.

> +
> +        if m.group('comment'):
> +            result += f'{padding}{m.group("comment")}\n'
> +
> +        result += f'{padding}{vm}.cmd({name}'
> +
> +        if args:
> +            result += ','
> +
> +            if '\n' in args:
> +                m_args = re.search('(?P<pad> *).*$', args)
> +                assert m_args is not None
> +
> +                cur_padding = len(m_args.group('pad'))
> +                expected = len(f'{padding}{res} = {vm}.qmp(')
> +                drop = len(f'{res} = ')
> +                if cur_padding == expected - 1:
> +                    # tolerate this bad style
> +                    drop -= 1
> +                elif cur_padding < expected - 1:
> +                    # assume nothing to do
> +                    drop = 0
> +
> +                if drop:
> +                    args = re.sub('\n' + ' ' * drop, '\n', args)

Wow - you've done some nice work here to not just replace things but
to keep relevant style intact.

> +
> +            result += args
> +
> +        result += ')'
> +
> +        text = text[m.end():]
> +
> +    return result
> +
> +
> +for fname in sys.argv[1:]:
> +    print(fname)
> +    with open(fname) as f:
> +        t = f.read()
> +
> +    t = update(t)
> +
> +    with open(fname, 'w') as f:
> +        f.write(t)
> -- 
> 2.34.1
>

Reviewed-by: Eric Blake <eblake@redhat.com>

On 06.10.23 20:01, Eric Blake wrote:
> On Fri, Oct 06, 2023 at 06:41:24PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> A script, to update the pattern
>>
>>      result = self.vm.qmp(...)
>>      self.assert_qmp(result, 'return', {})
>>
>> (and some similar ones) into
>>
>>      self.vm.cmd(...)
>>
>> Used in the next commit
>>      "python: use vm.cmd() instead of vm.qmp() where appropriate"
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   scripts/python_qmp_updater.py | 136 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 136 insertions(+)
>>   create mode 100755 scripts/python_qmp_updater.py
>>
>> diff --git a/scripts/python_qmp_updater.py b/scripts/python_qmp_updater.py
>> new file mode 100755
>> index 0000000000..494a169812
>> --- /dev/null
>> +++ b/scripts/python_qmp_updater.py
>> @@ -0,0 +1,136 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Intended usage:
>> +#
>> +# git grep -l '\.qmp(' | xargs ./scripts/python_qmp_updater.py
>> +#
>> +
>> +import re
>> +import sys
>> +from typing import Optional
>> +
>> +start_reg = re.compile(r'^(?P<padding> *)(?P<res>\w+) = (?P<vm>.*).qmp\(',
>> +                       flags=re.MULTILINE)
>> +
>> +success_reg_templ = re.sub('\n *', '', r"""
>> +    (\n*{padding}(?P<comment>\#.*$))?
>> +    \n*{padding}
>> +    (
>> +        self.assert_qmp\({res},\ 'return',\ {{}}\)
>> +    |
>> +        assert\ {res}\['return'\]\ ==\ {{}}
>> +    |
>> +        assert\ {res}\ ==\ {{'return':\ {{}}}}
>> +    |
>> +        self.assertEqual\({res}\['return'\],\ {{}}\)
>> +    )""")
> 
> We may find other patterns, but this is a nice way to capture the most
> common ones and a simple place to update if we find another one.
> 
> I did a quick grep for 'assert.*return' and noticed things like:
> 
> tests/qemu-iotests/056:        self.assert_qmp(res, 'return', {})

this one is

         res = self.vm.qmp(cmd, **kwargs)
         if error:
             self.assert_qmp(res, 'error/desc', error)
             return False

so here we can't just use cmd()

> tests/qemu-iotests/056:        self.assert_qmp(res, 'return', [])

yes, that's a result of query- command, caller wants the exact result.
Actually that's a check for "no block-jobs".

> 
> This script only simplifies the {} form, not the []; but that makes
> sense: when we are testing a command known to return an array rather
> than nothing, we still want to check if the array is empty, and not
> just that the command didn't crash.  We are only simplifying the
> commands that check for nothing in particular returned, on the grounds
> that not crashing was probably good enough, and explicitly checking
> that nothing extra was returned is not worth the effort.
> 

[..]

>>
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Thanks