Re: [PATCH 12/14] iotests: remove qemu_img_pipe_and_status()

[John Snow]

On Thu, Mar 17, 2022 at 11:28 AM Hanna Reitz <hreitz@redhat.com> wrote:
>
> On 09.03.22 04:54, John Snow wrote:
> > With the exceptional 'create' calls removed in the prior commit, change
> > qemu_img_log() and img_info_log() to call qemu_img() directly
> > instead.
> >
> > In keeping with the spirit of diff-based tests, allow these calls to
> > qemu_img() to return an unchecked non-zero status code -- because any
> > error we'd see from the output is going into the log anyway.
>
> :(
>
> I’d prefer having an exception that points exactly to where in the test
> the offending qemu-img call was.  But then again, I dislike such
> log-based tests anyway, and this is precisely one reason for it...
>
> I think Kevin disliked my approach of just `assert qemu_img() == 0`
> mainly because you don’t get the stderr output with it.  But you’ve
> solved that problem now, so I don’t think there’s a reason why we
> wouldn’t want a raised exception.
>
> Hanna
>

I thought you and Kevin actually preferred diff-based tests, maybe I
misunderstood. I know that there was a strong dislike of the unittest
based tests, and that the new script-style was more preferred. I
thought inherent to that was an actual preference for diff-based
itself, but maybe not?

I'd say negative tests are easier with the diff-based as one benefit.
I'm a little partial to that kind of test. (I noticed that bitmap
tests were the most habitual user of negative tests involving
qemu-img, haha.) Otherwise, I guess I don't really care what the test
mechanism is provided that the error output is informative. Happy to
defer to consensus between you and Kevin.

Anyway, this patch (and the ones that follow it, I haven't read your
feedback on 13-14 yet) doesn't close the door on making everything
Except-by-default, it would just be further work that needs to happen
after the fact. How do you want to move forward?

- Replace calls to qemu_img_log() with qemu_img()
- Make qemu_img_log() raise by default, but log output on success cases
- Something else?

--js