Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Think about how to support per-method test groups #11

Closed
bitprophet opened this issue Nov 7, 2011 · 6 comments
Closed

Think about how to support per-method test groups #11

bitprophet opened this issue Nov 7, 2011 · 6 comments

Comments

@bitprophet
Copy link
Owner

As usual, inspired by RSpec -- we can assert about class behavior, but what if we wanted to group tests more granularly, e.g. Class.method should (x, y, z), there's no way to do that at the moment.

RSpec method asserts (IIRC) look like this:

describe Model do
    describe "method_one" do
        it "should accept xyz parameters"
        it "when given 5 returns 10"
    end
end

Syntactically it's a tough problem because we're not using nested blocks as in Ruby, but Python classes and methods.


One way to get 3 levels is to use modules differently than we do now: assert that if a module has classes in it, it's not the usual "module w/ functions" but a three-level setup where the module name, capitalized, is the class; classes are about methods (downcased I guess) and methods are the tests.

E.g., in foo.py:

class MethodOne(object):
    def accepts_xyz_parameters(self):
        pass

    def raises_error_on_none_input(self):
        pass

class AnotherMethod(object);
    def when_given_5_returns_10(self):
        pass

Which might translate into this output:

Foo
.method_one:
    - accepts xyz parameters
    - raises error on none input
.another_method:
    - when given 5 returns 10

(Actual indentation/formatting subject to change, of course -- just an example.)

This could be further enhanced, readability wise, by using lowercase class names so that no CamelCase to underscores conversion is required, though this then breaks with PEP8. Perhaps make that an option (i.e. perform CamelCaseConversion if required, and that's it.)


This could be an area where class or method decorators make more sense. E.g.:

class Model_(object):
    @tests('method_one')
    def accepts_xyz_parameters(self):
        pass

    @tests('method_one')
    def raises_error_on_none_input(self);
        pass

would result in identical output as the Model.method_one lines in the above output example.

This avoids the oddball semantics of the first solution, but results in a moderate amount of repetition/boilerplate (i.e. complex methods end up with a lot of tests all saying @tests('methodname'). It's effectively "tagging" the tests (and in fact might be worth integrating with Nose's existing tag plugin(s)).


Another "less code work but more repetition" solution is just to rely on the existing docstring behavior -- objects' docstrings are used for their descriptions in spec output whenever possible.

So one can fake it with that. This:

class ObjectMethod(object):
    "Object.method"
    def should_blah(self):
        pass

becomes this:

Object.method
- should blah

So this could theoretically be closed as WONTFIX, really. Depends how fancy we want to get and what anybody else says would work best for them.

@garybernhardt
Copy link

This may be a nitpick, but nested describe/context blocks are much more general than method descriptions. For example, I might say context "when the user is logged in" do, etc. Supporting only one level of them would be a little unfortunate.

+1 for breaking PEP 8. You're not using classes as classes; you're using them as a hack to get around Python's lack of blocks. The rules go out the window IMO. I posted to the TIP list about this recently.

-1 for the decorators. It's noisy and invites duplication.

-1 for the docstrings as well, actually. Docstrings in Nose and friends exist because spec-style definition isn't possible. Your goal is to make it possible. :)

Some people have proposed various hacks using with, like this:

with describe(MyClass):
    with context(".method"):
        with it("does a thing"):
            ...

Syntactically, I don't know whether I like this more or less than the class/method approach. It also would require some pretty evil hacks to turn those with statements into blobs of code that are stored for later. I think that the POC I saw relied on with_hacks.

On that note, have you considered nested classes? Is there any reason that that wouldn't work?

tl;dr: +1 break PEP 8, -1 decorators, -1 docstrings, -0 with statement, +1 nested classes?

(I hope at least some of this is useful... :)

@bitprophet
Copy link
Owner Author

No, that's all great feedback, thanks a lot :)

I agree that it'd be ideal if this could work for general specification style junk and not only describing classes+methods. But that does bring in the nesting problem, and I too am unsure whether I like the look of contextmanagers more or less than nested classes.

In terms of implementation I suspect nested (non-PEP8-compliantly-named, as you mentioned) classes would be a lot easier/cleaner, to say nothing of requiring less hacking-up of Nose's internals.

Either approach might need deeper modifications to Nose, though, because I am pretty sure Nose does not give us direct control of determining how to search for test objects, only the ability to veto what it gathers up by default. I've not looked at that in depth though, there may be more methods in Selector that we could override. Will take a look sometime.

@bitprophet
Copy link
Owner Author

I'm halfway there; nested classes' public methods are now picked up as tests and appear to be run OK. While I haven't tested it, this should recurse correctly as well.

The output side of things needs handling though, since they currently print as standalone test cases (i.e. as if they were an un-nested top-level class). Will probably take this in stages: first get it to the point where it can handle the simpler use case of "Object => method => tests about that method", then worry about the presentation of arbitrary groupings a la describe/context.

bitprophet added a commit that referenced this issue Dec 16, 2011
Uses this to print 1 level of 'nesting' for now.

Implementation needs cleanup/refactoring, esp re: detecting public class members in ~2 places.

Re #11
@bitprophet
Copy link
Owner Author

Have it at least printing e.g. Class.method for single-nested inner classes, for now. Uses a metaclass, meaning that nesting will require a specific superclass and not object; not a big deal IMO.

While the printing is simplistic (works for my current needs but not for arbitrary nesting), the entire hierarchy should be in place now, i.e.:

class Outer(SpecSuperclass):
    def should_foo(self):
        pass

    class methodname:
        def should_bar(self):
            pass

should print out like this:

Outer:
- should foo

Outer.methodname:
- should bar

Ideally I should be able to add 'level' data so we can recursively indent or whatever. Hard to say if that'll be difficult or not, the actual printing is done somewhat naively (just iterates over tests and only prints out the "headers" when the test context changes.)


TODO:

  • Rejigger shit so the superclass is named Spec (right now the plugin itself is named Spec, hopefully that's not a hard requirement of how plugin registration works.)
  • Clean up the code a bit, I think I have 2-3x functions floating around for testing/discovering/etc "attributes which are public classes".
  • Explore truly recursive/arbitrary nesting vs "Object.method".

bitprophet added a commit that referenced this issue Dec 17, 2011
bitprophet added a commit that referenced this issue Dec 19, 2011
bitprophet added a commit that referenced this issue Dec 19, 2011
@bitprophet
Copy link
Owner Author

@garybernhardt If you want to play around with it, this works in base cases now:

from spec import Spec
from nose import SkipTest


class outercontext(Spec):
    def top_level_test(self):
        pass

    def other_top_level_test(self):
        raise SkipTest

    class innercontext:
        def inner_test(self):
            raise SkipTest

        class deepcontext:
            def yet_another_test(self):
                ohnoes

Output looks like this.

(Heh, just noticed that nose or the older behavior of the spec plugin is stripping the trailing "test" from the output. meh!)


Only just got this done this morning so I haven't forged ahead to see how it falls down in real usage, but probably will soon.

bitprophet added a commit that referenced this issue Mar 3, 2012
@bitprophet
Copy link
Owner Author

I've been using this for a few other projects and it works fine at least in the "one level down" case -- haven't found a need to go deeper, partly because I'm still using it to model class methods or other concepts that apply directly to the class under test.

Gonna close for now, if somebody needs Nth level nesting and it doesn't work, we can make a new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants