Test failures in 0.11.7: UnicodeEncodeErrors in trac.util.tests.AtomicFileTestCase

[djc]

I just went to run the test suite on 0.11.7 on my Gentoo system, and ran into two of these test failures. Here are the full tracebacks:

======================================================================
ERROR: test_unicode_path (trac.util.tests.AtomicFileTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/tmp/portage/www-apps/trac-0.11.7/work/Trac-0.11.7/trac/util/tests/__init__.py", line 69, in test_unicode_path
    f = util.AtomicFile(self.path)
  File "/var/tmp/portage/www-apps/trac-0.11.7/work/Trac-0.11.7/trac/util/__init__.py", line 163, in __init__
    (fd, self._temp) = tempfile.mkstemp(prefix=name + '-', dir=dir)
  File "/usr/lib64/python2.6/tempfile.py", line 293, in mkstemp
    return _mkstemp_inner(dir, prefix, suffix, flags)
  File "/usr/lib64/python2.6/tempfile.py", line 228, in _mkstemp_inner
    fd = _os.open(file, flags, 0600)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 7: ordinal not in range(128)

======================================================================
ERROR: test_unicode_path (trac.util.tests.AtomicFileTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/tmp/portage/www-apps/trac-0.11.7/work/Trac-0.11.7/trac/util/tests/__init__.py", line 30, in tearDown
    os.unlink(self.path)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xe4' in position 7: ordinal not in range(128)

[Remy Blank]

The tests pass on Gentoo here, so this must be a difference in configuration. What's the output of "locale" on your end? I have the following here:

LANG=en_US.utf8
LC_CTYPE=de_CH.utf8
LC_NUMERIC=en_CH.utf8
LC_TIME=en_CH.utf8
LC_COLLATE=en_CH.utf8
LC_MONETARY=en_CH.utf8
LC_MESSAGES=en_US.utf8
LC_PAPER=en_CH.utf8
LC_NAME=en_CH.utf8
LC_ADDRESS=en_CH.utf8
LC_TELEPHONE=en_CH.utf8
LC_MEASUREMENT=en_CH.utf8
LC_IDENTIFICATION=en_CH.utf8
LC_ALL=

I suspect you use a C locale. Let's see… Yes, same failure here if I run the tests with LC_ALL=C (it's actually a single test case failing, the second error happens in tearDown()). Which is really to be expected, as in that case we shouldn't be passing non-ASCII characters in paths.

The test was intended to catch the erroneous use of the ASCII variants of MoveFileTransacted() et al. on Windows, but will obviously fail on non-Windows platforms with an ASCII locale.

We have a few options: run that particular test only on Windows, detect an ASCII locale and skip the test in that case, or remove it altogether. Thoughts?

[Christian Boos]

I'm not sure the test is legitimate. It seems AtomicFile is in the same league as the other utilities we define in trac.util, like rename, read_file, create_file and create_unique_file. None of these can take a unicode path in all situations (e.g. Unix with LC_ALL=C), due to the limitations of the os module.

So I'd say we should document all those functions to make clear they expect str path, and could fail when given unicode.

If this is not the intent, if we want to support unicode input parameter, then we should also document it, and make the test pass in all cases.

[Remy Blank]

Replying to cboos:

I'm not sure the test is legitimate. It seems AtomicFile is in the same league as the other utilities we define in trac.util, like rename, read_file, create_file and create_unique_file. None of these can take a unicode path in all situations (e.g. Unix with LC_ALL=C), due to the limitations of the os module.

Actually, all these functions are in turn in the same league as the functions in the os module, which allow unicode paths and convert them using the encoding given by sys.getfilesystemencoding(). So we should certainly document that.

Unfortunately, sys.setfilesystemencoding() was only introduced in Python 3, so there's currently no way to make the test pass in all cases. So I think that the test should be removed.

[Christian Boos]

So what about 0.11.8? Should I create it?

[Remy Blank]

I'm sure you know what I'm going to reply, but I'll say it anyway:

After 0.12!

;-)

More seriously, this particular ticket is only about a failing test, so I don't think it's high priority for a backport.

[Christian Boos]

Ok, we might release 0.11.8 only after 0.12, but if we release 0.11.8 (or 0.11.7.1), then I think it's better to have a test suite passing on all platforms.

[Remy Blank]

Replying to cboos:

Ok, we might release 0.11.8 only after 0.12, but if we release 0.11.8 (or 0.11.7.1), then I think it's better to have a test suite passing on all platforms.

Agreed.

[Remy Blank]

Test disabled in on 0.11-stable in [9397], and merged to trunk in [9398].