Laszlo Ersek <ler...@redhat.com> writes:
> On 03/02/17 09:25, Laszlo Ersek wrote:
>
> Regarding your other email ("New QMP command without a test -> automatic
> NAK"), Ben did write a small test suite for this feature:
>
> [Qemu-devel] [PATCH v8 7/8] tests: Add unit tests for the VM Generation
> ID feature
>
> and it contains a test called "/vmgenid/vmgenid/query-monitor".
>
> That patch is not part of the pull request because the functional tests
> in the same suite only pass if you have an updated SeaBIOS blob in the
> tree as well. While the SeaBIOS patches have been committed, the update
> for the blob bundled with QEMU is still in progress. Thus the patch with
> the tests is being delayed.
Posting the testing part that depends on in-progress work only as RFC
sounds like a perfectly acceptable case of the "not practical" exception
mentioned "New QMP command without a test -> automatic NAK".
> So, it wasn't negligence, we simply missed this failure case because we
> were so focused on the long-awaited functionality. We didn't miss other
> failure cases that were a bit closer to the functionality.
>
> BTW, now that I look at said "/vmgenid/vmgenid/query-monitor" test case,
> it also doesn't seem to catch this error -- the monitor command is
> apparently not called without the device present.
I expect you to try to cover all QMP command error cases with automated
tests. I don't expect you to always succeed. We're all human :)
> So yeah, review
> focused specifically on QMP / QAPI bits is always welcome.
It's a struggle, to be honest. Bits of QMP and QAPI are often buried
deep in patches dealing with other stuff. QMP can usually be separated
into its own patch, but QAPI is just infrastructure, it *wants* to be
used.
