On 12/07/2017 09:51 AM, Vladimir Sementsov-Ogievskiy wrote: The subject says what, but there is no commit body that says why.
Presumably this makes writing the test in the next patch easier, but some details in the commit message would make this obvious. > Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> > --- > tests/qemu-iotests/iotests.py | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > My python is not strong; it looks good overall, although I have a few questions that may warrant a v3 before I give R-b. > +class QemuIoInteractive: > + def __init__(self, *args): > + self.args = qemu_io_args + list(args) > + self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE, > + stdout=subprocess.PIPE, > + stderr=subprocess.STDOUT) Why STDOUT instead of STDERR? Is the redirection intentional? > + def _read_output(self): > + pattern = 'qemu-io> ' > + n = len(pattern) > + pos = 0 > + s = [] > + while pos != n: > + c = self._p.stdout.read(1) > + #check unexpected EOF pep8 doesn't like this comment style (it wants space after #). The file is already unclean under pep8, but you shouldn't make it worse. > + assert c != '' Is assert really the nicest control flow when early EOF is present? Or is it because we are only using this in tests, where we don't expect early EOF, so asserting is just fine for our usage? > + s.append(c) > + if c == pattern[pos]: > + pos += 1 > + else: > + pos = 0 > + > + return ''.join(s[:-n]) Is it necessary to use join() on the empty string here, compared to just returning the array slice directly? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
