Code review comment for lp:~vila/bzr/321320-isolate-doc-tests

Revision history for this message
Vincent Ladeuil (vila) wrote :

Nice catch !

Thinking more about it, I think the issue is that giving a value to a variable via isolated_environ is just wrong.

If set HOME to None there (but still set it to os.getcwd() for TestCase), I get no failures. I didn't dare before but this makes sense. A test that *requires* HOME to be set for whatever reason, should make sure it's set or fail.

Re-running the test suite without setting HOME *at all* ....<drum roll> pass !

The other variables set here are:

- EMAIL: root cause for the branchbuilder doctest failure, this one is under discussion about whether the *feature* of making whoami mandatory is a good one. Setting it here masked the issue and *users* faced the consequences, I'll leave it for now, but I'm convinced we should set it to None and let the related tests themselves care about it (including the currently hidden failures),
- TERM, LINES, COLUMNS, BZR_COLUMNS: again, the tests needing these settings should set these variables explicitly, we'll get a better tracking of which tests really care about them.

That being said, I'll investigate further and submit a more robust solution, two reviewers agreeing on the same issue is worth addressing isn't it ? :D

« Back to merge proposal