MaddieM4 / EJTP-lib-python

Encrypted JSON Transport Protocol library for Python 2.6-3.3.
GNU Lesser General Public License v3.0
23 stars 10 forks source link

Use unittests instead of doctests #45

Closed MoritzS closed 11 years ago

MoritzS commented 11 years ago

As this project gets bigger I think doctests are a huge disadvantge. In my opinion they are not intuitive to write and make a mess in the docstrings. Another advantage using unittests is that you can write code portable between different python versions much easier. I propose to add a new directory called "unittests" and placing all test within. Right now there are 162 doctests. We should rewrite them to unittests before it's too late!

MaddieM4 commented 11 years ago

I strongly disagree, but at the same time, I don't want to use my position as project lead to undermine the project. If there are enough people other than me that agree on this, I'll grudgingly trust the hivemind. But I did choose doctests, and I had reasons to do so, which I still believe in at this time.

Doctests are a fantastic way to keep examples in the code documentation, and just as importantly, keep them up to date. They are equal parts example code and regression prevention, two birds with one stone, and any editor that colorizes them correctly makes them very clearly and easily distinct from the body of the code, and the textual documentation.

I agree that the current doctests in this project are messy, but if I'm going to go on a crusade to fix our testing system, it's going to be to clean up our doctests and break them up more evenly between the appropriate functions and classes, not scrap the doctest architecture altogether. I've actually been planning to do this for some time, once fewer other needs are pressing urgently for attention.

Again, if there's a clear consensus that switching from doctests to another test framework is the right thing to do, I will concede, because even though I've found doctest to be the most intuitive and useful way to insulate myself against regression, inaccurate documentation, and accidentally leaving modules out of __init__ lists, I accept that there may be better solutions out there, and the crowd probably has a bit more wisdom than any one person (such as myself) does.

EDIT: Marking this WONTFIX for now, but not closing the issue.

MoritzS commented 11 years ago

Doctests sure are very handy and easier to read or implement, but a huge disadvantage is that if you want to reach 100% code coverage with the tests, the docstrings get reeeeally messy and multiple times longer than the code itself. Another advantage of doctests is that you can not only check the code itself for errors but also you'll see if the documentation (we need more btw!) matches the implementation. What about having some code examples in the docstrings (that's a good way to understand the docs, anyway) but using unittest for the full and comprehensive testing?

iurisilvio commented 11 years ago

I agree with @MoritzS. Doctests are awesome to show code examples, but tests are really more complex than that.

I didn't read all your tests, but if you want to mock things, it already does not make sense to be a doctest.

MaddieM4 commented 11 years ago

The mocking is what sways me. I've already got a semi-hacky pattern going that "solves" the mocking problem, which I started using in the DEJE library, and I ended up recreating to a smaller degree in EJTP to soothe the pain of the identity merge. But proper mocking would be fantastic. Not saying the other reasons are without merit, or that mocking on its own is enough motivation - it was simply that the combination of reasons was finally enough to get me over the hill.

We won't try to switch over for v0.9.2. In fact, for any code that's going to be in that window, what needs testing should be done in doctests, so that we know to convert it later. We'll attack the conversion process as part of work on v0.9.3, breaking it up into distinct issues so that we can switch over in an incremental and parallel way. We won't get rid of doctest support, but we'll only use it to keep our code examples up-to-date, rather than having doctest bear the weight of being the testing suite for the project. This means much smaller docstrings, focused on serving the needs of developer readability, not the test framework.

EDIT: As I break up the conversion job into paid chunks, an issue each, I'll link each of those issues in the comments here. When all of them are done, I'll close this issue.

MoritzS commented 11 years ago

A good example why we need unittests is this: a7831ed31d07734c9900dcfdf19ee49d11033dd6 You added test code that could be implemented using unittest rather than putting it in the source files directly yourself!

MaddieM4 commented 11 years ago

@MoritzS: What do you think about Nose as a test framework? What turned me off of using the unittest module in the first place was its "heavy" infrastructure, like it was trying to emulate all the worst, most enterprise-y aspects of Java. Nose, on the other hand, looks beautifully simple, and even as doctest fogey, I'd feel perfectly comfortable writing tests for Nose.

Another benefit is that supposedly it supports doctests, so we can phase those out gracefully, even as we switch immediately to a new testing system. This is cool, because we don't have to separately run DoctestAll for documentation maintenance, but that's not a vital feature, just a nice secondary.

Obviously you have a lot more experience with this sort of thing, though, which is why I'd really love your opinion on this.

MoritzS commented 11 years ago

@campadrenalin: Well you are right, the unittest framework is somewhat heavy, because you cant just write your code but have to use the unittest.TestCase class. In nose that's easier because you use the (built-in) assert. But in my opinion it does not make a huge difference between using self.assertTrue(func()) (unittest) or assert func() is True (nose). But there comes another huge advantage with using unittest: It's a python built-in module so you won't have to use another framework/library/whatever. And well, you can keep using doctests with builtin python, of course ;-)

MaddieM4 commented 11 years ago

Finally finished! Technically, frame is still left to be done, but it's being handled incidentally by a separate refactor, so I'm not counting it.

Seriously, good job people. @iurisilvio and @MoritzS, you do magnificent work, and you were the ones who really pushed this to completion.