post-incident logging breaks trial unit tests

[Brian Warner]

The timer that is used to record post-Incident log events causes problems with trial unit tests: if any incidents are triggered during the tests (perhaps due to intentionally induced failures), this timer will probably still be running when the test finishes, which means trial will flunk the test with an "Unclean Reactor" error.

One workaround would be to have any unit test suite which is likely to trigger Incidents to also modify foolscap.logging.incident.IncidentReporter.TRAILING_DELAY to None, which would disable this timer, or to have the unit test suite register a replacement IncidentReporter subclass with this same change. This might need to be done from any test which had the potential to trigger incidents, though, and there could be a lot of them.

[Brian Warner]

We already require that applications shut down their Tubs at the end of their trial tests (in tearDown, by using stopService) to get Foolscap to clean up its usage of the reactor. It would be nice if this were also sufficient to terminate any lingering Incident reporters.

The trouble is that the logging framework exists outside the context of any single Tub, since we want to support the use of multiple Tubs in a single application (for reference lifetime management, among other things).

Listeners use a scheme in which the Listener is kept alive by a reference by any tub, but when the last Tub is shut down, the Listener is finally shut down too. Perhaps we could use a scheme like that. We need logging to work before any Tubs are created, but we could declare that all Incident Reporters (or at least those which involve trailing delays) come under the auspices of a collection of Tubs, and when the last Tub is shut down, the trailing delay is cancelled.

OTOH, I saw the Tahoe unit tests take much longer (perhaps twice as long) when incidents were enabled, probably because unit tests intentionally cause a lot of error conditions that would be treated as Incidents under normal operations. Maybe this is a hint that incident processing needs to be faster, but maybe it just means that unit tests should not enable incident processing.

[Brian Warner]

I'm not sure what I want to do about this: some sort of Tub.shutdown should kill off any outstanding incidents, but since loggers are outside the context of any Tubs, that's not quite right.

I'm ok with pushing this out past 0.3.0 .