On Thu, Mar 17, 2022 at 11:28 AM Hanna Reitz <hre...@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
