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

scenario result not passed to before/after hooks #891

Closed
charlierudolph opened this issue Aug 9, 2017 · 19 comments · Fixed by #919
Closed

scenario result not passed to before/after hooks #891

charlierudolph opened this issue Aug 9, 2017 · 19 comments · Fixed by #919
Assignees
Labels
🐛 bug Defect / Bug

Comments

@charlierudolph
Copy link
Member

No description provided.

@charlierudolph charlierudolph self-assigned this Aug 9, 2017
@jbpros jbpros added the 🐛 bug Defect / Bug label Aug 9, 2017
@jbpros
Copy link
Member

jbpros commented Aug 9, 2017

Got bitten by this. Our upgrade to 3 on cucumber pro is on hold because of this.

Is

scenarioResult: this.scenarioResult,
the culprit?

@charlierudolph
Copy link
Member Author

charlierudolph commented Aug 9, 2017

What part of this is needed exactly? Do you use just the status predicates? That appears to be the only thing used in the docs surrounding this

@jbpros
Copy link
Member

jbpros commented Aug 9, 2017

We currently use scenarioResult.status, scenarioResult.scenario.uri and scenarioResult.scenario.line.

@charlierudolph
Copy link
Member Author

@jbpros does your use case follow the examples we currently have surrounding this (saving a screenshot when a scenario fails)? I always thought it was really weird that the scenario was passed into hooks and would like to find a different way of handling this.

Currently, I'm thinking we add a new support code function:onFailedTestStep which lets you set a function to be called with a test step fails. It would be called with the same world argument as the failed step and thus you could attach an image or something like that. We could create a dedicated module for parsing attachments out of event protocol formatter and copying them into a folder.

@yaronassa
Copy link

As a tinker-user, I would want any such function / handler / object to have as much context as possible. Even without something grand as the entire past-and-future-run-tree (#875), it would be extremely useful to know what's the current step and scenario for each failure.
For example, this allows me to control rollback procedures according to tags, or simply seeing what other steps were executed before the failed one.

I would urge you to consider passing these to onFailedTestStep, if that's the solution you'd go after.

@aslakhellesoy
Copy link
Contributor

The Hook Docs need to be updated as well - the first paragraph's link to ScenarioResult is a 404.

@aslakhellesoy
Copy link
Contributor

Rather than onFailedTestStep, why not define a hook called AfterStep? Cucumber-Ruby has had this for a long time and as you know - consistency is important.

@charlierudolph
Copy link
Member Author

@aslakhellesoy I want to focus on the use case and be sure we are solving it the right way.

Consistency is important but that doesn't mean we should copy everything just because it exists elsewhere. We should be evolving as we determine better ways to handle things

Can you or @mattwynne or someone from the ruby team shed some light on what AfterStep was built for and what it is used for?

@redi-wadash
Copy link

I'm not from the ruby team so I don't know what AfterStep was built for, but I personally use AfterStep to do various operations. Here are some examples:

  1. Take a screenshot and output browser log on failed scenarios
  2. Do additional logging for some project regardless of if it failed or not
  3. Clean up

For us, we will need the scenario and the result to be passed into AfterStep anyway to do additional logging. I am sure there are other use cases that need them in AfterStep, too, that we haven't even considered.

This means if there were something like onFailedTestStep, we will have more than one way to achieve the same thing: one to use AfterStep and the other to use onFailedTestStep.

This can cause inconsistency within projects too depending on who implemented the hook; some may use AfterStep while others may use onFailedTestStep. I don't think either of those are desirable.

@charlierudolph

I want to understand where you are coming from. Would you mind elaborating why you think the following?

I always thought it was really weird that the scenario was passed into hooks

@charlierudolph
Copy link
Member Author

charlierudolph commented Aug 16, 2017

I never really liked the mixing of the two paradigms. Before / After hooks are related to setup / teardown with actually running the tests. Using it for also reporting seems like something that should be taken care of separately

Take a screenshot and output browser log on failed scenarios

If you are doing this in AfterStep this is being done for failed steps, not failed scenarios. This is what all the examples in the repo are using the scenario object for and thats why I want to make a new hook for this specific use case. I haven't heard any other use compelling use case.

Do additional logging for some project regardless of if it failed or not

What type of additional logging do you do?

Clean up

What is there to clean up after a step?

For us, we will need the scenario and the result to be passed into AfterStep anyway to do additional logging. I am sure there are other use cases that need them in AfterStep, too, that we haven't even considered.

I am trying to collect those use cases. Sometimes people may be using it when there are other ways to do it or as a workaround for something that we could build in better support for.

This means if there were something like onFailedTestStep, we will have more than one way to achieve the same thing: one to use AfterStep and the other to use onFailedTestStep.

Cucumber-js currently has no AfterStep and I don't plan on implementing both.

@darrinholst
Copy link
Contributor

I'm in the process of trying to get cucumber 3 support in protractor-cucumber-framework and I could really use an AfterStep. I realize that a library that glues two other libraries together is not the most common of use cases, but it would make it a lot simpler. I'm trying to figure out how to do it with the new event-protocol stuff, but I'm getting no feature/scenario/step context in any of the *-finished events. We use the names, uris, and line numbers from the feature, scenario and steps to report back to protractor and give IDE integrations (Intellij) something to show and navigate to in their tree. My $0.02 anyway.

@yaronassa
Copy link

yaronassa commented Aug 17, 2017

Just to mention that all the discussed functionality (and much more) can be accomplished with setDefinitionFunctionWrapper.
You can wrap all steps with pre/post actions, that not only allow you to react to a step's inputs and outputs, but even mutate them.

So, for example, I use it to implement a "The next step should fail with error XYZ" step, which allows me to quickly implement deep negative checks and error simulations without implementing duplicate negative steps or special flippable error handling in each and every step.

I think there's nothing an AfterStep hook can do that setDefinitionFunctionWrapper can't.

@paul-phillips-ark
Copy link

Any updates on this? Trying to upgrade from 2.0.0 to 3.0.0 at work and still getting undefined for scenarioResult

@aslakhellesoy
Copy link
Contributor

@paul-phillips-ark yes, see #905. If you want to help move it forward you can branch off and submit a new pr that addresses @charlierudolph's comments.

@paul-phillips-ark
Copy link

When I console.log(scenarioResult) I'm still getting undefined in both Before and After functions? Am I missing something? Apologies if above comment is dense sounding.

@likerRr
Copy link

likerRr commented Aug 25, 2017

My use cases currently are following:

On before each scenario I request fixtures which fill my database for testing. Every feature has it's own set of fixtures needed for it. Fixture's path on server repeats feature's path, so I need uri option too:

Before({ timeout: 60 * 1000 }, scenarioResult => {
    ...
    const file = scenarioResult.scenario.feature.uri;
    ...
});

On After hook as many of us I make a screenshot:

After(scenarioResult => {
    if (scenarioResult.status === 'failed') {
      driverUtils.takeScreenShot(scenarioResult.scenario.name);
    }
});

@mobidev111
Copy link

@charlierudolph could you cut a release with pull request #905 as interim solution? it allows us to finish the transition from v2 to v3 - which is currently stuck due to this regression/change. your proposed refactoring could go into another pull request.

@charlierudolph
Copy link
Member Author

There are some issues for #905 but I will try to get a fix in and do a release this weekend.

@lock
Copy link

lock bot commented Oct 24, 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 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛 bug Defect / Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants