[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 14/15] scripts: add python_qmp_updater.py
From: |
Eric Blake |
Subject: |
Re: [PATCH v7 14/15] scripts: add python_qmp_updater.py |
Date: |
Fri, 6 Oct 2023 12:01:22 -0500 |
User-agent: |
NeoMutt/20230517 |
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>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
- Re: [PATCH v7 05/15] python/qemu: rename command() to cmd(), (continued)
- [PATCH v7 08/15] iotests: add some missed checks of qmp result, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 09/15] iotests: refactor some common qmp result checks into generic pattern, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 10/15] iotests: drop some extra semicolons, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 06/15] python/machine.py: upgrade vm.cmd() method, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 11/15] iotests: drop some extra ** in qmp() call, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 12/15] iotests.py: pause_job(): drop return value, Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 13/15] tests/vm/basevm.py: use cmd() instead of qmp(), Vladimir Sementsov-Ogievskiy, 2023/10/06
- [PATCH v7 14/15] scripts: add python_qmp_updater.py, Vladimir Sementsov-Ogievskiy, 2023/10/06
- Re: [PATCH v7 14/15] scripts: add python_qmp_updater.py,
Eric Blake <=
- [PATCH v7 15/15] python: use vm.cmd() instead of vm.qmp() where appropriate, Vladimir Sementsov-Ogievskiy, 2023/10/06
- Re: [PATCH v7 00/15] iotests: use vm.cmd(), John Snow, 2023/10/11