curses.setupterm must only be called once per process

[therve]

The class _AnsiColorizer in twisted.trial.reporter calls it for each supported() call. But this function seems to leak memory, so it should only be called if necessary. Unfortunately, there is no way to check if it has been called expect by trying to call a curses function needing it.

[therve]

(In [23812]) Branching to 'curses-setupterm-3269'

[therve]

Please review.

[exarkun]

os.environ is really terrible. I think the tests need to do something slightly different with respect to it. Consider:

>>> import os
>>> os.system('echo $LESSOPEN')
| /usr/bin/lesspipe %s
0
>>> os.environ.pop('LESSOPEN')
'| /usr/bin/lesspipe %s'
>>> os.system('echo $LESSOPEN')
| /usr/bin/lesspipe %s
0
>>> os.environ['LESSOPEN'] = 'hello, world'
>>> os.system('echo $LESSOPEN')
hello, world
0
>>> os.environ = {'LESSOPEN': 'foo bar'}
>>> os.system('echo $LESSOPEN')
hello, world
0
>>>

Also, in setUp, I think you probably meant to save os.environ.copy() instead of just os.environ, but I don't think that helps at all.

I feel like _AnsiColorizer should be using a terminfo database instead of curses (or it should be using curses to do the coloring as well, not producing color codes on its own). That may be a different ticket, though. I don't know if there's even a good way to read terminfo from Python, anyway.

[therve]

(In [23847]) Remove cygwin related code.

Refs #3269

[therve]

I removed the code related to cygwin. This is not a supported platform, and I have no idea on how that works (I tried using cygwin with a win32 python installed, and that worked flawlessly). Hopefully someone interested later could add tests for that.

Please review.

[exarkun]

Changes look good. I noticed a pre-existing behavior which may be a defect. _AnsiColorizer checks sys.stderr.isatty() but the reporter writes to sys.stdout. This means trial foo > bar will probably write colors to bar. I doubt this is intentional or desirable. I rather suspect that supported should accept the file-like object which is going to be written to (__init__ already does) and make the check against that, instead.

I'd like to take another look if you fix the stdout/stderr issue in this branch. If you'd rather file a separate ticket for that, cool and go ahead and merge this.

[therve]

Back to review.

[exarkun]

• _Win32Colorizer still checks stdout, even if it is passed something else. I don't really care much about this. I don't even know how you'd get trial to be writing to something other than stdout on Windows. You could file another ticket for it, if you want.
• twisted.trial.test.test_reporter.AnsiColorizerTests.test_supportedSetupTerm and twisted.trial.test.test_reporter.AnsiColorizerTests.test_supportedTigetNumErrors fail if run when stdout is not a pty.
• MockColorizer.supported has a signature incompatible with the other colorizers. Since nothing fails because of this, I suppose that means it is never called, so maybe we don't need it

[therve]

Points 2 and 3 addressed. I don't really care about 1 either.

[exarkun]

Looks good. Merge if  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/curses-setupterm-3269&num_builds=1 goes green.

[therve]

(In [24169]) Merge curses-setupterm-3269

Author: therve Reviewer: exarkun Fixes #3269

Fix trial AnsiColorizer so that it doesn't call curses.setupterm uselessly: it caused a (small) memory leak in the reporter when using trial --until-failure.