On Thu, Oct 08, 2020 at 04:27:13PM -0400, Eduardo Habkost wrote: > Add a simple test case that will run QEMU directly (without QMP) > just to check for crashes when using `-object`. > > Signed-off-by: Eduardo Habkost <[email protected]> > --- > Cc: Cleber Rosa <[email protected]> > Cc: "Philippe Mathieu-Daudé" <[email protected]> > Cc: Wainer dos Santos Moschetta <[email protected]> > Cc: [email protected] > --- > tests/acceptance/object_option.py | 49 +++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > create mode 100644 tests/acceptance/object_option.py > > diff --git a/tests/acceptance/object_option.py > b/tests/acceptance/object_option.py > new file mode 100644 > index 0000000000..2b8bd00db1 > --- /dev/null > +++ b/tests/acceptance/object_option.py > @@ -0,0 +1,49 @@ > +# Copyright (c) 2020 Red Hat, Inc. > +# > +# Author: > +# Eduardo Habkost <[email protected]> > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# later. See the COPYING file in the top-level directory. > + > +import avocado_qemu > +import subprocess > +import shlex > + > +class ObjectOption(avocado_qemu.Test): > + """Check if ``-object`` option behaves as expected""" > + > + def run(self, cmd, *args, **kwargs): > + cmdstr = ' '.join(shlex.quote(c) for c in cmd) > + self.log.info("Command: %s", cmdstr)
Maybe "Running command: %s" is clearer?
> + return subprocess.run(cmd, encoding='utf-8', *args, **kwargs)
I'd just use `universal_newlines=True` (equivalent to `text`, but
that's Python 3.7+ only), so the current encoding will be respected.
I understand "utf-8" is a safe choice, but IMO, if someone sets a
different default, it should be respected, unless there's a clear
reason not to do so.
> +
> + def devices(self):
Maybe call it `get_devices()` or make it a property?
> + out = self.run([self.qemu_bin, '-object', 'help'],
> + check=True, stdout=subprocess.PIPE).stdout
> + lines = out.split('\n')
> + return [l.strip() for l in lines[1:] if l.strip()]
> +
In case the `CalledProcessError` is raised on subprocess.run(), the
following test status will be ERROR, which is a condition you were not
testing for, something unexpected. Given that you're using `check=True`,
I'd decorate this test with:
@avocado.fail_on(subprocess.CalledProcessError)
> + def test_help(self):
> + """Check if ``-object ...,help`` behaves as expected"""
> + for device in self.devices():
> + self.run([self.qemu_bin, '-object', '%s,help' % (device)],
> + check=True,
> + stdout=subprocess.DEVNULL)
> +
> + def test_crash(self):
> + """Check for crashes when using ``-object ...``"""
Maybe change the wording here, given that this is really checking that
QEMU doesn't crash, right? So something like "Checks that QEMU
doesn't crash ..." seems clearer to me.
> + for device in self.devices():
> + r = self.run([self.qemu_bin, '-object',
> + '%s,id=obj0' % (device),
> + '-monitor', 'stdio'],
> + input='quit\n',
> + stdout=subprocess.DEVNULL,
> + stderr=subprocess.PIPE)
I know adding command line options to QEMU (specially at this stage)
is, at the very least, frowned upon, but I can't help to think that an
option similar to Python's "-c" would be very helpful in testing
scenarios.
> + if r.returncode not in (0, 1):
> + self.log.warn("QEMU stderr: %s", r.stderr)
> + self.log.warn("QEMU exit code: %d", r.returncode)
> + if r.returncode < 0:
> + self.fail("QEMU crashed")
> + else:
> + self.fail("Unexpected exit code")
> --
> 2.26.2
>
This looks fine, but this test is clearly provocative (at least to
me), given at it's a bit more barebones than the approached used in
"empty_cpu_model.py". It seems to show the need for:
1) a custom test class (similar to avocado_qemu.Test) that can be
more suitable to more barebones tests like this
2) a custom QEMU "varianter" implementation we've talked about in the
past, with which the test code could be simplified. For instance,
setting hypothetical configuration "qemu-varianter=objects" could
produce:
[ {"object": "authz-list"},
{"object": "authz-list-file"},
...
{"object": "tls-creds-x509"} ]
I had not attempted this before, because Avocado was limited to
applying variants globally to a job. Starting with version 81.0
(released a month or so ago), this is no longer a limitation.
A simple PoC is here:
https://gitlab.com/cleber.gnu/qemu/-/commit/30f26b662326502c2d82aabca225009ccdebe6aa
Can be run with:
./tests/venv/bin/python job_object_option.py
And the tests themselves would look like this:
https://paste.centos.org/view/4c5f413d
Let me know if any of this makes sense.
Thanks,
- Cleber.
signature.asc
Description: PGP signature
