Adam Litke has posted comments on this change.

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


Patch Set 6:

(6 comments)

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

Line 277:         job = TestingJob(exception=exception.VdsmException())
Line 278:         self.run_job(job)
Line 279:         self.assertEqual(jobs.STATUS.FAILED, job.status)
Line 280:         self.assertIsInstance(job.error, exception.VdsmException)
Line 281: 
> Fragile, use fake scheduler instead.
Done
Line 282:     @permutations(((None, ), (Exception(),)))
Line 283:     def test_autodelete_when_finished(self, error):
Line 284:         cfg = make_config([('jobs', 'autodelete_delay', '10')])
Line 285:         job = AutodeleteJob(exception=error)


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

Line 75: class FakeScheduler(object):
Line 76: 
Line 77:     def __init__(self):
Line 78:         self.calls = []
Line 79: 
> This works, but this does not test that callable will delete job.
Good idea.  Changed.
Line 80:     def schedule(self, delay, callable):
Line 81:         self.calls.append((delay, callable))
Line 82:         return schedule.ScheduledCall(delay, callable)
Line 83: 


Line 281: 
Line 282:     @permutations(((None, ), (Exception(),)))
Line 283:     def test_autodelete_when_finished(self, error):
Line 284:         cfg = make_config([('jobs', 'autodelete_delay', '10')])
Line 285:         job = AutodeleteJob(exception=error)
> This does not verify that the job was removed after the configured delay.
Applied your suggestion from above.
Line 286:         jobs.add(job)
Line 287:         with MonkeyPatchScope([(jobs, 'config', cfg)]):
Line 288:             self.run_job(job)
Line 289:             self.assertEqual(1, len(self.scheduler.calls))


Line 289:             self.assertEqual(1, len(self.scheduler.calls))
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()
> Same
Same.
Line 294:             self.assertNotIn(job.id, jobs.info())
Line 295: 
Line 296:     def test_autodelete_when_aborted(self):
Line 297:         cfg = make_config([('jobs', 'autodelete_delay', '10')])


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
Thanks!
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 jo
done.
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