Code review comment for lp:~edwin-grubbs/launchpad/bug-615654-jobqueue-cron-script

Waiting for children to exit will mean that no new jobs can be processed until all the jobs complete. This may severely impair responsiveness. I suggest starting the subprocesses and then exiting.

The NoErrorOptionParser should not exist. Option handling should be taken care of in the ProcessJobSource class. If the layering is getting in our way, we should fix it, not work around it.

Unless you are specifically testing stringification, you should not stringify objects in tests, so that your tests are not fragile.

In test_setstatus_admin, what's the purpose of transaction.commit?

Since you do person_set.getByEmail('<email address hidden>'), you can use login_person or with person_logged_in instead of login. Also, lifeless requested that you use the constant for <email address hidden>' instead of spelling it out.

Please use lp.testing.run_script instead of rolling your own.

In test_processed, I don't think you really care what the database id of the job is, so use assertTextMatchesExpressionIgnoreWhitespace. (You'll need to move it to TestCaseWithFactory.)

get isn't required to run jobs under the non-twisted job runner, so it seems like an unnecessary requirement on IJobSource.

Perhaps ProcessJobSource should go in lp.services.job.runner ?

test_empty_queue looks fragile because it includes the directory of the lock file. Perhaps another case for assertTextMatchesExpressionIgnoreWhitespace?

Perhaps PYTHONPATH and LPCONFIG should be handled by common code, eg:
for name in ['PYTHONPATH', 'LPCONFIG']:
    if name in os.environ:
        env[name] = os.environ[name]