Code review comment for lp:~msapiro/mailman/bug_949924

Thanks for backporting this fix to mm3 Mark! Here are some comments:

I noticed a lot of unused imports in test_approved.py. You might want to look into pyflakes (what I use) or pylint as tools to help find these things. I'll clean them up before I commit though.

I usually like to add a comment to the test_foo() method to explain exactly what the test is checking. Sometimes it's obvious, but you'd be surprised when looking at a test from the outside. :) Also, the comment can refer to the bug #. Since you expect the execution of the rule to not raise an exception, the test does not need to transform UnicodeError and UnicodeWarnings into AssertionErrors. Just let any UnicodeError that occurs cause the test to fail. So really, all you're checking is that the rule returns False, for which you can just use self.assertFalse().

I rewrote this line in approved.py:

cset = part.get_content_charset('us-ascii')

instead of using the 'or'.

All minor stuff. Thanks! Branch approved and I'll land it on trunk.

(Aside: I noticed that we're assuming the moderator password is stored in the clear, so after I land your branch I'm going to be sure it's hashed properly. I'll do that in a separate commit though.)