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

Move the step evaluation into a hook #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vodik
Copy link

@vodik vodik commented Jul 18, 2017

Putting this up to get some feedback and see how receptive people are of the idea before I spend much more time on polishing this.

I'm currently using this library to try and bring this style of testing to our regression, but lots of our code is heavily built around asyncio.

I want to have control over how steps are evaluated, so I can automatically jump in when there's a coroutine and evaluate the step automatically in the current event loop. I'm currently using something like this to demo pytest-bdd internally:

@pytest.hookimpl
def pytest_bdd_call_step(request, feature, scenario, step, step_func, step_func_args):
    if not inspect.iscoroutinefunction(step_func):
        return

    loop = asyncio.get_event_loop()
    loop.run_until_complete(step_func(**step_func_args))
    return True

I don't expect this merged as is, as its is still missing unit tests, documentation, and some quality of life stuff to suppress pluggy from the traceback should a step fail.

Also not sure if you guys want to keep the before and after call hooks too, then, since technically they're now equivalent to having a hook wrapper around this new hook.


This change is Reviewable

By moving it into a hook, it mirrors pytest's pytest_pyfunc_call,
allowing a user to control how a step ends up being evaluated.
@anxuae
Copy link

anxuae commented Dec 18, 2017

Is it possible to integrate this feature in the next release?

@ggens
Copy link

ggens commented Dec 21, 2017

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@ggens
Copy link

ggens commented Dec 21, 2017

work very well for me . good job.


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

@vodik
Copy link
Author

vodik commented Dec 25, 2017

Okay, if that's the case, do you guys want to merge it in its current state? Is there documentation you might want me to update? Should I mark the before/after hooks as deprecated?

@vodik
Copy link
Author

vodik commented Apr 4, 2018

Any chance we can get something like this in for the next release?

@Victor-Savu
Copy link

I would also be interested in finding out the status for this PR. @bubenkoff and @olegpidsadnyi in your opinion, what would be the remaining steps to finish this pull request?

Copy link
Member

@bubenkoff bubenkoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document this new hook in the readme

@pjz
Copy link

pjz commented Aug 9, 2019

Any idea when/if this will get merged?

@pjz
Copy link

pjz commented Aug 13, 2019

FWIW, I rebased this on master and used it with some asyncio steps. It seemed to break on example tables, however, making all calls with the literal <fieldname> value instead of one of the values in the column.

@jruizaranguren
Copy link

@bubenkoff, @youtux, @olegpidsadnyi, It seems that the only thing to merge this improvement is just to document the hook. Would it be merged if I complete that documentation?

Copy link
Contributor

@youtux youtux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we would probably merge this once there are tests and documentation added

Comment on lines +75 to +76
step_func(**step_func_args)
return True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably return the result of the step_func

Suggested change
step_func(**step_func_args)
return True
return step_func(**step_func_args)

@@ -70,6 +70,12 @@ def pytest_bdd_before_step(request, feature, scenario, step, step_func):
reporting.before_step(request, feature, scenario, step, step_func)


@pytest.mark.trylast
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this is a trylast rather than a tryfirst

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.

8 participants