Fix some minor issues with functional test cases

[Ryan J Ollos]

This ticket proposes to fix the following issues with the functional test cases:

1. Attempting to execute a functional test module directly while working in a virtualenv results in:

   (t11258)user@ubuntu:~/Workspace/t11258/teo-rjollos.git$ ./trac/admin/tests/functional.py 
   Traceback (most recent call last):
     File "./trac/admin/tests/functional.py", line 2, in <module>
       from trac.tests.functional import *
   ImportError: No module named trac.tests.functional
   

   With a replacement #!/usr/bin/python → #!/usr/bin/env python, it will be possible to execute those test modules directly.
2. The module trac/ticket/tests/functional.py can't be run alone, failing with (first mentioned in comment:5:ticket:11176):

   (t11259)user@ubuntu:~/Workspace/t11259/teo-rjollos.git$ ./trac/ticket/tests/functional.py 
   SKIP: validation of XHTML output in functional tests (no lxml installed)
   .....................................................................FFFFFFFFFFFFFFFFFFFFFFFFFF.
   ======================================================================
   FAIL: runTest (__main__.RegressionTestTicket4630a)
   Test for regression of http://trac.edgewall.org/ticket/4630 a
   ----------------------------------------------------------------------
   Traceback (most recent call last):
     File "./trac/ticket/tests/functional.py", line 1216, in runTest
       self._tester.login(u'joé')
     File "/home/user/Workspace/t11259/teo-rjollos.git/trac/tests/functional/tester.py", line 56, in login
       tc.find("logged in as %s" % username)
     File "/home/user/Workspace/t11259/teo-rjollos.git/trac/tests/functional/better_twill.py", line 203, in better_find
       raise twill.errors.TwillAssertionError(*args)
   TwillAssertionError: ("no match to 'logged in as jo\xc3\xa9'", '/home/user/Workspace/t11259/teo-rjollos.git/testenv/trac/log/RegressionTestTicket4630a.html')
   

3. tags/trac-1.0.1/trac/db/tests/functional.py​ doesn't have the executable property set on checkout from SVN or after cloning the Git repository.
4. Add license headers to all test case files.

[Ryan J Ollos]

Fix for (3) committed to SVN in [11875]. I'm imagining that the Git mirror picks up this change, but either way I'll find out shortly enough. I'll merge this change to the trunk when all of the changes are committed for this ticket.

[Ryan J Ollos]

Should I make the #!/usr/bin/python → /usr/bin/env python replacement for these files as well?

./contrib/workflow/migrate_original_to_basic.py:#!/usr/bin/python
./contrib/emailfilter.py:#!/usr/bin/python
./contrib/cgi-bin/trac.fcgi:#!/usr/bin/python
./contrib/cgi-bin/trac.cgi:#!/usr/bin/python
./contrib/checkwiki.py:#!/usr/bin/python
./contrib/htpasswd.py:#!/usr/bin/python

To solve (2), I just added another user "joe" in FunctionalTestEnvironment, and used that user rather than "joé" in RegressionTestTicket4630a, RegressionTestTicket4630b and RegressionTestTicket5394a.

Changes can be found in repos:rjollos.git:t11259.

[Jun Omae]

Changes can be found in repos:rjollos.git:t11259.

Looks good.

[Ryan J Ollos]

Committed to 1.0-stable in [11904:11907] and merged to trunk in [11908] and [11909] (and [11910:11911], note to self - always run tests right before committing!).

However this isn't working the way I had thought and the tests are failing due to issue (2). I'll probably have a follow-up here shortly.

Sorry for the messy change history. I'll be more careful in the future.

[Ryan J Ollos]

Two tests are still failing because the user joé may or may not exist, depending on which tests have been run prior to the two test cases that now expect joé to not exist.

Doctest: trac.test.Mock ... ok

======================================================================
FAIL: Test for regression of http://trac.edgewall.org/ticket/4630 b
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11259/teo-rjollos.git/trac/ticket/tests/functional.py", line 1227, in runTest
    self.assertEqual(sorted(users), ['admin', 'joe', 'user'])
AssertionError: [u'admin', u'joe', u'jo\xe9', u'user'] != ['admin', 'joe', 'user']

======================================================================
FAIL: Test for regression of http://trac.edgewall.org/ticket/5394 a
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11259/teo-rjollos.git/trac/ticket/tests/functional.py", line 1274, in runTest
    tc.find(to_utf8(options), 's')
  File "/home/user/Workspace/t11259/teo-rjollos.git/trac/tests/functional/better_twill.py", line 216, in better_find
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no match to \'id="action_reassign_reassign_owner"><option[^>]*>admin</option><option[^>]*>alan</option><option[^>]*>alice</option><option[^>]*>bob</option><option[^>]*>charlie</option><option[^>]*>jane</option><option[^>]*>joe</option><option[^>]*>john</option><option[^>]*>user</option><option[^>]*>zorro</option>\'', '/home/user/Workspace/t11259/teo-rjollos.git/testenv/trac/log/RegressionTestTicket5394a.html')

----------------------------------------------------------------------
Ran 1334 tests in 192.166s

FAILED (failures=2)

I think the solution here is to remove the user joé at the end of RegressionTestTicket6318​. Typically we clean up the test environment at the end of a test case (e.g. revoking a permission that was granted in the test case) in order to maintain a known state. Since we have test cases that depend on comparing to a known list of users, I propose that all users that persist across test cases should be created in FunctionalTestEnvironment.

I've made too many mistakes tonight, so I'm proposing the changes in repos:rjollos.git:t11259.2, and will follow up on it tomorrow.

Side note: Is there a special reason to add user "user" in RegressionTestTicket6318 (self._testenv.adduser(u'user'))? That user is already created in FunctionalTestEnvironment​. Maybe we can either delete that adduser call, or change to a different ascii username and also delete the different ascii user at the end of the testcase. I don't think this is causing any problems, I just found it to be confusing. Even just a clarifying comment might be the right "fix" here.

[Ryan J Ollos]

Fix described in comment:5 was committed to 1.0-stable in [11913:11915] and merged to trunk (along with the almost forgotten [11875]) in [11916].

[Christian Boos]

Well, I still have the failures mentioned in comment:5 when running all functional tests with latest 1.0-stable (r), on Windows… only (i.e. works as expected on Linux, or on Windows when running only the ticket functional tests).

It must be deluser (r11914) not working as expected.

More specifically, it's the deletion from the session table. After running the tests:

$ PYTHONPATH=. python trac/admin/console.py testenv/trac/ session list

SID      Auth  Last Visit  Name                  Email
------------------------------------------------------------------
admin    1     2013-08-12  System Administrator  admin@example.com
alan     1     2013-08-12
alice    1     2013-08-12
bob      1     2013-08-12
charlie  1     2013-08-12
jane     1     2013-08-12
joe      1     2013-08-12
john     1     2013-08-12
joé      1     2013-08-12
user     1     2013-08-12
zorro    1     2013-08-12

Ok, seems FunctionalTestEnvironment._tracadmin on Windows doesn't like UTF-8 input. For me, using getpreferredencoding() seems to provide the correct encoding (cp1252) i.e. the one expected by _tracadmin (see trac.admin.console.TracAdmin.onecmd()).

trac/tests/functional/testenv.py

@@ -30 +30 @@
 from trac.tests.functional.better_twill import tc, ConnectError
 from trac.util import terminate
 from trac.util.compat import close_fds
-from trac.util.text import to_utf8
+from trac.util.text import getpreferredencoding, to_utf8
 
 try:
     from configobj import ConfigObj
@@ -166 +166 @@
 
     def deluser(self, user):
         """Delete a user from the environment."""
+        self._tracadmin('session', 'delete',
+                        user.encode(getpreferredencoding()))
         user = to_utf8(user)
-        self._tracadmin('session', 'delete', user)
         if call([sys.executable, os.path.join(self.trac_src, 'contrib',
                  'htpasswd.py'), '-D', self.htpasswd, user],
                 close_fds=close_fds, cwd=self.command_cwd):

I'm interested to see if this also works on Eastern Asian installations of Windows (Jun, could you please try?). If this proves to be problematic, maybe better to just use a SQL statement.

[Ryan J Ollos]

Coincidentally, I have some additional minor changes to push against this ticket. I thought I had reopened it yesterday, but I must have dreamed that.

[Jun Omae]

Replying to cboos:

I'm interested to see if this also works on Eastern Asian installations of Windows (Jun, could you please try?). If this proves to be problematic, maybe better to just use a SQL statement.

The proposal patch doesn't work on Japanese installation of Windows XP. Even if Greek (cp1253) and Hebrew‎ (cp1255), it probably doesn't work. The ANSI code pages is from ​National Language Support (NLS) API Reference.

# -*- coding: utf-8 -*-

username = u'joé'
for encoding in 'cp1250 cp1251 cp1252 cp1253 cp1254 cp1255 cp1256 cp1257 cp1258 cp874 cp932 cp936 cp949 cp950'.split():
    try:
        roundtrip = username.encode(encoding).decode(encoding) == username
        if roundtrip:
            print encoding, 'ok'
        else:
            print encoding, 'not round-trip'
    except UnicodeError, e:
        print encoding, e.__class__.__name__, unicode(e)

> python25 C:\tmp\test.py
cp1250 ok
cp1251 UnicodeEncodeError 'charmap' codec can't encode character u'\xe9' in position 2: character maps to <undefined>
cp1252 ok
cp1253 UnicodeEncodeError 'charmap' codec can't encode character u'\xe9' in position 2: character maps to <undefined>
cp1254 ok
cp1255 UnicodeEncodeError 'charmap' codec can't encode character u'\xe9' in position 2: character maps to <undefined>
cp1256 ok
cp1257 ok
cp1258 ok
cp874 UnicodeEncodeError 'charmap' codec can't encode character u'\xe9' in position 2: character maps to <undefined>
cp932 UnicodeEncodeError 'cp932' codec can't encode character u'\xe9' in position 2: illegal multibyte sequence
cp936 ok
cp949 UnicodeEncodeError 'cp949' codec can't encode character u'\xe9' in position 2: illegal multibyte sequence
cp950 UnicodeEncodeError 'cp950' codec can't encode character u'\xe9' in position 2: illegal multibyte sequence

[Jun Omae]

In [751887b0/jomae.git], we can pass utf-8 command via stdin of trac-admin process. All functional tests pass with Python 2.5 and 2.6 on Windows XP (Japanese) and Linux.

[Christian Boos]

Replying to jomae:

In [751887b0/jomae.git], we can pass utf-8 command via stdin of trac-admin process. All functional tests pass with Python 2.5 and 2.6 on Windows XP (Japanese) and Linux.

Great fix, works for me as well (Python 2.7, Windows 7). Please commit!

[Jun Omae]

Thanks for your confirming! Applied in [11947] and merged into trunk in [11948].

[Ryan J Ollos]

Replying to rjollos:

Coincidentally, I have some additional minor changes to push against this ticket. I thought I had reopened it yesterday, but I must have dreamed that.

I eventually created a new ticket for this issue, #11332. Following the changes committed in that ticket, the tests in trac.tests.functional.testcases can be executed without executing the entire test suite. Now each of the 8 functional test cases modules can be executed independently.