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 hook and Before hooks are not running if a failure occrurs #824

Open
3 tasks done
dmbartle opened this issue Sep 9, 2022 · 14 comments
Open
3 tasks done

After hook and Before hooks are not running if a failure occrurs #824

dmbartle opened this issue Sep 9, 2022 · 14 comments
Labels

Comments

@dmbartle
Copy link

dmbartle commented Sep 9, 2022

Current behavior

The Before and After hooks will only run when tests pass.

Desired behavior

We want to be able to use the cucumber style After hook to run some test cleanup, even when tests fail. But currently the After hook will only run when the test passes.

Using the cypress afterEach hook is an option but it would be nice to use the cucumber After hook as it supports @tags

Test code to reproduce

Here is a link to an older test feature file from the cypress repo which illustrates the issue. The scenarios that illustrate this problem are commented out as they cause steps to fail, (Scenario: With Untagged After having test errors in steps part 1 & 2). The steps file only needed a minor tweak to pull { Given, Then, After, Before } from the right place.

https://github.com/badeball/cypress-cucumber-preprocessor/blob/77dbacb65741d559ae0eedd461f02970249f75d8/cypress/integration/BeforeAndAfterSteps.feature

Versions

  • Cypress version: cypress@10.7.0
  • Preprocessor version: @badeball/cypress-cucumber-preprocessor@12.1.0
  • ** esbuild version**: @bahmutov/cypress-esbuild-preprocessor@2.1.3
  • Node version: v16.14.0

Checklist

  • I've read the FAQ.
  • I've read Instructions for logging issues.
  • I'm not using cypress-cucumber-preprocessor@4.3.1 (package name has changed and it is no longer the most recent version, see #689).
@dmbartle
Copy link
Author

dmbartle commented Sep 9, 2022

Related issue:

#538

@badeball
Copy link
Owner

badeball commented Sep 9, 2022

The Before and After hooks will only run when tests pass.

This is by design and matches behavior found in Cucumber.

If you want afterEach behavior then use afterEach. You can use doesFeatureMatch(..) for conditions, as seen here.

@badeball badeball closed this as completed Sep 9, 2022
@dmbartle
Copy link
Author

dmbartle commented Sep 9, 2022

Okay, thanks for the quick reply.

@hammzj
Copy link

hammzj commented Mar 29, 2023

Hello, I believe we need to reopen this. The behavior of an After hook is to run post-scenario, regardless of whether the scenario passed or failed.

See here:

After hooks run after the last step of each scenario, even when the step result is failed, undefined, pending, or skipped.

While that above is for Cucumber-Java, the same behavior works for Cucumber-JS where a tagged hook will still run post-scenario. I can reproduce this behavior if needed.

@badeball
Copy link
Owner

Re-opening, as this does in fact seem to the behavior exhibited by Cucumber, despite what I was thinking earlier.

This is likely a pretty substantial change and I don't have the ability to look into it right now. Anyone feeling particularly compelled can take a shot at it meanwhile.

@badeball badeball reopened this Mar 29, 2023
badeball added a commit that referenced this issue May 1, 2023
Messages are no longer lingering in the runtime, but passed along to the
backend using tasks immediately. This is a prerequisite for the
following issues:

 - 810 [1]
 - 824 [2]
 - 944 [3]

This is not to say that these will be fixed shortly or even at all, but
at least now it will be remotely possible.

Furthermore, this removes any doubt regarding whether reports work in
interactive mode. The experimental flag simply turns out to be too
broken [4-5]. Interactive mode is now detected and reports turned off.

Lastly, detecting interactive mode is a somewhat broken feature [6]. The
workaround (using `isTextTerminal`) is only available in Cypress v10 and
beyond. Thus is Cypress v9 now unsupported.

[1] #810
[2] #824
[3] #944
[4] cypress-io/cypress#18955
[5] cypress-io/cypress#26634
[6] cypress-io/cypress#20789
badeball added a commit that referenced this issue May 1, 2023
Messages are no longer lingering in the runtime, but passed along to the
backend using tasks immediately. This is a prerequisite for the
following issues:

 - 810 [1]
 - 824 [2]
 - 944 [3]

This is not to say that these will be fixed shortly or even at all, but
at least now it will be remotely possible.

Furthermore, this removes any doubt regarding whether reports work in
interactive mode. The experimental flag simply turns out to be too
broken [4-5]. Interactive mode is now detected and reports turned off.

Lastly, detecting interactive mode is a somewhat broken feature [6]. The
workaround (using `isTextTerminal`) is only available in Cypress v10 and
beyond. Thus is Cypress v9 now unsupported.

[1] #810
[2] #824
[3] #944
[4] cypress-io/cypress#18955
[5] cypress-io/cypress#26634
[6] cypress-io/cypress#20789
badeball added a commit that referenced this issue May 1, 2023
Messages are no longer lingering in the runtime, but passed along to the
backend using tasks immediately. This is a prerequisite for the
following issues:

 - 810 [1]
 - 824 [2]
 - 944 [3]

This is not to say that these will be fixed shortly or even at all, but
at least now it will be remotely possible.

Furthermore, this removes any doubt regarding whether reports work in
interactive mode. The experimental flag simply turns out to be too
broken [4-5]. Interactive mode is now detected and reports turned off.

Lastly, detecting interactive mode is a somewhat broken feature [6]. The
workaround (using `isTextTerminal`) is only available in Cypress v10 and
beyond. Thus is Cypress v9 now unsupported.

[1] #810
[2] #824
[3] #944
[4] cypress-io/cypress#18955
[5] cypress-io/cypress#26634
[6] cypress-io/cypress#20789
badeball added a commit that referenced this issue May 1, 2023
Messages are no longer lingering in the runtime, but passed along to the
backend using tasks immediately. This is a prerequisite for the
following issues:

 - 810 [1]
 - 824 [2]
 - 944 [3]

This is not to say that these will be fixed shortly or even at all, but
at least now it will be remotely possible.

Furthermore, this removes any doubt regarding whether reports work in
interactive mode. The experimental flag simply turns out to be too
broken [4-5]. Interactive mode is now detected and reports turned off.

Lastly, detecting interactive mode is a somewhat broken feature [6]. The
workaround (using `isTextTerminal`) is only available in Cypress v10 and
beyond. Thus is Cypress v9 now unsupported.

[1] #810
[2] #824
[3] #944
[4] cypress-io/cypress#18955
[5] cypress-io/cypress#26634
[6] cypress-io/cypress#20789
@badeball badeball added the bug label May 24, 2023
@badeball
Copy link
Owner

As per now, the plugin will register a single afterEach hook. If a step fails, this hook will ensure that appropriate messages are sent to the backend (example).

In order to ensure that After hooks are run despite step failure, they would have to be part of an afterEach block. This is just due to how Cypress works - there isn't any try-catch here.

However, errors ocurring in afterEach blocks are "uncatchable", as in they will cause the spec to finish early. In other words, you only have one opportunity to handle failures in Cypress and this is used by the plugin to create appropriate messages / reports.

Thus, if After hooks were to run as part of the internal afterEach, then failures in such hooks could not be handled properly and failures here would finish the spec early.

For this reason, I believe it is unlikely that this issue will be fixed.

@hammzj
Copy link

hammzj commented Jul 26, 2023

Hey, so I am coming back to this as I had a few questions.

For Cucumber, the After hook is necessarily the same as a Mocha afterEach hook since they both run immediately after a test. Also, a Cucumber After hook can be skipped or included based on tag patterns, unlike an afterEach hook.

For a failure to occur, I believe this means within the test steps and not within the hooks. A failure in the hooks is understandable, but it does not make sense that an After hook is skipped based on a test failing, when an afterEach hook is still run.

Instead, the function within an After hook could be included into a wrapped function body within that single afterEach hook, where the first function will still send Cucumber messages, and all other hook functions can be run within that hook body.

Skipping an After hook because a test step fails doesn't feel correct when we separate the execution hooks from the execution of the test body.

I'd like to look into this if you can point me in the right direction -- I believe something can be done.

@badeball
Copy link
Owner

The following tests, test output in case of failures in After hooks:

This is the behavior I believe you will have some trouble retaining when moving After hook execution inside the internal afterEach hook. I also believe this would be much more probable had one had test:after:run hooks.

In any case, the code you want to look into resides in

The former is run in the browser and it's there you will find the internal afterEach hook. The latter is run within setupNodeEvents() { .. } through addCucumberPreprocessorPlugin().

@badeball
Copy link
Owner

For Cucumber, the After hook is necessarily the same as a Mocha afterEach hook since they both run immediately after a test. Also, a Cucumber After hook can be skipped or included based on tag patterns, unlike an afterEach hook.

FYI, you can use doesFeatureMatch(..) in afterEach hooks to run code conditionally based on tags.

@hammzj
Copy link

hammzj commented Jul 26, 2023

Could we register the function of an After hook inside of the internal afterEach? I've done function-wrapping before, and it works in WebDriverIO for the use-case I have. The separation of node environment and browser environment makes Cypress so tricky to use, but I think I have an idea if this is possible.

Edit: I reread that and then looked at the code -- I see what you mean. Why are Cucumber hooks ran separately from the Mocha hooks? If After hook could essentially be recreated as a Mocha hook like such:

//This is essentially the same
After({tags: "@tag1"}, function() {
 console.log('Hook executed');
})


//as this
afterEach(function(){
  if(doesFeatureMatch("@tag1")){
    console.log('Hook executed');
  }
});

would it make sense that all Cucumber hooks are instead just a proxy for Mocha hooks?

Like, creating an After hook be written as

function After(tags, fn) {
  return afterEach(function()) {
    if(featureDoesMatch(tags){
      fn()
    }
  }
}

@badeball
Copy link
Owner

The problem with proxying After to afterEach, is that failures in such hooks would cause the rest of the spec (test file) to be skipped, including other After hooks. In other words, you still wouldn't be able to provide the guarantee that cucumber-js offers, where all After hooks are tried, despite their status.

@hammzj
Copy link

hammzj commented Jul 27, 2023

Darn, that's tough. Would the same true if an afterEach hook fails when it uses doesFeatureMatch?

@badeball
Copy link
Owner

That would skip the rest of the spec (test file), yes. This is simply just how Cypress works, I have no control nor influence over this.

@hammzj
Copy link

hammzj commented Jul 27, 2023

I understand -- just needed clarity. Thanks for updating; I think we should leave this issue open in the case that Cypress itself is updated to allow for better try/catch logic around hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants