On Fri, Feb 17, 2012 at 2:09 AM, eli.bendersky
<python-checkins@python.org> wrote:
> diff --git a/Lib/test/test_xml_etree_c.py b/Lib/test/test_xml_etree_c.py
> --- a/Lib/test/test_xml_etree_c.py
> +++ b/Lib/test/test_xml_etree_c.py
> @@ -53,8 +53,8 @@
> � � � � # actual class. In the Python version it's a class.
> � � � � self.assertNotIsInstance(cET.Element, type)
>
> - � �def test_correct_import_cET_alias(self):
> - � � � �self.assertNotIsInstance(cET_alias.Element, type)
> + � �#def test_correct_import_cET_alias(self):
> + � � � �#self.assertNotIsInstance(cET_alias.Element, type)

While this one was fixed quickly, *please* don't comment tests out
without some kind of explanation in the code (not just in the checkin
message).

Even better is to use the expected_failure() decorator or the skip() decorator.

I just saw this test failing in some bots and wanted to fix it ASAP, without spending time on a real investigation. The follow-up fix came less than 2 hours later. But yes, I agree that commenting out wasn't a good choice - I should've just deleted it for the time I was working on a fix.
">

(original) (raw)

On Fri, Feb 17, 2012 at 05:50, Nick Coghlan <ncoghlan@gmail.com> wrote:
On Fri, Feb 17, 2012 at 2:09 AM, eli.bendersky
<python-checkins@python.org> wrote:
> diff --git a/Lib/test/test\_xml\_etree\_c.py b/Lib/test/test\_xml\_etree\_c.py
> --- a/Lib/test/test\_xml\_etree\_c.py
> +++ b/Lib/test/test\_xml\_etree\_c.py
> @@ -53,8 +53,8 @@
> � � � � # actual class. In the Python version it's a class.
> � � � � self.assertNotIsInstance(cET.Element, type)
>
> - � �def test\_correct\_import\_cET\_alias(self):
> - � � � �self.assertNotIsInstance(cET\_alias.Element, type)
> + � �#def test\_correct\_import\_cET\_alias(self):
> + � � � �#self.assertNotIsInstance(cET\_alias.Element, type)

While this one was fixed quickly, \*please\* don't comment tests out
without some kind of explanation in the code (not just in the checkin
message).

Even better is to use the expected\_failure() decorator or the skip() decorator.

I just saw this test failing in some bots and wanted to fix it ASAP, without spending time on a real investigation. The follow-up fix came less than 2 hours later. But yes, I agree that commenting out wasn't a good choice - I should've just deleted it for the time I was working on a fix.


By the way, I later discussed the failing test with Florent and http://bugs.python.org/issue14035 is the result. That failure had made no sense until Florent got deeper into import_fresh_module.


Eli



�