unified-font-object / ufoNormalizer

A tool that will normalize the XML and other data inside of a UFO.
Other
51 stars 19 forks source link

Move doctests to unittests #17

Closed moyogo closed 8 years ago

moyogo commented 8 years ago
typesupply commented 8 years ago

Why shouldn't these be in doctests?

moyogo commented 8 years ago

It’s fine to have example tests in the docstring but unittest is better for thorough testing, doctest is more appropriate for documenting or basic testing. The docstrings should be short and to the point, they end up in .doc. Having the unit tests in a separate file makes the main script lighter.

There are also several issues with using only doctests, unittests are just more flexible and can be more atomic.

anthrotype commented 8 years ago

http://bemusement.org/doctests-arent-code

typesupply commented 8 years ago

Yes, but...

This isn't a typical module that needs atomic documentation. There is commandline behavior and a single public function. The rest is all internal.

I don't have anything against unittests. In this case, I put everything in doctests because it made sense to put the tests next to the code that they are evaluating. In almost all of the functions in this file the tests were the documentation. Now things like xmlConvertInt have no documentation or information about how they are expected to handle certain situations. The file may be cleaner, but it is going to be harder to make changes without cross checking.

I'll think about this switch. I'm not rejecting it. I just need to think about it.

typesupply commented 8 years ago

Alright, I decided to merge this just to keep the main file shorter. Could you resolve the conflicts and update the PR?

moyogo commented 8 years ago

OK. I'll update the PR next week.

moyogo commented 8 years ago

@typesupply OK, the branch has been rebased and adff343 reflect the changes made in the doctests.