Re: [libvirt] [PATCH 02/18] qemumonitorjsontest: Test qemuMonitorJSONSystemPowerdown

On 02.10.2013 20:08, Daniel P. Berrange wrote: > On Wed, Oct 02, 2013 at 11:42:13AM -0600, Eric Blake wrote: >> On 10/02/2013 11:09 AM, Michal Privoznik wrote: >>> Right now, we are testing qemuMonitorSystemPowerdown instead of >>> qemuMonitorJSONSystemPowerdown. It makes no harm, as both functions have >>> the same header and the former is just a wrapper over the latter. But we >>> should be consistent as we're testing the JSON functions only in here. >>> >>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>> --- >>> tests/qemumonitorjsontest.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> Actually, I think we should do the opposite, and test only the wrapper >> functions that the rest of qemu_driver.c and friends will be calling. >> For example, testQemuMonitorJSONGetVersion() calls into >> qemuMonitorGetVersion, not qemuMonitorJSONGetVersion. > > Well this test suite was specifically targetting only the JSON monitor > impl, not the text mode impl, so calling the JSON functions is > right IMHO. > > There is separate testing for the text mode monitor > > Daniel > Does this mean ACK, esp. if other patches calling JSON functions were acked?