On 07/30/2018 08:32 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> On 07/27/2018 11:46 AM, Thomas Huth wrote: >>> On 07/27/2018 05:13 PM, Markus Armbruster wrote: >>>> qtest_qmp_discard_response(...) is shorthand for >>>> qobject_unref(qtest_qmp(...), except it's not actually shorter. >>> >>> But the latter is IMHO harder to read. > > Doing things sloppily looks a bit uglier now. That's a feature. > >> Maybe, but then it lends itself well to: >> >> QObject *rsp = qtest_qmp(...); >> qobject_unref(rsp); >> >> which is where you do insert tests for valid responses. >> >>> And it might be shorter in the compiled binary (one function call vs. two). > > I'd be quite sympathetic to this argument... > >> The size of the test binaries is not our biggest concern. > > ... outside tests/. > >>>> Moreover, the presence of these functions encourage sloppy testing. >>> >>> Shouldn't we then rather fix the tests to check for valid responses >>> instead of replacing this function with harder-to-read code? > > I'd welcome such patches, but this series is already pretty long.
Then maybe rather drop this patch from this series, and fix the issues in a separate series instead? Thomas
