Nir Soffer has posted comments on this change.

Change subject: jobs: Autodelete
......................................................................


Patch Set 6: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/62002/6/tests/jobsTests.py
File tests/jobsTests.py:

Line 290:             delay, callable = self.scheduler.calls[0]
Line 291:             self.assertEqual(10, delay)
Line 292:             self.assertIn(job.id, jobs.info())
Line 293:             callable()
Line 294:             self.assertNotIn(job.id, jobs.info())
Nice tests
Line 295: 
Line 296:     def test_autodelete_when_aborted(self):
Line 297:         cfg = make_config([('jobs', 'autodelete_delay', '10')])
Line 298:         job = AutodeleteJob()


Line 303:             delay, callable = self.scheduler.calls[0]
Line 304:             self.assertEqual(10, delay)
Line 305:             self.assertIn(job.id, jobs.info())
Line 306:             callable()
Line 307:             self.assertNotIn(job.id, jobs.info())
Both tests are exactly the same except the line when we run or abort the job. 
We can refactor the common parts, maybe the code for checking that autodelete 
was called?
Line 308: 
Line 309:     def test_autodelete_disabled(self):
Line 310:         cfg = make_config([('jobs', 'autodelete_delay', '-1')])
Line 311:         job = AutodeleteJob()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4aafd2271397bd9bc4289dc1aab723398d475add
Gerrit-PatchSet: 6
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