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

Ensure that errors happening after the test suite is done still get thown #2096

Closed
wants to merge 1 commit into from

Conversation

glenjamin
Copy link
Contributor

Most notably this happens if there are problems in a reporter's summary output, such as that described in #2089

I'd like to include a test for this - but I'm not sure where best to put it. Any thoughts?

Here's a sample which will reproduce the error even after #2094 is fixed:

var assert = require('assert');

describe('x', function() {
  it('should', function() {
    var d = new Date();
    // Force an error during rendering
    d.toISOString = function() { throw new Error("Can't display"); };
    assert.deepEqual([d], ['a']);
  });
});

@danielstjules
Copy link
Contributor

I'd like to include a test for this - but I'm not sure where best to put it. Any thoughts?

If it occurs in all reporters, you could add a fixture and integration spec that tested for the presence of the error.

Most notably this happens if there are problems in a reporter's
summary output
@glenjamin glenjamin force-pushed the errors-outside-tests branch from f3ad859 to c011d85 Compare February 20, 2016 12:37
@glenjamin
Copy link
Contributor Author

I've added a test into the regression section, as that seemed to be where it fit best.

I was briefly going to add tests into runner.js, as that's where the fix is - but couldn't find a good way to tie it in. I did start adding some useful tests before giving up on that approach - I've left those in this PR as I think they don't hurt, but I'm happy to remove them if desired.

@hollomancer hollomancer added the status: needs review a maintainer should (re-)review this pull request label Aug 28, 2016
@boneskull
Copy link
Contributor

this was fixed in v5.0.2

@boneskull boneskull closed this Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review a maintainer should (re-)review this pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants