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

after.each only executes if examples pass #96

Closed
jes5e opened this issue Jan 5, 2018 · 7 comments
Closed

after.each only executes if examples pass #96

jes5e opened this issue Jan 5, 2018 · 7 comments

Comments

@jes5e
Copy link
Contributor

jes5e commented Jan 5, 2018

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
Copy link
Contributor Author

jes5e commented Jan 5, 2018

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 added a commit to jes5e/mamba that referenced this issue Jan 5, 2018
…ass. There was an if statement that checked for errors and only ran code in after.each if there were none. Removed this if check.
jes5e added a commit to jes5e/mamba that referenced this issue Jan 5, 2018
…ass. There was an if statement that checked for errors and only ran code in after.each if there were none. Removed this if check.
@jes5e
Copy link
Contributor Author

jes5e commented Jan 5, 2018

Just made a pull request to fix this

@angelsanzn
Copy link
Contributor

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
Copy link
Contributor

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

@jes5e
Copy link
Contributor Author

jes5e commented Jan 6, 2018

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
Copy link
Owner

nestorsalceda commented Jan 9, 2018

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 :)

nestorsalceda added a commit that referenced this issue Jan 9, 2018
Fixes issue #96 - after.each only executes if examples pass.
@jes5e
Copy link
Contributor Author

jes5e commented Jan 12, 2018

Awesome, thanks!

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

3 participants