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

Handle exceptions in hooks #210

Closed
wants to merge 2 commits into from

Conversation

mupkoo
Copy link
Contributor

@mupkoo mupkoo commented Jul 27, 2018

beforeEach and afterEach hooks are promised based and are called one after the other. If one of them raises an exception, the execution stops at that moment. This causes problems as it will prevent the tearing down of the test.

Related to #209

PS I had to update the test selectors, because I was not able to install them

`beforeEach` and `afterEach` hooks are promised based
and are called one after the other. If one of them raises
an exception, the execution stops at that moment. This
causes problems as it will prevent the tearing down of
the test.
@mupkoo mupkoo force-pushed the handle-exceptions-in-hooks branch from 1a0733a to 907b1d6 Compare July 27, 2018 13:22
return promise
.then(fn.bind(context))
// eslint-disable-next-line no-console
.catch((error) => console.error('Hook throws an error: ', error));
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this will prevent getting a "break on exceptions" style breakpoint in your offending hook...

Can you double check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pause on exception is working even if the exception is caught.

screen shot 2018-07-27 at 22 58 41

screen shot 2018-07-27 at 23 01 56

@rwjblue let me know if you have a better idea, how we can do this.

Copy link
Member

Choose a reason for hiding this comment

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

In the screenshot you are using the checkbox for “pause on caught exceptions”. Maybe that’s just what you always have had to do (due to mochas usage of exceptions for flow control)?

Copy link
Contributor Author

@mupkoo mupkoo Jul 28, 2018

Choose a reason for hiding this comment

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

Sorry, it looks like I misunderstood you.

One thing that we can do is to just make sure that the teardown happens in all cases. Something like

afterEach(function() {
  return chainHooks(afterEachHooks, this)
    .finally(() => teardownContext(this))
    .finally(() => {
      // delete any extraneous properties
      for (let key in this) {
        if (!(key in originalContext)) {
          delete this[key];
        }
      }

      // copy over the original values
      _assign(this, originalContext);
    })
    .catch((error) => { throw error });
});

This way the errors that happen in the hooks will be returned to mocha, but we would have cleared the test context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2018-07-28 at 11 30 31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a fan of the idea?

@mupkoo mupkoo closed this Aug 18, 2020
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.

2 participants