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

Unhandled async errors crash Jest. #2059

Closed
cpojer opened this Issue Nov 6, 2016 · 45 comments

Comments

Projects
None yet
@cpojer
Contributor

cpojer commented Nov 6, 2016

On master, I changed one of the tests in jest-haste-map from toBeDefined() or something to not.toBeDefined(). It would then go on to crash Jest:

[cpojer: ~/Projects/jest (master)]$ jest "haste-map.*index"

 RUNS  packages/jest-haste-map/src/__tests__/index-test.js

/Users/cpojer/Projects/jest/packages/jest-jasmine2/node_modules/jest-matchers/build/index.js:110
        throw error;
        ^
Error: expect(received).toBeNull()

Expected value to be null, instead received
  undefined
    at HasteMap.hasteMap.once (/Users/cpojer/Projects/jest/packages/jest-haste-map/src/__tests__/index-test.js:685:62)
    at HasteMap.g (events.js:291:16)
    at emitOne (events.js:96:13)
    at HasteMap.emit (events.js:188:7)
    at Timeout.emitChange (/Users/cpojer/Projects/jest/packages/jest-haste-map/src/index.js:562:14)
    at ontimeout (timers.js:365:14)
    at tryOnTimeout (timers.js:237:5)
    at Timer.listOnTimeout (timers.js:207:5)

@dmitriiabramov what have we done?

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Nov 7, 2016

Contributor

Oh, it might have been because I didn't return the promise but instead am using Jasmine's async work. Nevertheless, we should hook up an unhandled promise handler in Jest.

Contributor

cpojer commented Nov 7, 2016

Oh, it might have been because I didn't return the promise but instead am using Jasmine's async work. Nevertheless, we should hook up an unhandled promise handler in Jest.

@cpojer cpojer changed the title from Thrown promises crash Jest to Unhandled async errors crash Jest. Nov 29, 2016

@mweststrate

This comment has been minimized.

Show comment
Hide comment
@mweststrate

mweststrate Nov 29, 2016

Contributor

Currently we use in our tests a utility to work around this in a quite cumbersome way:

/**
 * In async tests, JEST will die (in watch mode) if an exception is thrown from a callback. This utility will catch
 * the errors instead and report the test as failed in these case *
 *
 * @param {jest.DoneCallback} done
 * @param {T} callback
 * @returns {T}
 */
export function catchErrors<T extends Function>(done: jest.DoneCallback, callback: T): T {
	return function() {
		try {
			callback.apply(this, arguments)
		} catch (e) {
			done.fail(e)
		}
	} as any as T
}

which then can be used as follows:

describe("suite", () => {
	it("should handle async exceptions", (done) => {
                setTimeout(catchErrors(done, (err, res) => {
                        throw "Catched by catchErrors so that our tests properly fail!"
                        done() 
                }), 100)
        })
})

Any cleaner approach to this would be appreciated!

Contributor

mweststrate commented Nov 29, 2016

Currently we use in our tests a utility to work around this in a quite cumbersome way:

/**
 * In async tests, JEST will die (in watch mode) if an exception is thrown from a callback. This utility will catch
 * the errors instead and report the test as failed in these case *
 *
 * @param {jest.DoneCallback} done
 * @param {T} callback
 * @returns {T}
 */
export function catchErrors<T extends Function>(done: jest.DoneCallback, callback: T): T {
	return function() {
		try {
			callback.apply(this, arguments)
		} catch (e) {
			done.fail(e)
		}
	} as any as T
}

which then can be used as follows:

describe("suite", () => {
	it("should handle async exceptions", (done) => {
                setTimeout(catchErrors(done, (err, res) => {
                        throw "Catched by catchErrors so that our tests properly fail!"
                        done() 
                }), 100)
        })
})

Any cleaner approach to this would be appreciated!

@mmcgahan

This comment has been minimized.

Show comment
Hide comment
@mmcgahan

mmcgahan Dec 1, 2016

Contributor

this sounds somewhat similar to #1873, where done() is unreachable after failing expect calls in Promises

Contributor

mmcgahan commented Dec 1, 2016

this sounds somewhat similar to #1873, where done() is unreachable after failing expect calls in Promises

@julien-f

This comment has been minimized.

Show comment
Hide comment
@julien-f

julien-f Dec 19, 2016

Any news on this?

I also have a repro repo here if that can help.

Any news on this?

I also have a repro repo here if that can help.

@lucasconstantino

This comment has been minimized.

Show comment
Hide comment
@lucasconstantino

lucasconstantino Dec 30, 2016

This is some huge drawback for me :( Trying to find a way to circumvent this properly...

This is some huge drawback for me :( Trying to find a way to circumvent this properly...

@theogravity

This comment has been minimized.

Show comment
Hide comment
@theogravity

theogravity Jan 13, 2017

this is quite frustrating as I have no idea where it's dying

this is quite frustrating as I have no idea where it's dying

@capaj

This comment has been minimized.

Show comment
Hide comment
@capaj

capaj Jan 24, 2017

Contributor

@mweststrate this should be in the jest core. Seriously, this has been lying here for two months and no one opened a PR yet? 😖

Contributor

capaj commented Jan 24, 2017

@mweststrate this should be in the jest core. Seriously, this has been lying here for two months and no one opened a PR yet? 😖

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Jan 24, 2017

Contributor

@capaj seems like you are on the hook to make that PR. Jest is an open source project, if you'd like something to be fixed, please start contributing. The attitude you are showing on this issue tracker is not appropriate.

Contributor

cpojer commented Jan 24, 2017

@capaj seems like you are on the hook to make that PR. Jest is an open source project, if you'd like something to be fixed, please start contributing. The attitude you are showing on this issue tracker is not appropriate.

@capaj

This comment has been minimized.

Show comment
Hide comment
@capaj

capaj Jan 24, 2017

Contributor

@cpojer sorry I am just very enthusiastic about my favorite OSS tools. I wasn't aware that displaying negative emotions about negative things is prohibited in jest repo. Next time, I will try to write my comments like a robot, deprived of any emotions.

Contributor

capaj commented Jan 24, 2017

@cpojer sorry I am just very enthusiastic about my favorite OSS tools. I wasn't aware that displaying negative emotions about negative things is prohibited in jest repo. Next time, I will try to write my comments like a robot, deprived of any emotions.

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Jan 24, 2017

Contributor

I'd prefer if you channeled your enthusiasm into code which will help make this project better for everyone or channel it into productive conversation. Negative comments don't help anybody; if that's the only thing you are contributing to this project I'd prefer to see you using a different testing platform.

Contributor

cpojer commented Jan 24, 2017

I'd prefer if you channeled your enthusiasm into code which will help make this project better for everyone or channel it into productive conversation. Negative comments don't help anybody; if that's the only thing you are contributing to this project I'd prefer to see you using a different testing platform.

@capaj

This comment has been minimized.

Show comment
Hide comment
@capaj

capaj Jan 24, 2017

Contributor

@cpojer I beg to disagree, but that's a discussion better suited for some other place/platform

Contributor

capaj commented Jan 24, 2017

@cpojer I beg to disagree, but that's a discussion better suited for some other place/platform

@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Jan 24, 2017

Contributor

That's ok, we can disagree on this but the Jest project doesn't need your negativity and we won't accept this behavior here.

Contributor

cpojer commented Jan 24, 2017

That's ok, we can disagree on this but the Jest project doesn't need your negativity and we won't accept this behavior here.

@jeffbski

This comment has been minimized.

Show comment
Hide comment
@jeffbski

jeffbski Feb 28, 2017

In mocha, the done callback takes an optional error (standard Node.js cb style).

If you call done with a truthy value then it will be used as an error.

promise.then(x => {
  // do something
  done(); // successful
}).catch(err => {
  // do something with error
  done(err); // hand error to test runner
});

In mocha, the done callback takes an optional error (standard Node.js cb style).

If you call done with a truthy value then it will be used as an error.

promise.then(x => {
  // do something
  done(); // successful
}).catch(err => {
  // do something with error
  done(err); // hand error to test runner
});
@IPWright83

This comment has been minimized.

Show comment
Hide comment
@IPWright83

IPWright83 Mar 10, 2017

Having a look through this there isn't a reproduction example, so I thought I'd include one. In here the first test will fail reporting

Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

while the other two tests correctly reporting back the failed expectation. Correctly works in Jasmine.

describe("Error Reporting", function() {

     it("Async", function(done) {
       setTimeout(function() {
          expect(true).toBe(false);
          done();
       }, 100);
    });

    it("Sync", function() {
        expect(true).toBe(false);
    });

    it("Fake Async", function(done) {
        expect(true).toBe(false);
        done();
    });
});

Having a look through this there isn't a reproduction example, so I thought I'd include one. In here the first test will fail reporting

Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.

while the other two tests correctly reporting back the failed expectation. Correctly works in Jasmine.

describe("Error Reporting", function() {

     it("Async", function(done) {
       setTimeout(function() {
          expect(true).toBe(false);
          done();
       }, 100);
    });

    it("Sync", function() {
        expect(true).toBe(false);
    });

    it("Fake Async", function(done) {
        expect(true).toBe(false);
        done();
    });
});
@evansiroky

This comment has been minimized.

Show comment
Hide comment
@evansiroky

evansiroky Mar 10, 2017

I submitted #1897 that aslo included a reproducible example: https://github.com/evansiroky/jest-exec-bug

I submitted #1897 that aslo included a reproducible example: https://github.com/evansiroky/jest-exec-bug

evocateur added a commit to evocateur/lerna that referenced this issue Mar 20, 2017

Re-wrap async done() in try-catch
Due to facebook/jest#2059, turns out we need
those try/catch blocks I tried to remove in the previous PR.

Partially reverts #682

@evocateur evocateur referenced this issue Mar 20, 2017

Merged

Re-wrap async done() in try-catch #695

4 of 9 tasks complete

evocateur added a commit to lerna/lerna that referenced this issue Mar 20, 2017

Re-wrap async done() in try-catch (#695)
Due to facebook/jest#2059, turns out we need
those try/catch blocks I tried to remove in the previous PR.

Partially reverts #682
@wilomgfx

This comment has been minimized.

Show comment
Hide comment
@wilomgfx

wilomgfx Mar 27, 2017

@jeffbski indeed thats the most convenient way of doing this! I thought we would be able
to do the same thing with JEST, but instead we have to do : done.fail(ARG)

@jeffbski indeed thats the most convenient way of doing this! I thought we would be able
to do the same thing with JEST, but instead we have to do : done.fail(ARG)

@jeffbski

This comment has been minimized.

Show comment
Hide comment
@jeffbski

jeffbski Mar 28, 2017

done.fail(arg) would work fine as long as it checks the truthiness of the arg before considering it an error, otherwise it would just complete, but I assume that if done.fail is called it always indicates a failure regardless of arg.

So to work with Node.js callback code we would need to continually create functional wrappers like

// create cb wrapper that checks the error and calls done or done.fail
function cb(err) {
  if (err) { return done.fail(err); }
  done();
}

Could we add another method to jest that does this for us so we're not creating this in every test?

Maybe something like done.cb(err) or done.nodeCb(err) that works in this fashion?

done.fail(arg) would work fine as long as it checks the truthiness of the arg before considering it an error, otherwise it would just complete, but I assume that if done.fail is called it always indicates a failure regardless of arg.

So to work with Node.js callback code we would need to continually create functional wrappers like

// create cb wrapper that checks the error and calls done or done.fail
function cb(err) {
  if (err) { return done.fail(err); }
  done();
}

Could we add another method to jest that does this for us so we're not creating this in every test?

Maybe something like done.cb(err) or done.nodeCb(err) that works in this fashion?

@bf

This comment has been minimized.

Show comment
Hide comment
@bf

bf May 9, 2017

Any chance to fix this bug? Why isn't this basic node functionality supported?

bf commented May 9, 2017

Any chance to fix this bug? Why isn't this basic node functionality supported?

@venantius

This comment has been minimized.

Show comment
Hide comment
@venantius

venantius May 18, 2017

Just another voice chiming in here - this (slash, #1873) is a surprisingly easy issue to run into and caused me to spend quite a bit of time trying to figure out why my tests were reporting a timeout failure rather than the actual root cause of the test failure.

Just another voice chiming in here - this (slash, #1873) is a surprisingly easy issue to run into and caused me to spend quite a bit of time trying to figure out why my tests were reporting a timeout failure rather than the actual root cause of the test failure.

@pvande

This comment has been minimized.

Show comment
Hide comment
@pvande

pvande Jul 4, 2017

At the very least, someone might want to update the documentation.

From https://facebook.github.io/jest/docs/en/asynchronous.html#callbacks:

test('the data is peanut butter', done => {
  function callback(data) {
    expect(data).toBe('peanut butter');
    done();
  }

  fetchData(callback);
});

(If that expectation fails, the test will still fail, but only because the done() call is never reached.)

pvande commented Jul 4, 2017

At the very least, someone might want to update the documentation.

From https://facebook.github.io/jest/docs/en/asynchronous.html#callbacks:

test('the data is peanut butter', done => {
  function callback(data) {
    expect(data).toBe('peanut butter');
    done();
  }

  fetchData(callback);
});

(If that expectation fails, the test will still fail, but only because the done() call is never reached.)

@dignifiedquire dignifiedquire referenced this issue Jul 11, 2017

Merged

[WIP] Aegir@next #134

19 of 25 tasks complete
@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jul 12, 2017

Contributor

I have started working on it and have a proof of concept. Now will need to clean it up and prepare so it can be made into a PR

screen shot 2017-07-12 at 14 21 26

Contributor

dignifiedquire commented Jul 12, 2017

I have started working on it and have a proof of concept. Now will need to clean it up and prepare so it can be made into a PR

screen shot 2017-07-12 at 14 21 26

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Jul 12, 2017

@aaronabramov

This comment has been minimized.

Show comment
Hide comment
@aaronabramov

aaronabramov Jul 12, 2017

Member

i remember running into it when i was updating jest at fb.
https://github.com/facebook/jest/pull/3880/files got me half way through, but there were a few other crashes and timeouts.

jest-circus should solve these issues, but i don't think we'll ship it for at least another month

Member

aaronabramov commented Jul 12, 2017

i remember running into it when i was updating jest at fb.
https://github.com/facebook/jest/pull/3880/files got me half way through, but there were a few other crashes and timeouts.

jest-circus should solve these issues, but i don't think we'll ship it for at least another month

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jul 12, 2017

Contributor

@aaronabramov does circus do proper uncaughtException and unhandledRejection capturing? I couldn't find any references when I looked and if it doesn't check those I am pretty sure there are still a few errors that will escape the test runner

Contributor

dignifiedquire commented Jul 12, 2017

@aaronabramov does circus do proper uncaughtException and unhandledRejection capturing? I couldn't find any references when I looked and if it doesn't check those I am pretty sure there are still a few errors that will escape the test runner

@aaronabramov

This comment has been minimized.

Show comment
Hide comment
@aaronabramov

aaronabramov Jul 12, 2017

Member

@dignifiedquire it doesn't do it yet actually! and i think work that you're doing in https://github.com/facebook/jest/pull/4016/files can be reused in both

Member

aaronabramov commented Jul 12, 2017

@dignifiedquire it doesn't do it yet actually! and i think work that you're doing in https://github.com/facebook/jest/pull/4016/files can be reused in both

@mo

This comment has been minimized.

Show comment
Hide comment
@mo

mo Jul 18, 2017

Contributor

@dignifiedquire thanks for working on this issue

Contributor

mo commented Jul 18, 2017

@dignifiedquire thanks for working on this issue

@ambroselittle

This comment has been minimized.

Show comment
Hide comment
@ambroselittle

ambroselittle Aug 3, 2017

Something of a rabbit hole led me here, so I may be in the wrong place.. can you tell me if this is the same issue I'm seeing?

I'm using the new-ish .rejects and getting similar behavior:

it('rejects when an error is thrown', () => {
  return expect(myPromiseFuncThatThrows()).rejects.toEqual(myExpectedError);
}

That results in the jasmine timeout error. OTOH, if I have myPromiseFuncThatThrows catch and then reject(ex), it works. But I don't want to add special catch-reject handling just for the tests.

Something of a rabbit hole led me here, so I may be in the wrong place.. can you tell me if this is the same issue I'm seeing?

I'm using the new-ish .rejects and getting similar behavior:

it('rejects when an error is thrown', () => {
  return expect(myPromiseFuncThatThrows()).rejects.toEqual(myExpectedError);
}

That results in the jasmine timeout error. OTOH, if I have myPromiseFuncThatThrows catch and then reject(ex), it works. But I don't want to add special catch-reject handling just for the tests.

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Aug 6, 2017

Hi all! Does @dignifiedquire PR fulfill everyone's needs? Would love to see this fix come to reality as soon as possible, let me know if there is any way in which I can help to speed the process.

Thanks!

Hi all! Does @dignifiedquire PR fulfill everyone's needs? Would love to see this fix come to reality as soon as possible, let me know if there is any way in which I can help to speed the process.

Thanks!

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Aug 15, 2017

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Aug 16, 2017

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Aug 16, 2017

Contributor

@aaronabramov the PR should be ready now, it would be great to get some review on it so we can get this shipped :)

Contributor

dignifiedquire commented Aug 16, 2017

@aaronabramov the PR should be ready now, it would be great to get some review on it so we can get this shipped :)

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Aug 22, 2017

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Aug 22, 2017

dignifiedquire added a commit to dignifiedquire/jest that referenced this issue Aug 24, 2017

cpojer added a commit that referenced this issue Aug 24, 2017

Handle async errors (#4016)
* wip: start handling async errors

Ref #2059

* Implement basic cancelation

* Improve cancelation

* lint and flow happy, more tests

* snapshot updates

* happy tests

* add tests for before hooks

* log errors if they escape us

* use p-cancelable

* improve based on CR
@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Aug 24, 2017

Contributor

This is part of Jest 21 thanks to @dignifiedquire. Published jest@test (Jest 21 beta) so you can give this a try. Also enjoy a substantially faster Jest startup.

Contributor

cpojer commented Aug 24, 2017

This is part of Jest 21 thanks to @dignifiedquire. Published jest@test (Jest 21 beta) so you can give this a try. Also enjoy a substantially faster Jest startup.

@cpojer cpojer closed this Aug 24, 2017

midzelis added a commit to midzelis/jest that referenced this issue Aug 30, 2017

Handle async errors (#4016)
* wip: start handling async errors

Ref facebook#2059

* Implement basic cancelation

* Improve cancelation

* lint and flow happy, more tests

* snapshot updates

* happy tests

* add tests for before hooks

* log errors if they escape us

* use p-cancelable

* improve based on CR
@ron-liu

This comment has been minimized.

Show comment
Hide comment
@ron-liu

ron-liu Sep 25, 2017

@dignifiedquire @cpojer I still got timeout for the following test. I am sure I installed version 21.1.0 and I also tried version 21.0.2.
Did I miss anything ?

test.spec.js

it('exception promise', (done) => {
        setTimeout(()=>{
                expect(true).toEqual(false)
                done()
        }, 10)
})

package.json

{
  "name": "testJest",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "jest": "21.1.0"
  },
  "scripts": {
    "test": "jest --debug"
  }
}

image

ron-liu commented Sep 25, 2017

@dignifiedquire @cpojer I still got timeout for the following test. I am sure I installed version 21.1.0 and I also tried version 21.0.2.
Did I miss anything ?

test.spec.js

it('exception promise', (done) => {
        setTimeout(()=>{
                expect(true).toEqual(false)
                done()
        }, 10)
})

package.json

{
  "name": "testJest",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "jest": "21.1.0"
  },
  "scripts": {
    "test": "jest --debug"
  }
}

image

@kimjoar

This comment has been minimized.

Show comment
Hide comment
@kimjoar

kimjoar Sep 26, 2017

Contributor

Looks like ^ fails when using jsdom, but not when using the node env.

Using node:

$ npx jest --env node
 FAIL  test/index.test.js
  ● exception promise

    expect(received).toEqual(expected)
    
    Expected value to equal:
      false
    Received:
      true
      
      at Timeout.setTimeout [as _onTimeout] (test/index.test.js:3:18)
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (18ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.216s, estimated 6s
Ran all test suites.

Using jsdom:

$ npx jest --env jsdom
 FAIL  test/index.test.js (5.114s)
  ● exception promise

    Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
      
      at node_modules/jest-jasmine2/build/queue_runner.js:65:21
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (5003ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        5.733s
Ran all test suites.

Minimal repro: https://github.com/kjbekkelund/jest-timeout-issue-jsdom

Contributor

kimjoar commented Sep 26, 2017

Looks like ^ fails when using jsdom, but not when using the node env.

Using node:

$ npx jest --env node
 FAIL  test/index.test.js
  ● exception promise

    expect(received).toEqual(expected)
    
    Expected value to equal:
      false
    Received:
      true
      
      at Timeout.setTimeout [as _onTimeout] (test/index.test.js:3:18)
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (18ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.216s, estimated 6s
Ran all test suites.

Using jsdom:

$ npx jest --env jsdom
 FAIL  test/index.test.js (5.114s)
  ● exception promise

    Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
      
      at node_modules/jest-jasmine2/build/queue_runner.js:65:21
      at ontimeout (timers.js:469:11)
      at tryOnTimeout (timers.js:304:5)
      at Timer.listOnTimeout (timers.js:264:5)

  ✕ exception promise (5003ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        5.733s
Ran all test suites.

Minimal repro: https://github.com/kjbekkelund/jest-timeout-issue-jsdom

@ron-liu

This comment has been minimized.

Show comment
Hide comment
@ron-liu

ron-liu Sep 26, 2017

@kjbekkelund yeah, thank you. --env node solved the issue, I did miss this.
Alos check the jest official help .

ron-liu commented Sep 26, 2017

@kjbekkelund yeah, thank you. --env node solved the issue, I did miss this.
Alos check the jest official help .

@SimenB

This comment has been minimized.

Show comment
Hide comment
@SimenB

SimenB Dec 6, 2017

Collaborator

I think that last case might be fixed in #4669

Collaborator

SimenB commented Dec 6, 2017

I think that last case might be fixed in #4669

@bobrosoft

This comment has been minimized.

Show comment
Hide comment
@bobrosoft

bobrosoft Dec 11, 2017

@dignifiedquire hey there. In which Jest version that was fixed? Using Jest 21.1.6 and it's still there. My particular case is there https://stackoverflow.com/a/47760009/5086732, crashing if remove try..catch wrapper from there.

@dignifiedquire hey there. In which Jest version that was fixed? Using Jest 21.1.6 and it's still there. My particular case is there https://stackoverflow.com/a/47760009/5086732, crashing if remove try..catch wrapper from there.

@shdwjk

This comment has been minimized.

Show comment
Hide comment
@shdwjk

shdwjk Jan 10, 2018

I'm still seeing this with v20.0.4. In my case with a check inside a redux store reducer.

shdwjk commented Jan 10, 2018

I'm still seeing this with v20.0.4. In my case with a check inside a redux store reducer.

@bf

This comment has been minimized.

Show comment
Hide comment
@bf

bf Jan 10, 2018

Seems to be a gift that gives on giving...

bf commented Jan 10, 2018

Seems to be a gift that gives on giving...

@SimenB

This comment has been minimized.

Show comment
Hide comment
@SimenB

SimenB Jan 10, 2018

Collaborator

Still happening in 22?

Collaborator

SimenB commented Jan 10, 2018

Still happening in 22?

@shdwjk

This comment has been minimized.

Show comment
Hide comment
@shdwjk

shdwjk Jan 10, 2018

Ah, is working in 22.0.5. Great, thanks!

shdwjk commented Jan 10, 2018

Ah, is working in 22.0.5. Great, thanks!

@rawroland

This comment has been minimized.

Show comment
Hide comment
@rawroland

rawroland Jan 16, 2018

I am still experiencing this in 22.1.1 in the following test:

it('replies with the correct message': done => {
  // Initialise a bot with the MS botbuilder framework
  bot.on('send', message => {
     expect(message.text).toEqual('Is everything ok?');
     done();
   });
});

I am still experiencing this in 22.1.1 in the following test:

it('replies with the correct message': done => {
  // Initialise a bot with the MS botbuilder framework
  bot.on('send', message => {
     expect(message.text).toEqual('Is everything ok?');
     done();
   });
});
@SimenB

This comment has been minimized.

Show comment
Hide comment
@SimenB

SimenB Jan 16, 2018

Collaborator

Can you provide a repro I can pull down to test?

Collaborator

SimenB commented Jan 16, 2018

Can you provide a repro I can pull down to test?

@rawroland

This comment has been minimized.

Show comment
Hide comment
@rawroland

rawroland Jan 16, 2018

Thanks for the quick reply. Yes, will do so within the next 30 mins.

Thanks for the quick reply. Yes, will do so within the next 30 mins.

@rawroland

This comment has been minimized.

Show comment
Hide comment
@rawroland

rawroland Jan 16, 2018

Here is a repo to reproduce the issue: https://github.com/rawroland/async-jest-test.

Here is a repo to reproduce the issue: https://github.com/rawroland/async-jest-test.

@SimenB

This comment has been minimized.

Show comment
Hide comment
@SimenB

SimenB Jan 26, 2018

Collaborator

@rawroland that's expected, you're basically seeing #3917.

I'd recommend using promise instead of callback based code, then you don't have to worry about that sort of code flow. E.g.

it('replies with the correct message', async () => {
  // Initialise a bot with the MS botbuilder framework
  const result = await new Promise(resolve => bot.on('send', resolve));

  expect(result.text).toEqual('Is everything ok?');
});
Collaborator

SimenB commented Jan 26, 2018

@rawroland that's expected, you're basically seeing #3917.

I'd recommend using promise instead of callback based code, then you don't have to worry about that sort of code flow. E.g.

it('replies with the correct message', async () => {
  // Initialise a bot with the MS botbuilder framework
  const result = await new Promise(resolve => bot.on('send', resolve));

  expect(result.text).toEqual('Is everything ok?');
});
@rawroland

This comment has been minimized.

Show comment
Hide comment
@rawroland

rawroland Feb 1, 2018

Thank you for the information.

Thank you for the information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment