Milan Zamazal has posted comments on this change.
Change subject: tests: add tests for sampling.VMBulkSampler
......................................................................
Patch Set 36: Code-Review-1
(3 comments)
The tests take relatively long time to run. It would be nice to make them faster.
https://gerrit.ovirt.org/#/c/40053/36/tests/bulkSamplingTests.py
File tests/bulkSamplingTests.py:
Line 206: self._perform_sampling(sampler, cache, times=3,
Line 207: timeout=timeout, flush_at=1)
Line 208:
Line 209: cache.done.wait(timeout * 2)
Line 210: self.assertEqual(conn.calls, ['FAST', 'SLOW',
'FAST'])
This fails but it's not a bug in the test, see _perform_sampling.
Line 211: self.assertVmsInBulkStats(vms.keys(), cache.data[-1][0])
Line 212:
Line 213: def _perform_sampling(self, sampler, cache, times, timeout=1,
Line 214: flush_at=None):
Line 216:
Line 217: for i in range(times):
Line 218: self.exc.dispatch(sampler, timeout)
Line 219: settle_timeout += timeout
Line 220: if flush_at is not None and i == flush_at:
This doesn't work as intended: The flush is immediate here, while the dispatched tasks
are executed asynchronously.
Line 221: sampler.clear()
Line 222:
Line 223: self.assertTrue(cache.done.wait(settle_timeout))
Line 224:
https://gerrit.ovirt.org/#/c/40053/36/vdsm/virt/sampling.py
File vdsm/virt/sampling.py:
Line 489: self._sampling = threading.Semaphore() # used as glorified counter
Line 490: self._log = logging.getLogger("virt.sampling.VMBulkSampler")
Line 491:
Line 492: # for test purposes only
Line 493: def clear(self):
Is it really needed, as a public method? I'd prefer not polluting the public interface
and calling a private method from the tests.
Line 494: self._skip_doms.clear()
Line 495:
Line 496: def __call__(self):
Line 497: timestamp = self._stats_cache.clock()
--
To view, visit
https://gerrit.ovirt.org/40053
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id66dbd420ca29d08ae4063dc83b858be34b8940f
Gerrit-PatchSet: 36
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes