Code review comment for lp://staging/~jtv/launchpad/bug-523926

= Bug 520651 =

Sometimes when running tests, a daemon fails to start up. In that case you'll get a message referring to the daemon's log file—which would usually help except it does not exist.

This happens because TacHandler's tearDown deletes the logfile. Yes, log files do need to be deleted before the same dæmon is started again later. Magic in the way TacHandler polls for daemon startup makes that necessary. But there's no real need to clean up the logfile at the end of a run. The amount of garbage is limited, with a single fixed filename, and it's in /var/tmp/ where it won't usually bother anyone.

So I fixed this by deleting the log file in setUp, but not in tearDown. In addition to this I did some drive-by cleanups. In the buildd test harness, which stubbornly refused to leave logfiles behind, I moved the logfile out of /var/tmp/buildd/ which was being deleted during both setup and teardown.

I also rewrote that daemon startup polling loop to be, I hope, a bit easier to maintain.

There is one pylint notice:
{{{
lib/canonical/launchpad/daemons/tachandler.py
    6: [W0105] String statement has no effect
}}}

This seems to be because the docstring has to come after the import for the "with" statement. Not quite enough to disable the warning; the extra import will go away as we upgrade python further anyway.

To Q/A, run some tests that start daemons (e.g. librarian and/or buildd) twice in succession. You should see /var/tmp/librarian.log and /var/tmp/build-slave.log after each run that exercises the respective daemon. Run these log files through "wc" after the first run and the second. Their stats should remain about the same as the re-run cleans up the old logs and the tests then write the same logs again. (In my case the "wc" output is _exactly_ identical, with just the timestamps changed AFAICS, but I wouldn't promise it).

Jeroen