Nir Soffer has posted comments on this change.

Change subject: Pass scheduler to jobs.start
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/62000/2/tests/jobsTests.py
File tests/jobsTests.py:

Line 63: class JobsTests(VdsmTestCase):
Line 64:     TIMEOUT = 1
Line 65: 
Line 66:     def setUp(self):
Line 67:         scheduler = schedule.Scheduler()
I would not create a real scheduler, this will make it much harder to test - 
you will have to wait until the scheduled call run, so we will have slow or 
racy (or both) tests, or need to mock stuff to wait for events.

Using a fake scheduler, we can check which call where scheduled, and we can 
simulate scheduled events by calling the scheduled functions.

For this patch, passing None, or object() is just fine. If you want to test 
start or stop, fake scheduler is better, but there is not much to test anyway.
Line 68:         jobs.start(scheduler)
Line 69: 
Line 70:     def tearDown(self):
Line 71:         jobs.stop()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5004b49b992ae0f7171a0f0549465cc8782d05da
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to