Francesco Romani 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. will drop default argument. 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"? Vdsm before this patch. This comment is confusing, will rephrase 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? none. I was fooled by different ordering. 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 I need to make sure that (stats_after['block.0.rd.reqs'] - stats_before['block.0.rd.reqs']) > 0 same for 'block.0.rd.bytes'. 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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
