Code review comment for lp:~adiroiban/launchpad/bug-509252-take-2

Revision history for this message
Adi Roiban (adiroiban) wrote :

În data de Mi, 24-02-2010 la 18:27 +0000, Henning Eggers a scris:
> Review: Approve code
> > Conflict solved.
>
> Cool, looks good. ;)
>
>
> > Are you saying that we should not write unit tests using doctest format?
>
> Yes, at least that's what I do. They execute slower, don't they? But
> as you pointed out, the test was already there, so you just extended
> it. That's what I do, too ... ;) So you can keep it as it is if you
> want to.
OK. This is not the only unit test written in doctest format.

$ ls lib/lp/translations/browser/tests/*.txt
lib/lp/translations/browser/tests/distroseries-views.txt
lib/lp/translations/browser/tests/language-views.txt
lib/lp/translations/browser/tests/poexport-request-views.txt
lib/lp/translations/browser/tests/pofile-base-views.txt
lib/lp/translations/browser/tests/pofile-views.txt
lib/lp/translations/browser/tests/potemplate-views.txt
lib/lp/translations/browser/tests/productseries-views.txt
lib/lp/translations/browser/tests/translationimportqueue-views.txt
lib/lp/translations/browser/tests/translationmessage-views.txt
lib/lp/translations/browser/tests/translator-views.txt

Maybe we should open a bug to convert all those test from doctest to
"pure" python code.

> Thanks for the extra comments in the code, btw. And all the clean-up
> work!
> You have a double "allowed allowed" in there somewhere, btw.

Typo fixed and pushed.

When you have time, can you please send this branch to ec2 test?

Kindest regards,

--
Adi Roiban

« Back to merge proposal