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

Fixes #261: calls before/after hooks for each outline example. #437

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

michelts
Copy link
Contributor

Fixes issue #261.

@michelts michelts changed the title Fixes 261: calls before/after hooks for each outline example. Fixes #261: calls before/after hooks for each outline example. Apr 10, 2014
@danni
Copy link
Collaborator

danni commented Apr 11, 2014

Hi, this is wrong because it breaks the outputters. In the new-parser branch we have before/after.each_example which implements what you want without breaking the API. You could backport that here.

@michelts
Copy link
Contributor Author

Sorry the late response. I didn't find time to dig some more until now. Thanks for help me out @danni.

It is not 100% yet, but I am with some questions and I think @gabrielfalcao and maybe @ajtack can help me with decisions I can't take myself:

Tests count:

The tests counts how many scenarios was executed counting how many times a hook is called. I changed the counts, now that each outline scenario is considered a scenario. Another approach would be just call it for first outline scenario (see my last commits).

Before/after.outline:

I started from #261 and didn't see in details #181 and #131. There is the undocumented after.outline that also fits my case, but @ajtack suggests that before/after.outline gets executed before/after all outline examples get executed.

Without that, it seems before/after.outline just repeat before/after.each_scenario.

I agree with @ajtack, before/after.each_scenario are used to cleanup tests (e.g. django flush db), I don't want to repeat the cleanup in the first example by calling cleanup in before.each_scenario and before.outline.

I can make before/each.scenario encapsulate all examples, can I go further?

@danni
Copy link
Collaborator

danni commented Apr 23, 2014

Hi, it's still wrong, because it still breaks the outputters, you need both before/after scenario and before/after example.

This is because each scenario example calls before.each_scenario,
AFAIK responsible for show the failed message.
@michelts
Copy link
Contributor Author

In fact, the outputters were right. The problem was with a subunit test that checks for undefined steps.

Now that each outline example triggers before/after.each_scenario, it seems the undefined message is shown for each example. I don't think this is an error at all, so I just updated the test to reflect this new condition.

Sorry again to make the pull request without check if tests were passing.

@michelts
Copy link
Contributor Author

Hi @danni,

Can I help with something else? Let me know if I missed anything.

Thanks!

@rtkrruvinskiy
Copy link
Contributor

It would be really great to see this merged!

@throwable-one
Copy link
Contributor

michelts, thank you! I believe @before.each_step should also work with steps in outline. It does not work now (called only for the first step)

@danni
Copy link
Collaborator

danni commented Aug 14, 2014

This is still the wrong approach. It breaks the API. Have a look at how this has been approached in the new-parser branch.

@michelts
Copy link
Contributor Author

Hi @danni, you are right, the output is broken. Sorry I didn't realize that before.

I didn't find time to dig more yet. I'm using another approach as an alternative, in addition to the @before.each_scenario, I also bind the @before.outline in my reset hook:

@before.each_scenario
@before.outline
def reset_data(scenario, *args):
    call_command('flush', interactive=False, verbosity=0)
    call_command('loaddata', 'some_test_data.json', verbosity=0)

The only problem is that the reset is called twice for the first scenario outline, but its ok for me. @rtkrruvinskiy and @throwable-one, is @before.outline an option for you?

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

Successfully merging this pull request may close these issues.

4 participants