Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

#9128 closed defect (fixed)

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

Reported by: djc Owned by: Remy Blank
Priority: normal Milestone: 0.12
Component: general Version:
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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)

Attachments (0)

Change History (9)

comment:1 by Remy Blank, 12 years ago

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?

comment:2 by Christian Boos, 12 years ago

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.

in reply to:  2 comment:3 by Remy Blank, 12 years ago

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.

comment:4 by Remy Blank, 12 years ago

Milestone: 0.12
Owner: set to Remy Blank

comment:5 by Christian Boos, 12 years ago

So what about 0.11.8? Should I create it?

comment:6 by Remy Blank, 12 years ago

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.

comment:7 by Christian Boos, 12 years ago

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.

in reply to:  7 comment:8 by Remy Blank, 12 years ago

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.

comment:9 by Remy Blank, 12 years ago

Resolution: fixed
Status: newclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.