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

Pass scenario to hook as a second argument #143

Closed
wants to merge 5 commits into from

Conversation

vectart
Copy link
Contributor

@vectart vectart commented Nov 29, 2013

Allows to determine scenario name and so make hooks more useful and
scenario-oriented.

Allows to determine scenario name and so make hooks more useful and
scenario-oriented.
@vectart
Copy link
Contributor Author

vectart commented Nov 29, 2013

For example, I want to create a hook that will save a screenshot after each scenario.
But the problem is that only world variable is passed to hook function and I'm not able to determine details of that scenario.

@jbpros
Copy link
Member

jbpros commented Nov 29, 2013

Thanks @vectart. I think we should follow the conventions and keep our callback as the last argument. I know this will not be backward-compatible but we're still on 0.x, right?

@vectart
Copy link
Contributor Author

vectart commented Nov 29, 2013

@jbpros You're absolutely right—backward compatibly was a reason why I kept that order.
I committed an update.

@jbpros
Copy link
Member

jbpros commented Nov 29, 2013

Thank you. I'll merge it and bump the version to 0.4 in the next release.

I was thinking maybe we could inspect the hook function signature and conditionally pass the scenario object to it. The only issue with that people won't be able to do arguments-based magic in the hook function but I don't really see why one would do that.

this.Before(function () {
  arguments[1](); // <- calls back
});

Or they still can if we inspect the function and pass the scenario objects in all cases except when exactly one parameter is exposed on its signature:

if (code.length === 1) 
  code.call(world, callback);
else
  code.call(world, scenario, callback);

It seems pretty good to me and would stay backward-compatible for most (all?) people. WDYT?

@vectart
Copy link
Contributor Author

vectart commented Nov 29, 2013

@jbpros I like your solution and committed the improvement.

@jbpros jbpros closed this in ae4a29a Nov 29, 2013
ldegen pushed a commit to ldegen/cucumber-js that referenced this pull request Jan 13, 2014
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants