zopefoundation / zope.testing

Other
4 stars 13 forks source link

doctests in TestCase classes #4

Closed jimfulton closed 9 years ago

jimfulton commented 9 years ago

Added support for creating doctests as methods of unittest.TestCase classes so that they can found automatically by test runners, like nose that ignore test suites.

mgedmin commented 9 years ago

nose has --with-doctest and --doctest-tests. Granted, that has some disadvantages (e.g. you cannot specify custom checkers or optionflags), so there's room for other solutions.

I have two reservations for this PR:

jimfulton commented 9 years ago

The biggest problems I have wrt current nose support is the poor support for tst-specific test fixtures, which I find unusable.

WRT naming, I'm open to other suggestions. I was originally going to use doctest, but then that would have been awkward when importing the standard doctest module to get at option flags and othe rmodule contents. doc_test would look weird too. At least camel-case make sense because this is a constructor.

WRT module globals, I have mixed feelings about this. Would you expose module globals for test files too? I assume not.

A bigger question I had was whether to include test-case instance dictionary data in the doctest globals. Eventually decided it would be better to expose a test object, which would give access to test case methods.

jimfulton commented 9 years ago

Oh, and another missing feature is the ability to declare execution of doctests from a module.

I'll add that in a later PR. I'm kinda anxious to get the PR done cuz U need it for work. :)

jimfulton commented 9 years ago

wrt naming: doctestcase?

jimfulton commented 9 years ago

I think having access to module globals makes more sense for traditional doctests that exist in non-test modules.

I'm inclined, for now, to not include module globals, which are somewhat less likely to be interesting in test modules.

It's interesting to note that nose bothers not to look in test modules for doctests, by default.

mgedmin commented 9 years ago

As for naming, yeah, the most sense-making choice (@doctest) conflicts with using doctest.ELLIPSIS etc. Good point there.

I can get used to @DocTest. I've been using @Lazy for a long time without complaining.

As for module globals: right, sharing globals with separate files feels Wrong, just like not letting doctests in docstring access globally defined names feels Wrong to me. I'd prefer consistency with the stdlib doctest module.

I'm tempted to ask for an option @DocTest(globals=True), but that's a bad way to design APIs, so I won't ;).

leorochael commented 9 years ago

I mildly dislike the DWIMing of the first argument acting as either a filename or the test itself, but I can't convince myself that using differently named decorators or different named parameters would make it any better, so I won't complain.

WRT globals, I do think that sharing globals into inlined doctests is important (both the 1st argument tests and the decorated method docstrings), mainly because if I'm mixing doctests with regular test methods in the same test file I'd find it annoying to have to reimport modules that were imported globally only inside the inlined doctests and not in the test methods.

The external doctests I agree shouldn't share globals with the test module.

(I guess a case could be made here that the "test" global should be called (or aliased) "self" in the inlined tests)

WRT naming, @doctestcase seems wrong since the decorator creates a test (method), not a test-case (which is a class with several tests, a setup and a teardown). I'd also prefer a lowercased name, but anything I can think of would be weirder or less convenient than @DocTest.

All in all, I really like this PR for allowing doctests being able to reuse TestCase setup/teardown and being locatable by nose!

jimfulton commented 9 years ago

After reading Leonardo's comments, separate function names is growing on me:

docteststring doctestfile doctestmethod

doctestmethod and docteststring would inject module globals.

jimfulton commented 9 years ago

I prefer test for referring to the test. Having 'self' at the top level of doctest code seems pretty weird.

Also, if a test defines a class, the self in the example class would hide access to the test.

But perhaps this could work differently for doctestfile vs docteststring and doctestmethod.

leorochael commented 9 years ago

In my mind, self in the doctest doesn't look so weird when it's, for instance, the docstring of the test method where the developer just did self.something = something_else to prepare for the test. But I agree it only makes sense in the docteststring/method cases.

And the hiding of self would be undesirable indeed, so if it's there it should be an alias to test.

Again, this is minor IMO, and maybe not worth the extra effort.

But speaking of globals, it would be extra nice (perhaps in another pull request) if there was a way of passing new top-level names into the doctest from the decorated function.

jimfulton commented 9 years ago

I've rewritten this based on comments:

jimfulton commented 9 years ago

BTW, I plan to submit a Python PR after we've used this in zope.testing for a while.

jimfulton commented 9 years ago

What do y'all think? :)

mgedmin commented 9 years ago

Looks good to me (other than some typos)!

I didn't mention that some docstings ought to have a period at the end of the first sentence (as per PEP-257), since I didn't want to appear too nitpickery.

jimfulton commented 9 years ago

Thanks for the comments! I've addresses all of them.

leorochael commented 9 years ago

Didn't do as thorough a review as @mgedmin, but looks nice.

The independent functions are nicer and more readable than before.