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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 03, 2013 at 12:44:20PM +0200, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
> >>> ---
> >>>  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?

Yes, sorry, ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]