Change in vdsm[master]: tests: add tests for sampling.VMBulkSampler

mpolednik at redhat.com mpolednik at redhat.com
Tue Apr 19 07:56:49 UTC 2016


Martin Polednik has posted comments on this change.

Change subject: tests: add tests for sampling.VMBulkSampler
......................................................................


Patch Set 4: Code-Review-1

(5 comments)

Some questions inside, not sure I prefer the context managers to previous code (names such as prepare_wait don't help code clarity either for someone unfamiliar in sampling code).

https://gerrit.ovirt.org/#/c/55856/4/tests/bulk_sampling_test.py
File tests/bulk_sampling_test.py:

PS4, Line 64: def test_collect_fast_path_as_default(self):
What is our slow test threshold? 2 and 4 second tests imho belong to the same category.


PS4, Line 113: vms['1'].ready = False
How does it affect this test? domainListGetStats isn't affected by the event set in FakeConnection -> we'll have expected=2 anyway?


PS4, Line 133: vms['1'].ready = False
Similar to q above - how do you expect this to affect the test?


PS4, Line 151: vms['1'].ready = True
Same.


PS4, Line 228:     for idx in range(1, num+1):
             :         vm = FakeVM(str(idx))
Using 'index' wouldn't hurt anything here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ec7d5f2fc3b9b5c3efbffe874ad6ceefe7a3406
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani 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: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list