Change in vdsm[master]: tests: improve vmstats.disks coverage

nsoffer at redhat.com nsoffer at redhat.com
Thu Jan 7 10:17:27 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: tests: improve vmstats.disks coverage
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.ovirt.org/#/c/50598/1/tests/vmStatsTests.py
File tests/vmStatsTests.py:

Line 234: 
Line 235:     def assertNameIsAt(self, stats, group, idx, name):
Line 236:         self.assertEqual(stats['%s.%d.name' % (group, idx)], name)
Line 237: 
Line 238:     def assertStatsHaveKeys(self, stats, keys=None):
We can use keys=_EXPECTED_KEYS in the function definition.

But it is better to keep the code simple and let the caller specify the keys always.
Line 239:         if keys is None:
Line 240:             keys = self._EXPECTED_KEYS
Line 241:         for key in keys:
Line 242:             self.assertIn(key, stats)


Line 385:         'readBytes',
Line 386:         'writtenBytes',
Line 387:     )
Line 388: 
Line 389:     # this is what old VDSM did. No clear rationale.
What is "old VDSM"?
Line 390:     _MINIMAL_KEYS = (
Line 391:         'apparentsize',
Line 392:         'flushLatency',
Line 393:         'imageID',


Line 386:         'writtenBytes',
Line 387:     )
Line 388: 
Line 389:     # this is what old VDSM did. No clear rationale.
Line 390:     _MINIMAL_KEYS = (
What is the difference between this list and _EXPECTED_KEYS?
Line 391:         'apparentsize',
Line 392:         'flushLatency',
Line 393:         'imageID',
Line 394:         'readBytes',


Line 410:         stats_after = copy.deepcopy(self.bulk_stats)
Line 411:         stats_before['block.0.rd.reqs'] = 0
Line 412:         stats_after['block.0.rd.reqs'] = 1024
Line 413:         stats_before['block.0.rd.bytes'] = 0
Line 414:         stats_after['block.0.rd.bytes'] = 128 * 1024
Are you adding keys or changing the values - not clear what  are you trying to test here.
Line 415:         vmstats.disks(testvm, stats,
Line 416:                       stats_before, stats_after,
Line 417:                       interval)
Line 418:         self.assertRepeatedStatsHaveKeys(drives, stats['disks'])


-- 
To view, visit https://gerrit.ovirt.org/50598
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I931d080d66b8a6087b05f01fbdce8fee0cd99039
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list