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

🚀 Feature: Allow hooks to easily set "allowUncaught" flag #1985

Open
indexzero opened this issue Nov 27, 2015 · 13 comments
Open

🚀 Feature: Allow hooks to easily set "allowUncaught" flag #1985

indexzero opened this issue Nov 27, 2015 · 13 comments
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal

Comments

@indexzero
Copy link

Both suite.allowUncaught and test.allowUncaught are completely ignored by the mocha.Runner instance being used at any time. (See code: https://github.com/mochajs/mocha/blob/master/lib/runner.js#L413-L416).

  if (this.allowUncaught) {
    test.allowUncaught = true;
    return test.run(fn);
  }

Which really should be something like:

  if (this.allowUncaught || test.parent.allowUncaught || test.allowUncaught) {
    test.allowUncaught = true;
    return test.run(fn);
  }

Now I'm not really sure the best way to traverse up the tree of suites to find all of the parents so the test.parent.allowUncaught is pretty naive, but the second test.allowUncaught check allows for a (somewhat) elegant work-around:

describe('Anything', function () {
  this.on('test', function (test) {
    test.allowUncaught = true;
  });
});

The super-hack work-around until I have the time to make a pull-request for this is:

//
// This is an awful and fragile hack that
// needs to be changed ASAP.
//
var _runTest = mocha.Runner.prototype.runTest;
mocha.Runner.prototype.runTest = function () {
  this.allowUncaught = true;
  _runTest.apply(this, arguments);
};

describe('Anything', function () {

  after(function () {
    mocha.Runner.prototype.runTest = _runTest;
  });
});

Worth noting that the --allowUncaught CLI option also appears to be completely broken right now. My use-case is slightly different, however, since I only want to set allowUncaught to true for some test suites (not all).

indexzero added a commit to winstonjs/winston that referenced this issue Nov 27, 2015
…ha#1985). Move some tests into `test/logger-legacy.test.js`
@kevinoid
Copy link
Contributor

Per-test or per-suite support for .allowUncaught would be great for my uses as well.

@TimBeyer
Copy link

I could also really use this right now.
I need to assert that my server does the correct shutdown procedure and logs the errors when encountering an uncaught exception.

@mlucool
Copy link
Contributor

mlucool commented Mar 21, 2016

👍

1 similar comment
@ghost
Copy link

ghost commented May 22, 2016

👍

@spalger
Copy link

spalger commented May 30, 2016

I'm currently setting this on the runner and it works great:

mocha.setup(...)
requireAllZeTests()
const runner = mocha.run()
runner.allowUncaught = true

@doublerebel
Copy link

Related: #1801

@boneskull
Copy link
Contributor

@spalger has a workaround, and here's another:

beforeEach(function () {
  this.runnable().allowUncaught = true;
});

it('should be uncaught', function () {
  throw new Error();
});

It's not possible to have allowUncaught be true on a per-test-case basis, because we need to know whether or not to allow the uncaught exception before we run the test. So it has to be in a beforeEach() (this.runnable() when run in a before() is undefined).

@boneskull boneskull added type: bug a defect, confirmed by a maintainer status: waiting for author waiting on response from OP - more information needed confirmed labels Sep 17, 2016
@boneskull
Copy link
Contributor

I'm calling this a "bug" insofar as it's a "missing feature".

However, I don't have enough understanding of the use case(s) for allowUncaught, and why it's intended to only be run in the browser. How does it help debugging in Chrome, specifically?

@qrohlf
Copy link

qrohlf commented Sep 30, 2016

@boneskull I use 'stop on uncaught errors' to automatically stop execution so I can debug a failing test where the exception occurs, with all local variables intact. It's quite helpful.

@boneskull
Copy link
Contributor

@qrohlf What I was trying to get to was "why is this only a browser thing"?

@qrohlf
Copy link

qrohlf commented Oct 3, 2016

@boneskull woops, missed that dimension to your question. I agree that allowUncaught should be usable in CLI/node environments.

@boneskull
Copy link
Contributor

That would seem reasonable 😄, but there's some reason the intent of the feature was limited to browsers. Basically someone needs to get to the bottom of it by inspecting issues and/or changeset history.

@boneskull boneskull removed the status: waiting for author waiting on response from OP - more information needed label Oct 3, 2016
indexzero added a commit to winstonjs/winston that referenced this issue Nov 2, 2016
…ha#1985). Move some tests into `test/logger-legacy.test.js`
indexzero added a commit to winstonjs/winston that referenced this issue Mar 13, 2017
…ha#1985). Move some tests into `test/logger-legacy.test.js`
indexzero added a commit to winstonjs/winston that referenced this issue Mar 13, 2017
…ha#1985). Move some tests into `test/logger-legacy.test.js`
indexzero added a commit to winstonjs/winston that referenced this issue Sep 26, 2017
…ha#1985). Move some tests into `test/logger-legacy.test.js`
indexzero added a commit to winstonjs/winston that referenced this issue Sep 27, 2017
…ha#1985). Move some tests into `test/logger-legacy.test.js`
indexzero added a commit to winstonjs/winston that referenced this issue Oct 2, 2017
…ha#1985). Move some tests into `test/logger-legacy.test.js`
@boneskull boneskull added type: feature enhancement proposal status: accepting prs Mocha can use your help with this one! labels Jan 17, 2018
@boneskull boneskull changed the title Cannot set allowUncaught anywhere. allow hooks to easily set "allowUncaught" flag Jan 17, 2018
@boneskull boneskull removed the type: bug a defect, confirmed by a maintainer label Jan 17, 2018
@boneskull
Copy link
Contributor

--allow-uncaught in Node.js was added by #2793; this is now a feature request to make it easier to set the flag.

I believe it is possible to set this flag from within a test, but not without some code changes.
Right now, it's only possible to use in beforeEach.

Basically the result should be:

it('should set allowUncaught', function () {
  this.allowUncaught();
  throw new Error('boom crash exit process');
});

@JoshuaKGoldberg JoshuaKGoldberg changed the title allow hooks to easily set "allowUncaught" flag 🚀 Feature: Allow hooks to easily set "allowUncaught" flag Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests

8 participants