nestorsalceda / mamba

The definitive testing tool for Python. Born under the banner of Behavior Driven Development (BDD).
http://nestorsalceda.github.io/mamba
MIT License
518 stars 65 forks source link

after.each only executes if examples pass #96

Closed jes5e closed 6 years ago

jes5e commented 6 years ago

As a heavy ruby/rspec user, I'm really happy that mamba exists. Having a couple of issues though, and this is the biggest for me:

When a test fails, the code in after.each never executes. This seems like a bug to me. In my test suite I need to do things after every test (verify doubles, roll back session, etc.), but those things only happen when a test passes. Am I missing something here?

Simple example:

from expects import *
from mamba import description, before, after, it

with description("after each test"):
    with after.each:
        print("this should print twice but only prints once")

    with it("should fail"):
        expect(True).to(equal(False))

    with it("should pass"):
        pass
jes5e commented 6 years ago

Seems like the issue is possibly the if stmt on line 30 in example.py:

if self.error is None:
    self.parent.execute_hook('after_each', execution_context)

should maybe by just:

self.parent.execute_hook('after_each', execution_context)

without the if?

jes5e commented 6 years ago

Just made a pull request to fix this

angelsanzn commented 6 years ago

Essentially a dupe of #71 but thanks for the contribution!. I believe this is a behaviour change which would need discussion - it doesn't really feel like a bug because the if you mention is very explicit about its intent.

angelsanzn commented 6 years ago

On a side note, how come you consider your test "passed" before verifying doubles?

jes5e commented 6 years ago

Thanks for the reply! Sorry, didn't see #71.

To answer your question, I use a doubles library that allows code like:

my_instance = MyClass()
doubles.expect(my_instance).my_method.and_return('mock results of my method')
my_instance.my_method()
doubles.verify()

Which requires either having to call verify() in every single test, or having an after.each to put it in. Actually, it would be super helpful if there was a way to define after.each outside of specific examples so you don't have to put your teardown() call in every example of your test suite (but I saw there's already an open issue for this).

If you don't mind, could you explain how it's desired behavior to have code in after.each not execute if the example fails? In my case, we're using sqlalchemy and using transactions for test isolation. This is pretty common practice I think and really requires the behavior of after.each always executing. If you're curious, we're doing something along the lines of:

http://docs.sqlalchemy.org/en/latest/orm/session_transaction.html#joining-a-session-into-an-external-transaction-such-as-for-test-suites

You could of course do something like delete all of the data out of your database in a before.each, but from experience I can say that this approach can be considerably slower than using transactions in even a modest size application with a modest sized test suite.

nestorsalceda commented 6 years ago

Yeah!

Thanks for the PR. As I said in #71 let me do some research about this stuff with rspec and others.

I do not use mock + verify behaviour, and this error wasn't too painful for me. But I hope it will be resolved today.

Thanks for the PR @jes5e and thanks @angelsanz for caring :)

jes5e commented 6 years ago

Awesome, thanks!