0

I have the following (simplified for the sake of scoping down problem) code:

function pushPromises() {
  const promises = [];

  promises.push(firstPromise('something'))
  promises.push(secondPromise('somethingelse'))

  return promises;
}

export default handlePromises(async (c) => {
  const results = await Promise.all(pushPromises())
  c.success(results);
});

My test mocks those firstPromise and secondPromise to check if they were called with the right arguments. This works (assume mock set up is properly done):

jest.mock('src/firstPromise');
jest.mock('src/secondPromise');

describe('test', () => {
  let firstMock;
  let secondMock;


  beforeEach(() => {
    require('src/firstPromise').default = firstMock;
    require('src/secondPromise').default = secondMock;
  })

  it('test', async () => {
    await handlePromises(context);
    expect(firstPromiseMock).toHaveBeenCalledTimes(1);
    expect(secondPromiseMock).toHaveBeenCalledTimes(1);
  });
});

Now, if I add handlers to the promises such as:

function pushPromises() {
      const promises = [];

      promises.push(
        firstPromise('something')
        .then(r => console.log(r))
        .catch(e => console.log(e))
      )
      promises.push(
        secondPromise('somethingelse')
        .then(r => console.log(r))
        .catch(e => console.log(e))
      )

      return promises;
    }

The first expectation passes, but the second one doesn't. It looks like the second promise is no longer called.

How can I modify the test/code so that adding handlers on the promises don't make the test break? It looks like it is just finishing the execution on the first promise handler and does never get to the second one.

EDIT:

I have modified the function to return a Promise.all without await:

 export default handlePromises(async (c) => {
      return Promise.all(pushPromises())
 });

But I'm having the exact same issue. The second promise is not called if the first one has a .then.

7
  • 1
    How're those mocks set up?
    – raina77ow
    Mar 26, 2019 at 10:15
  • await handlePromises(context); handlePromises doesn't look like a promise to me, so the await will have no effect.
    – Keith
    Mar 26, 2019 at 10:15
  • handlePromises(context) here context must have a callback named success and then you will have to move your expect statements in success callback.
    – AZ_
    Mar 26, 2019 at 10:18
  • You have two very different ways that you call handlePromises. In your real code, you call it with an async function and export its result, most likely a promise. In your test code, you are calling it with your context, which most likely is an object. Mar 26, 2019 at 10:19
  • Added mock set up @raina77ow Mar 26, 2019 at 10:22

2 Answers 2

1

In your edit, your handlePromises is still not a promise..

Try the following. ->

it('test', async () => {
  await Promise.all(pushPromises());
  expect(firstPromiseMock).toHaveBeenCalledTimes(1);
  expect(secondPromiseMock).toHaveBeenCalledTimes(1);
});
4
  • Why do I even need to do this do? If in my code (not my edit) I'm already doing const results = await Promise.all(pushPromises()) doesn't that guarantee that all promises will be run (from a test perspective)? Mar 26, 2019 at 11:36
  • @HommerSmith No, because your handlePromises is not a promise, so when you do await handlePromises() it's not going to wait. And this explains why your test fails, as the first promise might have resolved by the time your first expect is called, but your second promise might still be pending when your second expect is called.
    – Keith
    Mar 26, 2019 at 11:40
  • What I don't understand Keith is why this just happens when I introduce then and catch in my promises. Anyway, if I need to have that context (the edit would not be valid because I really need to return that context.success) how would I do it? Mar 26, 2019 at 11:54
  • @HommerSmith when I introduce then and catch It's just a timing thing,. Sometimes the Javascript engine might optimise away a next tick, but it's something you should never rely on, in fact I'm pretty sure in early implementations of promises it was guaranteed a next tick would fire. I'm not sure what context is and why needed, but all you need to do is make sure handlePromises() returns as a promise as currently it's not. Looking at AZ_ where he's returning a promise with promise.all and piggy backing the then method to update the context looks like what your after.
    – Keith
    Mar 26, 2019 at 12:02
0

Your handlePromises function accepts a callback but you are handling it as it returns a promise, it is not a good way to go but you can test your method using callback as following

Modify your Test as=>

it('test', (done) => {
    handlePromises(context);
    context.success = (results) => {
        console.log(results)
        expect(firstPromiseMock).toHaveBeenCalledTimes(1);
        expect(secondPromiseMock).toHaveBeenCalledTimes(1);
        done();
    }
});

Or modify your code as =>

function pushPromises() {
  const promises = [];

  promises.push(firstPromise('something'))
  promises.push(secondPromise('somethingelse'))

  return promises;
}

export default handlePromises = (context) => {
  return Promise.all(pushPromises()).then((data) => context.success(data))
};

//with async
export default handlePromises = async (context) => {
  let data = await Promise.all(pushPromises());
  context.success(data)
};
11
  • Why does it work when I don't add the then and catch? And how is this solving that issue? Mar 26, 2019 at 10:30
  • I already tried modifying my code to not use the context and instead returning Promise.all(pushPromises()) and I see the same issue. The second promise is not being called if I add a then in the first promise. Mar 26, 2019 at 10:32
  • @HommerSmith returning Promise.all(pushPromises()) Could you show us that version, as that's the one that should work. The then / catch should have no effect on the promise return.
    – Keith
    Mar 26, 2019 at 10:37
  • It solves the issue by making you tests to wait for all the promises to get resolved first, I am not very sure How it worked without then block @HommerSmith
    – AZ_
    Mar 26, 2019 at 10:46
  • it must be export default handlePromises = () => { return Promise.all(pushPromises()) }); @HommerSmith
    – AZ_
    Mar 26, 2019 at 10:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.